Re: [PATCH net] net: stmmac: dwmac-sun8i: reset the EMAC when opening, not when probing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch fails to fix the EMAC soft reset timeout bug for boards with an internal PHY that are configured to use an external PHY.
- [High] Uncoordinated MAC soft reset triggered by MDIO mux switching causes silent network interface hang.
--

--- Patch [1]: [PATCH net] net: stmmac: dwmac-sun8i: reset the EMAC when opening, not when probing ---
commit 5f11ae1c8c7c87b062343e837c4c5de6f8022782
Author: Pedro Santos <hartmnn.p@gmail.com>

net: stmmac: dwmac-sun8i: reset the EMAC when opening, not when probing

This patch moves the EMAC soft reset from the probe function to the DMA
reset hook (called during interface open). This ensures the PHY is powered
up and driving the RX clock before the MAC reset is triggered, fixing
reset timeouts on boards with external PHYs after a warm reboot.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 48c52eb962335..748ebabd933cd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c

[ ... ]

> @@ -740,23 +766,12 @@ static void sun8i_dwmac_flow_ctrl(struct mac_device_info *hw,
>  
>  static int sun8i_dwmac_reset(struct stmmac_priv *priv)
>  {
> -	u32 v;
> -	int err;
> -
> -	v = readl(priv->ioaddr + EMAC_BASIC_CTL1);
> -	writel(v | 0x01, priv->ioaddr + EMAC_BASIC_CTL1);
> +	int err = sun8i_dwmac_soft_reset(priv->ioaddr);

[Severity: High]
This isn't a bug introduced by this patch, but does the MDIO mux switch
callback cause a silent network interface hang?

If userspace queries the inactive PHY's MDIO bus (for example, via ethtool
or /dev/mdio), the multiplexer invokes mdio_mux_syscon_switch_fn(), which
unconditionally calls sun8i_dwmac_reset().

This calls sun8i_dwmac_soft_reset(), asserting the hardware soft reset bit
and wiping all MAC registers, filters, and DMA engine state. Since the stmmac
core driver is completely unaware of this reset, will it fail to reinitialize
the hardware, resulting in all traffic being dropped until the interface is
administratively restarted?

>  
> -	/* The timeout was previously set to 10ms, but some board (OrangePI0)
> -	 * need more if no cable plugged. 100ms seems OK
> -	 */
> -	err = readl_poll_timeout(priv->ioaddr + EMAC_BASIC_CTL1, v,
> -				 !(v & 0x01), 100, 100000);
> -
> -	if (err) {
>  		dev_err(priv->device, "EMAC reset timeout\n");
> -		return err;
> -	}
> -	return 0;
> +
> +	return err;
>  }

[ ... ]

> @@ -1217,10 +1232,6 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
>  			dev_err(&pdev->dev, "Failed to register mux\n");
>  			goto dwmac_mux;
>  		}
> -	} else {
> -		ret = sun8i_dwmac_reset(priv);
> -		if (ret)
> -			goto dwmac_remove;
>  	}

[Severity: High]
This is a pre-existing issue, but does this patch miss the reset timeout
for boards with an internal PHY that are configured to use an external PHY?

When sun8i_dwmac_probe() calls sun8i_dwmac_register_mdio_mux() just above
this block, mdio_mux_init() synchronously scans the child buses. This invokes
mdio_mux_syscon_switch_fn() for the first time, which then unconditionally
calls sun8i_dwmac_reset().

Since this still happens at probe time, wouldn't the external PHY still be
powered down from a warm reboot, causing the soft reset to time out here
just like it did before?

>  
>  	pm_runtime_put(&pdev->dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822045641.19282-1-hartmnn.p@gmail.com?part=1