[PATCH net-next] net: phy: Synchronize c45_ids to phy_id

[PATCH net-next] net: phy: Synchronize c45_ids to phy_id

[Yajun Deng]

The phy_id_show() function emit the phy_id for the phy device. If the phy
device is a c45 device, the phy_id is empty. In other words, phy_id_show()
only works with the c22 device.

Synchronize c45_ids to phy_id, phy_id_show() will work with both the c22
and c45 devices.

Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 drivers/net/phy/phy_device.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2eb735e68dd8..6fed3e84e1a6 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -690,8 +690,12 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
 	dev->pma_extable = -ENODATA;
 	dev->is_c45 = is_c45;
 	dev->phy_id = phy_id;
-	if (c45_ids)
+	if (c45_ids) {
 		dev->c45_ids = *c45_ids;
+		dev->phy_id = dev->c45_ids.device_ids[MDIO_MMD_PMAPMD];
+		if (!dev->phy_id)
+			dev->phy_id = dev->c45_ids.device_ids[MDIO_MMD_PCS];
+	}
 	dev->irq = bus->irq[addr];
 
 	dev_set_name(&mdiodev->dev, PHY_ID_FMT, bus->id, addr);
-- 
2.25.1

Re: [PATCH net-next] net: phy: Synchronize c45_ids to phy_id

[Russell King (Oracle)]

On Thu, May 22, 2025 at 09:19:18PM +0800, Yajun Deng wrote:
> The phy_id_show() function emit the phy_id for the phy device. If the phy
> device is a c45 device, the phy_id is empty. In other words, phy_id_show()
> only works with the c22 device.

This is correct.

> Synchronize c45_ids to phy_id, phy_id_show() will work with both the c22
> and c45 devices.

This is incorrect, as there may (and are in some cases) be multiple
different IDs in a C45 PHY.

I think we've had patches like this before and turned them down.

-- 
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: Synchronize c45_ids to phy_id

[Andrew Lunn]

On Thu, May 22, 2025 at 09:19:18PM +0800, Yajun Deng wrote:
> The phy_id_show() function emit the phy_id for the phy device. If the phy
> device is a c45 device, the phy_id is empty. In other words, phy_id_show()
> only works with the c22 device.
> 
> Synchronize c45_ids to phy_id, phy_id_show() will work with both the c22
> and c45 devices.

First off, they are different things. A device can have both a C22 and
a collection of C45 IDs. So they should not be mixed up in one sysfs
attribute.

The second point has already been made by Russell, there are lots of
different C45 IDs, and phylib will try to find a driver based on any
of them.

If you want to export the C45 IDs, please think about adding new sysfs
attributes.

	Andrew

Re: [PATCH net-next] net: phy: Synchronize c45_ids to phy_id

[Yajun Deng]

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



> 
> On Thu, May 22, 2025 at 09:19:18PM +0800, Yajun Deng wrote:
> 
> > 
> > The phy_id_show() function emit the phy_id for the phy device. If the phy
> > 
> >  device is a c45 device, the phy_id is empty. In other words, phy_id_show()
> > 
> >  only works with the c22 device.
> > 
> >  
> > 
> >  Synchronize c45_ids to phy_id, phy_id_show() will work with both the c22
> > 
> >  and c45 devices.
> > 
> 
> First off, they are different things. A device can have both a C22 and
> 
> a collection of C45 IDs. So they should not be mixed up in one sysfs
> 
> attribute.

In get_phy_device(), only one of get_phy_c45_ids() or get_phy_c22_id()
is called. Even if a device has both a c22 and c45 IDs, only one of the
phy_id and the c45_ids has values.

> 
> The second point has already been made by Russell, there are lots of
> 
> different C45 IDs, and phylib will try to find a driver based on any
> 
> of them.

I noticed that. I tested the BCM89890, 88X3310 and 88Q2110 PHY devices,
and the ID is always the same in different MMDs.

> 
> If you want to export the C45 IDs, please think about adding new sysfs
> 
> attributes.
> 

Okay.

>  Andrew
>

Re: [PATCH net-next] net: phy: Synchronize c45_ids to phy_id

[Andrew Lunn]

> > The second point has already been made by Russell, there are lots of
> > 
> > different C45 IDs, and phylib will try to find a driver based on any
> > 
> > of them.
> 
> I noticed that. I tested the BCM89890, 88X3310 and 88Q2110 PHY devices,
> and the ID is always the same in different MMDs.

Try a Marvel 10G PHY. They have different IDs. It has been speculated
Marvell licensed part of the PHY from a different vendor, and you see
the vendors OUI in some locations.

	Andrew

Re: [PATCH net-next] net: phy: Synchronize c45_ids to phy_id

[Russell King (Oracle)]

On Fri, May 23, 2025 at 03:03:43PM +0200, Andrew Lunn wrote:
> 
> > > The second point has already been made by Russell, there are lots of
> > > 
> > > different C45 IDs, and phylib will try to find a driver based on any
> > > 
> > > of them.
> > 
> > I noticed that. I tested the BCM89890, 88X3310 and 88Q2110 PHY devices,
> > and the ID is always the same in different MMDs.
> 
> Try a Marvel 10G PHY. They have different IDs. It has been speculated
> Marvell licensed part of the PHY from a different vendor, and you see
> the vendors OUI in some locations.

The 88X3310 was listed, but incorrectly as always having the same ID in
each MMD. This is false as stated previously.

Maybe Marvell have updated the 88x3310 to use the same ID throughout
in a later revision, but it certainly is not true with the versions
I have seen.

-- 
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: Synchronize c45_ids to phy_id

[Russell King (Oracle)]

On Fri, May 23, 2025 at 02:10:00AM +0000, Yajun Deng wrote:
> I noticed that. I tested the BCM89890, 88X3310 and 88Q2110 PHY devices,
> and the ID is always the same in different MMDs.

The 88x3310 PHY uses:

	Device ID	Package ID	OUI
MMD 1: 0x002b09aa	0x002b09aa	00:0a:c2
MMD 3: 0x002b09aa	0x002b09aa	00:0a:c2
MMD 4: 0x01410daa	0x01410daa	00:50:43
MMD 7: 0x002b09aa	0x002b09aa	00:0a:c2

other MMDs do not contain an ID.

According to https://hwaddress.com/oui-iab/00-0A-C2/, 00:0a:c2 is/was
Wuhan FiberHome Digital Technology Co.

00:50:43 is Marvell Semiconductor.

Not all PHYs have a single ID. IEEE 802.3 allows for this.

-- 
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: Synchronize c45_ids to phy_id

[Yajun Deng]

May 24, 2025 at 1:37 AM, "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:



> 
> On Fri, May 23, 2025 at 02:10:00AM +0000, Yajun Deng wrote:
> 
> > 
> > I noticed that. I tested the BCM89890, 88X3310 and 88Q2110 PHY devices,
> > 
> >  and the ID is always the same in different MMDs.
> > 
> 
> The 88x3310 PHY uses:
> 
>  Device ID Package ID OUI
> 
> MMD 1: 0x002b09aa 0x002b09aa 00:0a:c2
> 
> MMD 3: 0x002b09aa 0x002b09aa 00:0a:c2
> 
> MMD 4: 0x01410daa 0x01410daa 00:50:43
> 
> MMD 7: 0x002b09aa 0x002b09aa 00:0a:c2
> 
> other MMDs do not contain an ID.
> 
> According to https://hwaddress.com/oui-iab/00-0A-C2/, 00:0a:c2 is/was
> 
> Wuhan FiberHome Digital Technology Co.
> 
> 00:50:43 is Marvell Semiconductor.
> 
> Not all PHYs have a single ID. IEEE 802.3 allows for this.
> 

Okay, thank you. I'll add the c45_ids entry for the c45 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: Synchronize c45_ids to phy_id

[kernel test robot]

Hello,

kernel test robot noticed "WARNING:at_drivers/net/phy/phy.c:#_phy_state_machine" on:

commit: 5ba1925a399bc2393c503a70406edea488b549ff ("[PATCH net-next] net: phy: Synchronize c45_ids to phy_id")
url: https://github.com/intel-lab-lkp/linux/commits/Yajun-Deng/net-phy-Synchronize-c45_ids-to-phy_id/20250522-212043
base: https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git 3da895b23901964fcf23450f10b529d45069f333
patch link: https://lore.kernel.org/all/20250522131918.31454-1-yajun.deng@linux.dev/
patch subject: [PATCH net-next] net: phy: Synchronize c45_ids to phy_id

in testcase: rcutorture
version: 
with following parameters:

	runtime: 300s
	test: cpuhotplug
	torture_type: tasks



config: x86_64-randconfig-121-20250524
compiler: clang-20
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)


+------------------------------------------------------+------------+------------+
|                                                      | 3da895b239 | 5ba1925a39 |
+------------------------------------------------------+------------+------------+
| WARNING:at_drivers/net/phy/phy.c:#_phy_state_machine | 0          | 18         |
| RIP:_phy_state_machine                               | 0          | 18         |
+------------------------------------------------------+------------+------------+


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202505281424.ee78ec7d-lkp@intel.com


[   12.608484][   T25] ------------[ cut here ]------------
[ 12.609643][ T25] _phy_start_aneg+0x0/0xf0: returned: -1 
[ 12.610888][ T25] WARNING: CPU: 1 PID: 25 at drivers/net/phy/phy.c:1350 _phy_state_machine (drivers/net/phy/phy.c:1350) 
[   12.612870][   T25] Modules linked in:
[   12.613775][   T25] CPU: 1 UID: 0 PID: 25 Comm: kworker/1:0 Tainted: G                T   6.15.0-rc6-01307-g5ba1925a399b #1 PREEMPT(voluntary)
[   12.614298][    T7] e1000: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX
[   12.616470][   T25] Tainted: [T]=RANDSTRUCT
[   12.619076][   T25] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[   12.624259][    T1] dsa-loop fixed-0:1f lan2: configuring for phy/gmii link mode
[   12.626882][   T25] Workqueue: events_power_efficient phy_state_machine
[   12.628568][    T7] ------------[ cut here ]------------
[ 12.629750][ T25] RIP: 0010:_phy_state_machine (drivers/net/phy/phy.c:1350) 
[ 12.630847][ T7] _phy_start_aneg+0x0/0xf0: returned: -1 
[ 12.632047][ T25] Code: 49 c7 c6 c0 e3 1d 82 bd 01 00 00 00 83 f8 ed 0f 84 21 01 00 00 85 c0 79 6b 48 c7 c7 96 7b ec 83 4c 89 f6 89 c2 e8 21 7e 0f ff <0f> 0b 48 8d bb f8 05 00 00 e8 63 43 17 ff 84 c0 0f 84 0c 01 00 00
All code
========
   0:	49 c7 c6 c0 e3 1d 82 	mov    $0xffffffff821de3c0,%r14
   7:	bd 01 00 00 00       	mov    $0x1,%ebp
   c:	83 f8 ed             	cmp    $0xffffffed,%eax
   f:	0f 84 21 01 00 00    	je     0x136
  15:	85 c0                	test   %eax,%eax
  17:	79 6b                	jns    0x84
  19:	48 c7 c7 96 7b ec 83 	mov    $0xffffffff83ec7b96,%rdi
  20:	4c 89 f6             	mov    %r14,%rsi
  23:	89 c2                	mov    %eax,%edx
  25:	e8 21 7e 0f ff       	call   0xffffffffff0f7e4b
  2a:*	0f 0b                	ud2		<-- trapping instruction
  2c:	48 8d bb f8 05 00 00 	lea    0x5f8(%rbx),%rdi
  33:	e8 63 43 17 ff       	call   0xffffffffff17439b
  38:	84 c0                	test   %al,%al
  3a:	0f 84 0c 01 00 00    	je     0x14c

Code starting with the faulting instruction
===========================================
   0:	0f 0b                	ud2
   2:	48 8d bb f8 05 00 00 	lea    0x5f8(%rbx),%rdi
   9:	e8 63 43 17 ff       	call   0xffffffffff174371
   e:	84 c0                	test   %al,%al
  10:	0f 84 0c 01 00 00    	je     0x122
