From: Alvin Šipraga <a...@bang-olufsen.dk> barebox deep-probe will walk the device tree to ensure dependent devices have been probed. In so doing, it uses the device_node::dev pointer to check whether a given node has a device; if not, a device is created on demand. The behaviour is recursive, so parent nodes without an associated device will also have devices created on their behalf. The recursion stops when a parent with a device is found.
Weird behaviour can ensure if, when devices with a device_node are registered, the device_node::dev field is not populated. This patch addresses a niche, benign, but also noisy error observed as a result of this behaviour. One mostly harmless consequence is that spurious devices can get created when a suitable one already exists. In my case, I have an MDIO-connected Ethernet switch with an internal MDIO bus and some internal PHYs. With deep-probe, but without these changes, the devinfo output looks as follows: `-- 30be0000.ether...@30be0000.of `-- eth0 `-- miibus0 `-- mdio0-dev1d `-- 0x00000000-0x0000003f ( 64 Bytes): /dev/mdio0-phy1d `-- 30be0000.ethernet@30be0000:mdio:ethernet-switch@1d:ports:p...@6.of `-- 30be0000.ethernet@30be0000:mdio:ethernet-switch@1d:ports:p...@0.of `-- eth1 `-- 30be0000.ethernet@30be0000:mdio:ethernet-switch@1d:ports:p...@1.of `-- eth2 `-- 30be0000.ethernet@30be0000:mdio:ethernet-switch@1d:ports:p...@2.of `-- eth3 `-- 30be0000.ethernet@30be0000:mdio:ethernet-switch@1d:ports:p...@3.of `-- eth4 `-- miibus1 `-- mdio1-phy00 `-- 0x00000000-0x0000003f ( 64 Bytes): /dev/mdio1-phy00 `-- mdio1-phy01 `-- 0x00000000-0x0000003f ( 64 Bytes): /dev/mdio1-phy01 `-- mdio1-phy02 `-- 0x00000000-0x0000003f ( 64 Bytes): /dev/mdio1-phy02 `-- mdio1-phy03 `-- 0x00000000-0x0000003f ( 64 Bytes): /dev/mdio1-phy03 `-- 30be0000.ethernet@30be0000:mdio.of `-- 30be0000.ethernet@30be0000:mdio:ethernet-swi...@1d.of `-- 30be0000.ethernet@30be0000:mdio:ethernet-switch@1d:mdio.of Notice the last three devices with generic names; they are spurious creations of the deep-probe algorithm. In fact, the devices have already been created: real dev spurious dev -------- ------------ miibus0 30be0000.ethernet@30be0000:mdio.of mdio0-devid1d 30be0000.ethernet@30be0000:mdio:ethernet-swi...@1d.of miibus1 30be0000.ethernet@30be0000:mdio:ethernet-switch@1d:mdio.of The only reason there aren't even more spurious devices created is that deep-probe stopped at 30be0000.ether...@30be0000.of, a platform device whose device_node had its dev field populated correctly. But these so-called real devices are all created by mdio_bus, which only links one way, i.e. sets device::of_node, but not device_node::dev. This issue probably goes unnoticed on most boards because, while the call to of_device_ensure_probed() returns -ENODEV on the bottom listed node, the code in phy.c doesn't check the return code, and the real devices have already been probed, so no harm is done. I observed much more surprising behaviour on my board because the switch I am using, an RTL8365MB, has multiple possible management interfaces, for which barebox has two drivers: realtek-smi and realtek-mdio (for SMI- and MDIO-connected switches, respectively). The compatible strings are the same, "realtek,rtl8365mb", but the bus types are different: the former is a platform driver, the latter a phy (read: mdio) driver. In my bootloader I have both drivers built in, because some of my boards use SMI while others use MDIO. When I would run the command 'boot bnet', bringing up my network interface, DSA would call phy_device_connect() to connect my edev with its corresponding phy, and a deep-probe would kick off because the phy's parent's device_node::dev field was not populated. And during the creation of the spurious device for my (already probed) Ethernet switch, the platform code would find a compatible string match with realtek-smi and try to probe the device with that driver: Booting entry 'bnet' ERROR: gpiolib: _gpio_request: gpio-233 (30be0000.ethernet@30be0000:mdio:ethernet-swi...@1d.of-reset) status -16 ERROR: realtek-smi 30be0000.ethernet@30be0000:mdio:ethernet-swi...@1d.of: error Device or resource busy: failed to get RESET GPIO ERROR: realtek-smi 30be0000.ethernet@30be0000:mdio:ethernet-swi...@1d.of: probe failed: Device or resource busy As stated above, my switch is connected over MDIO, not SMI, so I really should not be seeing any "realtek-smi" in my log. Nevertheless, because phy.c doesn't check for errors, the errors are benign and my network boot runs just fine. Dumping the stack around the above error print in the realtek-smi driver reveals exactly why things are going wrong: ERROR: gpiolib: _gpio_request: gpio-233 (30be0000.ethernet@30be0000:mdio:ethernet-swi...@1d.of-reset) status -16 Call trace: [<7dd8f400>] (unwind_backtrace+0x0/0x90) [<7dd8f4a0>] (dump_stack+0x10/0x18) [<7dd2d4d4>] (realtek_smi_probe+0x16c/0x2f4) [<7dd1da28>] (platform_probe+0x48/0x4c) [<7dd1cf24>] (device_probe+0x80/0x13c) [<7dd1d028>] (match+0x48/0x58) [<7dd1d3c8>] (register_device+0x178/0x184) [<7dd1da80>] (platform_device_register+0x18/0x20) [<7dd5501c>] (of_platform_device_create+0x2b8/0x2e0) [<7dd55170>] (of_device_create_on_demand+0x84/0xc0) [<7dd5513c>] (of_device_create_on_demand+0x50/0xc0) [<7dd552e0>] (of_device_ensure_probed+0x40/0x6c) [<7dd21398>] (phy_device_connect+0x174/0x230) [<7dd1fd74>] (dsa_port_start+0x60/0x1f0) [<7dd7a0e4>] (eth_open+0x24/0x44) [<7dd7d7e4>] (ifup_edev+0x9c/0x2ec) [<7dd7dbe4>] (ifup_all+0x110/0x204) [<7dd7dd2c>] (do_ifup+0x54/0xd4) [<7dd09e78>] (execute_command+0x44/0x8c) ... The whole mess is rectified if the proper linking is done by mdio_bus, since it is responsible for creating all of the devices to begin with. So, make sure that the device_node::dev field is populated in all cases there. Note that while I also make this change in of_mdiobus_register_phy(), I did not actually observe any unwanted behaviour by not doing so - the problem above is specifically because it's not done in of_mdiobus_register() or of_mdiobus_register_device(). But this is only because of the way phys are looked up to begin with, so it seemed reasonable to do it for them as well. Signed-off-by: Alvin Šipraga <a...@bang-olufsen.dk> --- drivers/net/phy/mdio_bus.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 332db6c05b11..94123ef6141c 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -119,6 +119,7 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi * Associate the OF node with the device structure so it * can be looked up later */ + child->dev = &phy->dev; phy->dev.of_node = child; /* @@ -145,6 +146,7 @@ static int of_mdiobus_register_device(struct mii_bus *mdio, if (IS_ERR(mdiodev)) return PTR_ERR(mdiodev); + child->dev = &mdiodev->dev; mdiodev->dev.of_node = child; ret = mdio_register_device(mdiodev); @@ -315,6 +317,8 @@ int mdiobus_register(struct mii_bus *bus) pr_info("%s: probed\n", dev_name(&bus->dev)); if (bus->dev.of_node) { + bus->dev.of_node->dev = &bus->dev; + /* Register PHY's as child node to mdio node */ of_mdiobus_register(bus, bus->dev.of_node); } -- 2.43.0