Re: [PATCH net v5 3/6] can: rockchip_canfd: cancel bus-off restart before closing the device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] can: rockchip_canfd: missing mask flush before free_irq() risks storm
--

commit 761946c971cc50dc5c7a42016ab27b61a872001e
Author: Cunhao Lu <1579567540@qq.com>

can: rockchip_canfd: cancel bus-off restart before closing the device

This patch fixes issues in the driver's close and restart paths by reordering
operations in ndo_stop(). It synchronously cancels restart work earlier, masks
device interrupts, and synchronizes IRQs without disabling the shared IRQ line.

> diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
> index 37c1c22c40c9f..4b3db07242395 100644
> --- a/drivers/net/can/rockchip/rockchip_canfd-core.c
> +++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
[ ... ]
> @@ -356,7 +357,18 @@ static int rkcanfd_set_mode(struct net_device *ndev,
>  
>  	switch (mode) {
>  	case CAN_MODE_START:
> +		rkcanfd_chip_interrupts_disable(priv);
> +		/* Flush the interrupt mask write before suspending the handler. */
> +		rkcanfd_read(priv, RKCANFD_REG_INT_MASK);
> +		/* Keep shared IRQs from accessing the device during restart. */
> +		WRITE_ONCE(priv->irq_suspended, true);
> +		/* Wait for handlers that already passed the suspended check. */
> +		synchronize_irq(ndev->irq);
> +		rkcanfd_timestamp_stop_sync(priv);
> +
>  		rkcanfd_chip_start(priv);
[ ... ]
> @@ -758,6 +775,7 @@ static int rkcanfd_stop(struct net_device *ndev)
>  {
>  	struct rkcanfd_priv *priv = netdev_priv(ndev);
>  
> +	close_candev_cancel_restart_sync(ndev);
>  	netif_stop_queue(ndev);
>  
>  	rkcanfd_chip_interrupts_disable(priv);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does rkcanfd_stop() also need a readback flush after disabling hardware
interrupts, similar to the one added above in rkcanfd_set_mode()?

Without a readback flush before the subsequent free_irq() call, if the
shared IRQ line receives an interrupt before the posted write reaches
the bus, the CPU could receive the interrupt after the handler has been
deregistered. Could this cause an unhandled interrupt storm that disables
the shared IRQ line entirely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-rockchip-canfd-restart-close-v5-0-ae77327037c9@qq.com?part=3