As the link_caps array is efficient for <speed,duplex> lookups,
implement a function for speed/duplex lookups that matches a given
mask. This replicates to some extent the phy_lookup_settings()
behaviour, matching full link_capabilities instead of a single linkmode.

phy.c's phy_santize_settings() and phylink's
phylink_ethtool_ksettings_set() performs such lookup using the
phy_settings table, but are only interested in the actual speed/duplex
that were matched, rathet than the individual linkmode.

Similar to phy_lookup_settings(), the newly introduced phy_caps_lookup()
will run through the link_caps[] array by descending speed/duplex order.

If the link_capabilities for a given <speed/duplex> tuple intersects the
passed linkmodes, we consider that a match.

Similar to phy_lookup_settings(), we also allow passing an 'exact'
boolean, allowing non-exact match. Here, we MUST always match the
linkmodes mask, but we allow matching on lower speed settings.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/phy-caps.h |  4 ++++
 drivers/net/phy/phy.c      | 32 ++++++--------------------
 drivers/net/phy/phy_caps.c | 47 ++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/phylink.c  | 17 +++++++-------
 4 files changed, 67 insertions(+), 33 deletions(-)

diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
index 8833798f141f..aef4b54a8429 100644
--- a/drivers/net/phy/phy-caps.h
+++ b/drivers/net/phy/phy-caps.h
@@ -51,4 +51,8 @@ phy_caps_lookup_by_linkmode(const unsigned long *linkmodes);
 const struct link_capabilities *
 phy_caps_lookup_by_linkmode_rev(const unsigned long *linkmodes, bool fdx_only);
 
+const struct link_capabilities *
+phy_caps_lookup(int speed, unsigned int duplex, const unsigned long *supported,
+		bool exact);
+
 #endif /* __PHY_CAPS_H */
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 8df37d221fba..562acde89224 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -213,25 +213,6 @@ int phy_aneg_done(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_aneg_done);
 
