Received-SPF: pass (google.com: domain of linux-kernel+bounces-5557-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 19 Dec 2023 18:28:03 +0200
From: Vladimir Oltean <olteanv@gmail.com>
To: Serge Semin <fancer.lancer@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>, Russell King <linux@armlinux.org.uk>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Jose Abreu <joabreu@synopsys.com>,
	Jose Abreu <Jose.Abreu@synopsys.com>,
	Maxime Chevallier <maxime.chevallier@bootlin.com>,
	Tomer Maimon <tmaimon77@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	openbmc@lists.ozlabs.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 06/16] net: pcs: xpcs: Avoid creating dummy XPCS
 MDIO device
Message-ID: <20231219162803.kmb3jb6aevef3kj3@skbuf>
References: <20231205103559.9605-1-fancer.lancer@gmail.com>
 <20231205103559.9605-1-fancer.lancer@gmail.com>
 <20231205103559.9605-7-fancer.lancer@gmail.com>
 <20231205103559.9605-7-fancer.lancer@gmail.com>
 <20231205115234.7ntjvymurot5nnak@skbuf>
 <wnptneaxxe2tq2rf7ac6a72xtyluyggughvmtxbbg5qto64mpa@7gchl5e4qllu>
 <jlcyqvcw5hsjtmvf77sm7qni6tzihxudo6baoq7wi7zgqx3lvs@ykbsxgmjkvnf>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <jlcyqvcw5hsjtmvf77sm7qni6tzihxudo6baoq7wi7zgqx3lvs@ykbsxgmjkvnf>

On Tue, Dec 19, 2023 at 06:48:09PM +0300, Serge Semin wrote:
> > > Sorry, because the commit log lost me at the "context presentation" stage,
> > > I failed to understand the "what"s and the "why"s.
> > > 
> > > Are you basically trying to add xpcs support on top of an mdio_device
> > > where the mdio_device_create() call was made externally to the xpcs code,
> > > through mdiobus_register_board_info() and mdiobus_setup_mdiodev_from_board_info()?
> > 
> > Basically yes, but there is more of it. The main idea is to convert
> > the XPCS driver to using the already created non-PHY MDIO-devices
> > instead of manually creating a 'dummy'/'redundant' one. From my point
> > of view there are several reasons of doing so:
> > 
> > 1. mdiobus_register_board_info() provides a way to assign the device
> > platform data to being registered afterwards device. Thus we can pass
> > some custom data to the XPCS-device driver (whether it's just an
> > xpcs_create_*() call or a fully functional MDIO-device driver
> > registered by the mdio_driver_register() method). For instance it can
> > be utilized to drop the fake PHYSIDs implementation from
> > drivers/net/dsa/sja1105/sja1105_mdio.c .

Ok. Seeing an alternative to the NXP_SJA1110_XPCS_ID hack will be interesting.

FWIW, I'm looking at reworking the dsa_loop probing to use software nodes.
Since dsa_loop is the only current user of mdiobus_register_board_info(),
maybe that will lead to its deletion. It appears a matter of timing, but
the mechanism looks promising.

Maybe we can also use it somehow to add compatibility with existing
lynx-pcs device trees where there is no compatible string, so a struct
phy_device gets created. Device tree breakage was the fundamental reason
why Sean Anderson's patch set couldn't make forward progress.
https://patchwork.kernel.org/project/netdevbpf/cover/20221103210650.2325784-1-sean.anderson@seco.com/

> > 2. The MDIO-devices actually registered on the MDIO-bus will be
> > visible in sysfs with for instance useful IO statistics provided by
> > the MDIO-bus. Potentially (if it is required) at some point we'll be
> > able to convert the DW XPCS driver to being true MDIO-device driver
> > (bindable to the DW XPCS device) with less efforts.

Ok.

> > 3. Having an MDIO-device registered that way would make the DW XPCS
> > IO-device implementation unified after the fwnode-based XPCS
> > descriptor creation support is added in one of the subsequent patches.

Unified how? You mean that "XPCS will always operate as a driver bound
to an mdio_device"?

You're not planning to unify the mdio_device and MMIO register handling
by using regmap, right?

> > So based on the listed above I've got a question. Do you think all of
> > that is worth to be implemented? Andrew, Russell?
> > 
> > I am asking because the patchset advance depends on your answers. If
> > you do I'll need to fix the problem described in my first message,
> > implement some new mdiobus_register_board_info()-like but
> > MDIO-bus-specific interface function (so MDIO-device infos would be
> > attached to the allocated MDIO-bus and then used to register the
> > respective MDIO-devices on the MDIO-bus registration), then convert
> > the sja1105 and wangxun txgbe drivers to using it. If you don't I'll
> > get back the xpcs_create_mdiodev() implementation and just provide a
> > fwnode-based version of one.
> 
> Folks, this is the only issue left to settle so I could move on with
> the series fixing up. So the question is: taking my comment above into
> account is it worth to convert the xpcs_create_mdiodev() method to
> re-using the already registered MDIO-device instance instead of
> always creating a stub-like MDIO-device?

I can't exactly say "yes, this is worth it", because it also depends on
what the phylib/phylink maintainers say. So I haven't said anything.
But I also don't have any objection, as long as the conversion doesn't
break existing setups (in new ways; see the "unbind MDIO bus driver"
case which is already problematic).