netdev - Re: [RFC PATCH] net: phy: integrate reset-after-clock quirk into phy_init

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: <20260128202634.gqevi76o6wnf5xno@pengutronix.de>
Date: Wed, 28 Jan 2026 21:26:34 +0100
From: Marco Felsch <m.felsch@...gutronix.de>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Michael Nazzareno Trimarchi <michael@...rulasolutions.com>,
	Andrew Lunn <andrew@...n.ch>, Wei Fang <wei.fang@....com>,
	Shenwei Wang <shenwei.wang@....com>,
	Clark Wang <xiaoning.wang@....com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Heiner Kallweit <hkallweit1@...il.com>,
	"open list:FREESCALE IMX / MXC FEC DRIVER" <imx@...ts.linux.dev>,
	"open list:FREESCALE IMX / MXC FEC DRIVER" <netdev@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] net: phy: integrate reset-after-clock quirk into
 phy_init_hw

On 26-01-28, Russell King (Oracle) wrote:
> On Wed, Jan 28, 2026 at 02:33:24PM +0100, Michael Nazzareno Trimarchi wrote:
> > Hi Andrew
> > 
> > On Wed, Jan 28, 2026 at 2:25 PM Andrew Lunn <andrew@...n.ch> wrote:
> > >
> > > On Wed, Jan 28, 2026 at 10:46:41AM +0100, Michael Trimarchi wrote:
> > > > The current implementation of phy_reset_after_clk_enable requires MAC drivers
> > > > (like fec_main.c) to manually track PHY probing states and trigger resets
> > > > at specific points in their clock management flow and was created just for a SoC
> > > > vendor.
> > >
> > > We try to keep workarounds for specific SoC vendors out of the core,
> > > when possible. It just makes the core more complex for an edge case
> > > which most people don't care about. It is better to hide the
> > > complexity away in the driver which needs it.
> > 
> > We should not merge things in the core that are for one Soc and
> > after d65af21842f8a7821fc3b28dc043c2f92b2b312c, I need to anyway to do:
> > 
> > net: phy: smsc: Fix functionality of lan8710 in combination with NXP fec
> > 
> > Revert "net: phy: smsc: LAN8710/20: remove PHY_RST_AFTER_CLK_EN flag"
> > d65af21842
> 
> That commit is an example of a poor commit description. It doesn't
> describe the problem, nor which hardware this affects, so when we end
> up with this kind of situation, we're lacking the context behind why
> the change was made.

I need to say that this commit lacks a bit of context which is part of
my previous commit. I thought, that I made that clear with:

|   The same behaviour can be archived now by specifying the refclk.

To be honest, I should have made it more clear.

> So, how do we know that reverting that commit is not going to cause a
> regression for whatever platform the patch was proposed for?

If I get the Michael's patch correct, he tries to move the quirk
handling into the core. I really don't know why Michael requires the
speical reset handling at all. The commit adding the
PHY_RST_AFTER_CLK_EN flag says:

|    phylib: add reset after clk enable support
|
|   Some PHYs need the refclk to be a continuous clock. Therefore they don't
|   allow turning it off and on again during operation. Nonetheless such a
|   clock switching is performed by some ETH drivers (namely FEC [1]) for
|   power saving reasons. An example for an affected PHY is the
|   SMSC/Microchip LAN8720 in "REF_CLK In Mode".

This can be achieved with commit
bedd8d78aba300860cec3f85d6ff549b3b7f2679 if specified accordingly.

> If you read the preceeding commit (which is referenced by the commit
> you intend to revert) itgives more information about the problem, and
> I would expect that reverting it will cause a regression with
> interrupts.

Yes, my bad I thought this reference is enough.

> Adding Marco to this thread for comment.

Thanks.

The issue was with the out-of-band reset coming from the FEC driver
which doesn't honor the phy state-machine. It shouldn't cause a
regression (TM) if it would be part of the state-machine (aka this RFC
patch) but it needs to be tested!

As said above, I'm not sure why PHY_RST_AFTER_CLK_EN is required in the
first place.

> Also, maybe you could clearly state what the issue that you are trying
> to address - why do you need special reset handling for the combination
> of FEC + this PHY. Consider including that description in the commit
> message for any proposed solution so that - as is the case here looking
> back at Macro's commit - we can see _why_ the change is being made.

With the context of the previous commit it should be clear.

Regards,
  Marco

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

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |