netdev - Re: [PATCH net-next v2 5/7] net: dsa: realtek: Migrate user_mii

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: <467b56ec-ecc1-4d76-bc00-b78e844b6a61@arinc9.com>
Date: Fri, 5 Jan 2024 21:43:32 +0300
From: Arınç ÜNAL <arinc.unal@...nc9.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Luiz Angelo Daros de Luca <luizluca@...il.com>, linus.walleij@...aro.org,
 alsi@...g-olufsen.dk, andrew@...n.ch, f.fainelli@...il.com,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 5/7] net: dsa: realtek: Migrate user_mii_bus
 setup to realtek-dsa

On 3.01.2024 21:44, Vladimir Oltean wrote:
> On Fri, Dec 22, 2023 at 07:56:48PM +0300, Arınç ÜNAL wrote:
>> We should also align all DSA subdrivers with this understanding. I will
>> modify the MDIO bus patch I submitted for the MT7530 DSA subdriver
>> accordingly.
> 
> I began working on this, and I do have some patches. But returns start
> to diminish very quickly. Some drivers are just not worth it to change.
> So I will also respin the documentation patch set to at least advise to
> not continue the pattern to new drivers.

I've seen your patch series regarding this. I like that you've thought to
skip registering the bus if its node is explicitly disabled. I will
implement that on the MT7530 subdriver as well.

> 
>> I was wondering of moving the MDIO bus registration from DSA subdrivers to
>> the DSA core driver but probably it's not generic enough across switch
>> models with multiple MDIO buses and whatnot to manage this.
> 
> Actually this is the logic after which everything starts to unravel -
> "multiple DSA switches have internal MDIO buses, so let's make DSA
> assist with their registration".
> 
> If you can't do a good job at it, it's more honest to not even try -
> and you gave the perfect example of handling multiple internal MDIO buses.

Makes sense. Setting the interrupts is another good example. Currently, DSA
registers the bus non-OF-based but won't set the interrupts, as far as I
can see.

> 
> I just don't want to maintain stuff that I am really clueless about.
> If registering an MDIO bus is so hard that DSA has to help with it,
> make the MDIO API better.
> 
> Where things would be comfortable for me is if the optional ds->user_mii_bus
> pointer could be always provided by individual subdrivers, and never allocated
> by the framework. So that dsa_switch_ops :: phy_read() and :: phy_write()
> would not exist at all.

I agree. Why don't we do this? These are the subdrivers that we need to
deal with before getting rid of dsa_switch_ops :: phy_read() and ::
phy_write(), and the code block for registering the MDIO bus on the DSA
core driver:

drivers/net/dsa/b53/b53_common.c:
- The DSA subdriver lets the DSA driver register the bus.

drivers/net/dsa/microchip/ksz_common.c:
- The DSA subdriver lets the DSA driver register the bus when "mdio" child
   node is not defined.

drivers/net/dsa/realtek/realtek-mdio.c:
- The DSA subdriver lets the DSA driver register the bus.

This won't be the case after "[PATCH net-next v3 0/8] net: dsa: realtek:
variants to drivers, interfaces to a common module" is applied.

drivers/net/dsa/lan9303-core.c:
- The DSA subdriver lets the DSA driver register the bus.

drivers/net/dsa/vitesse-vsc73xx-core.c:
- The DSA subdriver lets the DSA driver register the bus.

All these subdrivers populate dsa_switch_ops :: phy_read() and ::
phy_write() and won't populate ds->user_mii_bus.

Arınç