[ 12.633319][ T7] WARNING: CPU: 0 PID: 7 at drivers/net/phy/phy.c:1350 _phy_state_machine (drivers/net/phy/phy.c:1350) 
[   12.637175][   T25] RSP: 0018:ffff888102e5fdc8 EFLAGS: 00010246
[   12.638506][    T7] Modules linked in:
[   12.639158][   T25]
[   12.639576][    T7] CPU: 0 UID: 0 PID: 7 Comm: kworker/0:1 Tainted: G                T   6.15.0-rc6-01307-g5ba1925a399b #1 PREEMPT(voluntary)
[   12.639823][   T25] RAX: 0000000000000000 RBX: ffff88813a422000 RCX: 0000000000000000
[   12.641190][    T7] Tainted: [T]=RANDSTRUCT
[   12.642046][   T25] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[   12.642049][   T25] RBP: 0000000000000001 R08: 0000000000000000 R09: 0000000000000000
[   12.642052][   T25] R10: 0000000000000000 R11: 0000000000000000 R12: ffff888100079800
[   12.642054][   T25] R13: ffff88842fdea840 R14: ffffffff821de3c0 R15: 0000000000000004
[   12.642057][   T25] FS:  0000000000000000(0000) GS:ffff8884aa5fc000(0000) knlGS:0000000000000000
[   12.642060][   T25] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   12.642062][   T25] CR2: 00007fc38285a160 CR3: 0000000004296000 CR4: 00000000000006b0
[   12.642067][   T25] Call Trace:
[   12.642072][   T25]  <TASK>
[ 12.642079][ T25] ? process_scheduled_works (kernel/workqueue.c:3214) 
[ 12.642086][ T25] phy_state_machine (drivers/net/phy/phy.c:1612) 
[ 12.642093][ T25] ? process_scheduled_works (kernel/workqueue.c:3214) 
[ 12.642097][ T25] process_scheduled_works (kernel/workqueue.c:3243) 
[ 12.642117][ T25] worker_thread (include/linux/list.h:373 kernel/workqueue.c:946 kernel/workqueue.c:3401) 
[ 12.642127][ T25] ? pr_cont_work (kernel/workqueue.c:3346) 
[ 12.642130][ T25] kthread (kernel/kthread.c:466) 
[ 12.642138][ T25] ? kthread_unuse_mm (kernel/kthread.c:413) 
[ 12.642144][ T25] ret_from_fork (arch/x86/kernel/process.c:159) 
[ 12.642149][ T25] ? kthread_unuse_mm (kernel/kthread.c:413) 
[ 12.642155][ T25] ret_from_fork_asm (arch/x86/entry/entry_64.S:258) 
[   12.642178][   T25]  </TASK>
[   12.642180][   T25] irq event stamp: 14831
[ 12.642182][ T25] hardirqs last enabled at (14837): vprintk_emit (arch/x86/include/asm/irqflags.h:26 arch/x86/include/asm/irqflags.h:109 arch/x86/include/asm/irqflags.h:151 kernel/printk/printk.c:2043 kernel/printk/printk.c:2449) 
[ 12.642187][ T25] hardirqs last disabled at (14842): vprintk_emit (kernel/printk/printk.c:2022) 
[ 12.642192][ T25] softirqs last enabled at (14648): __irq_exit_rcu (arch/x86/include/asm/jump_label.h:36 kernel/softirq.c:682) 
[ 12.642197][ T25] softirqs last disabled at (14643): __irq_exit_rcu (arch/x86/include/asm/jump_label.h:36 kernel/softirq.c:682) 
[   12.642201][   T25] ---[ end trace 0000000000000000 ]---
[   12.645728][    T1] dsa-loop fixed-0:1f lan3: configuring for phy/gmii link mode
[   12.648032][    T1] dsa-loop fixed-0:1f lan4: configuring for phy/gmii link mode
[   12.648206][    T7] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250528/202505281424.ee78ec7d-lkp@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki