[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aDENYfL7Hstsswml@shell.armlinux.org.uk>
Date: Sat, 24 May 2025 01:05:53 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Sean Anderson <sean.anderson@...ux.dev>
Cc: Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Lei Wei <quic_leiwei@...cinc.com>,
Christian Marangi <ansuelsmth@...il.com>,
Kory Maincent <kory.maincent@...tlin.com>,
Simon Horman <horms@...nel.org>,
Daniel Golle <daniel@...rotopia.org>,
Vineeth Karumanchi <vineeth.karumanchi@....com>,
linux-kernel@...r.kernel.org, Ioana Ciornei <ioana.ciornei@....com>,
Vladimir Oltean <vladimir.oltean@....com>, imx@...ts.linux.dev,
linux-stm32@...md-mailman.stormreply.com
Subject: Re: [net-next PATCH v5 05/10] net: pcs: lynx: Convert to an MDIO
driver
On Fri, May 23, 2025 at 06:07:16PM -0400, 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@...ux.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.
You're reading the documentation wrongly.
Take this example:
config A
select B
config B
select C
depends on E
config C
select D
config D
config E
Enabling A leads to B, C and D all being enabled, but it does *not*
lead to E being enabled - kconfig will issue a warning if E is not
already enabled.
E is a dependency of B. There are no other dependencies, but there
are reverse dependencies, and reverse dependencies are propagated.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists