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 11:09:16, Markus Schneider-Pargmann wrote:
> >> --- a/drivers/net/can/m_can/m_can.c
> >> +++ b/drivers/net/can/m_can/m_can.c

[...]

> >> +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?
> 
> After reading the other users of the above function, I do see why this
> was added. I am still wondering if there is a way to return to
> ERROR_ACTIVE once the errors are cleared from the error register.

Sorry, I don't get what you mean.

> Also looking at all the users added for the function above, could you
> read the register inside the function? Currently you are adding a
> reg variable and a read call for each call to this function.
> m_can_handle_state_errors() also doesn't need the psr value with your
> refactoring.

That makes sense.

regards,
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