Re: [PATCH net v2 3/3] 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:
- [Medium] can: rockchip_canfd: disable_irq() called on shared IRQ
--

commit 580402c6870b4c8a3e1db89526b629150785fd7a
Author: Cunhao Lu <1579567540@qq.com>

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

This commit cancels restart_work during the close path before closing the device
to prevent a running restart from re-enabling the controller. It also uses
disable_irq() during the restart callback to prevent the IRQ handler from
running while the hardware is being reinitialized.

> diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
> --- a/drivers/net/can/rockchip/rockchip_canfd-core.c
> +++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
> @@ -356,8 +356,10 @@ static int rkcanfd_set_mode(struct net_device *ndev,
>  
>  	switch (mode) {
>  	case CAN_MODE_START:
> +		disable_irq(ndev->irq);

[Severity: Medium]
Is it safe to call disable_irq() here on a shared interrupt line?

In rkcanfd_open(), the IRQ is explicitly registered as shared:

    err = request_irq(ndev->irq, rkcanfd_irq, IRQF_SHARED, ndev->name, priv);

Masking a shared IRQ line globally delays or drops interrupts for all other
devices sharing that line.

Could this driver instead use a software flag paired with synchronize_irq()
or spinlocks to exclude the handler, rather than disabling the physical line?

>  		rkcanfd_chip_start(priv);
>  		rkcanfd_chip_interrupts_enable(priv);
> +		enable_irq(ndev->irq);
>  		netif_wake_queue(ndev);
>  		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_196A2433801631CD2FA42998465E26316105@qq.com?part=3