[PATCH net] can: rockchip_canfd: stop bus-off restart before closing the device

[Marc Kleine-Budde]

On 13.10.2026 21:06:21, Cunhao Lu wrote:
> Hi Marc,
>
> > AFAICS this is a systematic problem and should IMHO not worked around in
> > one driver only.
>
> Agreed. I initially only noticed the problem in the Rockchip CAN-FD
> driver and did not consider the common CAN code.

Thanks to your analysis, it was easy to reach that conclusion. :)

> > netif_running() tests __LINK_STATE_START, which is removed in
> > __dev_close_many() prior to calling the driver's ndo_stop().
> >
> > Can we make use of netif_running() prior to calling do_set_mode(dev,
> > CAN_MODE_START)? from can_restart()?
>
> If you have no objections, I think it would be better to put the
> netif_running() check after the existing !priv->do_set_mode check and
> before netif_carrier_ok():
>
>         if (!priv->do_set_mode)
>                 return -EOPNOTSUPP;
>
>         if (!netif_running(dev))
>                 return -ENETDOWN;
>
> This would preserve the existing error precedence for devices that do
> not support restart.

ACK

> > can_bus_off() should not schedule_delayed_work() if !netif_running().
>
> Agreed. Although this would change the behavior of some drivers, I think
> it should help improve stability.

ok

> > We still have to call cancel_delayed_work_sync(&priv->can.restart_work)
> > from each driver, though.

Let's wrap that into a function and put this into linux/can/dev.h. It's
name could be something like close_candev_cancel_restart_sync().

> I can make changes to the other drivers. However, I do not have the
> corresponding hardware and would not be able to test the changes on
> real devices, so I am concerned about the risk. I would like to confirm:
> do I need to audit and fix the restart behavior of every CAN device
> driver?

I think it's OK to fix the rockchip driver only.

> > What do you think?
>
> I will address these comments in v2. Thanks for your review.

Please split the changes into several patches:
- fix the existing infrastructure
- add new close_candev_cancel_restart_sync() function
- use this function in the rockchip driver

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   |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20261013/2cf04bb0/attachment.sig>