Received-SPF: pass (google.com: domain of linux-kernel+bounces-67928-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 16 Feb 2024 03:00:53 +0200
From: Vladimir Oltean <olteanv@gmail.com>
To: arinc.unal@arinc9.com
Cc: Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	mithat.guner@xeront.com, erkin.bozoglu@xeront.com,
	Luiz Angelo Daros de Luca <luizluca@gmail.com>,
	Alvin =?utf-8?Q?=C5=A0ipraga?= <ALSI@bang-olufsen.dk>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v2] net: dsa: remove OF-based MDIO bus
 registration from DSA core
Message-ID: <20240216010053.asbu75kdi65jjuky@skbuf>
References: <20240213-for-netnext-dsa-mdio-bus-v2-1-0ff6f4823a9e@arinc9.com>
 <20240213-for-netnext-dsa-mdio-bus-v2-1-0ff6f4823a9e@arinc9.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240213-for-netnext-dsa-mdio-bus-v2-1-0ff6f4823a9e@arinc9.com>
 <20240213-for-netnext-dsa-mdio-bus-v2-1-0ff6f4823a9e@arinc9.com>

On Tue, Feb 13, 2024 at 10:29:05AM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.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@gmail.com>
> Acked-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.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@gmail.com>