-/**
- * phy_find_valid - find a PHY setting that matches the requested parameters
- * @speed: desired speed
- * @duplex: desired duplex
- * @supported: mask of supported link modes
- *
- * Locate a supported phy setting that is, in priority order:
- * - an exact match for the specified speed and duplex mode
- * - a match for the specified speed, or slower speed
- * - the slowest supported speed
- * Returns the matched phy_setting entry, or %NULL if no supported phy
- * settings were found.
- */
-static const struct phy_setting *
-phy_find_valid(int speed, int duplex, unsigned long *supported)
-{
-	return phy_lookup_setting(speed, duplex, supported, false);
-}
-
 /**
  * phy_supported_speeds - return all speeds currently supported by a phy device
  * @phy: The phy device to return supported speeds of.
@@ -274,13 +255,14 @@ EXPORT_SYMBOL(phy_check_valid);
  */
 static void phy_sanitize_settings(struct phy_device *phydev)
 {
-	const struct phy_setting *setting;
+	const struct link_capabilities *c;
+
+	c = phy_caps_lookup(phydev->speed, phydev->duplex, phydev->supported,
+			    false);
 
-	setting = phy_find_valid(phydev->speed, phydev->duplex,
-				 phydev->supported);
-	if (setting) {
-		phydev->speed = setting->speed;
-		phydev->duplex = setting->duplex;
+	if (c) {
+		phydev->speed = c->speed;
+		phydev->duplex = c->duplex;
 	} else {
 		/* We failed to find anything (no supported speeds?) */
 		phydev->speed = SPEED_UNKNOWN;
diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
index c39f38c12ef2..0366feee2912 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -170,6 +170,53 @@ phy_caps_lookup_by_linkmode_rev(const unsigned long *linkmodes, bool fdx_only)
 	return NULL;
 }
 
+/**
+ * phy_caps_lookup() - Lookup capabilities by speed/duplex that matches a mask
+ * @speed: Speed to match
+ * @duplex: Duplex to match
+ * @supported: Mask of linkmodes to match
+ * @exact: Perform an exact match or not.
+ *
+ * Lookup a link_capabilities entry that intersect the supported linkmodes mask,
+ * and that matches the passed speed and duplex.
+ *
+ * When @exact is set, an exact match is performed on speed and duplex, meaning
+ * that if the linkmodes for the given speed and duplex intersect the supported
+ * mask, this capability is returned, otherwise we don't have a match and return
+ * NULL.
+ *
+ * When @exact is not set, we return either an exact match, or matching capabilities
+ * at lower speed, or the lowest matching speed, or NULL.
+ *
+ * Returns: a matched link_capabilities according to the above process, NULL
+ *	    otherwise.
+ */
+const struct link_capabilities *
+phy_caps_lookup(int speed, unsigned int duplex, const unsigned long *supported,
+		bool exact)
+{
+	const struct link_capabilities *lcap, *last = NULL;
+
+	for_each_link_caps_desc_speed(lcap) {
+		if (linkmode_intersects(lcap->linkmodes, supported)) {
+			last = lcap;
+			/* exact match on speed and duplex*/
+			if (lcap->speed == speed && lcap->duplex == duplex) {
+				return lcap;
+			} else if (!exact) {
+				if (lcap->speed <= speed)
+					return lcap;
+			}
+		}
+	}
+
+	if (!exact)
+		return last;
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(phy_caps_lookup);
+
 /**
  * phy_caps_linkmode_max_speed() - Clamp a linkmodes set to a max speed
  * @max_speed: Speed limit for the linkmode set
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index a3f64b6d2d34..cf9f019382ad 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -20,6 +20,7 @@
 #include <linux/timer.h>
 #include <linux/workqueue.h>
 
+#include "phy-caps.h"
 #include "sfp.h"
 #include "swphy.h"
 
@@ -2852,8 +2853,8 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
 				  const struct ethtool_link_ksettings *kset)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(support);
+	const struct link_capabilities *c;
 	struct phylink_link_state config;
-	const struct phy_setting *s;
 
 	ASSERT_RTNL();
 
@@ -2896,23 +2897,23 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
 		/* Autonegotiation disabled, select a suitable speed and
 		 * duplex.
 		 */
-		s = phy_lookup_setting(kset->base.speed, kset->base.duplex,
-				       pl->supported, false);
-		if (!s)
+		c = phy_caps_lookup(kset->base.speed, kset->base.duplex,
+				    pl->supported, false);
+		if (!c)
 			return -EINVAL;
 
 		/* If we have a fixed link, refuse to change link parameters.
 		 * If the link parameters match, accept them but do nothing.
 		 */
 		if (pl->req_link_an_mode == MLO_AN_FIXED) {
-			if (s->speed != pl->link_config.speed ||
-			    s->duplex != pl->link_config.duplex)
+			if (c->speed != pl->link_config.speed ||
+			    c->duplex != pl->link_config.duplex)
 				return -EINVAL;
 			return 0;
 		}
 
-		config.speed = s->speed;
-		config.duplex = s->duplex;
+		config.speed = c->speed;
+		config.duplex = c->duplex;
 		break;
 
 	case AUTONEG_ENABLE:
-- 
2.48.1

on 2025/3/8 1:36, Maxime Chevallier wrote:
> As the link_caps array is efficient for <speed,duplex> lookups,
> implement a function for speed/duplex lookups that matches a given
> mask. This replicates to some extent the phy_lookup_settings()
> behaviour, matching full link_capabilities instead of a single linkmode.
>
> phy.c's phy_santize_settings() and phylink's
> phylink_ethtool_ksettings_set() performs such lookup using the
> phy_settings table, but are only interested in the actual speed/duplex
> that were matched, rathet than the individual linkmode.
>
> Similar to phy_lookup_settings(), the newly introduced phy_caps_lookup()
> will run through the link_caps[] array by descending speed/duplex order.
>
> If the link_capabilities for a given <speed/duplex> tuple intersects the
> passed linkmodes, we consider that a match.
>
> Similar to phy_lookup_settings(), we also allow passing an 'exact'
> boolean, allowing non-exact match. Here, we MUST always match the
> linkmodes mask, but we allow matching on lower speed settings.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
>   drivers/net/phy/phy-caps.h |  4 ++++
>   drivers/net/phy/phy.c      | 32 ++++++--------------------
>   drivers/net/phy/phy_caps.c | 47 ++++++++++++++++++++++++++++++++++++++
>   drivers/net/phy/phylink.c  | 17 +++++++-------
>   4 files changed, 67 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
> index 8833798f141f..aef4b54a8429 100644
> --- a/drivers/net/phy/phy-caps.h
> +++ b/drivers/net/phy/phy-caps.h
> @@ -51,4 +51,8 @@ phy_caps_lookup_by_linkmode(const unsigned long *linkmodes);
>   const struct link_capabilities *
>   phy_caps_lookup_by_linkmode_rev(const unsigned long *linkmodes, bool fdx_only);
>   
> +const struct link_capabilities *
> +phy_caps_lookup(int speed, unsigned int duplex, const unsigned long *supported,
> +		bool exact);
> +
>   #endif /* __PHY_CAPS_H */
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 8df37d221fba..562acde89224 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -213,25 +213,6 @@ int phy_aneg_done(struct phy_device *phydev)
>   }
>   EXPORT_SYMBOL(phy_aneg_done);
>   
> -/**
> - * phy_find_valid - find a PHY setting that matches the requested parameters
> - * @speed: desired speed
> - * @duplex: desired duplex
> - * @supported: mask of supported link modes
> - *
> - * Locate a supported phy setting that is, in priority order:
> - * - an exact match for the specified speed and duplex mode
> - * - a match for the specified speed, or slower speed
> - * - the slowest supported speed
> - * Returns the matched phy_setting entry, or %NULL if no supported phy
> - * settings were found.
> - */
> -static const struct phy_setting *
> -phy_find_valid(int speed, int duplex, unsigned long *supported)
> -{
> -	return phy_lookup_setting(speed, duplex, supported, false);
> -}
> -
>   /**
>    * phy_supported_speeds - return all speeds currently supported by a phy device
>    * @phy: The phy device to return supported speeds of.
> @@ -274,13 +255,14 @@ EXPORT_SYMBOL(phy_check_valid);
>    */
>   static void phy_sanitize_settings(struct phy_device *phydev)
>   {
> -	const struct phy_setting *setting;
> +	const struct link_capabilities *c;
> +
> +	c = phy_caps_lookup(phydev->speed, phydev->duplex, phydev->supported,
> +			    false);
>   
> -	setting = phy_find_valid(phydev->speed, phydev->duplex,
> -				 phydev->supported);
> -	if (setting) {
> -		phydev->speed = setting->speed;
> -		phydev->duplex = setting->duplex;
> +	if (c) {
> +		phydev->speed = c->speed;
> +		phydev->duplex = c->duplex;
>   	} else {
>   		/* We failed to find anything (no supported speeds?) */
>   		phydev->speed = SPEED_UNKNOWN;
> diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
> index c39f38c12ef2..0366feee2912 100644
> --- a/drivers/net/phy/phy_caps.c
> +++ b/drivers/net/phy/phy_caps.c
> @@ -170,6 +170,53 @@ phy_caps_lookup_by_linkmode_rev(const unsigned long *linkmodes, bool fdx_only)
>   	return NULL;
>   }
>   
> +/**
> + * phy_caps_lookup() - Lookup capabilities by speed/duplex that matches a mask
> + * @speed: Speed to match
> + * @duplex: Duplex to match
> + * @supported: Mask of linkmodes to match
> + * @exact: Perform an exact match or not.
> + *
> + * Lookup a link_capabilities entry that intersect the supported linkmodes mask,
> + * and that matches the passed speed and duplex.
> + *
> + * When @exact is set, an exact match is performed on speed and duplex, meaning
> + * that if the linkmodes for the given speed and duplex intersect the supported
> + * mask, this capability is returned, otherwise we don't have a match and return
> + * NULL.
> + *
> + * When @exact is not set, we return either an exact match, or matching capabilities
> + * at lower speed, or the lowest matching speed, or NULL.
> + *
> + * Returns: a matched link_capabilities according to the above process, NULL
> + *	    otherwise.
> + */
> +const struct link_capabilities *
> +phy_caps_lookup(int speed, unsigned int duplex, const unsigned long *supported,
> +		bool exact)
> +{
> +	const struct link_capabilities *lcap, *last = NULL;
> +
> +	for_each_link_caps_desc_speed(lcap) {
> +		if (linkmode_intersects(lcap->linkmodes, supported)) {
> +			last = lcap;
> +			/* exact match on speed and duplex*/
> +			if (lcap->speed == speed && lcap->duplex == duplex) {
> +				return lcap;
> +			} else if (!exact) {
> +				if (lcap->speed <= speed)
> +					return lcap;
> +			}
> +		}
> +	}
> +
> +	if (!exact)
> +		return last;
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(phy_caps_lookup);
> +
>   /**
>    * phy_caps_linkmode_max_speed() - Clamp a linkmodes set to a max speed
>    * @max_speed: Speed limit for the linkmode set
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index a3f64b6d2d34..cf9f019382ad 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -20,6 +20,7 @@
>   #include <linux/timer.h>
>   #include <linux/workqueue.h>
>   
> +#include "phy-caps.h"
>   #include "sfp.h"
>   #include "swphy.h"
>   
> @@ -2852,8 +2853,8 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
>   				  const struct ethtool_link_ksettings *kset)
>   {
>   	__ETHTOOL_DECLARE_LINK_MODE_MASK(support);
> +	const struct link_capabilities *c;
>   	struct phylink_link_state config;
> -	const struct phy_setting *s;
>   
>   	ASSERT_RTNL();
>   
> @@ -2896,23 +2897,23 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
>   		/* Autonegotiation disabled, select a suitable speed and
>   		 * duplex.
>   		 */
> -		s = phy_lookup_setting(kset->base.speed, kset->base.duplex,
> -				       pl->supported, false);
> -		if (!s)
> +		c = phy_caps_lookup(kset->base.speed, kset->base.duplex,
> +				    pl->supported, false);
> +		if (!c)
>   			return -EINVAL;



Hi Maxime,  fc81e257d19f ("net: phy: phy_caps: Allow looking-up link caps based on speed and duplex") might have different behavior than the modification.
My case is set 10M Half with disable autoneg both sides and I expect it is
link in 10M Half. But now, it is link in 10M Full，which is not what I
expect.

I used followed command and trace how phy worked.
	ethtool -s eth1 autoneg off speed 10 duplex half
The log is showed as followed:
ethtool-13127	[067]	6164.771853: phy_ethtool_ksettings set: (phy_ethtool ksettings set+0x0/0x200) duplex=0 speed=10
kworker/u322:2-11096	[070]	6164.771853:	_phy_start_aneq: ( _phy_start_aneg+0x0/0xb8) duplex=0 speed=10
kworker/u322:2-11096	[070]	6164.771854:	phy_caps_lookup: (phy_caps_lookup+0x0/0xf0) duplex=0 speed=10
kworker/u322:2-11096	[070]	6164.771855:	phy_config_aneg: (phy_config_aneg+0x0/0x70) duplex=1 speed=10
kworker/u322:2-11096	[070]	6164.771856:	genphy_config_aneg:	(__genphy_config_aneg+0X0/0X270) duplex=1 speed=10

I also try to fixed it and it works. Do you have any idea about it.

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 0e762fc3a529..2986c41c42a8 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -258,7 +258,7 @@ static void phy_sanitize_settings(struct phy_device *phydev)
         const struct link_capabilities *c;

         c = phy_caps_lookup(phydev->speed, phydev->duplex, phydev->supported,
-                           false);
+                           true);

         if (c) {
                 phydev->speed = c->speed;



>   
>   		/* If we have a fixed link, refuse to change link parameters.
>   		 * If the link parameters match, accept them but do nothing.
>   		 */
>   		if (pl->req_link_an_mode == MLO_AN_FIXED) {
> -			if (s->speed != pl->link_config.speed ||
> -			    s->duplex != pl->link_config.duplex)
> +			if (c->speed != pl->link_config.speed ||
> +			    c->duplex != pl->link_config.duplex)
>   				return -EINVAL;
>   			return 0;
>   		}
>   
> -		config.speed = s->speed;
> -		config.duplex = s->duplex;
> +		config.speed = c->speed;
> +		config.duplex = c->duplex;
>   		break;
>   
>   	case AUTONEG_ENABLE:

Hello again,

On Thu, 29 May 2025 17:36:11 +0800
Jijie Shao <shaojijie@huawei.com> wrote:

>
> 
> Hi Maxime,  fc81e257d19f ("net: phy: phy_caps: Allow looking-up link caps based on speed and duplex") might have different behavior than the modification.
> My case is set 10M Half with disable autoneg both sides and I expect it is
> link in 10M Half. But now, it is link in 10M Full，which is not what I
> expect.
> 
> I used followed command and trace how phy worked.
> 	ethtool -s eth1 autoneg off speed 10 duplex half
> The log is showed as followed:
> ethtool-13127	[067]	6164.771853: phy_ethtool_ksettings set: (phy_ethtool ksettings set+0x0/0x200) duplex=0 speed=10
> kworker/u322:2-11096	[070]	6164.771853:	_phy_start_aneq: ( _phy_start_aneg+0x0/0xb8) duplex=0 speed=10
> kworker/u322:2-11096	[070]	6164.771854:	phy_caps_lookup: (phy_caps_lookup+0x0/0xf0) duplex=0 speed=10
> kworker/u322:2-11096	[070]	6164.771855:	phy_config_aneg: (phy_config_aneg+0x0/0x70) duplex=1 speed=10
> kworker/u322:2-11096	[070]	6164.771856:	genphy_config_aneg:	(__genphy_config_aneg+0X0/0X270) duplex=1 speed=10

I managed to get a bit of time to look into this, and indeed I was able
to reproduce the issue locally.

I'll send a patch ASAP, based on the the prototype patch I gave in the
previous reply.

Thanks for reporting,

Maxime

On Thu, 29 May 2025 17:36:11 +0800
Jijie Shao <shaojijie@huawei.com> wrote:

> on 2025/3/8 1:36, Maxime Chevallier wrote:
> > As the link_caps array is efficient for <speed,duplex> lookups,
> > implement a function for speed/duplex lookups that matches a given
> > mask. This replicates to some extent the phy_lookup_settings()
> > behaviour, matching full link_capabilities instead of a single linkmode.
> >
> > phy.c's phy_santize_settings() and phylink's
> > phylink_ethtool_ksettings_set() performs such lookup using the
> > phy_settings table, but are only interested in the actual speed/duplex
> > that were matched, rathet than the individual linkmode.
> >
> > Similar to phy_lookup_settings(), the newly introduced phy_caps_lookup()
> > will run through the link_caps[] array by descending speed/duplex order.
> >
> > If the link_capabilities for a given <speed/duplex> tuple intersects the
> > passed linkmodes, we consider that a match.
> >
> > Similar to phy_lookup_settings(), we also allow passing an 'exact'
> > boolean, allowing non-exact match. Here, we MUST always match the
> > linkmodes mask, but we allow matching on lower speed settings.
> >
> > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > ---
> >   drivers/net/phy/phy-caps.h |  4 ++++
> >   drivers/net/phy/phy.c      | 32 ++++++--------------------
> >   drivers/net/phy/phy_caps.c | 47 ++++++++++++++++++++++++++++++++++++++
> >   drivers/net/phy/phylink.c  | 17 +++++++-------
> >   4 files changed, 67 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
> > index 8833798f141f..aef4b54a8429 100644
> > --- a/drivers/net/phy/phy-caps.h
> > +++ b/drivers/net/phy/phy-caps.h
> > @@ -51,4 +51,8 @@ phy_caps_lookup_by_linkmode(const unsigned long *linkmodes);
> >   const struct link_capabilities *
> >   phy_caps_lookup_by_linkmode_rev(const unsigned long *linkmodes, bool fdx_only);
> >   
> > +const struct link_capabilities *
> > +phy_caps_lookup(int speed, unsigned int duplex, const unsigned long *supported,
> > +		bool exact);
> > +
> >   #endif /* __PHY_CAPS_H */
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 8df37d221fba..562acde89224 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -213,25 +213,6 @@ int phy_aneg_done(struct phy_device *phydev)
> >   }
> >   EXPORT_SYMBOL(phy_aneg_done);
> >   
> > -/**
> > - * phy_find_valid - find a PHY setting that matches the requested parameters
> > - * @speed: desired speed
> > - * @duplex: desired duplex
> > - * @supported: mask of supported link modes
> > - *
> > - * Locate a supported phy setting that is, in priority order:
> > - * - an exact match for the specified speed and duplex mode
> > - * - a match for the specified speed, or slower speed
> > - * - the slowest supported speed
> > - * Returns the matched phy_setting entry, or %NULL if no supported phy
> > - * settings were found.
> > - */
> > -static const struct phy_setting *
> > -phy_find_valid(int speed, int duplex, unsigned long *supported)
> > -{
> > -	return phy_lookup_setting(speed, duplex, supported, false);
> > -}
> > -
> >   /**
> >    * phy_supported_speeds - return all speeds currently supported by a phy device
> >    * @phy: The phy device to return supported speeds of.
> > @@ -274,13 +255,14 @@ EXPORT_SYMBOL(phy_check_valid);
> >    */
> >   static void phy_sanitize_settings(struct phy_device *phydev)
> >   {
> > -	const struct phy_setting *setting;
> > +	const struct link_capabilities *c;
> > +
> > +	c = phy_caps_lookup(phydev->speed, phydev->duplex, phydev->supported,
> > +			    false);
> >   
> > -	setting = phy_find_valid(phydev->speed, phydev->duplex,
> > -				 phydev->supported);
> > -	if (setting) {
> > -		phydev->speed = setting->speed;
> > -		phydev->duplex = setting->duplex;
> > +	if (c) {
> > +		phydev->speed = c->speed;
> > +		phydev->duplex = c->duplex;
> >   	} else {
> >   		/* We failed to find anything (no supported speeds?) */
> >   		phydev->speed = SPEED_UNKNOWN;
> > diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
> > index c39f38c12ef2..0366feee2912 100644
> > --- a/drivers/net/phy/phy_caps.c
> > +++ b/drivers/net/phy/phy_caps.c
> > @@ -170,6 +170,53 @@ phy_caps_lookup_by_linkmode_rev(const unsigned long *linkmodes, bool fdx_only)
> >   	return NULL;
> >   }
> >   
> > +/**
> > + * phy_caps_lookup() - Lookup capabilities by speed/duplex that matches a mask
> > + * @speed: Speed to match
> > + * @duplex: Duplex to match
> > + * @supported: Mask of linkmodes to match
> > + * @exact: Perform an exact match or not.
> > + *
> > + * Lookup a link_capabilities entry that intersect the supported linkmodes mask,
> > + * and that matches the passed speed and duplex.
> > + *
> > + * When @exact is set, an exact match is performed on speed and duplex, meaning
> > + * that if the linkmodes for the given speed and duplex intersect the supported
> > + * mask, this capability is returned, otherwise we don't have a match and return
> > + * NULL.
> > + *
> > + * When @exact is not set, we return either an exact match, or matching capabilities
> > + * at lower speed, or the lowest matching speed, or NULL.
> > + *
> > + * Returns: a matched link_capabilities according to the above process, NULL
> > + *	    otherwise.
> > + */
> > +const struct link_capabilities *
> > +phy_caps_lookup(int speed, unsigned int duplex, const unsigned long *supported,
> > +		bool exact)
> > +{
> > +	const struct link_capabilities *lcap, *last = NULL;
> > +
> > +	for_each_link_caps_desc_speed(lcap) {
> > +		if (linkmode_intersects(lcap->linkmodes, supported)) {
> > +			last = lcap;
> > +			/* exact match on speed and duplex*/
> > +			if (lcap->speed == speed && lcap->duplex == duplex) {
> > +				return lcap;
> > +			} else if (!exact) {
> > +				if (lcap->speed <= speed)
> > +					return lcap;
> > +			}
> > +		}
> > +	}
> > +
> > +	if (!exact)
> > +		return last;
> > +
> > +	return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(phy_caps_lookup);
> > +
> >   /**
> >    * phy_caps_linkmode_max_speed() - Clamp a linkmodes set to a max speed
> >    * @max_speed: Speed limit for the linkmode set
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index a3f64b6d2d34..cf9f019382ad 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> > @@ -20,6 +20,7 @@
> >   #include <linux/timer.h>
> >   #include <linux/workqueue.h>
> >   
> > +#include "phy-caps.h"
> >   #include "sfp.h"
> >   #include "swphy.h"
> >   
> > @@ -2852,8 +2853,8 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
> >   				  const struct ethtool_link_ksettings *kset)
> >   {
> >   	__ETHTOOL_DECLARE_LINK_MODE_MASK(support);
> > +	const struct link_capabilities *c;
> >   	struct phylink_link_state config;
> > -	const struct phy_setting *s;
> >   
> >   	ASSERT_RTNL();
> >   
> > @@ -2896,23 +2897,23 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
> >   		/* Autonegotiation disabled, select a suitable speed and
> >   		 * duplex.
> >   		 */
> > -		s = phy_lookup_setting(kset->base.speed, kset->base.duplex,
> > -				       pl->supported, false);
> > -		if (!s)
> > +		c = phy_caps_lookup(kset->base.speed, kset->base.duplex,
> > +				    pl->supported, false);
> > +		if (!c)
> >   			return -EINVAL;  
> 
> 
> 
> Hi Maxime,  fc81e257d19f ("net: phy: phy_caps: Allow looking-up link caps based on speed and duplex") might have different behavior than the modification.
> My case is set 10M Half with disable autoneg both sides and I expect it is
> link in 10M Half. But now, it is link in 10M Full，which is not what I
> expect.
> 
> I used followed command and trace how phy worked.
> 	ethtool -s eth1 autoneg off speed 10 duplex half
> The log is showed as followed:
> ethtool-13127	[067]	6164.771853: phy_ethtool_ksettings set: (phy_ethtool ksettings set+0x0/0x200) duplex=0 speed=10
> kworker/u322:2-11096	[070]	6164.771853:	_phy_start_aneq: ( _phy_start_aneg+0x0/0xb8) duplex=0 speed=10
> kworker/u322:2-11096	[070]	6164.771854:	phy_caps_lookup: (phy_caps_lookup+0x0/0xf0) duplex=0 speed=10
> kworker/u322:2-11096	[070]	6164.771855:	phy_config_aneg: (phy_config_aneg+0x0/0x70) duplex=1 speed=10
> kworker/u322:2-11096	[070]	6164.771856:	genphy_config_aneg:	(__genphy_config_aneg+0X0/0X270) duplex=1 speed=10
> 
> I also try to fixed it and it works. Do you have any idea about it.

The !exact match logic in the rework is wrong indeed, sorry...

For non-exact matches we return a non-exact match too early without
giving the chance for a potentially exact half-duplex match...

As for the fix, can you try this out :

diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
index 703321689726..d80f6a37edf1 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -195,7 +195,7 @@ const struct link_capabilities *
 phy_caps_lookup(int speed, unsigned int duplex, const unsigned long *supported,
 		bool exact)
 {
-	const struct link_capabilities *lcap, *last = NULL;
+	const struct link_capabilities *lcap, *match = NULL, *last = NULL;
 
 	for_each_link_caps_desc_speed(lcap) {
 		if (linkmode_intersects(lcap->linkmodes, supported)) {
@@ -204,16 +204,19 @@ phy_caps_lookup(int speed, unsigned int duplex, const unsigned long *supported,
 			if (lcap->speed == speed && lcap->duplex == duplex) {
 				return lcap;
 			} else if (!exact) {
-				if (lcap->speed <= speed)
-					return lcap;
+				if (!match && lcap->speed <= speed)
+					match = lcap;
+
+				if (lcap->speed < speed)
+					break;
 			}
 		}
 	}
 
-	if (!exact)
-		return last;
+	if (!match && !exact)
+		match = last;
 
-	return NULL;
+	return match;
 }
 EXPORT_SYMBOL_GPL(phy_caps_lookup);

On Thu, May 29, 2025 at 05:36:11PM +0800, Jijie Shao wrote:
> Hi Maxime,  fc81e257d19f ("net: phy: phy_caps: Allow looking-up link caps based on speed and duplex") might have different behavior than the modification.
> My case is set 10M Half with disable autoneg both sides and I expect it is
> link in 10M Half. But now, it is link in 10M Full，which is not what I
> expect.
> 
> I used followed command and trace how phy worked.
> 	ethtool -s eth1 autoneg off speed 10 duplex half
> The log is showed as followed:
> ethtool-13127	[067]	6164.771853: phy_ethtool_ksettings set: (phy_ethtool ksettings set+0x0/0x200) duplex=0 speed=10
> kworker/u322:2-11096	[070]	6164.771853:	_phy_start_aneq: ( _phy_start_aneg+0x0/0xb8) duplex=0 speed=10
> kworker/u322:2-11096	[070]	6164.771854:	phy_caps_lookup: (phy_caps_lookup+0x0/0xf0) duplex=0 speed=10
> kworker/u322:2-11096	[070]	6164.771855:	phy_config_aneg: (phy_config_aneg+0x0/0x70) duplex=1 speed=10
> kworker/u322:2-11096	[070]	6164.771856:	genphy_config_aneg:	(__genphy_config_aneg+0X0/0X270) duplex=1 speed=10
> 
> I also try to fixed it and it works. Do you have any idea about it.
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 0e762fc3a529..2986c41c42a8 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -258,7 +258,7 @@ static void phy_sanitize_settings(struct phy_device *phydev)
>         const struct link_capabilities *c;
> 
>         c = phy_caps_lookup(phydev->speed, phydev->duplex, phydev->supported,
> -                           false);
> +                           true);

This isn't the correct fix, as:

+ * When @exact is not set, we return either an exact match, or matching capabilities
+ * at lower speed, or the lowest matching speed, or NULL.

So it isn't returning the exact match but apparently ignoring the
duplex.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!