Hi Vladimir,

On 8/17/21 4:52 PM, Vladimir Oltean wrote:

It seems that of_find_compatible_node has a weird calling convention in
which it calls of_node_put() on the "from" node argument, instead of
leaving that up to the caller. This comes from the fact that
of_find_compatible_node with a non-NULL "from" argument it only supposed
to be used as the iterator function of for_each_compatible_node(). OF
iterator functions call of_node_get on the next OF node and of_node_put()
on the previous one.

When of_find_compatible_node calls of_node_put, it actually never
expects the refcount to drop to zero, because the call is done under the
atomic devtree_lock context, and when the refcount drops to zero it
triggers a kobject and a sysfs file deletion, which assume blocking
context.

So any driver call to of_find_compatible_node is probably buggy because
an unexpected of_node_put() takes place.

What should be done is to use the of_get_compatible_child() function.

I have an observation that's slightly out of the scope of your patch, 
but I'll post here on the off chance that you find it relevant. 
Apologies if it's out of place.

Do these integrated NXP PHYs use a specific PHY driver, or do they just 
use the Generic PHY driver? If the former is the case, do you experience 
that the PHY driver fails to get probed during mdiobus registration if 
the kernel uses fw_devlink=on?

In my case I am writing a new subdriver for realtek-smi, a DSA driver 
which registers an internal MDIO bus analogously to sja1105, which is 
why I'm asking. I noticed a deferred probe of the PHY driver because the 
supplier (ethernet-switch) is not ready - presumably because all of this 
is happening in the probe of the switch driver. See below:

[   83.653213] device_add:3270: device: 'SMI-0': device_add
[   83.653905] device_pm_add:136: PM: Adding info for No Bus:SMI-0
[   83.654055] device_add:3270: device: 
'platform:ethernet-switch--mdio_bus:SMI-0': device_add
[   83.654224] device_link_add:843: mdio_bus SMI-0: Linked as a sync 
state only consumer to ethernet-switch
[   83.654291] libphy: SMI slave MII: probed
...
[   83.659809] device_add:3270: device: 'SMI-0:00': device_add
[   83.659883] bus_add_device:447: bus: 'mdio_bus': add device SMI-0:00
[   83.659970] device_pm_add:136: PM: Adding info for mdio_bus:SMI-0:00
[   83.660122] device_add:3270: device: 
'platform:ethernet-switch--mdio_bus:SMI-0:00': device_add
[   83.660274] devices_kset_move_last:2701: devices_kset: Moving 
SMI-0:00 to end of list
[   83.660282] device_pm_move_last:203: PM: Moving mdio_bus:SMI-0:00 to 
end of list
[   83.660293] device_link_add:859: mdio_bus SMI-0:00: Linked as a 
consumer to ethernet-switch
[   83.660350] __driver_probe_device:736: bus: 'mdio_bus': 
__driver_probe_device: matched device SMI-0:00 with driver RTL8365MB-VC 
Gigabit Ethernet
[   83.660365] device_links_check_suppliers:1001: mdio_bus SMI-0:00: 
probe deferral - supplier ethernet-switch not ready
[   83.660376] driver_deferred_probe_add:138: mdio_bus SMI-0:00: Added 
to deferred list

It's not necessarily fatal because phy_attach_direct will just use the 
Generic PHY driver as a fallback, but it's obviously not the intended 
behaviour.

Perhaps this affects your driver too? Due to lack of hardware I am not 
in a position to test, but a static code analysis suggests it may be if 
you are expecting anything but Generic PHY.

Kind regards,
Alvin