netdev - Re: [PATCH net-next v2] net: dsa: remove OF-based MDIO bus registration from DSA core

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]

Date: Fri, 16 Feb 2024 03:00:53 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: arinc.unal@...nc9.com
Cc: Andrew Lunn <andrew@...n.ch>, 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>,
	mithat.guner@...ont.com, erkin.bozoglu@...ont.com,
	Luiz Angelo Daros de Luca <luizluca@...il.com>,
	Alvin Šipraga <ALSI@...g-olufsen.dk>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2] net: dsa: remove OF-based MDIO bus
 registration from DSA core

On Tue, Feb 13, 2024 at 10:29:05AM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@...nc9.com>
> 
> The code block under the "!ds->user_mii_bus && ds->ops->phy_read" check
> under dsa_switch_setup() populates ds->user_mii_bus. The use of
> ds->user_mii_bus is inappropriate when the MDIO bus of the switch is
> described on the device tree [1].
> 
> For this reason, use this code block only for switches [with MDIO bus]
> probed on platform_data, and OF which the switch MDIO bus isn't described
> on the device tree. Therefore, remove OF-based MDIO bus registration as
> it's useless for these cases.
> 
> These subdrivers which control switches [with MDIO bus] probed on OF, will
> lose the ability to register the MDIO bus OF-based:
> 
> drivers/net/dsa/b53/b53_common.c
> drivers/net/dsa/lan9303-core.c
> drivers/net/dsa/vitesse-vsc73xx-core.c
> 
> These subdrivers let the DSA core driver register the bus:
> - ds->ops->phy_read() and ds->ops->phy_write() are present.
> - ds->user_mii_bus is not populated.
> 
> The commit fe7324b93222 ("net: dsa: OF-ware slave_mii_bus") which brought
> OF-based MDIO bus registration on the DSA core driver is reasonably recent
> and, in this time frame, there have been no device trees in the Linux
> repository that started describing the MDIO bus, or dt-bindings defining
> the MDIO bus for the switches these subdrivers control. So I don't expect
> any devices to be affected.
> 
> The logic we encourage is that all subdrivers should register the switch
> MDIO bus on their own [2]. And, for subdrivers which control switches [with
> MDIO bus] probed on OF, this logic must be followed to support all cases
> properly:
> 
> No switch MDIO bus defined: Populate ds->user_mii_bus, register the MDIO
> bus, set the interrupts for PHYs if "interrupt-controller" is defined at
> the switch node. This case should only be covered for the switches which
> their dt-bindings documentation didn't document the MDIO bus from the
> start. This is to keep supporting the device trees that do not describe the
> MDIO bus on the device tree but the MDIO bus is being used nonetheless.
> 
> Switch MDIO bus defined: Don't populate ds->user_mii_bus, register the MDIO
> bus, set the interrupts for PHYs if ["interrupt-controller" is defined at
> the switch node and "interrupts" is defined at the PHY nodes under the
> switch MDIO bus node].
> 
> Switch MDIO bus defined but explicitly disabled: If the device tree says
> status = "disabled" for the MDIO bus, we shouldn't need an MDIO bus at all.
> Instead, just exit as early as possible and do not call any MDIO API.
> 
> After all subdrivers that control switches with MDIO buses are made to
> register the MDIO buses on their own, we will be able to get rid of
> dsa_switch_ops :: phy_read() and :: phy_write(), and the code block for
> registering the MDIO bus on the DSA core driver.
> 
> Link: https://lore.kernel.org/netdev/20231213120656.x46fyad6ls7sqyzv@skbuf/ [1]
> Link: https://lore.kernel.org/netdev/20240103184459.dcbh57wdnlox6w7d@skbuf/ [2]
> Suggested-by: Luiz Angelo Daros de Luca <luizluca@...il.com>
> Acked-by: Luiz Angelo Daros de Luca <luizluca@...il.com>
> Signed-off-by: Arınç ÜNAL <arinc.unal@...nc9.com>
> ---
> Changes in v2:
> - Remove mention to drivers/net/dsa/realtek/realtek-mdio.c as it now
>   registers the MDIO bus OF-based on its own, and now under
>   drivers/net/dsa/realtek/rtl83xx.c. I've waited until this happened
>   because if this patch was applied beforehand, there would be no way to
>   set IRQs on PHYs as the subdriver doesn't do that for the MDIO bus
>   registered non-OF-based.
> - Link to v1: https://lore.kernel.org/r/20240122053348.6589-1-arinc.unal@arinc9.com
> ---

Reviewed-by: Vladimir Oltean <olteanv@...il.com>