Re: [PATCH net-next v10 3/3] w5100: detect carrier state using link status bit and optional interrupt

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: w5100: race in w5100_suspend due to incorrect detach ordering and missing work flushes
--

commit 4b50b6b7047dc53f16eedfdba4f4a537ef54c0fd
Author: Arthur Crepin Leblond <arthur@marmottus.net>

w5100: detect carrier state using link status bit and optional interrupt

This commit adds support for detecting the carrier state on the w5500 Ethernet
controller using the Link Status bit and an optional interrupt. It also
refactors the bring-up and teardown operations, including changes to when
net device registration and work queue cancellations occur.

> diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
> --- a/drivers/net/ethernet/wiznet/w5100.c
> +++ b/drivers/net/ethernet/wiznet/w5100.c
[ ... ]
> @@ -839,9 +997,20 @@ static int w5100_suspend(struct device *dev)
>  	struct w5100_priv *priv = netdev_priv(ndev);
>  
>  	if (netif_running(ndev)) {
> -		netif_carrier_off(ndev);
> +		disable_irq(priv->irq);
> +		if (priv->link_irq > 0)
> +			disable_irq(priv->link_irq);
> +
> +		cancel_work_sync(&priv->restart_work);
> +
>  		netif_device_detach(ndev);

[Severity: High]
Does this code leave a race condition in w5100_suspend() due to the detach
ordering?

If a TX timeout occurs between cancel_work_sync(&priv->restart_work) and
netif_device_detach(), wouldn't restart_work be re-scheduled because the
netdev watchdog is still active?

Also, rx_work, tx_work, setrx_work, and napi do not appear to be flushed or
disabled in w5100_suspend(), unlike in w5100_stop(). Could this allow them
to execute concurrently with or after w5100_hw_close(priv) and access
suspended SPI hardware?

>  
> +		if (priv->link_irq > 0) {
> +			mutex_lock(&priv->link_lock);
> +			netif_carrier_off(ndev);
> +			mutex_unlock(&priv->link_lock);
> +		}
> +
>  		w5100_hw_close(priv);
>  	}
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-wiznet-link-gpio-v10-0-5874a7a147a2@marmottus.net?part=3