Re: [PATCH net v9 05/10] can: dev: add close_candev_cancel_restart_sync()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch adds close_candev_cancel_restart_sync() as a static
  inline in include/linux/can/dev.h and moves close_candev() onto it.

--

can: dev: add close_candev_cancel_restart_sync()

This commit adds a static inline helper, close_candev_cancel_restart_sync(),
to include/linux/can/dev.h. The helper synchronously cancels the CAN core
bus-off restart work, and close_candev() now calls it. Drivers can then
wait for recovery to finish before they tear down IRQ and hardware
resources, and echo skb cleanup stays in close_candev().

> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> index 346faddee02eb..2a286f906394b 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h

[ ... ]

> @@ -115,6 +116,22 @@ struct can_priv *safe_candev_priv(struct net_device *dev);
>  
>  int open_candev(struct net_device *dev);
>  void close_candev(struct net_device *dev);
> +
> +/**
> + * close_candev_cancel_restart_sync() - cancel bus-off restart synchronously
> + * @dev: CAN network device
> + *
> + * Call before tearing down resources used by the restart callback.
> + * The driver must quiesce restart work producers before calling
> + * close_candev() for the final cleanup.
> + */
> +static inline void close_candev_cancel_restart_sync(struct net_device *dev)
> +{
> +	struct can_priv *priv = netdev_priv(dev);
> +
> +	cancel_delayed_work_sync(&priv->restart_work);
> +}

[Severity: Low]
This isn't a runtime bug, but should this patch carry a stable tag? The
alternative would be for the following Rockchip fix to list it as a
prerequisite.

The next patch in the series, "can: rockchip_canfd: cancel bus-off restart
before closing the device", has Fixes: ff60bfbaf67f and
Cc: stable@xxxxxxxxxxxxxxx. It calls the new helper at the start of
rkcanfd_stop() in drivers/net/can/rockchip/rockchip_canfd-core.c:

    close_candev_cancel_restart_sync(ndev);

This commit has no Cc: stable tag, and the Rockchip fix does not name it
as a dependency.

If stable picks up only the tagged Rockchip fix, would the backport fail
to build? close_candev_cancel_restart_sync() would be undefined there.

Documentation/process/stable-kernel-rules.rst gives this form for
prerequisites:

    Cc: <stable@xxxxxxxxxxxxxxx> # x.y.z: <sha>: <subject>

Other patches in the series already state their backport dependencies.
The earlier core patch "can: dev: check netif_running() before bus-off
recovery" is also tagged for stable.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111-rockchip-canfd-restart-close-v9-0-6643f3825ba3%40qq.com