Re: [PATCH 3/7] can: m_can: m_can_handle_state_errors(): fix CAN state transition to Error Active

[Marc Kleine-Budde]

On 20.08.2025 10:49:35, Markus Schneider-Pargmann wrote:
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -812,6 +812,10 @@ static int m_can_handle_state_change(struct net_device *dev,
> >  	u32 timestamp = 0;
> >  
> >  	switch (new_state) {
> > +	case CAN_STATE_ERROR_ACTIVE:
> > +		/* error active state */
> 
> This comment and the one below don't really help IMHO.

both removed

> > +		cdev->can.state = CAN_STATE_ERROR_ACTIVE;
> > +		break;
> >  	case CAN_STATE_ERROR_WARNING:
> >  		/* error warning state */
> >  		cdev->can.can_stats.error_warning++;
> > @@ -841,6 +845,13 @@ static int m_can_handle_state_change(struct net_device *dev,
> >  	__m_can_get_berr_counter(dev, &bec);
> >  
> >  	switch (new_state) {
> > +	case CAN_STATE_ERROR_ACTIVE:
> > +		/* error active state */
> > +		cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
> > +		cf->data[1] = CAN_ERR_CRTL_ACTIVE;
> > +		cf->data[6] = bec.txerr;
> > +		cf->data[7] = bec.rxerr;
> > +		break;
> >  	case CAN_STATE_ERROR_WARNING:
> >  		/* error warning state */
> >  		cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
> > @@ -877,30 +888,29 @@ static int m_can_handle_state_change(struct net_device *dev,
> >  	return 1;
> >  }
> >  
> > +static enum can_state
> > +m_can_can_state_get_by_psr(const u32 psr)
> > +{
> > +	if (psr & PSR_BO)
> > +		return CAN_STATE_BUS_OFF;
> > +	if (psr & PSR_EP)
> > +		return CAN_STATE_ERROR_PASSIVE;
> > +	if (psr & PSR_EW)
> > +		return CAN_STATE_ERROR_WARNING;
> 
> Why should m_can_handle_state_errors() should be called if none of these
> flags are set?
> 
> m_can_handle_state_errors() seems to only be called if IR_ERR_STATE
> which is defined as:
>   #define IR_ERR_STATE	(IR_BO | IR_EW | IR_EP)
> 
> This is the for the interrupt register but will the PSR register bits be
> set without the interrupt register being set?

The IR_ERR_STATE will be set if there was a _change_ in corresponding
PSR register's bits.

With successful transmission/reception the corresponding error counter
decrease and if a threshold is crossed the state changes.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Attachment: signature.asc
Description: PGP signature