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] [day] [month] [year] [list]
Message-ID: <20240110163551.ceemrjwfvavmodqi@skbuf>
Date: Wed, 10 Jan 2024 18:35:51 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: Luiz Angelo Daros de Luca <luizluca@...il.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Andrew Lunn <andrew@...n.ch>,
	Florian Fainelli <f.fainelli@...il.com>,
	Alvin Šipraga <alsi@...g-olufsen.dk>,
	Linus Walleij <linus.walleij@...aro.org>,
	Florian Fainelli <florian.fainelli@...adcom.com>,
	Hauke Mehrtens <hauke@...ke-m.de>,
	Christian Marangi <ansuelsmth@...il.com>,
	Arınç ÜNAL <arinc.unal@...nc9.com>
Subject: Re: [PATCH net-next 05/10] net: dsa: qca8k: skip MDIO bus creation
 if its OF node has status = "disabled"

On Thu, Jan 04, 2024 at 11:19:20PM -0300, Luiz Angelo Daros de Luca wrote:
> Don't you still need to put the node that is not available? Just put
> it unconditionally whenever you exit this function after you get it.
> of_node_put() can handle even NULL.

You're right. I've prepared a patch to handle this case correctly.
I don't think it's worth sending to 'net' now that 'net-next' has
closed, because as you say below, it's quite possible that the
!of_device_is_available() code path is never exercised by existing
device trees. So the bug is like the tree that falls in the forest but
nobody hears it. I will submit the correction when net-next reopens,
together with Alvin's suggested "err" -> "ret" renaming.

> I'm not sure if this and other simple switches can be useful without a
> valid MDIO.

Will that always be the case? As implausible as this may sound, I've
received DSA questions from people using the sja1105 as a two-port
adapter between MII on the CPU side and RMII on the PHY side. It was the
cheapest way of adapting their SoC to RMII, using a switch as not even a
port multiplier. I see that AR8237 has 1x RGMII and 1x SerDes, so maybe
somebody would want to use it that way, and sidestep the internal PHYs?
I don't know.

> Anyway, wouldn't it be equivalent to having an empty mdio
> node? It looks like it would work as well but without a specific code
> path.

I guess you could also express this that way too. Although, in case it
matters, an 'empty node' has to pass schema validation (has to have all
required properties), and a disabled one doesn't.

The idea with this patch was to deliberately change the status = "disabled"
handling that the driver already had, to make things more consistent
across the board. Each driver binding has its own unique interpretation
of an absent MDIO OF node already. Some consider the OF node optional
and thus register it anyway, some say that absent means it's not needed.
But I think status = "disabled" should be an unambiguous way to specify
through DT that the hardware component is disabled. This is not how
qca8k was interpreting it prior to this change.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