Re: [PATCH net-next v2 5/7] net: dsa: realtek: Migrate user_mii_bus setup to realtek-dsa

[Vladimir Oltean <olteanv@gmail.com>]

On Fri, Dec 22, 2023 at 05:03:38PM -0300, Luiz Angelo Daros de Luca wrote:
> > Having the MDIO bus described in OF, but no phy-handle to its children
> > is a semantically broken device tree, we should make no effort whatsoever
> > to support it.
> 
> OK. I was trying to keep exactly that setup working.

Which setup exactly?

> Should I keep the check and bail out with an error like:
> 
> +       dsa_switch_for_each_user_port(dp, ds) {
> +               phy_node = of_parse_phandle(dp->dn, "phy-handle", 0);
> +               of_node_put(phy_node);
> +               if (phy_node)
> +                       continue;
> +               dev_err(priv->dev,
> +                       "'%s' is missing phy-handle",
> +                       dp->name);
> +               return -EINVAL;
> +       }
> 
> or should I simply let it break silently? The device-tree writers
> might like some feedback if they are doing it wrong. I guess neither
> DSA nor MDIO bus will say a thing about the missing phy-handle.

FWIW, it will not break silently, but like this (very easy to test, no need to guess):

[    7.196687] mscc_felix 0000:00:00.5 swp3 (uninitialized): failed to connect to PHY: -ENODEV
[    7.205168] mscc_felix 0000:00:00.5 swp3 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 3

If you have no other decision to make in the driver based on the
presence/absence of "phy-handle", it doesn't make much sense to bloat
the driver with dubious logic just to get an arguably prettier error.
I'm saying "dubious" because my understanding is that rtl8365mb "extint"
ports can also serve as user ports, and can additionally be fixed-links.
But arbitrary logic like this breaks that.

The cost/benefit ratio does not seem too favorable for this addition.