linux-kernel - Re: [net-next PATCH 1/2] net: phy: Add support for new Aeonsemi PHYs

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: <Z-Mh7TlJgStma9ln@shell.armlinux.org.uk>
Date: Tue, 25 Mar 2025 21:36:45 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, 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>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [net-next PATCH 1/2] net: phy: Add support for new Aeonsemi PHYs

On Tue, Mar 25, 2025 at 01:04:30PM +0100, Christian Marangi wrote:
> On Mon, Mar 24, 2025 at 04:16:09PM +0100, Andrew Lunn wrote:
> > On Mon, Mar 24, 2025 at 03:16:08PM +0100, Christian Marangi wrote:
> > > On Mon, Mar 24, 2025 at 03:03:51PM +0100, Andrew Lunn wrote:
> > > > > Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1,
> > > > > AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1,
> > > > > AS21210PB1 that all register with the PHY ID 0x7500 0x7500
> > > > > before the firmware is loaded.
> > > > 
> > > > Does the value change after the firmware is loaded? Is the same
> > > > firmware used for all variants?
> > > >
> > > 
> > > Yes It does... Can PHY subsystem react on this? Like probe for the
> > > generic one and then use the OPs for the real PHY ID?
> > 
> > Multiple thoughts here....
> > 
> > If the firmware is in SPI, i assume by the time the MDIO bus is
> > probed, the PHY has its 'real' IDs. So you need entries for those as
> > well as the dummy ID.
> > 
> > I think this is the first PHY which changes its IDs at runtime. So we
> > don't have a generic solution to this. USB and PCI probably have
> > support for this, so maybe there is something we can copy. It could be
> > they support hotplug, so the device disappears and returns. That is
> > not really something we have in MDIO. So i don't know if we can reuse
> > ideas from there.
> > 
> > When the firmware is running, do the different variants all share the
> > same ID? Is there a way to tell them apart. We always have
> > phy_driver->match_phy_device(). It could look for 0x75007500, download
> > the firmware, and then match on the real IDs.
> 
> Ok update on this... The PHY report 7500 7500 but on enabling PTP clock,
> a more specific ""family"" ID is filled in MMD that is 0x7500 0x9410.
> 
> They all use the same firmware so matching for the family ID might not
> be a bad idea... The alternative is either load the firmware in
> match_phy_device or introduce some additional OPs to handle this
> correctly...
> 
> Considering how the thing are evolving with PHY I really feel it's time
> we start introducing specific OP for firmware loading and we might call
> this OP before PHY ID matching is done (or maybe do it again).

You're basically talking there about modifying the core driver model of
the kernel, which I think you're going to have an uphill battle with.

The match_phy_device() op is called by the core driver model when a new
device/driver is added to find a driver for an unbound device. It does
this calling the bus_type's .match() method for each unbound device
on the bus_type, and every device driver that is bound to the bus type.

In the case of a MDIO device, which phylib's PHYs are a sub-class of,
this is populated with mdio_bus_match(), which then goes on to call
the MDIO device's ->bus_match() method. For a PHY, that's
phy_bus_match(), which will either call the PHY driver's
.match_phy_device() method or will attempt to match the hardware ID
against what is given in the PHY driver structure.

So, the core driver model is responsible for trying each device driver
on the bus to find one that matches each device. This isn't a phylib
thing.

At the moment, I don't see how adding another PHY driver OP helps. We
still need to find the right PHY driver struct somehow. It seems to me
to be a chicken-and-egg problem.

That's why I suggested that maybe board firmware should be loading at
least _some_ kind of firmware to get the PHY to the point that the
kernel can properly identify it - the kernel can then read the
loaded firmware, compare it with the version that's locally available,
and if different, replace the firmware. The main thing is that board
firmware gets the PHY to the point that it can be properly identified.

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