[PATCH net-next] net: phy: Add c45_phy_ids sysfs entry

[PATCH net-next] net: phy: Add c45_phy_ids sysfs entry

[Yajun Deng]

The phy_id only shows the PHY ID of the c22 device, and the c45 device
didn't store the PHY ID in the phy_id.

Export c45_phy_ids for the c45 device.

Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 .../ABI/testing/sysfs-class-net-phydev         | 10 ++++++++++
 drivers/net/phy/phy_device.c                   | 18 ++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-net-phydev b/Documentation/ABI/testing/sysfs-class-net-phydev
index ac722dd5e694..f6194fd6927c 100644
--- a/Documentation/ABI/testing/sysfs-class-net-phydev
+++ b/Documentation/ABI/testing/sysfs-class-net-phydev
@@ -26,6 +26,16 @@ Description:
 		This ID is used to match the device with the appropriate
 		driver.
 
+What:		/sys/class/mdio_bus/<bus>/<device>/c45_phy_ids
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	netdev@vger.kernel.org
+Description:
+		This attribute contains the 32-bit PHY Identifier as reported
+		by the device during bus enumeration, encoded in hexadecimal.
+		These C45 IDs are used to match the device with the appropriate
+		driver.
+
 What:		/sys/class/mdio_bus/<bus>/<device>/phy_interface
 Date:		February 2014
 KernelVersion:	3.15
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 781dfa6680eb..eecd8273111c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -581,6 +581,23 @@ phy_id_show(struct device *dev, struct device_attribute *attr, char *buf)
 }
 static DEVICE_ATTR_RO(phy_id);
 
+static ssize_t
+c45_phy_ids_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct phy_device *phydev = to_phy_device(dev);
+	const int num_ids = ARRAY_SIZE(phydev->c45_ids.device_ids);
+	unsigned int i;
+	size_t len = 0;
+
+	for (i = 1; i < num_ids; i++)
+		len += sysfs_emit_at(buf, len, "0x%.8lx ",
+				(unsigned long)phydev->c45_ids.device_ids[i]);
+	buf[len - 1] = '\n';
+
+	return len;
+}
+static DEVICE_ATTR_RO(c45_phy_ids);
+
 static ssize_t
 phy_interface_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
