Re: [PATCH 4/7] can: m_can: m_can_chip_config(): bring up interface in correct state

[Marc Kleine-Budde]

On 20.08.2025 11:00:51, Markus Schneider-Pargmann wrote:
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index b485d2f3d971..310a907cbb7e 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -1607,6 +1607,7 @@ static int m_can_chip_config(struct net_device *dev)
> >  static int m_can_start(struct net_device *dev)
> >  {
> >  	struct m_can_classdev *cdev = netdev_priv(dev);
> > +	u32 reg_psr;
> >  	int ret;
> >  
> >  	/* basic m_can configuration */
> > @@ -1617,7 +1618,8 @@ static int m_can_start(struct net_device *dev)
> >  	netdev_queue_set_dql_min_limit(netdev_get_tx_queue(cdev->net, 0),
> >  				       cdev->tx_max_coalesced_frames);
> >  
> > -	cdev->can.state = CAN_STATE_ERROR_ACTIVE;
> > +	reg_psr = m_can_read(cdev, M_CAN_PSR);
> > +	cdev->can.state = m_can_can_state_get_by_psr(reg_psr);
> 
> Previous patch makes sense for use here. But how is the state set back
> in operation after mcan was in an error state? Maybe I missed the path
> back to CAN_STATE_ERROR_ACTIVE somewhere?

Sorry, I don't exactly get what you mean here.

> Also CAN_STATE_ERROR_ACTIVE is set in resume() as well, should that also
> read the PSR instead?

see next patch :)

> Ans lastly I don't like the function name, because of the repeated can,
> maybe something like m_can_error_state_by_psr()?

It's m_can_state_get_by_psr() now.

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