netdev - Re: [PATCH net] net: phy: smsc: Skip soft reset when a hardware reset GPIO is provided

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aRoFB3MunCS-_Qvl@shell.armlinux.org.uk>
Date: Sun, 16 Nov 2025 17:08:23 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Fabio Estevam <festevam@...il.com>
Cc: kuba@...nel.org, andrew@...n.ch, hkallweit1@...il.com,
	manfred.schlaegl@...zinger.com, netdev@...r.kernel.org,
	edumazet@...gle.com, pabeni@...hat.com, f.fainelli@...il.com
Subject: Re: [PATCH net] net: phy: smsc: Skip soft reset when a hardware
 reset GPIO is provided

On Sun, Nov 16, 2025 at 12:48:24PM -0300, Fabio Estevam wrote:
> On platforms using the LAN8720 in RMII mode, issuing a soft reset through
> genphy_soft_reset() can temporarily disrupt the PHY output clock (REF_CLK).
> 
> Boards that source ENET_REF_CLK from the LAN8720 are therefore sensitive
> to PHY soft resets, as the MAC receives an unstable or missing RMII clock
> during the transition.
> 
> When a "reset-gpios" property is present, the MDIO core already performs a
> hardware reset using this GPIO before calling the driver's ->reset() hook.
> Issuing an additional soft reset in smsc_phy_reset() is redundant and may
> result in RX CRC/frame errors, packet loss, and general link instability at
> 100 Mbps.
> 
> Change smsc_phy_reset() so that:
> 
> - If reset-gpios is present: rely solely on the hardware reset and skip
> the soft reset.
> - If reset-gpios is absent: fall back to genphy_soft_reset(), preserving
> the existing behavior.
> 
> The soft reset to remove the PHY from power down is kept, as this is
> a requirement mentioned in the LAN8720 datasheet.
> 
> This fixes packet loss observed on i.MX6 platforms using LAN8720 without
> breaking boards that rely on the existing soft reset path.
> 
> Fixes: fc0f7e3317c5 ("net: phy: smsc: reintroduced unconditional soft reset")
> Signed-off-by: Fabio Estevam <festevam@...il.com>
> ---
>  drivers/net/phy/smsc.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> index 48487149c225..3840b658a996 100644
> --- a/drivers/net/phy/smsc.c
> +++ b/drivers/net/phy/smsc.c
> @@ -54,6 +54,7 @@ struct smsc_phy_priv {
>  	unsigned int edpd_mode_set_by_user:1;
>  	unsigned int edpd_max_wait_ms;
>  	bool wol_arp;
> +	bool reset_gpio;
>  };
>  
>  static int smsc_phy_ack_interrupt(struct phy_device *phydev)
> @@ -136,6 +137,7 @@ EXPORT_SYMBOL_GPL(smsc_phy_config_init);
>  
>  static int smsc_phy_reset(struct phy_device *phydev)
>  {
> +	struct smsc_phy_priv *priv = phydev->priv;
>  	int rc = phy_read(phydev, MII_LAN83C185_SPECIAL_MODES);
>  	if (rc < 0)
>  		return rc;
> @@ -147,9 +149,17 @@ static int smsc_phy_reset(struct phy_device *phydev)
>  		/* set "all capable" mode */
>  		rc |= MII_LAN83C185_MODE_ALL;
>  		phy_write(phydev, MII_LAN83C185_SPECIAL_MODES, rc);
> +		/* reset the phy */

This would be a more useful comment here:

		/* The LAN7820 datasheet states that a soft reset causes
		 * the PHY to reconfigure according to the MODE bits in
		 * MII_LAN83C185_SPECIAL_MODES. Thus, a soft reset is
		 * necessary for the above write to take effect.
		 */

Please also insert a blank line prior to the comment to make the code
more readable.

> +		return genphy_soft_reset(phydev);
>  	}
>  
> -	/* reset the phy */
> +	/* If the reset-gpios property exists, hardware reset will be
> +	 * performed by the MDIO core, so do NOT issue a soft reset here.
> +	 */
> +	if (priv->reset_gpio)
> +		return 0;

Have you tried adding a 1ms delay before the soft reset, in case the
hard reset hasn't completed?

As Andrew's feedback states to the thread that we were discussing it
(and now we have a forked discussion which is far from ideal) we
still don't know "why" the PHY is failing, and without knowing why,
we don't know whether someone else will run into the same issue and
end up patching the kernel in a different way (e.g. the network
driver.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!