Re: [net-next PATCH v5 05/10] net: pcs: lynx: Convert to an MDIO driver

[Sean Anderson <sean.anderson@linux.dev>]

On 5/23/25 18:07, Sean Anderson wrote:
> On 5/23/25 17:39, Sean Anderson wrote:
>> On 5/23/25 17:33, Heiner Kallweit wrote:
>>> On 23.05.2025 22:33, Sean Anderson wrote:
>>>> This converts the lynx PCS driver to a proper MDIO driver.
>>>> This allows using a more conventional driver lifecycle (e.g. with a
>>>> probe and remove). It will also make it easier to add interrupt support.
>>>> 
>>>> The existing helpers are converted to bind the MDIO driver instead of
>>>> creating the PCS directly. As lynx_pcs_create_mdiodev creates the PCS
>>>> device, we can just set the modalias. For lynx_pcs_create_fwnode, we try
>>>> to get the PCS the usual way, and if that fails we edit the devicetree
>>>> to add a compatible and reprobe the device.
>>>> 
>>>> To ensure my contributions remain free software, remove the BSD option
>>>> from the license. This is permitted because the SPDX uses "OR".
>>>> 
>>>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>>>> ---
>>>> 
>>>> Changes in v5:
>>>> - Use MDIO_BUS instead of MDIO_DEVICE
>>>> 
>>>> Changes in v4:
>>>> - Add a note about the license
>>>> - Convert to dev-less pcs_put
>>>> 
>>>> Changes in v3:
>>>> - Call devm_pcs_register instead of devm_pcs_register_provider
>>>> 
>>>> Changes in v2:
>>>> - Add support for #pcs-cells
>>>> - Remove unused variable lynx_properties
>>>> 
>>>>  drivers/net/dsa/ocelot/Kconfig                |   4 +
>>>>  drivers/net/dsa/ocelot/felix_vsc9959.c        |  11 +-
>>>>  drivers/net/dsa/ocelot/seville_vsc9953.c      |  11 +-
>>>>  drivers/net/ethernet/altera/Kconfig           |   2 +
>>>>  drivers/net/ethernet/altera/altera_tse_main.c |   7 +-
>>>>  drivers/net/ethernet/freescale/dpaa/Kconfig   |   2 +-
>>>>  drivers/net/ethernet/freescale/dpaa2/Kconfig  |   3 +
>>>>  .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  11 +-
>>>>  drivers/net/ethernet/freescale/enetc/Kconfig  |   2 +
>>>>  .../net/ethernet/freescale/enetc/enetc_pf.c   |   8 +-
>>>>  .../net/ethernet/freescale/enetc/enetc_pf.h   |   1 -
>>>>  .../freescale/enetc/enetc_pf_common.c         |   4 +-
>>>>  drivers/net/ethernet/freescale/fman/Kconfig   |   4 +-
>>>>  .../net/ethernet/freescale/fman/fman_memac.c  |  25 ++--
>>>>  drivers/net/ethernet/stmicro/stmmac/Kconfig   |   3 +
>>>>  .../ethernet/stmicro/stmmac/dwmac-socfpga.c   |   6 +-
>>>>  drivers/net/pcs/Kconfig                       |  11 +-
>>>>  drivers/net/pcs/pcs-lynx.c                    | 110 ++++++++++--------
>>>>  include/linux/pcs-lynx.h                      |  13 ++-
>>>>  19 files changed, 128 insertions(+), 110 deletions(-)
>>>> 
>>>> diff --git a/drivers/net/dsa/ocelot/Kconfig b/drivers/net/dsa/ocelot/Kconfig
>>>> index 081e7a88ea02..907c29d61c14 100644
>>>> --- a/drivers/net/dsa/ocelot/Kconfig
>>>> +++ b/drivers/net/dsa/ocelot/Kconfig
>>>> @@ -42,7 +42,9 @@ config NET_DSA_MSCC_FELIX
>>>>  	select NET_DSA_TAG_OCELOT_8021Q
>>>>  	select NET_DSA_TAG_OCELOT
>>>>  	select FSL_ENETC_MDIO
>>>> +	select PCS
>>>>  	select PCS_LYNX
>>>> +	select MDIO_BUS
>>> 
>>> This shouldn't be needed. NET_DSA selects PHYLINK, which selects PHYLIB,
>>> which selects MDIO_BUS. There are more places in this series where the
>>> same comment applies.
>> 
>> select does not transitively enable dependencies. See the note in
>> Documentation/kbuild/kconfig-language.rst for details. Therefore we must
>> select the dependencies of things we select in order to ensure we do not
>> trip sym_warn_unmet_dep.
> 
> OK, I see what you mean here. But of course NET_DSA is missing selects for
> PHYLIB and MDIO_BUS. And PHYLINK is also missing a select for MDIO_BUS. Actually,
> this bug is really endemic. Maybe we should just get rid of PHYLIB as a config
> and just make everything depend on ETHERNET instead.

After some experimentation, I think what that note means is that select
is recursive for select but not for depends on. So I think I only have
to select the "forward" dependencies, which is certainly easier.

--Sean