Re: [PATCH net-next 10/16] net: pcs: xpcs: Add generic DW XPCS MDIO-device support

Serge Semin
2023-12-05T12:12:55Z

Hi Vladimir

On Tue, Dec 05, 2023 at 01:13:51PM +0200, Vladimir Oltean wrote:
> On Tue, Dec 05, 2023 at 01:35:31PM +0300, Serge Semin wrote:
> > @@ -1436,21 +1480,32 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
> > struct dw_xpcs *xpcs;
> > int ret;
> >
> > + ret = device_attach(&mdiodev->dev);
> > + if (ret < 0 && ret != -ENODEV)
> > + return ERR_PTR(ret);
> > +
> > xpcs = xpcs_create_data(mdiodev);
> > if (IS_ERR(xpcs))
> > return xpcs;
> >
> > + ret = xpcs_init_clks(xpcs);
> > + if (ret)
> > + goto out_free_data;
> > +
> > ret = xpcs_init_id(xpcs);
> > if (ret)
> > - goto out;
> > + goto out_clear_clks;
> >
> > ret = xpcs_init_iface(xpcs, interface);
> > if (ret)
> > - goto out;
> > + goto out_clear_clks;
> >
> > return xpcs;
>
> [ 4.083518] Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0
> [ 4.092356] Mem abort info:
> [ 4.095164] ESR = 0x0000000096000004
> [ 4.098932] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 4.104277] SET = 0, FnV = 0
> [ 4.107352] EA = 0, S1PTW = 0
> [ 4.110505] FSC = 0x04: level 0 translation fault
> [ 4.115408] Data abort info:
> [ 4.118296] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [ 4.123807] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [ 4.128877] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [ 4.134214] [00000000000000d0] user address but active_mm is swapper
> [ 4.140595] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> [ 4.146882] Modules linked in:
> [ 4.149944] CPU: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.7.0-rc3-00719-g75be5ea8e111-dirty #1551
> [ 4.164524] Workqueue: events_unbound deferred_probe_work_func
> [ 4.177372] pc : __device_attach+0x3c/0x1bc
> [ 4.181570] lr : __device_attach+0x38/0x1bc
> [ 4.185767] sp : ffff8000800f3800
> [ 4.189087] x29: ffff8000800f3820 x28: 0000000000000001 x27: ffff063781bda150
> [ 4.196252] x26: ffff063781bda150 x25: ffff063780827480 x24: ffffcb9a08138a40
> [ 4.203416] x23: ffff063781114080 x22: 0000000000000000 x21: 0000000000000004
> [ 4.210579] x20: ffff06378123a400 x19: ffff06378123a480 x18: ffffcb9a07c703a0
> [ 4.217743] x17: ffffcb9a07c703a4 x16: 00000000000000d4 x15: ffffcb9a07be70fc
> [ 4.224906] x14: ffffcb9a08299638 x13: 0000000000000053 x12: ffff003000000200
> [ 4.232069] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
> [ 4.239233] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 000000000000003a
> [ 4.246396] x5 : ffff0637809a037b x4 : ffffcb9a087b0d47 x3 : ffff10300000020f
> [ 4.253560] x2 : ffffcb9a0910c561 x1 : 0000000000000000 x0 : ffff06378123a480
> [ 4.260724] Call trace:
> [ 4.263172] __device_attach+0x3c/0x1bc
> [ 4.267020] device_attach+0x14/0x20
> [ 4.270606] xpcs_create+0x24/0x384
> [ 4.274107] xpcs_create_byaddr+0x74/0xa0
> [ 4.278129] sja1105_mdiobus_register+0xf8/0x478
> [ 4.282763] sja1105_setup+0xb4/0x1194
> [ 4.286524] dsa_register_switch+0xab0/0x11f8
> [ 4.290895] sja1105_probe+0x2bc/0x2e4
> [ 4.294654] spi_probe+0xa4/0xc4
> [ 4.297890] really_probe+0x16c/0x3fc
> [ 4.301564] __driver_probe_device+0xa4/0x168
> [ 4.305935] driver_probe_device+0x3c/0x220
> [ 4.310131] __device_attach_driver+0x128/0x1cc
> [ 4.314676] bus_for_each_drv+0xf4/0x14c
> [ 4.318610] __device_attach+0xfc/0x1bc
> [ 4.322457] device_initial_probe+0x14/0x20
> [ 4.326654] bus_probe_device+0x94/0x100
> [ 4.330587] deferred_probe_work_func+0xa0/0xfc
> [ 4.335132] process_scheduled_works+0x210/0x318
> [ 4.339764] worker_thread+0x28c/0x450
> [ 4.343523] kthread+0xfc/0x184
> [ 4.346669] ret_from_fork+0x10/0x20
> [ 4.350256] Code: 2a0103f6 f81f83a8 9431ccd8 f9402688 (39434109)
> [ 4.356366] ---[ end trace 0000000000000000 ]---
>
> I haven't looked at the code at all, but disassembling drivers/base/dd.lst,
> I think the NPD is at dev->p->dead (0xa68 + 0x3c = 0xaa4).
>
> 0000000000000a68 <__device_attach>:
> ; {
> a68: d503233f paciasp
> a6c: d10143ff sub sp, sp, #0x50
> a70: a9027bfd stp x29, x30, [sp, #0x20]
> a74: a90357f6 stp x22, x21, [sp, #0x30]
> a78: a9044ff4 stp x20, x19, [sp, #0x40]
> a7c: 910083fd add x29, sp, #0x20
> a80: d5384108 mrs x8, SP_EL0
> ; mutex_lock(&dev->mutex);
> a84: 91020013 add x19, x0, #0x80
> a88: f9423508 ldr x8, [x8, #0x468]
> a8c: aa0003f4 mov x20, x0
> a90: aa1303e0 mov x0, x19
> a94: 2a0103f6 mov w22, w1
> a98: f81f83a8 stur x8, [x29, #-0x8]
> a9c: 94000000 bl 0xa9c <__device_attach+0x34>
> 0000000000000a9c: R_AARCH64_CALL26 mutex_lock
> ; if (dev->p->dead) {
> aa0: f9402688 ldr x8, [x20, #0x48]
> aa4: 39434109 ldrb w9, [x8, #0xd0]
> aa8: 37000129 tbnz w9, #0x0, 0xacc <__device_attach+0x64>
> ; } else if (dev->driver) {
> aac: f9403689 ldr x9, [x20, #0x68]
> ab0: b40003e9 cbz x9, 0xb2c <__device_attach+0xc4>
> ; return dev->p && klist_node_attached(&dev->p->knode_driver);
> ab4: b40002a8 cbz x8, 0xb08 <__device_attach+0xa0>
> ab8: 91012100 add x0, x8, #0x48
> abc: 94000000 bl 0xabc <__device_attach+0x54>
> 0000000000000abc: R_AARCH64_CALL26 klist_node_attached
> ; if (device_is_bound(dev)) {
> ac0: 34000240 cbz w0, 0xb08 <__device_attach+0xa0>
> ac4: 52800035 mov w21, #0x1
> ac8: 14000002 b 0xad0 <__device_attach+0x68>
> acc: 2a1f03f5 mov w21, wzr
> ; mutex_unlock(&dev->mutex);
> ad0: aa1303e0 mov x0, x19
> ad4: 94000000 bl 0xad4 <__device_attach+0x6c>
> 0000000000000ad4: R_AARCH64_CALL26 mutex_unlock
> ad8: d5384108 mrs x8, SP_EL0
> adc: f9423508 ldr x8, [x8, #0x468]
> ae0: f85f83a9 ldur x9, [x29, #-0x8]
> ae4: eb09011f cmp x8, x9
> ae8: 540008c1 b.ne 0xc00 <__device_attach+0x198>
> ; return ret;
> aec: 2a1503e0 mov w0, w21
> af0: a9444ff4 ldp x20, x19, [sp, #0x40]
> af4: a94357f6 ldp x22, x21, [sp, #0x30]
> af8: a9427bfd ldp x29, x30, [sp, #0x20]
> afc: 910143ff add sp, sp, #0x50
> b00: d50323bf autiasp
> b04: d65f03c0 ret
> ; ret = driver_sysfs_add(dev);
> b08: aa1403e0 mov x0, x20
> b0c: 97ffff21 bl 0x790 <driver_sysfs_add>
> ; if (!ret) {
> b10: 340006c0 cbz w0, 0xbe8 <__device_attach+0x180>
> ; bus_notify(dev, BUS_NOTIFY_DRIVER_NOT_BOUND);
> b14: aa1403e0 mov x0, x20
> b18: 528000e1 mov w1, #0x7
> b1c: 94000000 bl 0xb1c <__device_attach+0xb4>
> 0000000000000b1c: R_AARCH64_CALL26 bus_notify
> b20: 2a1f03f5 mov w21, wzr
> ; dev->driver = NULL;
> b24: f900369f str xzr, [x20, #0x68]
> b28: 17ffffea b 0xad0 <__device_attach+0x68>
> b2c: 120002c8 and w8, w22, #0x1
> ; if (dev->parent)
> b30: f9402280 ldr x0, [x20, #0x40]
> ; struct device_attach_data data = {
> b34: a900fff4 stp x20, xzr, [sp, #0x8]
> b38: 39004bff strb wzr, [sp, #0x12]
> b3c: 390043e8 strb w8, [sp, #0x10]
> ; if (dev->parent)
> b40: b4000060 cbz x0, 0xb4c <__device_attach+0xe4>
> ; return __pm_runtime_resume(dev, RPM_GET_PUT);
> b44: 52800081 mov w1, #0x4
> b48: 94000000 bl 0xb48 <__device_attach+0xe0>
> 0000000000000b48: R_AARCH64_CALL26 __pm_runtime_resume
> ; ret = bus_for_each_drv(dev->bus, NULL, &data,
> b4c: f9403280 ldr x0, [x20, #0x60]
> b50: 90000003 adrp x3, 0x0 <driver_deferred_probe_add>
> 0000000000000b50: R_AARCH64_ADR_PREL_PG_HI21 .text+0x17ac
> b54: 91000063 add x3, x3, #0x0
> 0000000000000b54: R_AARCH64_ADD_ABS_LO12_NC .text+0x17ac
> b58: 910023e2 add x2, sp, #0x8
> b5c: aa1f03e1 mov x1, xzr
> b60: 94000000 bl 0xb60 <__device_attach+0xf8>
> 0000000000000b60: R_AARCH64_CALL26 bus_for_each_drv
> b64: 39404be8 ldrb w8, [sp, #0x12]
> ; if (!ret && allow_async && data.have_async) {
> b68: 7100001f cmp w0, #0x0
> b6c: 1a9f07e9 cset w9, ne
> ; ret = bus_for_each_drv(dev->bus, NULL, &data,
> b70: 2a0003f5 mov w21, w0
> b74: 7100011f cmp w8, #0x0
> ; if (!ret && allow_async && data.have_async) {
> b78: 520002c8 eor w8, w22, #0x1
> b7c: 1a9f17ea cset w10, eq
> b80: 2a0a0108 orr w8, w8, w10
> b84: 2a080136 orr w22, w9, w8
> b88: 360000f6 tbz w22, #0x0, 0xba4 <__device_attach+0x13c>
> ; return __pm_runtime_idle(dev, RPM_ASYNC);
> b8c: aa1403e0 mov x0, x20
> b90: 52800021 mov w1, #0x1
> b94: 94000000 bl 0xb94 <__device_attach+0x12c>
> 0000000000000b94: R_AARCH64_CALL26 __pm_runtime_idle
> ; if (dev->parent)
> b98: f9402280 ldr x0, [x20, #0x40]
> b9c: b50000e0 cbnz x0, 0xbb8 <__device_attach+0x150>
> ba0: 14000008 b 0xbc0 <__device_attach+0x158>
> ; asm_volatile_goto(
> ba4: d503201f nop

Omg, thank you very much for testing the series straight away and
sorry for the immediate trouble it caused. I'll need some more time
for investigation. I'll get back to this topic a bit later on this
week.

-Serge(y)