netdev - Re: [PATCH net 3/4] docs: net: dsa: update user MDIO bus documentation

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: <20231211143513.n6ms3dlp6rrcqya6@skbuf>
Date: Mon, 11 Dec 2023 16:35:13 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: Alvin Šipraga <ALSI@...g-olufsen.dk>
Cc: "netdev@...r.kernel.org" <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>,
	Luiz Angelo Daros de Luca <luizluca@...il.com>,
	Madhuri Sripada <madhuri.sripada@...rochip.com>,
	Marcin Wojtas <mw@...ihalf.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Tobias Waldekranz <tobias@...dekranz.com>,
	Arun Ramadoss <arun.ramadoss@...rochip.com>,
	"Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
	Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH net 3/4] docs: net: dsa: update user MDIO bus
 documentation

On Sun, Dec 10, 2023 at 01:48:12PM +0000, Alvin Šipraga wrote:
> On Fri, Dec 08, 2023 at 09:35:17PM +0200, Vladimir Oltean wrote:
> > +When using OF, the ``ds->user_mii_bus`` can be seen as a legacy feature, rather
> > +than core functionality. Since 2014, the DSA OF bindings support the
> > +``phy-handle`` property, which is a universal mechanism to reference a PHY,
> > +be it internal or external.
> > +
> > +New switch drivers are encouraged to require the more universal ``phy-handle``
> > +property even for user ports with internal PHYs. This allows device trees to
> > +interoperate with simpler variants of the drivers such as those from U-Boot,
> > +which do not have the (redundant) fallback logic for ``ds->user_mii_bus``.
> 
> Considering this policy, should we not emphasize that ds->user_mii_bus
> and ds->ops->phy_{read,write}() ought to be left unpopulated by new
> drivers, with the remark that if a driver wants to set up an MDIO bus,
> it should store the corresponding struct mii_bus pointer in its own
> driver private data? Just to make things crystal clear.
> 
> Regardless I think this is good!
> 
> Reviewed-by: Alvin Šipraga <alsi@...g-olufsen.dk>

I think something that makes a limited amount of sense is for DSA to
probe on OF, but not describe the MDIO controller in OF. Then, you'd
need ds->user_mii_bus. But new drivers should probably not do that
either; they should look into the MFD model and make the MDIO controller
be separate from (not a child of) the DSA switch. Then use a phy-handle
to it. So for new drivers, even this doesn't make too much sense, and
neither is it best to allocate the mii_bus from driver private code.

What makes no sense whatsoever is commit fe7324b93222 ("net: dsa:
OF-ware slave_mii_bus"). Because DSA provides ds->user_mii_bus to do
something reasonable when the MDIO controller isn't described in OF,
but this change assumes that it _is_ described in OF!

I'm not sure how and where to best put in words "let's not make DSA a
library for everything, just keep it for the switch". I'll think about
it some more.