[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7da3983f-1321-49ea-afa4-83126f616b5d@csgroup.eu>
Date: Wed, 17 Sep 2025 09:01:04 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>, davem@...emloft.net
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, thomas.petazzoni@...tlin.com,
Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
Russell King <linux@...linux.org.uk>, linux-arm-kernel@...ts.infradead.org,
Herve Codina <herve.codina@...tlin.com>,
Florian Fainelli <f.fainelli@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>,
Vladimir Oltean <vladimir.oltean@....com>,
Köry Maincent <kory.maincent@...tlin.com>,
Marek Behún <kabel@...nel.org>,
Oleksij Rempel <o.rempel@...gutronix.de>,
Nicolò Veronese <nicveronese@...il.com>,
Simon Horman <horms@...nel.org>, mwojtas@...omium.org,
Antoine Tenart <atenart@...nel.org>, devicetree@...r.kernel.org,
Conor Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Rob Herring <robh@...nel.org>,
Romain Gantois <romain.gantois@...tlin.com>,
Daniel Golle <daniel@...rotopia.org>,
Dimitri Fedrau <dimitri.fedrau@...bherr.com>
Subject: Re: [PATCH net-next v12 09/18] net: phylink: Move sfp interface
selection and filtering to phy_caps
Le 09/09/2025 à 17:26, Maxime Chevallier a écrit :
> Phylink's helpers to get the interfaces usable on an SFP module based on
> speed and linkmodes can be modes to phy_caps, so that it can benefit to
> PHY-driver SFP support.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
Reviewed-by: Christophe Leroy <christophe.leroy@...roup.eu>
> ---
> drivers/net/phy/phy-caps.h | 6 ++++
> drivers/net/phy/phy_caps.c | 72 ++++++++++++++++++++++++++++++++++++++
> drivers/net/phy/phylink.c | 72 +++++---------------------------------
> 3 files changed, 86 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
> index ba81cd75e122..ebed340a2e77 100644
> --- a/drivers/net/phy/phy-caps.h
> +++ b/drivers/net/phy/phy-caps.h
> @@ -66,4 +66,10 @@ void phy_caps_medium_get_supported(unsigned long *supported,
> int lanes);
> u32 phy_caps_mediums_from_linkmodes(unsigned long *linkmodes);
>
> +void phy_caps_filter_sfp_interfaces(unsigned long *dst,
> + const unsigned long *interfaces);
> +phy_interface_t phy_caps_select_sfp_interface_speed(const unsigned long *interfaces,
> + u32 speed);
> +phy_interface_t phy_caps_choose_sfp_interface(const unsigned long *interfaces);
> +
> #endif /* __PHY_CAPS_H */
> diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
> index b38c567ec6ef..e4adc36e0fe3 100644
> --- a/drivers/net/phy/phy_caps.c
> +++ b/drivers/net/phy/phy_caps.c
> @@ -63,6 +63,22 @@ static int speed_duplex_to_capa(int speed, unsigned int duplex)
> #define for_each_link_caps_desc_speed(cap) \
> for (cap = &link_caps[__LINK_CAPA_MAX - 1]; cap >= link_caps; cap--)
>
> +static const phy_interface_t phy_caps_sfp_interface_preference[] = {
> + PHY_INTERFACE_MODE_100GBASEP,
> + PHY_INTERFACE_MODE_50GBASER,
> + PHY_INTERFACE_MODE_LAUI,
> + PHY_INTERFACE_MODE_25GBASER,
> + PHY_INTERFACE_MODE_USXGMII,
> + PHY_INTERFACE_MODE_10GBASER,
> + PHY_INTERFACE_MODE_5GBASER,
> + PHY_INTERFACE_MODE_2500BASEX,
> + PHY_INTERFACE_MODE_SGMII,
> + PHY_INTERFACE_MODE_1000BASEX,
> + PHY_INTERFACE_MODE_100BASEX,
> +};
> +
> +static DECLARE_PHY_INTERFACE_MASK(phy_caps_sfp_interfaces);
> +
> /**
> * phy_caps_init() - Initializes the link_caps array from the link_mode_params.
> *
> @@ -100,6 +116,10 @@ int phy_caps_init(void)
> __set_bit(i, link_caps[capa].linkmodes);
> }
>
> + for (int i = 0; i < ARRAY_SIZE(phy_caps_sfp_interface_preference); ++i)
> + __set_bit(phy_caps_sfp_interface_preference[i],
> + phy_caps_sfp_interfaces);
> +
> return 0;
> }
>
> @@ -520,3 +540,55 @@ int phy_caps_interface_max_speed(phy_interface_t interface)
> return SPEED_UNKNOWN;
> }
> EXPORT_SYMBOL_GPL(phy_caps_interface_max_speed);
> +
> +void phy_caps_filter_sfp_interfaces(unsigned long *dst,
> + const unsigned long *interfaces)
> +{
> + phy_interface_and(dst, interfaces, phy_caps_sfp_interfaces);
> +}
> +
> +phy_interface_t
> +phy_caps_select_sfp_interface_speed(const unsigned long *interfaces, u32 speed)
> +{
> + phy_interface_t best_interface = PHY_INTERFACE_MODE_NA;
> + phy_interface_t interface;
> + u32 max_speed;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(phy_caps_sfp_interface_preference); i++) {
> + interface = phy_caps_sfp_interface_preference[i];
> + if (!test_bit(interface, interfaces))
> + continue;
> +
> + max_speed = phy_caps_interface_max_speed(interface);
> +
> + /* The logic here is: if speed == max_speed, then we've found
> + * the best interface. Otherwise we find the interface that
> + * can just support the requested speed.
> + */
> + if (max_speed >= speed)
> + best_interface = interface;
> +
> + if (max_speed <= speed)
> + break;
> + }
> +
> + return best_interface;
> +}
> +EXPORT_SYMBOL_GPL(phy_caps_select_sfp_interface_speed);
> +
> +phy_interface_t phy_caps_choose_sfp_interface(const unsigned long *interfaces)
> +{
> + phy_interface_t interface;
> + size_t i;
> +
> + interface = PHY_INTERFACE_MODE_NA;
> + for (i = 0; i < ARRAY_SIZE(phy_caps_sfp_interface_preference); i++)
> + if (test_bit(phy_caps_sfp_interface_preference[i], interfaces)) {
> + interface = phy_caps_sfp_interface_preference[i];
> + break;
> + }
> +
> + return interface;
> +}
> +EXPORT_SYMBOL_GPL(phy_caps_choose_sfp_interface);
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 091b1ee5c49a..91111ea1b149 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -126,22 +126,6 @@ do { \
> })
> #endif
>
> -static const phy_interface_t phylink_sfp_interface_preference[] = {
> - PHY_INTERFACE_MODE_100GBASEP,
> - PHY_INTERFACE_MODE_50GBASER,
> - PHY_INTERFACE_MODE_LAUI,
> - PHY_INTERFACE_MODE_25GBASER,
> - PHY_INTERFACE_MODE_USXGMII,
> - PHY_INTERFACE_MODE_10GBASER,
> - PHY_INTERFACE_MODE_5GBASER,
> - PHY_INTERFACE_MODE_2500BASEX,
> - PHY_INTERFACE_MODE_SGMII,
> - PHY_INTERFACE_MODE_1000BASEX,
> - PHY_INTERFACE_MODE_100BASEX,
> -};
> -
> -static DECLARE_PHY_INTERFACE_MASK(phylink_sfp_interfaces);
> -
> /**
> * phylink_set_port_modes() - set the port type modes in the ethtool mask
> * @mask: ethtool link mode mask
> @@ -1922,8 +1906,7 @@ static int phylink_validate_phy(struct phylink *pl, struct phy_device *phy,
> /* If the PHY is on a SFP, limit the interfaces to
> * those that can be used with a SFP module.
> */
> - phy_interface_and(interfaces, interfaces,
> - phylink_sfp_interfaces);
> + phy_caps_filter_sfp_interfaces(interfaces, interfaces);
>
> if (phy_interface_empty(interfaces)) {
> phylink_err(pl, "SFP PHY's possible interfaces becomes empty\n");
> @@ -2643,34 +2626,16 @@ static phy_interface_t phylink_sfp_select_interface(struct phylink *pl,
> static phy_interface_t phylink_sfp_select_interface_speed(struct phylink *pl,
> u32 speed)
> {
> - phy_interface_t best_interface = PHY_INTERFACE_MODE_NA;
> phy_interface_t interface;
> - u32 max_speed;
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(phylink_sfp_interface_preference); i++) {
> - interface = phylink_sfp_interface_preference[i];
> - if (!test_bit(interface, pl->sfp_interfaces))
> - continue;
> -
> - max_speed = phy_caps_interface_max_speed(interface);
>
> - /* The logic here is: if speed == max_speed, then we've found
> - * the best interface. Otherwise we find the interface that
> - * can just support the requested speed.
> - */
> - if (max_speed >= speed)
> - best_interface = interface;
> -
> - if (max_speed <= speed)
> - break;
> - }
> + interface = phy_caps_select_sfp_interface_speed(pl->sfp_interfaces,
> + speed);
>
> - if (best_interface == PHY_INTERFACE_MODE_NA)
> + if (interface == PHY_INTERFACE_MODE_NA)
> phylink_err(pl, "selection of interface failed, speed %u\n",
> speed);
>
> - return best_interface;
> + return interface;
> }
>
> static void phylink_merge_link_mode(unsigned long *dst, const unsigned long *b)
> @@ -3450,17 +3415,7 @@ static void phylink_sfp_detach(void *upstream, struct sfp_bus *bus)
> static phy_interface_t phylink_choose_sfp_interface(struct phylink *pl,
> const unsigned long *intf)
> {
> - phy_interface_t interface;
> - size_t i;
> -
> - interface = PHY_INTERFACE_MODE_NA;
> - for (i = 0; i < ARRAY_SIZE(phylink_sfp_interface_preference); i++)
> - if (test_bit(phylink_sfp_interface_preference[i], intf)) {
> - interface = phylink_sfp_interface_preference[i];
> - break;
> - }
> -
> - return interface;
> + return phy_caps_choose_sfp_interface(intf);
> }
>
> static void phylink_sfp_set_config(struct phylink *pl, unsigned long *supported,
> @@ -3737,8 +3692,8 @@ static int phylink_sfp_connect_phy(void *upstream, struct phy_device *phy)
> phy_support_asym_pause(phy);
>
> /* Set the PHY's host supported interfaces */
> - phy_interface_and(phy->host_interfaces, phylink_sfp_interfaces,
> - pl->config->supported_interfaces);
> + phy_caps_filter_sfp_interfaces(phy->host_interfaces,
> + pl->config->supported_interfaces);
>
> /* Do the initial configuration */
> return phylink_sfp_config_phy(pl, phy);
> @@ -4166,16 +4121,5 @@ void phylink_mii_c45_pcs_get_state(struct mdio_device *pcs,
> }
> EXPORT_SYMBOL_GPL(phylink_mii_c45_pcs_get_state);
>
> -static int __init phylink_init(void)
> -{
> - for (int i = 0; i < ARRAY_SIZE(phylink_sfp_interface_preference); ++i)
> - __set_bit(phylink_sfp_interface_preference[i],
> - phylink_sfp_interfaces);
> -
> - return 0;
> -}
> -
> -module_init(phylink_init);
> -
> MODULE_LICENSE("GPL v2");
> MODULE_DESCRIPTION("phylink models the MAC to optional PHY connection");
Powered by blists - more mailing lists