Re: [PATCH net v2 2/2] net: stmmac: keep datapath state coherent after reinitialization failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] net: missing netif_device_attach() on successful MTU retry leaves interface detached
- [Medium] pm: missing hardware suspend on system sleep after failed MTU change
--

commit aa909a318d1e4379c0a0ad905cb393e855cd5906
Author: James Hilliard <james.hilliard1@gmail.com>
Date:   Tue Sep 22 23:27:41 2026 +0000

    net: stmmac: keep datapath state coherent after reinitialization failure
    
    An MTU change releases the running datapath before reopening it. If the
    reopen fails, its replacement DMA resources are freed, but the netdev is
    still administratively up. A later close repeats NAPI shutdown, IRQ
    release and DMA cleanup. Hardware resume failure has a different partial
    state: suspend disabled NAPI but retained the IRQs and DMA resources, so
    ordinary close can hang in a second napi_disable().

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 1fb5f804ea234..7b423c87314ca 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
[ ... ]
> @@ -6174,6 +6192,11 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
>  		if (ret) {
>  			free_dma_desc_resources(priv, dma_conf);
>  			kfree(dma_conf);
> +			/*
> +			 * Keep the administrative state and PHY/PM ownership until
> +			 * ndo_stop(), but prevent use of the released data path.
> +			 */
> +			netif_device_detach(dev);
>  			netdev_err(priv->dev, "failed reopening the interface after MTU change\n");
>  			return ret;
>  		}

[Severity: High]
Does stmmac_change_mtu() need to call netif_device_attach(dev) on the
success path?

If an MTU change fails, netif_device_detach(dev) administratively disables
the datapath while keeping netif_running() true. If the user then retries
changing the MTU and it succeeds, __stmmac_open() restores the datapath,
but netif_device_attach(dev) is never called.

Because the interface remains permanently detached (__LINK_STATE_PRESENT is
false), the networking core and XDP routines will refuse to queue or
transmit packets. The interface will silently drop traffic despite a
successful MTU configuration, requiring a manual down/up cycle to recover.

[ ... ]
> @@ -8198,26 +8246,24 @@ int stmmac_suspend(struct device *dev)
>  {
>  	struct net_device *ndev = dev_get_drvdata(dev);
>  	struct stmmac_priv *priv = netdev_priv(ndev);
> -	u8 chan;
>  
> -	if (!ndev || !netif_running(ndev))
> +	rtnl_lock();
> +	if (priv->datapath != STMMAC_DATAPATH_RUNNING) {
> +		rtnl_unlock();
>  		goto suspend_bsp;
> +	}

[Severity: Medium]
Will this bypass the hardware suspend sequence during system sleep if the
datapath is STMMAC_DATAPATH_DOWN following a failed MTU change?

If stmmac_change_mtu() fails, the datapath state becomes
STMMAC_DATAPATH_DOWN, but the interface remains administratively UP. The
error path of __stmmac_open() does not power down the SerDes, and
stmmac_change_mtu() intentionally retains PHY/PM ownership to allow a
retry.

When the system subsequently suspends, stmmac_suspend() skips the hardware
suspend sequence entirely because priv->datapath != STMMAC_DATAPATH_RUNNING.

Consequently, the device skips stmmac_legacy_serdes_power_down(),
pinctrl_pm_select_sleep_state(), and Wake-on-LAN configuration
(stmmac_pmt()). This leaves hardware actively powered during system sleep,
which could block deep sleep states and break Wake-on-LAN.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922-submit-stmmac-reset-fixes-v1-v2-0-d92e609be996@gmail.com?part=2