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 for Android: free password hash cracker in your pocket

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

Date: Wed, 13 Dec 2023 02:30:48 -0300
From: Luiz Angelo Daros de Luca <luizluca@...il.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: Alvin Šipraga <ALSI@...g-olufsen.dk>, 
	"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>, 
	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

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

Hello Vladimir,

Sorry for my lack of understanding but I still didn't get it.

You are telling us we should not use user_mii_bus when the user MDIO
is described in the OF. Is it only about the "generic DSA mii" or also
about the custom ones the current drivers have? If it is the latter, I
don't know how to connect the dots between phy_read/write functions
and the port.

We have some drivers that define ds->user_mii_bus (not the "generic
DSA mii") with the MDIO described in OF. Are they wrong?

Alvin asked if we should store the mii_bus internally and not in the
ds->user_mii_bus but I don't think you answered it. Is it about where
we store the bus (for dropping user_mii_bus in the future)?

You now also mention using the MFD model (shouldn't it be cited in the
docs too?) but I couldn't identify a DSA driver example that uses that
model, with mdio outside the switch. Do we have one already? Would the
OF compatible with the MDF model be something like this?

my_mfd {
    compatible "aaa";
    switch {
        compatible = "bbb";
        ports {
            port@0: {
              phy-handle = <&ethphy0>;
            }
        }
    }
    mdio {
         compatible = "ccc";
         ethphy0: ethernet-phy@0 {
         }
    }
}

And for MDIO-connected switches, something like this?

eth0 {
     mdio {
         my_mfd {
            switch{...}
            mdio{...}
         }
     }
}

Is it only a suggestion on how to write a new driver or should we
change the existing ones to fit both models?

Regards,

Luiz