netdev - Re: LAN8720: RX errors / packet loss when using smsc PHY driver on i.MX6Q

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] [day] [month] [year] [list]

Message-ID: <ca42d0de-404f-4ea5-b65d-5fe9d8813d64@lunn.ch>
Date: Sun, 16 Nov 2025 16:55:46 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Fabio Estevam <festevam@...il.com>
Cc: "Russell King (Oracle)" <linux@...linux.org.uk>,
	Heiner Kallweit <hkallweit1@...il.com>,
	edumazet <edumazet@...gle.com>, netdev <netdev@...r.kernel.org>
Subject: Re: LAN8720: RX errors / packet loss when using smsc PHY driver on
 i.MX6Q

On Sat, Nov 15, 2025 at 11:14:33PM -0300, Fabio Estevam wrote:
> On Sat, Nov 15, 2025 at 9:57 PM Fabio Estevam <festevam@...il.com> wrote:
> 
> > I have also tried describing it inside the ethernet-phy node with:
> > reset-assert-us; reset-deassert-us; and reset-gpios, but it did not help.
> 
> Ok, what do you think about the change below?
> 
> It will work when reset-gpios is described inside the ethernet-phy node.
> 
> It will not work when the reset GPIO is specified within the FEC node
> via the phy-reset-gpios property.
> 
> This is OK as 'phy-reset-gpios' is marked as deprecated in
> Documentation/devicetree/bindings/net/fsl, fec.yaml.
> 
> --- a/drivers/net/phy/smsc.c
> +++ b/drivers/net/phy/smsc.c
> @@ -147,9 +147,19 @@ 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, a hardware reset will be
> +        * performed by the PHY core, so do NOT issue a soft reset here.
> +        */
> +       if (phydev->mdio.dev.of_node &&
> +           of_property_present(phydev->mdio.dev.of_node, "reset-gpios"))
> +               return 0;
> +
> +       /* No reset GPIO: fall back to soft reset */
>         return genphy_soft_reset(phydev);

We are still missing the "Why?". Why does a combination of a hard and
soft reset mess up the PHY?

It would be good to understand this before accepting the patch. Or
extend the patch and the commit message to say that we have no idea
why this works, but testing suggests it does. That will help anybody
who comes later and has problems here to know this is a magical
workaround, not a fix.

	Andrew