@@ -618,6 +635,7 @@ static DEVICE_ATTR_RO(phy_dev_flags);
 
 static struct attribute *phy_dev_attrs[] = {
 	&dev_attr_phy_id.attr,
+	&dev_attr_c45_phy_ids.attr,
 	&dev_attr_phy_interface.attr,
 	&dev_attr_phy_has_fixups.attr,
 	&dev_attr_phy_dev_flags.attr,
-- 
2.25.1

Re: [PATCH net-next] net: phy: Add c45_phy_ids sysfs entry

[Andrew Lunn]

> +What:		/sys/class/mdio_bus/<bus>/<device>/c45_phy_ids
> +Date:		May 2025
> +KernelVersion:	6.16
> +Contact:	netdev@vger.kernel.org
> +Description:
> +		This attribute contains the 32-bit PHY Identifier as reported
> +		by the device during bus enumeration, encoded in hexadecimal.
> +		These C45 IDs are used to match the device with the appropriate
> +		driver.

https://docs.kernel.org/filesystems/sysfs.html#attributes

  Attributes should be ASCII text files, preferably with only one
  value per file. It is noted that it may not be efficient to contain
  only one value per file, so it is socially acceptable to express an
  array of values of the same type.

These are static values, so efficiency is not an issue.

It might be better to have a directory
/sys/class/mdio_bus/<bus>/<device>/c45_phy_ids and then for each MMD
create a file. I would also suggest using is_visible() == 0 for those
with an ID == 0.

	Andrew

Re: [PATCH net-next] net: phy: Add c45_phy_ids sysfs entry

[Yajun Deng]

May 23, 2025 at 9:55 PM, "Andrew Lunn" <andrew@lunn.ch> wrote:



> 
> > 
> > +What: /sys/class/mdio_bus/<bus>/<device>/c45_phy_ids
> > 
> >  +Date: May 2025
> > 
> >  +KernelVersion: 6.16
> > 
> >  +Contact: netdev@vger.kernel.org
> > 
> >  +Description:
> > 
> >  + This attribute contains the 32-bit PHY Identifier as reported
> > 
> >  + by the device during bus enumeration, encoded in hexadecimal.
> > 
> >  + These C45 IDs are used to match the device with the appropriate
> > 
> >  + driver.
> > 
> 
> https://docs.kernel.org/filesystems/sysfs.html#attributes
> 
>  Attributes should be ASCII text files, preferably with only one
> 
>  value per file. It is noted that it may not be efficient to contain
> 
>  only one value per file, so it is socially acceptable to express an
> 
>  array of values of the same type.
> 
> These are static values, so efficiency is not an issue.
> 
> It might be better to have a directory
> 
> /sys/class/mdio_bus/<bus>/<device>/c45_phy_ids and then for each MMD
> 
> create a file. I would also suggest using is_visible() == 0 for those
> 
> with an ID == 0.
> 

is_visible only hide files, not directory. It will look like this:

c45 device:
$ ls /sys/class/net/eth0/phydev/
attached_dev  driver  of_node         phy_id         power       subsystem
c45_phy_ids   hwmon   phy_has_fixups  phy_interface  statistics  uevent

$ ls /sys/class/net/eth0/phydev/c45_phy_ids
mmd10_device_id  mmd17_device_id  mmd23_device_id  mmd2_device_id   mmd7_device_id
mmd11_device_id  mmd18_device_id  mmd24_device_id  mmd30_device_id  mmd8_device_id
mmd12_device_id  mmd19_device_id  mmd25_device_id  mmd31_device_id  mmd9_device_id
mmd13_device_id  mmd1_device_id   mmd26_device_id  mmd3_device_id
mmd14_device_id  mmd20_device_id  mmd27_device_id  mmd4_device_id
mmd15_device_id  mmd21_device_id  mmd28_device_id  mmd5_device_id
mmd16_device_id  mmd22_device_id  mmd29_device_id  mmd6_device_id


c22 device:
$ ls /sys/class/net/eth0/phydev/
attached_dev  driver  of_node         phy_id         power       subsystem
c45_phy_ids   hwmon   phy_has_fixups  phy_interface  statistics  uevent

$ ls /sys/class/net/eth0/phydev/c45_phy_ids


So is that fine? 

>  Andrew
>

Re: [PATCH net-next] net: phy: Add c45_phy_ids sysfs entry

[Russell King (Oracle)]

On Mon, May 26, 2025 at 08:11:21AM +0000, Yajun Deng wrote:
> c45 device:
> $ ls /sys/class/net/eth0/phydev/
> attached_dev  driver  of_node         phy_id         power       subsystem
> c45_phy_ids   hwmon   phy_has_fixups  phy_interface  statistics  uevent
> 
> $ ls /sys/class/net/eth0/phydev/c45_phy_ids
> mmd10_device_id  mmd17_device_id  mmd23_device_id  mmd2_device_id   mmd7_device_id
> mmd11_device_id  mmd18_device_id  mmd24_device_id  mmd30_device_id  mmd8_device_id
> mmd12_device_id  mmd19_device_id  mmd25_device_id  mmd31_device_id  mmd9_device_id
> mmd13_device_id  mmd1_device_id   mmd26_device_id  mmd3_device_id
> mmd14_device_id  mmd20_device_id  mmd27_device_id  mmd4_device_id
> mmd15_device_id  mmd21_device_id  mmd28_device_id  mmd5_device_id
> mmd16_device_id  mmd22_device_id  mmd29_device_id  mmd6_device_id

I suspect you don't have a PHY that defines all these IDs. Are you sure
your .is_visible() is working properly?

-- 
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-next] net: phy: Add c45_phy_ids sysfs entry

[Yajun Deng]

May 26, 2025 at 4:19 PM, "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:



> 
> On Mon, May 26, 2025 at 08:11:21AM +0000, Yajun Deng wrote:
> 
> > 
> > c45 device:
> > 
> >  $ ls /sys/class/net/eth0/phydev/
> > 
> >  attached_dev driver of_node phy_id power subsystem
> > 
> >  c45_phy_ids hwmon phy_has_fixups phy_interface statistics uevent
> > 
> >  
> > 
> >  $ ls /sys/class/net/eth0/phydev/c45_phy_ids
> > 
> >  mmd10_device_id mmd17_device_id mmd23_device_id mmd2_device_id mmd7_device_id
> > 
> >  mmd11_device_id mmd18_device_id mmd24_device_id mmd30_device_id mmd8_device_id
> > 
> >  mmd12_device_id mmd19_device_id mmd25_device_id mmd31_device_id mmd9_device_id
> > 
> >  mmd13_device_id mmd1_device_id mmd26_device_id mmd3_device_id
> > 
> >  mmd14_device_id mmd20_device_id mmd27_device_id mmd4_device_id
> > 
> >  mmd15_device_id mmd21_device_id mmd28_device_id mmd5_device_id
> > 
> >  mmd16_device_id mmd22_device_id mmd29_device_id mmd6_device_id
> > 
> 
> I suspect you don't have a PHY that defines all these IDs. Are you sure
> 
> your .is_visible() is working properly?
> 

