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: <20231222220955.77j7nmvhbelv2t7a@skbuf>
Date: Sat, 23 Dec 2023 00:09:55 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Luiz Angelo Daros de Luca <luizluca@...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?

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