netdev - Re: [PATCH net-next v4 05/11] net: dsa: realtek: use phy

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: <Yea9oR0AteAMwjW2@lunn.ch>
Date:   Tue, 18 Jan 2022 14:16:17 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Luiz Angelo Daros de Luca <luizluca@...il.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        "open list:NETWORKING DRIVERS" <netdev@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Alvin Šipraga <alsi@...g-olufsen.dk>,
        Arınç ÜNAL <arinc.unal@...nc9.com>,
        Frank Wunderlich <frank-w@...lic-files.de>
Subject: Re: [PATCH net-next v4 05/11] net: dsa: realtek: use phy_read in
 ds->ops

> Thanks, Florian. You should be correct. It might call
> mdiobus_unregister() and mdiobus_free() twice, once inside the dsa
> code and another one by the devm (if I understood how devm functions
> work).
> 
> The issue is that the dsa switch is assuming that if slave_mii is
> allocated and ds->ops->phy_read is defined, it has allocated the
> slave_mii by itself and it should clean up the slave_mii during
> teardown.

Correct. Either the DSA core takes care of the mdiobus and uses the
phy_read and phy_write ops, or the driver internally registers its own
mdiobus, and phy_read and phy_write ops are not implemented. The core
is not designed to mix those together.

> if ds->ops->phy_read value should not tell if ds->slave_mii_bus should
> be cleaned by the DSA switch.
> 
> I would selfishly hope the correct one was the second option because
> it would make my code much cleaner. If not, that's a complex issue to
> solve without lots of duplications: realtek-smi drivers should not
> have ds->ops->phy_read defined while realtek-mdio requires it. I'll
> need to duplicate dsa_switch_ops for each subdriver only to unset
> phy_read and also duplicate realtek_variant for each interface only to
> reference that different dsa_switch_ops.

One option would be to provide a dummy mdiobus driver for
realtek-mdio, which simply passes the access through to the existing
MDIO bus.

     Andrew