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

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

[Fabio Estevam]

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@gmail.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 */
+		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;
+
+	/* No reset GPIO found: fall back to soft reset */
 	return genphy_soft_reset(phydev);
 }
 
@@ -671,6 +681,9 @@ int smsc_phy_probe(struct phy_device *phydev)
 	if (device_property_present(dev, "smsc,disable-energy-detect"))
 		priv->edpd_enable = false;
 
+	if (device_property_present(dev, "reset-gpios"))
+		priv->reset_gpio = true;
+
 	phydev->priv = priv;
 
 	/* Make clk optional to keep DTB backward compatibility. */
-- 
2.34.1

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

[Russell King (Oracle)]

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@gmail.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!

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

[Fabio Estevam]

On Sun, Nov 16, 2025 at 2:08 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:

> > +             /* 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.

Thanks, this is much better indeed.

>
> > +             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?

I can try it. Actually, I don't have physical access to the board, so
I need to ask someone to test it for me.

> 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.)

It seems that it is the i.MX6Q MAC that is failing, not the LAN8720.

After the hardware reset via GPIO, the LAN8720 generates a stable
50MHz clock to the i.MX6Q ENET_REF_CLK pin.

After the software reset is triggered, the following happens as per
the LAN8720 datasheet:

"For the first 16us after coming out of reset, the RMII interface will
run at 2.5 MHz. After this time, it will switch
to 25 MHz if auto-negotiation is enabled."

This glitch in the ENET_REF_CLK confuses the MAC, causing CRC and packet loss.

That's why avoiding the software reset in the case the LAN8720 is
driving the clock to i.MX6Q helps.