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@xxxxxxxx/ > > 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).
