netdev - Re: [PATCH net-next v2 5/7] net: dsa: realtek: Migrate user_mii

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJq09z4z_2mxTLCuojgLt5Sd_EMmB-O_RVP7mE85iaFW26geSA@mail.gmail.com>
Date: Fri, 22 Dec 2023 19:12:46 -0300
From: Luiz Angelo Daros de Luca <luizluca@...il.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: linus.walleij@...aro.org, alsi@...g-olufsen.dk, andrew@...n.ch, 
	f.fainelli@...il.com, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org, 
	arinc.unal@...nc9.com
Subject: Re: [PATCH net-next v2 5/7] net: dsa: realtek: Migrate user_mii_bus
 setup to realtek-dsa

> 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?

Ports without a phy-handle. But, as you said, it is a broken device
tree without a known device using it.

> > 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.

OK, I'll remove them. Thanks.

Regards,

Luiz