On 19.10.2023 18:35, Vladimir Oltean wrote:
Yikes, both these examples are actually broken, for a reason that was
extensively discussed with Arınç in the past, and that he tried to
automatically detect through dt-schema but was ultimately told it's too
complicated.
https://patchwork.kernel.org/project/netdevbpf/cover/20230916110902.234273-1-arinc.unal@xxxxxxxxxx/
True, and then I've figured that that approach was wrong from the start.
Read below.
Long story short: the "mdio" node is also the ds->slave_mii_bus (soon to
be ds->user_mii_bus after Florian's inclusivity changes). Having a
slave_mii_bus makes DSA know what to do with port nodes like this, which
don't have an explicit phy-handle:
port@3 {
reg = <3>;
label = "lan4";
};
but actually, call phy_connect() on the ds->slave_mii_bus at address 3
(the port "reg").
The issue is that phy_connect() won't work if ds->slave_mii_bus has an
OF presence, and ethernet-phy@3 isn't defined under it (which it isn't,
you only put ethernet-phy@9). The super detailed reason is that the
OF-based __of_mdiobus_register() does this:
/* Mask out all PHYs from auto probing. Instead the PHYs listed in
* the device tree are populated after the bus has been registered */
mdio->phy_mask = ~0;
So either:
- you delete the "mdio" node and the ethernet-phys under it, or
- you add all ethernet-phys under the mdio node, and put phy-handles
from ports to each of them, and phy-modes of "internal"
What you have now is exactly what won't work, i.e. an OF-based
slave_mii_bus with a non-OF-based phy_connect().
I don't want to see DT examples that are structurally broken, sorry,
because then we wonder why users are confused.
Personally, I would opt for adding the more modern explicit phy-handle
and phy-mode everywhere. Those also work with the U-Boot DM_DSA driver.
As can be seen on the conversation on the patch series you've referred to
above, this is ultimately what I've proposed to enforce on all ethernet
controllers (I hadn't mentioned phy-mode there yet). As long as Andrew
holds his stance, this won't happen. The whole conversation on that patch
series is a great read as to how a devicetree should be defined and how
drivers should interact with it.
But sure, they can at least be put on the examples for this specific
schema.
Arınç
