netdev - Re: [PATCH net] net: dsa: microchip: keep compatibility with device tree blobs with no phy-mode

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: <20220819164157.hubjxqczaxnoltjy@skbuf>
Date:   Fri, 19 Aug 2022 16:41:58 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
CC:     Vladimir Oltean <olteanv@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Woojung Huh <woojung.huh@...rochip.com>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Russell King <linux@...linux.org.uk>,
        Michael Grzeschik <m.grzeschik@...gutronix.de>,
        Oleksij Rempel <linux@...pel-privat.de>,
        Thorsten Leemhuis <regressions@...mhuis.info>,
        Alvin ┼áipraga <alsi@...g-olufsen.dk>,
        Craig McQueen <craig@...ueen.id.au>,
        "tharvey@...eworks.com" <tharvey@...eworks.com>
Subject: Re: [PATCH net] net: dsa: microchip: keep compatibility with device
 tree blobs with no phy-mode

On Fri, Aug 19, 2022 at 01:47:51PM +0200, Rasmus Villemoes wrote:
> > To give you an idea of how things work for user ports. If a user port
> > has a phy-handle, DSA will connect to that, irrespective of what OF-based
> > MDIO bus that is on. If not, DSA looks at whether ds->slave_mii_bus is
> > populated with a struct mii_bus by the driver. If it is, it connects in
> > a non-OF based way to a PHY address equal to the port number. If
> > ds->slave_mii_bus doesn't exist but the driver provides
> > ds->ops->phy_read and ds->ops->phy_write, DSA automatically creates
> > ds->slave_mii_bus where its ops are the driver provided phy_read and
> > phy_write, and it then does the same thing of connecting to the PHY in
> > that non-OF based way.
> 
> Thanks, that's quite useful. From quick grepping, it seems that ksz9567
> currently falls into the latter category?

So it would appear.

> No, but it's actually easier for me to just do that rather than carry an
> extra patch until the mainline fix hits 5.19.y.

Whatever suits you best.

> > there is something to be said about U-Boot compatibility. In U-Boot,
> > with DM_DSA, I don't intend to support any unnecessary complexity and
> > alternative ways of describing the same thing, so there, phy-mode and
> > one of phy-handle or fixed-link are mandatory for all ports. 
> 
> OK. I suppose that means the linux driver for the ksz9477 family  should
> learn to check if there's an "mdio" subnode and if so populate
> ds->slave_mii_bus, but unlike lan937x, absence of that node should not
> be fatal?

Yes.

> > U-Boot can pass its own device tree to Linux, it means Linux DSA drivers
> > might need to gradually gain support for OF-based phy-handle on user
> > ports as well. So see what Tim Harvey has done in drivers/net/ksz9477.c
> > in the U-Boot source code, and try to work with his device tree format,
> > as a starting point.
> 
> Hm. It does seem like that driver has the mdio bus optional (as in,
> probe doesn't fail just because the subnode isn't present). But I'm
> curious why ksz_probe_mdio() looks for a subnode called "mdios" rather
> than "mdio". Tim, just a typo?

No, definitely not a typo. I have to explain this for what seems like
the millionth time already, but the idea with an "mdios" container was
copied from the sja1105 driver, where there are actually multiple MDIO
buses. Documentation/devicetree/bindings/net/mdio.yaml wants the $nodename
to have the "^mdio(@.*)?" pattern, and:
- if you use "mdio" you can have a single node
- if you don't put the MDIO nodes under a container with an
  #address-cells != 0, you can't use the "mdio@...ething" syntax

https://lore.kernel.org/all/20210621234930.espjau5l2t5dr75y@skbuf/T/#me43cf6b1976a3c3aec8357f19ab967f98eea1f73

What I'm actually less clear about is whether ksz9477 actually needs this.
Luckily U-Boot should have less compatibility issues to worry about,
since the DT is appended to the bootloader image, so some corrections
can be made if necessary.