linux-kernel - Re: [net-next RFC PATCH v2 2/3] net: phy: Add support for Aeonsemi AS21xxx 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: <67e418a8.050a0220.3b7a97.7228@mx.google.com>
Date: Wed, 26 Mar 2025 16:09:25 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: 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>,
	Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [net-next RFC PATCH v2 2/3] net: phy: Add support for Aeonsemi
 AS21xxx PHYs

On Wed, Mar 26, 2025 at 03:56:15PM +0100, Andrew Lunn wrote:
> > +#define PHY_ID_AS21XXX			0x75009410
> > +/* AS21xxx ID Legend
> > + * AS21x1xxB1
> > + *     ^ ^^
> > + *     | |J: Supports SyncE/PTP
> > + *     | |P: No SyncE/PTP support
> > + *     | 1: Supports 2nd Serdes
> > + *     | 2: Not 2nd Serdes support
> > + *     0: 10G, 5G, 2.5G
> > + *     5: 5G, 2.5G
> > + *     2: 2.5G
> > + */
> > +#define PHY_ID_AS21011JB1		0x75009402
> > +#define PHY_ID_AS21011PB1		0x75009412
> > +#define PHY_ID_AS21010JB1		0x75009422
> > +#define PHY_ID_AS21010PB1		0x75009432
> > +#define PHY_ID_AS21511JB1		0x75009442
> > +#define PHY_ID_AS21511PB1		0x75009452
> > +#define PHY_ID_AS21510JB1		0x75009462
> > +#define PHY_ID_AS21510PB1		0x75009472
> > +#define PHY_ID_AS21210JB1		0x75009482
> > +#define PHY_ID_AS21210PB1		0x75009492
> > +#define PHY_VENDOR_AEONSEMI		0x75009400
> 
> O.K. This helps.
> 
> > +static struct phy_driver as21xxx_drivers[] = {
> > +	{
> > +		/* PHY expose in C45 as 0x7500 0x9410
> > +		 * before firmware is loaded.
> > +		 */
> > +		PHY_ID_MATCH_EXACT(PHY_ID_AS21XXX),
> > +		.name		= "Aeonsemi AS21xxx",
> > +		.probe		= as21xxx_probe,
> > +	},
> > +	{
> > +		PHY_ID_MATCH_EXACT(PHY_ID_AS21011JB1),
> > +		.name		= "Aeonsemi AS21011JB1",
> > +		.read_status	= as21xxx_read_status,
> > +		.led_brightness_set = as21xxx_led_brightness_set,
> > +		.led_hw_is_supported = as21xxx_led_hw_is_supported,
> > +		.led_hw_control_set = as21xxx_led_hw_control_set,
> > +		.led_hw_control_get = as21xxx_led_hw_control_get,
> > +		.led_polarity_set = as21xxx_led_polarity_set,
> > +	},
> 
> It is guaranteed by the current code that these entries are tried in
> the order listed here. If that was to change, other drivers would
> break.
> 
> So what you can do is have the first entry for PHY_ID_AS21XXX with
> as21xxx_probe, have the probe download the firmware and then return
> -ENODEV. PHY_ID_AS21XXX tells us there is no firmware, so this is what
> we need to do. The -ENODEV then tells the core that this driver entry
> does not match the hardware, try the next.
> 
> After the firmware download, the phylib core will still have the wrong
> ID values. So you cannot use PHY_ID_MATCH_EXACT(PHY_ID_AS21011JB1).
> But what you can do is have a .match_phy_device function. It will get
> called, and it can read the real ID from the device, and perform a
> match. If it does not match return -ENODEV, and the core will try the
> next entry.
> 
> You either need N match_phy_device functions, one per ID value, or you
> can make use of the .driver_data in phy_driver, and place the matching
> data there.
> 
> In the end you should have the correct phy_driver structure for the
> device. The core will still have the wrong ID values, which you should
> document with a comment. But that mostly only effects
> /sys/class/bus/mdio-bus/...
>

Thanks a lot for the hint on how to use .match_phy_device for this,
wasn't aware of the enforced PHY order.

I will investigate this but may I ask who creates the sysfs entry and
at what stage? After phy_register?

Cause if that is the case can't this be solved by making the PHY
function rescan the ID? Checking patch 1 of this series, it won't be
driver_release/attach but just a flag to read the PHY again and update
phy_device.

>From what I observed, updating the phy_id entry in phy_device (or
c45_ids) is harmless. A rescan totally deletes the problem of having to
use the match functions.

-- 
	Ansuel