I'm just determining if it's a c45 device and not filtering PHY ID content now.
I can add this condition. 


But the 'c45_phy_ids' directory could not be hidden if it's a c22 device.
If c45_phy_ids is a file and it has an array of values. We can hide this
file even if it's a c22 device.


> -- 
> 
> 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-next] net: phy: Add c45_phy_ids sysfs entry

[Andrew Lunn]

> But the 'c45_phy_ids' directory could not be hidden if it's a c22 device.
> If c45_phy_ids is a file and it has an array of values. We can hide this
> file even if it's a c22 device.

I think it is fine to have an empty directory. We don't hide the c22
id file if it is a C45 device.

	Andrew

Re: [PATCH net-next] net: phy: Add c45_phy_ids sysfs entry

[Russell King (Oracle)]

On Mon, May 26, 2025 at 08:52:12AM +0000, Yajun Deng wrote:
> May 26, 2025 at 4:19 PM, "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > On Mon, May 26, 2025 at 08:11:21AM +0000, Yajun Deng wrote:
> > >  $ ls /sys/class/net/eth0/phydev/c45_phy_ids
> > > 
> > >  mmd10_device_id mmd17_device_id mmd23_device_id mmd2_device_id mmd7_device_id
> > > 
> > >  mmd11_device_id mmd18_device_id mmd24_device_id mmd30_device_id mmd8_device_id
> > > 
> > >  mmd12_device_id mmd19_device_id mmd25_device_id mmd31_device_id mmd9_device_id
> > > 
> > >  mmd13_device_id mmd1_device_id mmd26_device_id mmd3_device_id
> > > 
> > >  mmd14_device_id mmd20_device_id mmd27_device_id mmd4_device_id
> > > 
> > >  mmd15_device_id mmd21_device_id mmd28_device_id mmd5_device_id
> > > 
> > >  mmd16_device_id mmd22_device_id mmd29_device_id mmd6_device_id
> > > 
> > 
> > I suspect you don't have a PHY that defines all these IDs. Are you sure
> > 
> > your .is_visible() is working properly?
> > 
> 
> I'm just determining if it's a c45 device and not filtering PHY ID content now.
> I can add this condition. 

I'm talking about listing all 31 entries, whether they're implemented in
the PHY or not. Look at mmds_present in struct phy_c45_device_ids to
determine which IDs should be exported as well as checking whether the
ID value you're exporting is not 0 or ~0.

-- 
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-next] net: phy: Add c45_phy_ids sysfs entry

[Yajun Deng]

May 28, 2025 at 9:08 PM, "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:



> 
> On Mon, May 26, 2025 at 08:52:12AM +0000, Yajun Deng wrote:
> 
> > 
> > May 26, 2025 at 4:19 PM, "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > 
> >  On Mon, May 26, 2025 at 08:11:21AM +0000, Yajun Deng wrote:
> > 
> >  > $ ls /sys/class/net/eth0/phydev/c45_phy_ids
> > 
> >  > 
> > 
> >  > mmd10_device_id mmd17_device_id mmd23_device_id mmd2_device_id mmd7_device_id
> > 
> >  > 
> > 
> >  > mmd11_device_id mmd18_device_id mmd24_device_id mmd30_device_id mmd8_device_id
> > 
> >  > 
> > 
> >  > mmd12_device_id mmd19_device_id mmd25_device_id mmd31_device_id mmd9_device_id
> > 
> >  > 
> > 
> >  > mmd13_device_id mmd1_device_id mmd26_device_id mmd3_device_id
> > 
> >  > 
> > 
> >  > mmd14_device_id mmd20_device_id mmd27_device_id mmd4_device_id
> > 
> >  > 
> > 
> >  > mmd15_device_id mmd21_device_id mmd28_device_id mmd5_device_id
> > 
> >  > 
> > 
> >  > mmd16_device_id mmd22_device_id mmd29_device_id mmd6_device_id
> > 
> >  > 
> > 
> >  
> > 
> >  I suspect you don't have a PHY that defines all these IDs. Are you sure
> > 
> >  
> > 
> >  your .is_visible() is working properly?
> > 
> >  
> > 
> >  
> > 
> >  I'm just determining if it's a c45 device and not filtering PHY ID content now.
> > 
> >  I can add this condition.
> > 
> 
> I'm talking about listing all 31 entries, whether they're implemented in
> 
> the PHY or not. Look at mmds_present in struct phy_c45_device_ids to
> 
> determine which IDs should be exported as well as checking whether the
> 
> ID value you're exporting is not 0 or ~0.
> 

I've already sent v2. Please review it.

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