netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file
@ 2025-03-03  9:03 Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 01/13] net: ethtool: Export the link_mode_params definitions Maxime Chevallier
                   ` (12 more replies)
  0 siblings, 13 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

Hello everyone,

This is V4 of the phy_caps series. In a nutshell, this series reworks the way
we maintain the list of speed/duplex capablities for each linkmode so that we
no longer have multiple definition of these associations.

That will help making sure that when people add new linkmodes in
include/uapi/linux/ethtool.h, they don't have to update phylib and phylink as
well, making the process more straightforward and less error-prone.

It also generalises the phy_caps interface to be able to lookup linkmodes
from phy_interface_t, which is needed for the multi-port work I've been working
on for a while.

This V4 addresses Russell's reviews, namely :

 - Introduce new iterators to traverse the link_caps array in ascending and
   descending order, making it more explicit and simple.
 - Remove an unneeded linkmode_zero in patch 9

V1 : https://lore.kernel.org/netdev/20250222142727.894124-1-maxime.chevallier@bootlin.com/
V2 : https://lore.kernel.org/netdev/20250226100929.1646454-1-maxime.chevallier@bootlin.com/
V3 : https://lore.kernel.org/netdev/20250228145540.2209551-1-maxime.chevallier@bootlin.com/

For context, The text below is an extract from the V1 cover :

Following the V4 of the phy_port series [1] we've discussed about attempting
to extract some of the linkmode <-> capabilities (speed/duplex) <-> interface
logic into a dedicated file, so that we can re-use that logic out of
phylink.

While trying to do that, I might have gotten a bit carried-away, and I'm
therefore submitting this series to rework the way we are currently
managing the linkmodes <-> capabilities handling.

We are currently defining all the possible Ethernet linkmodes in an
enum ethtool_link_mode_bit_indices value defined in uapi/linux/ethtool.h :

	ETHTOOL_LINK_MODE_10baseT_Half_BIT	= 0,
	ETHTOOL_LINK_MODE_10baseT_Full_BIT	= 1,
	...

Each of these modes represents a media-side link definition, and runs at
a given speed and duplex.

Specific attributes for each modes are stored in net/ethtool/common.c, as
an array of struct link_mode_info :

	struct link_mode_info {
		int				speed;
		u8				lanes;
		u8				duplex;
	}

The link_mode_params[] array is the canonical definition for these modes,
as (1) there are build-time checks to make sure any new linkmode
definition is also defined in this array and (2) this array is always
compiled-in, as it's part of the net stack (i.e. it is not phylib-specific).

This array is however not optimized for lookups, as it is not ordered in
any particular fashion (new modes go at the end, regardless of their speed
and duplex).

Phylib also includes a similar array, in the form of the phy_settings
array in drivers/net/phy/phy-core.c :

	struct phy_setting {
		u32 speed;
		u8 duplex;
		u8 bit; // The enum index for the linkmode
	};

The phy_settings array however is ordered by descending speeds. A variety
of helpers in phylib rely on that ordering to perform lookups, usually
to get one or any linkmode corresponding to a requested speed and duplex.

Finally, we have some helpers in phylink (phylink_caps_to_linkmodes) that
allows getting the list of linkmodes that match a set of speed and duplex
value, all at once.

While the phylink and phylib helpers allows for efficient lookups, they
have some drawbacks as well :

	(1) : It's easy to forget updating of all of these helpers and structures
		  when adding a new linkmode. New linkmodes are actually added fairly
		  often, lately either for slow BaseT1 flavours, or for crazy-fast
		  modes (800Gbps modes, but I guess people won't stop there)
		  
	(2) : Even though the phylink and phylib modes use carefull sorting
		  to speed-up the lookup process, the phylib lookups are usually
		  done in descending speed order and will therefore get slower
		  as people add even faster link speeds.
		  
This series introduces a new "link_capabilities" structure that is used
to build an array of link_caps :

	struct link_capabilities {
		int speed;                           
		unsigned int duplex;
		__ETHTOOL_DECLARE_LINK_MODE_MASK(linkmodes);
	};

We group these in an array, indexed with LINK_CAPA enums that are basically
identical to the phylink MAC_CAPS :

...
LINK_CAPA_1000HD,             
LINK_CAPA_1000FD,
LINK_CAPA_2500FD,
LINK_CAPA_5000FD,
...

We now have an associative array of <speed,duplex> <-> All compatible linkmodes

This array is initialized at phylib-init time based on the content of
the link_mode_params[] array from net/ethtool/common.c, that way it is
always up-to-date with new modes, and always properly ordered.

Patches 3 to 8 then convert all lookups from the phy_settings array into
lookups from this link_caps array, hopefully speeding-up lookups in the
meantime (we iterate over possible speeds instead of individual linkmodes)

This series is not meant to introduce changes in behaviour, however patches
9 and 10 do introduce functionnal changes. When configuring the advert
for speeds >= 1G in PHY devices, as well as when constructing the link
parameters for fixed links in phylink, we used to rely on phy_settings
lookups returning one, and only one, compatible linkmode. This series will
make so that the lookups will result on all matching linkmodes being
returned, and MAY cause advert/fixed-link configuring more linkmodes.

Patches 12 and 13 extract the conversion logic for interface <-> caps from
phylink.

There are cons as well for this, as this is a bit more init time for phylib,
but maybe more importantly, we lose in the precision for the lookups in
phy_settings. However, given all the uses for phy_settings (most are just
to get speed/duplex), I think this is actually ok, but any comment would
be very welcome.

This series was tested with :
 - 10/100/1000M links
 - 2,5, 5, 10G BaseT links
 - 1G Fixed link

I also made sure that this compiles with the following options :

CONFIG_PHYLIB=n

CNFIG_PHYLINK=m
CONFIG_PHYLIB=m

CNFIG_PHYLINK=m
CONFIG_PHYLIB=y

CNFIG_PHYLINK=y
CONFIG_PHYLIB=y

All the new helpers that were introduced (in drivers/net/phy/phy-caps.h)
are for internal use only (only users should be core stuff, such as phylib and
phylink, and in the future, phy_port).

Thanks,

Maxime

[1]: https://lore.kernel.org/netdev/20250213101606.1154014-1-maxime.chevallier@bootlin.com/

Maxime Chevallier (13):
  net: ethtool: Export the link_mode_params definitions
  net: phy: Use an internal, searchable storage for the linkmodes
  net: phy: phy_caps: Move phy_speeds to phy_caps
  net: phy: phy_caps: Move __set_linkmode_max_speed to phy_caps
  net: phy: phy_caps: Introduce phy_caps_valid
  net: phy: phy_caps: Implement link_capabilities lookup by linkmode
  net: phy: phy_caps: Allow looking-up link caps based on speed and
    duplex
  net: phy: phy_device: Use link_capabilities lookup for PHY aneg config
  net: phylink: Use phy_caps_lookup for fixed-link configuration
  net: phy: drop phy_settings and the associated lookup helpers
  net: phylink: Add a mapping between MAC_CAPS and LINK_CAPS
  net: phylink: Convert capabilities to linkmodes using phy_caps
  net: phy: phy_caps: Allow getting an phy_interface's capabilities

 drivers/net/phy/Makefile     |   2 +-
 drivers/net/phy/phy-caps.h   |  63 +++++++
 drivers/net/phy/phy-core.c   | 253 ++-----------------------
 drivers/net/phy/phy.c        |  37 +---
 drivers/net/phy/phy_caps.c   | 347 +++++++++++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c |  13 +-
 drivers/net/phy/phylink.c    | 338 +++++++++-------------------------
 include/linux/ethtool.h      |   8 +
 include/linux/phy.h          |  15 --
 net/ethtool/common.c         |   1 +
 net/ethtool/common.h         |   7 -
 11 files changed, 540 insertions(+), 544 deletions(-)
 create mode 100644 drivers/net/phy/phy-caps.h
 create mode 100644 drivers/net/phy/phy_caps.c

-- 
2.48.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 01/13] net: ethtool: Export the link_mode_params definitions
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 02/13] net: phy: Use an internal, searchable storage for the linkmodes Maxime Chevallier
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

link_mode_params contains a lookup table of all 802.3 link modes that
are currently supported with structured data about each mode's speed,
duplex, number of lanes and mediums.

As a preparation for a port representation, export that table for the
rest of the net stack to use.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: No changes

 include/linux/ethtool.h | 8 ++++++++
 net/ethtool/common.c    | 1 +
 net/ethtool/common.h    | 7 -------
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 7f222dccc7d1..8210ece94fa6 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -210,6 +210,14 @@ static inline u8 *ethtool_rxfh_context_key(struct ethtool_rxfh_context *ctx)
 
 void ethtool_rxfh_context_lost(struct net_device *dev, u32 context_id);
 
+struct link_mode_info {
+	int                             speed;
+	u8                              lanes;
+	u8                              duplex;
+};
+
+extern const struct link_mode_info link_mode_params[];
+
 /* declare a link mode bitmap */
 #define __ETHTOOL_DECLARE_LINK_MODE_MASK(name)		\
 	DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index ac8b6107863e..c9d6302e88c9 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -423,6 +423,7 @@ const struct link_mode_info link_mode_params[] = {
 	__DEFINE_LINK_MODE_PARAMS(800000, VR4, Full),
 };
 static_assert(ARRAY_SIZE(link_mode_params) == __ETHTOOL_LINK_MODE_MASK_NBITS);
+EXPORT_SYMBOL_GPL(link_mode_params);
 
 const char netif_msg_class_names[][ETH_GSTRING_LEN] = {
 	[NETIF_MSG_DRV_BIT]		= "drv",
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index a1088c2441d0..b4683d286a5a 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -15,12 +15,6 @@
 #define __SOF_TIMESTAMPING_CNT (const_ilog2(SOF_TIMESTAMPING_LAST) + 1)
 #define __HWTSTAMP_FLAG_CNT (const_ilog2(HWTSTAMP_FLAG_LAST) + 1)
 
-struct link_mode_info {
-	int				speed;
-	u8				lanes;
-	u8				duplex;
-};
-
 struct genl_info;
 struct hwtstamp_provider_desc;
 
@@ -33,7 +27,6 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN];
 extern const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN];
 extern const char link_mode_names[][ETH_GSTRING_LEN];
-extern const struct link_mode_info link_mode_params[];
 extern const char netif_msg_class_names[][ETH_GSTRING_LEN];
 extern const char wol_mode_names[][ETH_GSTRING_LEN];
 extern const char sof_timestamping_names[][ETH_GSTRING_LEN];
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 02/13] net: phy: Use an internal, searchable storage for the linkmodes
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 01/13] net: ethtool: Export the link_mode_params definitions Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-06  8:30   ` Paolo Abeni
  2025-03-03  9:03 ` [PATCH net-next v4 03/13] net: phy: phy_caps: Move phy_speeds to phy_caps Maxime Chevallier
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

The canonical definition for all the link modes is in linux/ethtool.h,
which is complemented by the link_mode_params array stored in
net/ethtool/common.h . That array contains all the metadata about each
of these modes, including the Speed and Duplex information.

Phylib and phylink needs that information as well for internal
management of the link, which was done by duplicating that information
in locally-stored arrays and lookup functions. This makes it easy for
developpers adding new modes to forget modifying phylib and phylink
accordingly.

However, the link_mode_params array in net/ethtool/common.c is fairly
inefficient to search through, as it isn't sorted in any manner. Phylib
and phylink perform a lot of lookup operations, mostly to filter modes
by speed and/or duplex.

We therefore introduce the link_caps private array in phy_caps.c, that
indexes linkmodes in a more efficient manner. Each element associated a
tuple <speed, duplex> to a bitfield of all the linkmodes runs at these
speed/duplex.

We end-up with an array that's fairly short, easily addressable and that
it optimised for the typical use-cases of phylib/phylink.

That array is initialized at the same time as phylib. As the
link_mode_params array is part of the net stack, which phylink depends
on, it should always be accessible from phylib.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: Ditch the __LINK_CAPA_LAST (no longer needed)

 drivers/net/phy/Makefile     |  2 +-
 drivers/net/phy/phy-caps.h   | 43 ++++++++++++++++++++
 drivers/net/phy/phy_caps.c   | 78 ++++++++++++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c |  2 +
 4 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/phy/phy-caps.h
 create mode 100644 drivers/net/phy/phy_caps.c

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index c8dac6e92278..7e800619162b 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -2,7 +2,7 @@
 # Makefile for Linux PHY drivers
 
 libphy-y			:= phy.o phy-c45.o phy-core.o phy_device.o \
-				   linkmode.o phy_link_topology.o
+				   linkmode.o phy_link_topology.o phy_caps.o
 mdio-bus-y			+= mdio_bus.o mdio_device.o
 
 ifdef CONFIG_MDIO_DEVICE
diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
new file mode 100644
index 000000000000..8a0dc2c486bb
--- /dev/null
+++ b/drivers/net/phy/phy-caps.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * link caps internal header, for link modes <-> capabilities <-> interfaces
+ * conversions.
+ */
+
+#ifndef __PHY_CAPS_H
+#define __PHY_CAPS_H
+
+#include <linux/ethtool.h>
+
+enum {
+	LINK_CAPA_10HD = 0,
+	LINK_CAPA_10FD,
+	LINK_CAPA_100HD,
+	LINK_CAPA_100FD,
+	LINK_CAPA_1000HD,
+	LINK_CAPA_1000FD,
+	LINK_CAPA_2500FD,
+	LINK_CAPA_5000FD,
+	LINK_CAPA_10000FD,
+	LINK_CAPA_20000FD,
+	LINK_CAPA_25000FD,
+	LINK_CAPA_40000FD,
+	LINK_CAPA_50000FD,
+	LINK_CAPA_56000FD,
+	LINK_CAPA_100000FD,
+	LINK_CAPA_200000FD,
+	LINK_CAPA_400000FD,
+	LINK_CAPA_800000FD,
+
+	__LINK_CAPA_MAX,
+};
+
+struct link_capabilities {
+	int speed;
+	unsigned int duplex;
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(linkmodes);
+};
+
+void phy_caps_init(void);
+
+#endif /* __PHY_CAPS_H */
diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
new file mode 100644
index 000000000000..367ca7110ddc
--- /dev/null
+++ b/drivers/net/phy/phy_caps.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/ethtool.h>
+#include <linux/linkmode.h>
+#include <linux/phy.h>
+
+#include "phy-caps.h"
+
+static struct link_capabilities link_caps[__LINK_CAPA_MAX] __ro_after_init = {
+	{ SPEED_10, DUPLEX_HALF, {0} }, /* LINK_CAPA_10HD */
+	{ SPEED_10, DUPLEX_FULL, {0} }, /* LINK_CAPA_10FD */
+	{ SPEED_100, DUPLEX_HALF, {0} }, /* LINK_CAPA_100HD */
+	{ SPEED_100, DUPLEX_FULL, {0} }, /* LINK_CAPA_100FD */
+	{ SPEED_1000, DUPLEX_HALF, {0} }, /* LINK_CAPA_1000HD */
+	{ SPEED_1000, DUPLEX_FULL, {0} }, /* LINK_CAPA_1000FD */
+	{ SPEED_2500, DUPLEX_FULL, {0} }, /* LINK_CAPA_2500FD */
+	{ SPEED_5000, DUPLEX_FULL, {0} }, /* LINK_CAPA_5000FD */
+	{ SPEED_10000, DUPLEX_FULL, {0} }, /* LINK_CAPA_10000FD */
+	{ SPEED_20000, DUPLEX_FULL, {0} }, /* LINK_CAPA_20000FD */
+	{ SPEED_25000, DUPLEX_FULL, {0} }, /* LINK_CAPA_25000FD */
+	{ SPEED_40000, DUPLEX_FULL, {0} }, /* LINK_CAPA_40000FD */
+	{ SPEED_50000, DUPLEX_FULL, {0} }, /* LINK_CAPA_50000FD */
+	{ SPEED_56000, DUPLEX_FULL, {0} }, /* LINK_CAPA_56000FD */
+	{ SPEED_100000, DUPLEX_FULL, {0} }, /* LINK_CAPA_100000FD */
+	{ SPEED_200000, DUPLEX_FULL, {0} }, /* LINK_CAPA_200000FD */
+	{ SPEED_400000, DUPLEX_FULL, {0} }, /* LINK_CAPA_400000FD */
+	{ SPEED_800000, DUPLEX_FULL, {0} }, /* LINK_CAPA_800000FD */
+};
+
+static int speed_duplex_to_capa(int speed, unsigned int duplex)
+{
+	if (duplex == DUPLEX_UNKNOWN ||
+	    (speed > SPEED_1000 && duplex != DUPLEX_FULL))
+		return -EINVAL;
+
+	switch (speed) {
+	case SPEED_10: return duplex == DUPLEX_FULL ?
+			      LINK_CAPA_10FD : LINK_CAPA_10HD;
+	case SPEED_100: return duplex == DUPLEX_FULL ?
+			       LINK_CAPA_100FD : LINK_CAPA_100HD;
+	case SPEED_1000: return duplex == DUPLEX_FULL ?
+				LINK_CAPA_1000FD : LINK_CAPA_1000HD;
+	case SPEED_2500: return LINK_CAPA_2500FD;
+	case SPEED_5000: return LINK_CAPA_5000FD;
+	case SPEED_10000: return LINK_CAPA_10000FD;
+	case SPEED_20000: return LINK_CAPA_20000FD;
+	case SPEED_25000: return LINK_CAPA_25000FD;
+	case SPEED_40000: return LINK_CAPA_40000FD;
+	case SPEED_50000: return LINK_CAPA_50000FD;
+	case SPEED_56000: return LINK_CAPA_56000FD;
+	case SPEED_100000: return LINK_CAPA_100000FD;
+	case SPEED_200000: return LINK_CAPA_200000FD;
+	case SPEED_400000: return LINK_CAPA_400000FD;
+	case SPEED_800000: return LINK_CAPA_800000FD;
+	}
+
+	return -EINVAL;
+}
+
+/**
+ * phy_caps_init() - Initializes the link_caps array from the link_mode_params.
+ */
+void phy_caps_init(void)
+{
+	const struct link_mode_info *linkmode;
+	int i, capa;
+
+	/* Fill the caps array from net/ethtool/common.c */
+	for (i = 0; i < __ETHTOOL_LINK_MODE_MASK_NBITS; i++) {
+		linkmode = &link_mode_params[i];
+		capa = speed_duplex_to_capa(linkmode->speed, linkmode->duplex);
+
+		if (capa < 0)
+			continue;
+
+		__set_bit(i, link_caps[capa].linkmodes);
+	}
+}
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index a38d399f244b..9c573555ac49 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -42,6 +42,7 @@
 #include <linux/unistd.h>
 
 #include "phylib-internal.h"
+#include "phy-caps.h"
 
 MODULE_DESCRIPTION("PHY library");
 MODULE_AUTHOR("Andy Fleming");
@@ -3795,6 +3796,7 @@ static int __init phy_init(void)
 	if (rc)
 		goto err_ethtool_phy_ops;
 
+	phy_caps_init();
 	features_init();
 
 	rc = phy_driver_register(&genphy_c45_driver, THIS_MODULE);
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 03/13] net: phy: phy_caps: Move phy_speeds to phy_caps
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 01/13] net: ethtool: Export the link_mode_params definitions Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 02/13] net: phy: Use an internal, searchable storage for the linkmodes Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 04/13] net: phy: phy_caps: Move __set_linkmode_max_speed " Maxime Chevallier
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

Use the newly introduced link_capabilities array to derive the list of
possible speeds when given a combination of linkmodes. As
link_capabilities is indexed by speed, we don't have to iterate the
whole phy_settings array.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: Switch to macro iterators for better readability (Russell)

 drivers/net/phy/phy-caps.h |  3 +++
 drivers/net/phy/phy-core.c | 15 ---------------
 drivers/net/phy/phy.c      |  3 ++-
 drivers/net/phy/phy_caps.c | 33 +++++++++++++++++++++++++++++++++
 include/linux/phy.h        |  2 --
 5 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
index 8a0dc2c486bb..52e2fb8cabab 100644
--- a/drivers/net/phy/phy-caps.h
+++ b/drivers/net/phy/phy-caps.h
@@ -40,4 +40,7 @@ struct link_capabilities {
 
 void phy_caps_init(void);
 
+size_t phy_caps_speeds(unsigned int *speeds, size_t size,
+		       unsigned long *linkmodes);
+
 #endif /* __PHY_CAPS_H */
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index b1c1670de23b..8533e57c3500 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -339,21 +339,6 @@ phy_lookup_setting(int speed, int duplex, const unsigned long *mask, bool exact)
 }
 EXPORT_SYMBOL_GPL(phy_lookup_setting);
 
-size_t phy_speeds(unsigned int *speeds, size_t size,
-		  unsigned long *mask)
-{
-	size_t count;
-	int i;
-
-	for (i = 0, count = 0; i < ARRAY_SIZE(settings) && count < size; i++)
-		if (settings[i].bit < __ETHTOOL_LINK_MODE_MASK_NBITS &&
-		    test_bit(settings[i].bit, mask) &&
-		    (count == 0 || speeds[count - 1] != settings[i].speed))
-			speeds[count++] = settings[i].speed;
-
-	return count;
-}
-
 static void __set_linkmode_max_speed(u32 max_speed, unsigned long *addr)
 {
 	const struct phy_setting *p;
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 16ffc00b419c..3128df03feda 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -37,6 +37,7 @@
 #include <net/sock.h>
 
 #include "phylib-internal.h"
+#include "phy-caps.h"
 
 #define PHY_STATE_TIME	HZ
 
@@ -245,7 +246,7 @@ unsigned int phy_supported_speeds(struct phy_device *phy,
 				  unsigned int *speeds,
 				  unsigned int size)
 {
-	return phy_speeds(speeds, size, phy->supported);
+	return phy_caps_speeds(speeds, size, phy->supported);
 }
 
 /**
diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
index 367ca7110ddc..827ded2729d8 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -57,6 +57,9 @@ static int speed_duplex_to_capa(int speed, unsigned int duplex)
 	return -EINVAL;
 }
 
+#define for_each_link_caps_asc_speed(cap) \
+	for (cap = link_caps; cap < &link_caps[__LINK_CAPA_MAX]; cap++)
+
 /**
  * phy_caps_init() - Initializes the link_caps array from the link_mode_params.
  */
@@ -76,3 +79,33 @@ void phy_caps_init(void)
 		__set_bit(i, link_caps[capa].linkmodes);
 	}
 }
+
+/**
+ * phy_caps_speeds() - Fill an array of supported SPEED_* values for given modes
+ * @speeds: Output array to store the speeds list into
+ * @size: Size of the output array
+ * @linkmodes: Linkmodes to get the speeds from
+ *
+ * Fills the speeds array with all possible speeds that can be achieved with
+ * the specified linkmodes.
+ *
+ * Returns: The number of speeds filled into the array. If the input array isn't
+ *	    big enough to store all speeds, fill it as much as possible.
+ */
+size_t phy_caps_speeds(unsigned int *speeds, size_t size,
+		       unsigned long *linkmodes)
+{
+	struct link_capabilities *lcap;
+	size_t count = 0;
+
+	for_each_link_caps_asc_speed(lcap) {
+		if (linkmode_intersects(lcap->linkmodes, linkmodes) &&
+		    (count == 0 || speeds[count - 1] != lcap->speed)) {
+			speeds[count++] = lcap->speed;
+			if (count >= size)
+				break;
+		}
+	}
+
+	return count;
+}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7bfbae51070a..5749aad96862 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1322,8 +1322,6 @@ struct phy_setting {
 const struct phy_setting *
 phy_lookup_setting(int speed, int duplex, const unsigned long *mask,
 		   bool exact);
-size_t phy_speeds(unsigned int *speeds, size_t size,
-		  unsigned long *mask);
 
 /**
  * phy_is_started - Convenience function to check whether PHY is started
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 04/13] net: phy: phy_caps: Move __set_linkmode_max_speed to phy_caps
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
                   ` (2 preceding siblings ...)
  2025-03-03  9:03 ` [PATCH net-next v4 03/13] net: phy: phy_caps: Move phy_speeds to phy_caps Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 05/13] net: phy: phy_caps: Introduce phy_caps_valid Maxime Chevallier
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

Convert the __set_linkmode_max_speed to use the link_capabilities array.
This makes it easy to clamp the linkmodes to a given max speed.
Introduce a new helper phy_caps_linkmode_max_speed to replace the
previous one that used phy_settings.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: Switch to macro iterators for better readability (Russell)

 drivers/net/phy/phy-caps.h |  2 ++
 drivers/net/phy/phy-core.c | 18 +++---------------
 drivers/net/phy/phy_caps.c | 19 +++++++++++++++++++
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
index 52e2fb8cabab..a44d983eaed8 100644
--- a/drivers/net/phy/phy-caps.h
+++ b/drivers/net/phy/phy-caps.h
@@ -42,5 +42,7 @@ void phy_caps_init(void);
 
 size_t phy_caps_speeds(unsigned int *speeds, size_t size,
 		       unsigned long *linkmodes);
+void phy_caps_linkmode_max_speed(u32 max_speed, unsigned long *linkmodes);
+
 
 #endif /* __PHY_CAPS_H */
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 8533e57c3500..f62bc1be67b2 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -7,6 +7,7 @@
 #include <linux/of.h>
 
 #include "phylib-internal.h"
+#include "phy-caps.h"
 
 /**
  * phy_speed_to_str - Return a string representing the PHY link speed
@@ -339,22 +340,9 @@ phy_lookup_setting(int speed, int duplex, const unsigned long *mask, bool exact)
 }
 EXPORT_SYMBOL_GPL(phy_lookup_setting);
 
-static void __set_linkmode_max_speed(u32 max_speed, unsigned long *addr)
-{
-	const struct phy_setting *p;
-	int i;
-
-	for (i = 0, p = settings; i < ARRAY_SIZE(settings); i++, p++) {
-		if (p->speed > max_speed)
-			linkmode_clear_bit(p->bit, addr);
-		else
-			break;
-	}
-}
-
 static void __set_phy_supported(struct phy_device *phydev, u32 max_speed)
 {
-	__set_linkmode_max_speed(max_speed, phydev->supported);
+	phy_caps_linkmode_max_speed(max_speed, phydev->supported);
 }
 
 /**
@@ -557,7 +545,7 @@ int phy_speed_down_core(struct phy_device *phydev)
 	if (min_common_speed == SPEED_UNKNOWN)
 		return -EINVAL;
 
-	__set_linkmode_max_speed(min_common_speed, phydev->advertising);
+	phy_caps_linkmode_max_speed(min_common_speed, phydev->advertising);
 
 	return 0;
 }
diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
index 827ded2729d8..132758d2eb8d 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -60,6 +60,9 @@ static int speed_duplex_to_capa(int speed, unsigned int duplex)
 #define for_each_link_caps_asc_speed(cap) \
 	for (cap = link_caps; cap < &link_caps[__LINK_CAPA_MAX]; cap++)
 
+#define for_each_link_caps_desc_speed(cap) \
+	for (cap = &link_caps[__LINK_CAPA_MAX - 1]; cap >= link_caps; cap--)
+
 /**
  * phy_caps_init() - Initializes the link_caps array from the link_mode_params.
  */
@@ -109,3 +112,19 @@ size_t phy_caps_speeds(unsigned int *speeds, size_t size,
 
 	return count;
 }
+
+/**
+ * phy_caps_linkmode_max_speed() - Clamp a linkmodes set to a max speed
+ * @max_speed: Speed limit for the linkmode set
+ * @linkmodes: Linkmodes to limit
+ */
+void phy_caps_linkmode_max_speed(u32 max_speed, unsigned long *linkmodes)
+{
+	struct link_capabilities *lcap;
+
+	for_each_link_caps_desc_speed(lcap)
+		if (lcap->speed > max_speed)
+			linkmode_andnot(linkmodes, linkmodes, lcap->linkmodes);
+		else
+			break;
+}
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 05/13] net: phy: phy_caps: Introduce phy_caps_valid
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
                   ` (3 preceding siblings ...)
  2025-03-03  9:03 ` [PATCH net-next v4 04/13] net: phy: phy_caps: Move __set_linkmode_max_speed " Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 06/13] net: phy: phy_caps: Implement link_capabilities lookup by linkmode Maxime Chevallier
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

With the link_capabilities array, it's trivial to validate a given mask
againts a <speed, duplex> tuple. Create a helper for that purpose, and
use it to replace a phy_settings lookup in phy_check_valid();

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: No changes

 drivers/net/phy/phy-caps.h |  1 +
 drivers/net/phy/phy.c      |  2 +-
 drivers/net/phy/phy_caps.c | 19 +++++++++++++++++++
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
index a44d983eaed8..f35ede4e557d 100644
--- a/drivers/net/phy/phy-caps.h
+++ b/drivers/net/phy/phy-caps.h
@@ -43,6 +43,7 @@ void phy_caps_init(void);
 size_t phy_caps_speeds(unsigned int *speeds, size_t size,
 		       unsigned long *linkmodes);
 void phy_caps_linkmode_max_speed(u32 max_speed, unsigned long *linkmodes);
+bool phy_caps_valid(int speed, int duplex, const unsigned long *linkmodes);
 
 
 #endif /* __PHY_CAPS_H */
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 3128df03feda..8df37d221fba 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -260,7 +260,7 @@ unsigned int phy_supported_speeds(struct phy_device *phy,
  */
 bool phy_check_valid(int speed, int duplex, unsigned long *features)
 {
-	return !!phy_lookup_setting(speed, duplex, features, true);
+	return phy_caps_valid(speed, duplex, features);
 }
 EXPORT_SYMBOL(phy_check_valid);
 
diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
index 132758d2eb8d..8ce7dca1acd0 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -128,3 +128,22 @@ void phy_caps_linkmode_max_speed(u32 max_speed, unsigned long *linkmodes)
 		else
 			break;
 }
+
+/**
+ * phy_caps_valid() - Validate a linkmodes set agains given speed and duplex
+ * @speed: input speed to validate
+ * @duplex: input duplex to validate. Passing DUPLEX_UNKNOWN is always not valid
+ * @linkmodes: The linkmodes to validate
+ *
+ * Returns: True if at least one of the linkmodes in @linkmodes can function at
+ *          the given speed and duplex, false otherwise.
+ */
+bool phy_caps_valid(int speed, int duplex, const unsigned long *linkmodes)
+{
+	int capa = speed_duplex_to_capa(speed, duplex);
+
+	if (capa < 0)
+		return false;
+
+	return linkmode_intersects(link_caps[capa].linkmodes, linkmodes);
+}
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 06/13] net: phy: phy_caps: Implement link_capabilities lookup by linkmode
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
                   ` (4 preceding siblings ...)
  2025-03-03  9:03 ` [PATCH net-next v4 05/13] net: phy: phy_caps: Introduce phy_caps_valid Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 07/13] net: phy: phy_caps: Allow looking-up link caps based on speed and duplex Maxime Chevallier
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

In several occasions, phylib needs to lookup a set of matching speed and
duplex against a given linkmode set. Instead of relying on the
phy_settings array and thus iterate over the whole linkmodes list, use
the link_capabilities array to lookup these matches, as we aren't
interested in the actual link setting that matches but rather the speed
and duplex for that setting.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: Switch to macro iterators for better readability (Russell)

 drivers/net/phy/phy-caps.h |  5 +++++
 drivers/net/phy/phy-core.c | 36 +++++++++++++-----------------
 drivers/net/phy/phy_caps.c | 45 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 21 deletions(-)

diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
index f35ede4e557d..7103cf508d7e 100644
--- a/drivers/net/phy/phy-caps.h
+++ b/drivers/net/phy/phy-caps.h
@@ -45,5 +45,10 @@ size_t phy_caps_speeds(unsigned int *speeds, size_t size,
 void phy_caps_linkmode_max_speed(u32 max_speed, unsigned long *linkmodes);
 bool phy_caps_valid(int speed, int duplex, const unsigned long *linkmodes);
 
+const struct link_capabilities *
+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);
 
 #endif /* __PHY_CAPS_H */
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index f62bc1be67b2..6cb8f857a7f1 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -468,16 +468,15 @@ EXPORT_SYMBOL_GPL(phy_resolve_aneg_pause);
 void phy_resolve_aneg_linkmode(struct phy_device *phydev)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
-	int i;
+	const struct link_capabilities *c;
 
 	linkmode_and(common, phydev->lp_advertising, phydev->advertising);
 
-	for (i = 0; i < ARRAY_SIZE(settings); i++)
-		if (test_bit(settings[i].bit, common)) {
-			phydev->speed = settings[i].speed;
-			phydev->duplex = settings[i].duplex;
-			break;
-		}
+	c = phy_caps_lookup_by_linkmode(common);
+	if (c) {
+		phydev->speed = c->speed;
+		phydev->duplex = c->duplex;
+	}
 
 	phy_resolve_aneg_pause(phydev);
 }
@@ -495,7 +494,8 @@ EXPORT_SYMBOL_GPL(phy_resolve_aneg_linkmode);
 void phy_check_downshift(struct phy_device *phydev)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
-	int i, speed = SPEED_UNKNOWN;
+	const struct link_capabilities *c;
+	int speed = SPEED_UNKNOWN;
 
 	phydev->downshifted_rate = 0;
 
@@ -505,11 +505,9 @@ void phy_check_downshift(struct phy_device *phydev)
 
 	linkmode_and(common, phydev->lp_advertising, phydev->advertising);
 
-	for (i = 0; i < ARRAY_SIZE(settings); i++)
-		if (test_bit(settings[i].bit, common)) {
-			speed = settings[i].speed;
-			break;
-		}
+	c = phy_caps_lookup_by_linkmode(common);
+	if (c)
+		speed = c->speed;
 
 	if (speed == SPEED_UNKNOWN || phydev->speed >= speed)
 		return;
@@ -523,17 +521,13 @@ void phy_check_downshift(struct phy_device *phydev)
 static int phy_resolve_min_speed(struct phy_device *phydev, bool fdx_only)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
-	int i = ARRAY_SIZE(settings);
+	const struct link_capabilities *c;
 
 	linkmode_and(common, phydev->lp_advertising, phydev->advertising);
 
-	while (--i >= 0) {
-		if (test_bit(settings[i].bit, common)) {
-			if (fdx_only && settings[i].duplex != DUPLEX_FULL)
-				continue;
-			return settings[i].speed;
-		}
-	}
+	c = phy_caps_lookup_by_linkmode_rev(common, fdx_only);
+	if (c)
+		return c->speed;
 
 	return SPEED_UNKNOWN;
 }
diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
index 8ce7dca1acd0..8160cb53b5ae 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -113,6 +113,51 @@ size_t phy_caps_speeds(unsigned int *speeds, size_t size,
 	return count;
 }
 
+/**
+ * phy_caps_lookup_by_linkmode() - Lookup the fastest matching link_capabilities
+ * @linkmodes: Linkmodes to match against
+ *
+ * Returns: The highest-speed link_capabilities that intersects the given
+ *	    linkmodes. In case several DUPLEX_ options exist at that speed,
+ *	    DUPLEX_FULL is matched first. NULL is returned if no match.
+ */
+const struct link_capabilities *
+phy_caps_lookup_by_linkmode(const unsigned long *linkmodes)
+{
+	struct link_capabilities *lcap;
+
+	for_each_link_caps_desc_speed(lcap)
+		if (linkmode_intersects(lcap->linkmodes, linkmodes))
+			return lcap;
+
+	return NULL;
+}
+
+/**
+ * phy_caps_lookup_by_linkmode_rev() - Lookup the slowest matching link_capabilities
+ * @linkmodes: Linkmodes to match against
+ * @fdx_only: Full duplex match only when set
+ *
+ * Returns: The lowest-speed link_capabilities that intersects the given
+ *	    linkmodes. When set, fdx_only will ignore half-duplex matches.
+ *	    NULL is returned if no match.
+ */
+const struct link_capabilities *
+phy_caps_lookup_by_linkmode_rev(const unsigned long *linkmodes, bool fdx_only)
+{
+	struct link_capabilities *lcap;
+
+	for_each_link_caps_asc_speed(lcap) {
+		if (fdx_only && lcap->duplex != DUPLEX_FULL)
+			continue;
+
+		if (linkmode_intersects(lcap->linkmodes, linkmodes))
+			return lcap;
+	}
+
+	return NULL;
+}
+
 /**
  * phy_caps_linkmode_max_speed() - Clamp a linkmodes set to a max speed
  * @max_speed: Speed limit for the linkmode set
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 07/13] net: phy: phy_caps: Allow looking-up link caps based on speed and duplex
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
                   ` (5 preceding siblings ...)
  2025-03-03  9:03 ` [PATCH net-next v4 06/13] net: phy: phy_caps: Implement link_capabilities lookup by linkmode Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 08/13] net: phy: phy_device: Use link_capabilities lookup for PHY aneg config Maxime Chevallier
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

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>
---
V4: Switch to macro iterators for better readability (Russell)

 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 7103cf508d7e..8ae8dc42494f 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 8160cb53b5ae..8b90fb6d6b15 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -158,6 +158,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 a3b186ab3854..6c67d5c9b787 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 08/13] net: phy: phy_device: Use link_capabilities lookup for PHY aneg config
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
                   ` (6 preceding siblings ...)
  2025-03-03  9:03 ` [PATCH net-next v4 07/13] net: phy: phy_caps: Allow looking-up link caps based on speed and duplex Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration Maxime Chevallier
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

When configuring PHY advertising with autoneg disabled, we lookd for an
exact linkmode to advertise and configure for the requested Speed and
Duplex, specially at or over 1G.

Using phy_caps_lookup allows us to build a list of the supported
linkmodes at that speed that we can advertise instead of the first mode
that matches.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: No changes

 drivers/net/phy/phy_device.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9c573555ac49..57b90ec6477e 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2357,7 +2357,7 @@ EXPORT_SYMBOL(genphy_check_and_restart_aneg);
 int __genphy_config_aneg(struct phy_device *phydev, bool changed)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(fixed_advert);
-	const struct phy_setting *set;
+	const struct link_capabilities *c;
 	unsigned long *advert;
 	int err;
 
@@ -2383,10 +2383,11 @@ int __genphy_config_aneg(struct phy_device *phydev, bool changed)
 	} else {
 		linkmode_zero(fixed_advert);
 
-		set = phy_lookup_setting(phydev->speed, phydev->duplex,
-					 phydev->supported, true);
-		if (set)
-			linkmode_set_bit(set->bit, fixed_advert);
+		c = phy_caps_lookup(phydev->speed, phydev->duplex,
+				    phydev->supported, true);
+		if (c)
+			linkmode_and(fixed_advert, phydev->supported,
+				     c->linkmodes);
 
 		advert = fixed_advert;
 	}
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
                   ` (7 preceding siblings ...)
  2025-03-03  9:03 ` [PATCH net-next v4 08/13] net: phy: phy_device: Use link_capabilities lookup for PHY aneg config Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-04 14:43   ` Maxime Chevallier
  2025-03-06  8:56   ` Paolo Abeni
  2025-03-03  9:03 ` [PATCH net-next v4 10/13] net: phy: drop phy_settings and the associated lookup helpers Maxime Chevallier
                   ` (3 subsequent siblings)
  12 siblings, 2 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

When phylink creates a fixed-link configuration, it finds a matching
linkmode to set as the advertised, lp_advertising and supported modes
based on the speed and duplex of the fixed link.

Use the newly introduced phy_caps_lookup to get these modes instead of
phy_lookup_settings(). This has the side effect that the matched
settings and configured linkmodes may now contain several linkmodes (the
intersection of supported linkmodes from the phylink settings and the
linkmodes that match speed/duplex) instead of the one from
phy_lookup_settings().

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: Remove unnecessary linklmode_zero in phylink_set_fixed_link(),
follwing Russell's comment

 drivers/net/phy/phylink.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 6c67d5c9b787..0b9585cb508e 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -805,9 +805,10 @@ static int phylink_validate(struct phylink *pl, unsigned long *supported,
 static int phylink_parse_fixedlink(struct phylink *pl,
 				   const struct fwnode_handle *fwnode)
 {
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(match) = { 0, };
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+	const struct link_capabilities *c;
 	struct fwnode_handle *fixed_node;
-	const struct phy_setting *s;
 	struct gpio_desc *desc;
 	u32 speed;
 	int ret;
@@ -879,8 +880,10 @@ static int phylink_parse_fixedlink(struct phylink *pl,
 	linkmode_copy(pl->link_config.advertising, pl->supported);
 	phylink_validate(pl, pl->supported, &pl->link_config);
 
-	s = phy_lookup_setting(pl->link_config.speed, pl->link_config.duplex,
-			       pl->supported, true);
+	c = phy_caps_lookup(pl->link_config.speed, pl->link_config.duplex,
+			    pl->supported, true);
+	if (c)
+		linkmode_and(match, pl->supported, c->linkmodes);
 
 	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, mask);
 	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, mask);
@@ -889,9 +892,10 @@ static int phylink_parse_fixedlink(struct phylink *pl,
 
 	phylink_set(pl->supported, MII);
 
-	if (s) {
-		__set_bit(s->bit, pl->supported);
-		__set_bit(s->bit, pl->link_config.lp_advertising);
+	if (c) {
+		linkmode_or(pl->supported, pl->supported, match);
+		linkmode_or(pl->link_config.lp_advertising,
+			    pl->link_config.lp_advertising, match);
 	} else {
 		phylink_warn(pl, "fixed link %s duplex %dMbps not recognised\n",
 			     pl->link_config.duplex == DUPLEX_FULL ? "full" : "half",
@@ -1879,21 +1883,20 @@ static int phylink_register_sfp(struct phylink *pl,
 int phylink_set_fixed_link(struct phylink *pl,
 			   const struct phylink_link_state *state)
 {
-	const struct phy_setting *s;
+	const struct link_capabilities *c;
 	unsigned long *adv;
 
 	if (pl->cfg_link_an_mode != MLO_AN_PHY || !state ||
 	    !test_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state))
 		return -EINVAL;
 
-	s = phy_lookup_setting(state->speed, state->duplex,
-			       pl->supported, true);
-	if (!s)
+	c = phy_caps_lookup(state->speed, state->duplex,
+			    pl->supported, true);
+	if (!c)
 		return -EINVAL;
 
 	adv = pl->link_config.advertising;
-	linkmode_zero(adv);
-	linkmode_set_bit(s->bit, adv);
+	linkmode_and(adv, pl->supported, c->linkmodes);
 	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, adv);
 
 	pl->link_config.speed = state->speed;
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 10/13] net: phy: drop phy_settings and the associated lookup helpers
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
                   ` (8 preceding siblings ...)
  2025-03-03  9:03 ` [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 11/13] net: phylink: Add a mapping between MAC_CAPS and LINK_CAPS Maxime Chevallier
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

The phy_settings array is no longer relevant as it has now been replaced
by the link_caps array and associated phy_caps helpers.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: No changes

 drivers/net/phy/phy-core.c | 184 -------------------------------------
 include/linux/phy.h        |  13 ---
 2 files changed, 197 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 6cb8f857a7f1..e7d0137abd48 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -156,190 +156,6 @@ int phy_interface_num_ports(phy_interface_t interface)
 }
 EXPORT_SYMBOL_GPL(phy_interface_num_ports);
 
-/* A mapping of all SUPPORTED settings to speed/duplex.  This table
- * must be grouped by speed and sorted in descending match priority
- * - iow, descending speed.
- */
-
-#define PHY_SETTING(s, d, b) { .speed = SPEED_ ## s, .duplex = DUPLEX_ ## d, \
-			       .bit = ETHTOOL_LINK_MODE_ ## b ## _BIT}
-
-static const struct phy_setting settings[] = {
-	/* 800G */
-	PHY_SETTING( 800000, FULL, 800000baseCR8_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseKR8_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseDR8_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseDR8_2_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseSR8_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseVR8_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseCR4_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseKR4_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseDR4_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseDR4_2_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseSR4_Full		),
-	PHY_SETTING( 800000, FULL, 800000baseVR4_Full		),
-	/* 400G */
-	PHY_SETTING( 400000, FULL, 400000baseCR8_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseKR8_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseLR8_ER8_FR8_Full	),
-	PHY_SETTING( 400000, FULL, 400000baseDR8_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseSR8_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseCR4_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseKR4_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseLR4_ER4_FR4_Full	),
-	PHY_SETTING( 400000, FULL, 400000baseDR4_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseSR4_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseCR2_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseKR2_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseDR2_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseDR2_2_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseSR2_Full		),
-	PHY_SETTING( 400000, FULL, 400000baseVR2_Full		),
-	/* 200G */
-	PHY_SETTING( 200000, FULL, 200000baseCR4_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseKR4_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseLR4_ER4_FR4_Full	),
-	PHY_SETTING( 200000, FULL, 200000baseDR4_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseSR4_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseCR2_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseKR2_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseLR2_ER2_FR2_Full	),
-	PHY_SETTING( 200000, FULL, 200000baseDR2_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseSR2_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseCR_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseKR_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseDR_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseDR_2_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseSR_Full		),
-	PHY_SETTING( 200000, FULL, 200000baseVR_Full		),
-	/* 100G */
-	PHY_SETTING( 100000, FULL, 100000baseCR4_Full		),
-	PHY_SETTING( 100000, FULL, 100000baseKR4_Full		),
-	PHY_SETTING( 100000, FULL, 100000baseLR4_ER4_Full	),
-	PHY_SETTING( 100000, FULL, 100000baseSR4_Full		),
-	PHY_SETTING( 100000, FULL, 100000baseCR2_Full		),
-	PHY_SETTING( 100000, FULL, 100000baseKR2_Full		),
-	PHY_SETTING( 100000, FULL, 100000baseLR2_ER2_FR2_Full	),
-	PHY_SETTING( 100000, FULL, 100000baseDR2_Full		),
-	PHY_SETTING( 100000, FULL, 100000baseSR2_Full		),
-	PHY_SETTING( 100000, FULL, 100000baseCR_Full		),
-	PHY_SETTING( 100000, FULL, 100000baseKR_Full		),
-	PHY_SETTING( 100000, FULL, 100000baseLR_ER_FR_Full	),
-	PHY_SETTING( 100000, FULL, 100000baseDR_Full		),
-	PHY_SETTING( 100000, FULL, 100000baseSR_Full		),
-	/* 56G */
-	PHY_SETTING(  56000, FULL,  56000baseCR4_Full	  	),
-	PHY_SETTING(  56000, FULL,  56000baseKR4_Full	  	),
-	PHY_SETTING(  56000, FULL,  56000baseLR4_Full	  	),
-	PHY_SETTING(  56000, FULL,  56000baseSR4_Full	  	),
-	/* 50G */
-	PHY_SETTING(  50000, FULL,  50000baseCR2_Full		),
-	PHY_SETTING(  50000, FULL,  50000baseKR2_Full		),
-	PHY_SETTING(  50000, FULL,  50000baseSR2_Full		),
-	PHY_SETTING(  50000, FULL,  50000baseCR_Full		),
-	PHY_SETTING(  50000, FULL,  50000baseKR_Full		),
-	PHY_SETTING(  50000, FULL,  50000baseLR_ER_FR_Full	),
-	PHY_SETTING(  50000, FULL,  50000baseDR_Full		),
-	PHY_SETTING(  50000, FULL,  50000baseSR_Full		),
-	/* 40G */
-	PHY_SETTING(  40000, FULL,  40000baseCR4_Full		),
-	PHY_SETTING(  40000, FULL,  40000baseKR4_Full		),
-	PHY_SETTING(  40000, FULL,  40000baseLR4_Full		),
-	PHY_SETTING(  40000, FULL,  40000baseSR4_Full		),
-	/* 25G */
-	PHY_SETTING(  25000, FULL,  25000baseCR_Full		),
-	PHY_SETTING(  25000, FULL,  25000baseKR_Full		),
-	PHY_SETTING(  25000, FULL,  25000baseSR_Full		),
-	/* 20G */
-	PHY_SETTING(  20000, FULL,  20000baseKR2_Full		),
-	PHY_SETTING(  20000, FULL,  20000baseMLD2_Full		),
-	/* 10G */
-	PHY_SETTING(  10000, FULL,  10000baseCR_Full		),
-	PHY_SETTING(  10000, FULL,  10000baseER_Full		),
-	PHY_SETTING(  10000, FULL,  10000baseKR_Full		),
-	PHY_SETTING(  10000, FULL,  10000baseKX4_Full		),
-	PHY_SETTING(  10000, FULL,  10000baseLR_Full		),
-	PHY_SETTING(  10000, FULL,  10000baseLRM_Full		),
-	PHY_SETTING(  10000, FULL,  10000baseR_FEC		),
-	PHY_SETTING(  10000, FULL,  10000baseSR_Full		),
-	PHY_SETTING(  10000, FULL,  10000baseT_Full		),
-	/* 5G */
-	PHY_SETTING(   5000, FULL,   5000baseT_Full		),
-	/* 2.5G */
-	PHY_SETTING(   2500, FULL,   2500baseT_Full		),
-	PHY_SETTING(   2500, FULL,   2500baseX_Full		),
-	/* 1G */
-	PHY_SETTING(   1000, FULL,   1000baseT_Full		),
-	PHY_SETTING(   1000, HALF,   1000baseT_Half		),
-	PHY_SETTING(   1000, FULL,   1000baseT1_Full		),
-	PHY_SETTING(   1000, FULL,   1000baseX_Full		),
-	PHY_SETTING(   1000, FULL,   1000baseKX_Full		),
-	/* 100M */
-	PHY_SETTING(    100, FULL,    100baseT_Full		),
-	PHY_SETTING(    100, FULL,    100baseT1_Full		),
-	PHY_SETTING(    100, HALF,    100baseT_Half		),
-	PHY_SETTING(    100, HALF,    100baseFX_Half		),
-	PHY_SETTING(    100, FULL,    100baseFX_Full		),
-	/* 10M */
-	PHY_SETTING(     10, FULL,     10baseT_Full		),
-	PHY_SETTING(     10, HALF,     10baseT_Half		),
-	PHY_SETTING(     10, FULL,     10baseT1L_Full		),
-	PHY_SETTING(     10, FULL,     10baseT1S_Full		),
-	PHY_SETTING(     10, HALF,     10baseT1S_Half		),
-	PHY_SETTING(     10, HALF,     10baseT1S_P2MP_Half	),
-	PHY_SETTING(     10, FULL,     10baseT1BRR_Full		),
-};
-#undef PHY_SETTING
-
-/**
- * phy_lookup_setting - lookup a PHY setting
- * @speed: speed to match
- * @duplex: duplex to match
- * @mask: allowed link modes
- * @exact: an exact match is required
- *
- * Search the settings array for a setting that matches the speed and
- * duplex, and which is supported.
- *
- * If @exact is unset, either an exact match or %NULL for no match will
- * be returned.
- *
- * If @exact is set, an exact match, the fastest supported setting at
- * or below the specified speed, the slowest supported setting, or if
- * they all fail, %NULL will be returned.
- */
-const struct phy_setting *
-phy_lookup_setting(int speed, int duplex, const unsigned long *mask, bool exact)
-{
-	const struct phy_setting *p, *match = NULL, *last = NULL;
-	int i;
-
-	for (i = 0, p = settings; i < ARRAY_SIZE(settings); i++, p++) {
-		if (p->bit < __ETHTOOL_LINK_MODE_MASK_NBITS &&
-		    test_bit(p->bit, mask)) {
-			last = p;
-			if (p->speed == speed && p->duplex == duplex) {
-				/* Exact match for speed and duplex */
-				match = p;
-				break;
-			} else if (!exact) {
-				if (!match && p->speed <= speed)
-					/* Candidate */
-					match = p;
-
-				if (p->speed < speed)
-					break;
-			}
-		}
-	}
-
-	if (!match && !exact)
-		match = last;
-
-	return match;
-}
-EXPORT_SYMBOL_GPL(phy_lookup_setting);
-
 static void __set_phy_supported(struct phy_device *phydev, u32 max_speed)
 {
 	phy_caps_linkmode_max_speed(max_speed, phydev->supported);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5749aad96862..4a59aa7254f4 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1310,19 +1310,6 @@ const char *phy_rate_matching_to_str(int rate_matching);
 
 int phy_interface_num_ports(phy_interface_t interface);
 
-/* A structure for mapping a particular speed and duplex
- * combination to a particular SUPPORTED and ADVERTISED value
- */
-struct phy_setting {
-	u32 speed;
-	u8 duplex;
-	u8 bit;
-};
-
-const struct phy_setting *
-phy_lookup_setting(int speed, int duplex, const unsigned long *mask,
-		   bool exact);
-
 /**
  * phy_is_started - Convenience function to check whether PHY is started
  * @phydev: The phy_device struct
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 11/13] net: phylink: Add a mapping between MAC_CAPS and LINK_CAPS
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
                   ` (9 preceding siblings ...)
  2025-03-03  9:03 ` [PATCH net-next v4 10/13] net: phy: drop phy_settings and the associated lookup helpers Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 12/13] net: phylink: Convert capabilities to linkmodes using phy_caps Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 13/13] net: phy: phy_caps: Allow getting an phy_interface's capabilities Maxime Chevallier
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

phylink allows MAC drivers to report the capabilities in terms of speed,
duplex and pause support. This is done through a dedicated set of enum
values in the form of the MAC_ capabilities. They are very close to what
the LINK_CAPA_xxx can express, with the difference that LINK_CAPA don't
have any information about Pause/Asym Pause support.

To prepare converting phylink to using the phy_caps, add the mapping
between MAC capabilities and phy_caps. While doing so, we move the
phylink_caps_params array up a bit to simplify future commits.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: No changes

 drivers/net/phy/phylink.c | 49 ++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 0b9585cb508e..9cb3beec9f47 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -292,6 +292,31 @@ static int phylink_interface_max_speed(phy_interface_t interface)
 	return SPEED_UNKNOWN;
 }
 
+static struct {
+	unsigned long mask;
+	int speed;
+	unsigned int duplex;
+	unsigned int caps_bit;
+} phylink_caps_params[] = {
+	{ MAC_400000FD, SPEED_400000, DUPLEX_FULL, BIT(LINK_CAPA_400000FD) },
+	{ MAC_200000FD, SPEED_200000, DUPLEX_FULL, BIT(LINK_CAPA_200000FD) },
+	{ MAC_100000FD, SPEED_100000, DUPLEX_FULL, BIT(LINK_CAPA_100000FD) },
+	{ MAC_56000FD,  SPEED_56000,  DUPLEX_FULL, BIT(LINK_CAPA_56000FD) },
+	{ MAC_50000FD,  SPEED_50000,  DUPLEX_FULL, BIT(LINK_CAPA_50000FD) },
+	{ MAC_40000FD,  SPEED_40000,  DUPLEX_FULL, BIT(LINK_CAPA_40000FD) },
+	{ MAC_25000FD,  SPEED_25000,  DUPLEX_FULL, BIT(LINK_CAPA_25000FD) },
+	{ MAC_20000FD,  SPEED_20000,  DUPLEX_FULL, BIT(LINK_CAPA_20000FD) },
+	{ MAC_10000FD,  SPEED_10000,  DUPLEX_FULL, BIT(LINK_CAPA_10000FD) },
+	{ MAC_5000FD,   SPEED_5000,   DUPLEX_FULL, BIT(LINK_CAPA_5000FD) },
+	{ MAC_2500FD,   SPEED_2500,   DUPLEX_FULL, BIT(LINK_CAPA_2500FD) },
+	{ MAC_1000FD,   SPEED_1000,   DUPLEX_FULL, BIT(LINK_CAPA_1000FD) },
+	{ MAC_1000HD,   SPEED_1000,   DUPLEX_HALF, BIT(LINK_CAPA_1000HD) },
+	{ MAC_100FD,    SPEED_100,    DUPLEX_FULL, BIT(LINK_CAPA_100FD) },
+	{ MAC_100HD,    SPEED_100,    DUPLEX_HALF, BIT(LINK_CAPA_100HD) },
+	{ MAC_10FD,     SPEED_10,     DUPLEX_FULL, BIT(LINK_CAPA_10FD) },
+	{ MAC_10HD,     SPEED_10,     DUPLEX_HALF, BIT(LINK_CAPA_10HD) },
+};
+
 /**
  * phylink_caps_to_linkmodes() - Convert capabilities to ethtool link modes
  * @linkmodes: ethtool linkmode mask (must be already initialised)
@@ -445,30 +470,6 @@ static void phylink_caps_to_linkmodes(unsigned long *linkmodes,
 	}
 }
 
-static struct {
-	unsigned long mask;
-	int speed;
-	unsigned int duplex;
-} phylink_caps_params[] = {
-	{ MAC_400000FD, SPEED_400000, DUPLEX_FULL },
-	{ MAC_200000FD, SPEED_200000, DUPLEX_FULL },
-	{ MAC_100000FD, SPEED_100000, DUPLEX_FULL },
-	{ MAC_56000FD,  SPEED_56000,  DUPLEX_FULL },
-	{ MAC_50000FD,  SPEED_50000,  DUPLEX_FULL },
-	{ MAC_40000FD,  SPEED_40000,  DUPLEX_FULL },
-	{ MAC_25000FD,  SPEED_25000,  DUPLEX_FULL },
-	{ MAC_20000FD,  SPEED_20000,  DUPLEX_FULL },
-	{ MAC_10000FD,  SPEED_10000,  DUPLEX_FULL },
-	{ MAC_5000FD,   SPEED_5000,   DUPLEX_FULL },
-	{ MAC_2500FD,   SPEED_2500,   DUPLEX_FULL },
-	{ MAC_1000FD,   SPEED_1000,   DUPLEX_FULL },
-	{ MAC_1000HD,   SPEED_1000,   DUPLEX_HALF },
-	{ MAC_100FD,    SPEED_100,    DUPLEX_FULL },
-	{ MAC_100HD,    SPEED_100,    DUPLEX_HALF },
-	{ MAC_10FD,     SPEED_10,     DUPLEX_FULL },
-	{ MAC_10HD,     SPEED_10,     DUPLEX_HALF },
-};
-
 /**
  * phylink_limit_mac_speed - limit the phylink_config to a maximum speed
  * @config: pointer to a &struct phylink_config
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 12/13] net: phylink: Convert capabilities to linkmodes using phy_caps
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
                   ` (10 preceding siblings ...)
  2025-03-03  9:03 ` [PATCH net-next v4 11/13] net: phylink: Add a mapping between MAC_CAPS and LINK_CAPS Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  2025-03-03  9:03 ` [PATCH net-next v4 13/13] net: phy: phy_caps: Allow getting an phy_interface's capabilities Maxime Chevallier
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

phylink_caps_to_linkmodes() is used to derive a list of linkmodes that
can be conceivably exposed using a given set of speeds and duplex
through phylink's MAC capabilities.

This list can be derived from the link_caps array in phy_caps, provided
we convert the MAC capabilities into a LINK_CAPA bitmask first.

Introduce an internal phylink helper phylink_caps_to_link_caps() to
convert from MAC capabilities into phy_caps, then  phy_caps_linkmodes()
to do the link_caps -> linkmodes conversion.

This avoids having to update phylink for every new linkmode.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: No changes

 drivers/net/phy/phy-caps.h |   1 +
 drivers/net/phy/phy_caps.c |  14 ++++
 drivers/net/phy/phylink.c  | 155 +++++--------------------------------
 3 files changed, 36 insertions(+), 134 deletions(-)

diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
index 8ae8dc42494f..db53f380392e 100644
--- a/drivers/net/phy/phy-caps.h
+++ b/drivers/net/phy/phy-caps.h
@@ -44,6 +44,7 @@ size_t phy_caps_speeds(unsigned int *speeds, size_t size,
 		       unsigned long *linkmodes);
 void phy_caps_linkmode_max_speed(u32 max_speed, unsigned long *linkmodes);
 bool phy_caps_valid(int speed, int duplex, const unsigned long *linkmodes);
+void phy_caps_linkmodes(unsigned long caps, unsigned long *linkmodes);
 
 const struct link_capabilities *
 phy_caps_lookup_by_linkmode(const unsigned long *linkmodes);
diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
index 8b90fb6d6b15..0a64289a161b 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -239,3 +239,17 @@ bool phy_caps_valid(int speed, int duplex, const unsigned long *linkmodes)
 
 	return linkmode_intersects(link_caps[capa].linkmodes, linkmodes);
 }
+
+/**
+ * phy_caps_linkmodes() - Convert a bitfield of capabilities into linkmodes
+ * @caps: The list of caps, each bit corresponding to a LINK_CAPA value
+ * @linkmodes: The set of linkmodes to fill. Must be previously initialized.
+ */
+void phy_caps_linkmodes(unsigned long caps, unsigned long *linkmodes)
+{
+	unsigned long capa;
+
+	for_each_set_bit(capa, &caps, __LINK_CAPA_MAX)
+		linkmode_or(linkmodes, linkmodes, link_caps[capa].linkmodes);
+}
+EXPORT_SYMBOL_GPL(phy_caps_linkmodes);
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 9cb3beec9f47..30f0ecb084ef 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -317,6 +317,24 @@ static struct {
 	{ MAC_10HD,     SPEED_10,     DUPLEX_HALF, BIT(LINK_CAPA_10HD) },
 };
 
+/**
+ * phylink_caps_to_link_caps() - Convert a set of MAC capabilities LINK caps
+ * @caps: A set of MAC capabilities
+ *
+ * Returns: The corresponding set of LINK_CAPA as defined in phy-caps.h
+ */
+static unsigned long phylink_caps_to_link_caps(unsigned long caps)
+{
+	unsigned long link_caps = 0;
+	int i;
+
+	for (i = 0; i <  ARRAY_SIZE(phylink_caps_params); i++)
+		if (caps & phylink_caps_params[i].mask)
+			link_caps |= phylink_caps_params[i].caps_bit;
+
+	return link_caps;
+}
+
 /**
  * phylink_caps_to_linkmodes() - Convert capabilities to ethtool link modes
  * @linkmodes: ethtool linkmode mask (must be already initialised)
@@ -328,146 +346,15 @@ static struct {
 static void phylink_caps_to_linkmodes(unsigned long *linkmodes,
 				      unsigned long caps)
 {
+	unsigned long link_caps = phylink_caps_to_link_caps(caps);
+
 	if (caps & MAC_SYM_PAUSE)
 		__set_bit(ETHTOOL_LINK_MODE_Pause_BIT, linkmodes);
 
 	if (caps & MAC_ASYM_PAUSE)
 		__set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, linkmodes);
 
-	if (caps & MAC_10HD) {
-		__set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10baseT1S_Half_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10baseT1S_P2MP_Half_BIT, linkmodes);
-	}
-
-	if (caps & MAC_10FD) {
-		__set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10baseT1S_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_100HD) {
-		__set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100baseFX_Half_BIT, linkmodes);
-	}
-
-	if (caps & MAC_100FD) {
-		__set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100baseT1_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100baseFX_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_1000HD)
-		__set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, linkmodes);
-
-	if (caps & MAC_1000FD) {
-		__set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_2500FD) {
-		__set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_2500baseX_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_5000FD)
-		__set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, linkmodes);
-
-	if (caps & MAC_10000FD) {
-		__set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10000baseR_FEC_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_25000FD) {
-		__set_bit(ETHTOOL_LINK_MODE_25000baseCR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_25000baseKR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_25000baseSR_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_40000FD) {
-		__set_bit(ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_50000FD) {
-		__set_bit(ETHTOOL_LINK_MODE_50000baseCR2_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_50000baseKR2_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_50000baseSR2_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_50000baseKR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_50000baseSR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_50000baseCR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
-			  linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_50000baseDR_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_56000FD) {
-		__set_bit(ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_56000baseCR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_100000FD) {
-		__set_bit(ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseSR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseLR4_ER4_Full_BIT,
-			  linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT,
-			  linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseKR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseSR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseLR_ER_FR_Full_BIT,
-			  linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseCR_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_100000baseDR_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_200000FD) {
-		__set_bit(ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT,
-			  linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_200000baseKR2_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_200000baseSR2_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_200000baseLR2_ER2_FR2_Full_BIT,
-			  linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_200000baseDR2_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_200000baseCR2_Full_BIT, linkmodes);
-	}
-
-	if (caps & MAC_400000FD) {
-		__set_bit(ETHTOOL_LINK_MODE_400000baseKR8_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_400000baseSR8_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_400000baseLR8_ER8_FR8_Full_BIT,
-			  linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_400000baseDR8_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_400000baseCR8_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_400000baseKR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_400000baseSR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_400000baseLR4_ER4_FR4_Full_BIT,
-			  linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_400000baseDR4_Full_BIT, linkmodes);
-		__set_bit(ETHTOOL_LINK_MODE_400000baseCR4_Full_BIT, linkmodes);
-	}
+	phy_caps_linkmodes(link_caps, linkmodes);
 }
 
 /**
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH net-next v4 13/13] net: phy: phy_caps: Allow getting an phy_interface's capabilities
  2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
                   ` (11 preceding siblings ...)
  2025-03-03  9:03 ` [PATCH net-next v4 12/13] net: phylink: Convert capabilities to linkmodes using phy_caps Maxime Chevallier
@ 2025-03-03  9:03 ` Maxime Chevallier
  12 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-03  9:03 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

Phylink has internal code to get the MAC capabilities of a given PHY
interface (what are the supported speed and duplex).

Extract that into phy_caps, but use the link_capa for conversion. Add an
internal phylink helper for the link caps -> mac caps conversion, and
use this in phylink_caps_to_linkmodes().

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: No changes

 drivers/net/phy/phy-caps.h |  4 ++
 drivers/net/phy/phy_caps.c | 92 ++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/phylink.c  | 90 ++++++-------------------------------
 3 files changed, 110 insertions(+), 76 deletions(-)

diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
index db53f380392e..e1fe19d11e5f 100644
--- a/drivers/net/phy/phy-caps.h
+++ b/drivers/net/phy/phy-caps.h
@@ -8,6 +8,7 @@
 #define __PHY_CAPS_H
 
 #include <linux/ethtool.h>
+#include <linux/phy.h>
 
 enum {
 	LINK_CAPA_10HD = 0,
@@ -32,6 +33,8 @@ enum {
 	__LINK_CAPA_MAX,
 };
 
+#define LINK_CAPA_ALL	GENMASK((__LINK_CAPA_MAX - 1), 0)
+
 struct link_capabilities {
 	int speed;
 	unsigned int duplex;
@@ -45,6 +48,7 @@ size_t phy_caps_speeds(unsigned int *speeds, size_t size,
 void phy_caps_linkmode_max_speed(u32 max_speed, unsigned long *linkmodes);
 bool phy_caps_valid(int speed, int duplex, const unsigned long *linkmodes);
 void phy_caps_linkmodes(unsigned long caps, unsigned long *linkmodes);
+unsigned long phy_caps_from_interface(phy_interface_t interface);
 
 const struct link_capabilities *
 phy_caps_lookup_by_linkmode(const unsigned long *linkmodes);
diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
index 0a64289a161b..555daaa41d52 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -253,3 +253,95 @@ void phy_caps_linkmodes(unsigned long caps, unsigned long *linkmodes)
 		linkmode_or(linkmodes, linkmodes, link_caps[capa].linkmodes);
 }
 EXPORT_SYMBOL_GPL(phy_caps_linkmodes);
+
+/**
+ * phy_caps_from_interface() - Get the link capa from a given PHY interface
+ * @interface: The PHY interface we want to get the possible Speed/Duplex from
+ *
+ * Returns: A bitmask of LINK_CAPA_xxx values that can be achieved with the
+ *          provided interface.
+ */
+unsigned long phy_caps_from_interface(phy_interface_t interface)
+{
+	unsigned long link_caps = 0;
+
+	switch (interface) {
+	case PHY_INTERFACE_MODE_USXGMII:
+		link_caps |= BIT(LINK_CAPA_10000FD) | BIT(LINK_CAPA_5000FD);
+		fallthrough;
+
+	case PHY_INTERFACE_MODE_10G_QXGMII:
+		link_caps |= BIT(LINK_CAPA_2500FD);
+		fallthrough;
+
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_PSGMII:
+	case PHY_INTERFACE_MODE_QSGMII:
+	case PHY_INTERFACE_MODE_QUSGMII:
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_GMII:
+		link_caps |= BIT(LINK_CAPA_1000HD) | BIT(LINK_CAPA_1000FD);
+		fallthrough;
+
+	case PHY_INTERFACE_MODE_REVRMII:
+	case PHY_INTERFACE_MODE_RMII:
+	case PHY_INTERFACE_MODE_SMII:
+	case PHY_INTERFACE_MODE_REVMII:
+	case PHY_INTERFACE_MODE_MII:
+		link_caps |= BIT(LINK_CAPA_10HD) | BIT(LINK_CAPA_10FD);
+		fallthrough;
+
+	case PHY_INTERFACE_MODE_100BASEX:
+		link_caps |= BIT(LINK_CAPA_100HD) | BIT(LINK_CAPA_100FD);
+		break;
+
+	case PHY_INTERFACE_MODE_TBI:
+	case PHY_INTERFACE_MODE_MOCA:
+	case PHY_INTERFACE_MODE_RTBI:
+	case PHY_INTERFACE_MODE_1000BASEX:
+		link_caps |= BIT(LINK_CAPA_1000HD);
+		fallthrough;
+	case PHY_INTERFACE_MODE_1000BASEKX:
+	case PHY_INTERFACE_MODE_TRGMII:
+		link_caps |= BIT(LINK_CAPA_1000FD);
+		break;
+
+	case PHY_INTERFACE_MODE_2500BASEX:
+		link_caps |= BIT(LINK_CAPA_2500FD);
+		break;
+
+	case PHY_INTERFACE_MODE_5GBASER:
+		link_caps |= BIT(LINK_CAPA_5000FD);
+		break;
+
+	case PHY_INTERFACE_MODE_XGMII:
+	case PHY_INTERFACE_MODE_RXAUI:
+	case PHY_INTERFACE_MODE_XAUI:
+	case PHY_INTERFACE_MODE_10GBASER:
+	case PHY_INTERFACE_MODE_10GKR:
+		link_caps |= BIT(LINK_CAPA_10000FD);
+		break;
+
+	case PHY_INTERFACE_MODE_25GBASER:
+		link_caps |= BIT(LINK_CAPA_25000FD);
+		break;
+
+	case PHY_INTERFACE_MODE_XLGMII:
+		link_caps |= BIT(LINK_CAPA_40000FD);
+		break;
+
+	case PHY_INTERFACE_MODE_INTERNAL:
+		link_caps |= LINK_CAPA_ALL;
+		break;
+
+	case PHY_INTERFACE_MODE_NA:
+	case PHY_INTERFACE_MODE_MAX:
+		break;
+	}
+
+	return link_caps;
+}
+EXPORT_SYMBOL_GPL(phy_caps_from_interface);
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 30f0ecb084ef..0162ce09f54d 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -335,6 +335,18 @@ static unsigned long phylink_caps_to_link_caps(unsigned long caps)
 	return link_caps;
 }
 
+static unsigned long phylink_link_caps_to_mac_caps(unsigned long link_caps)
+{
+	unsigned long caps = 0;
+	int i;
+
+	for (i = 0; i <  ARRAY_SIZE(phylink_caps_params); i++)
+		if (link_caps & phylink_caps_params[i].caps_bit)
+			caps |= phylink_caps_params[i].mask;
+
+	return caps;
+}
+
 /**
  * phylink_caps_to_linkmodes() - Convert capabilities to ethtool link modes
  * @linkmodes: ethtool linkmode mask (must be already initialised)
@@ -412,86 +424,12 @@ static unsigned long phylink_get_capabilities(phy_interface_t interface,
 					      unsigned long mac_capabilities,
 					      int rate_matching)
 {
+	unsigned long link_caps = phy_caps_from_interface(interface);
 	int max_speed = phylink_interface_max_speed(interface);
 	unsigned long caps = MAC_SYM_PAUSE | MAC_ASYM_PAUSE;
 	unsigned long matched_caps = 0;
 
-	switch (interface) {
-	case PHY_INTERFACE_MODE_USXGMII:
-		caps |= MAC_10000FD | MAC_5000FD;
-		fallthrough;
-
-	case PHY_INTERFACE_MODE_10G_QXGMII:
-		caps |= MAC_2500FD;
-		fallthrough;
-
-	case PHY_INTERFACE_MODE_RGMII_TXID:
-	case PHY_INTERFACE_MODE_RGMII_RXID:
-	case PHY_INTERFACE_MODE_RGMII_ID:
-	case PHY_INTERFACE_MODE_RGMII:
-	case PHY_INTERFACE_MODE_PSGMII:
-	case PHY_INTERFACE_MODE_QSGMII:
-	case PHY_INTERFACE_MODE_QUSGMII:
-	case PHY_INTERFACE_MODE_SGMII:
-	case PHY_INTERFACE_MODE_GMII:
-		caps |= MAC_1000HD | MAC_1000FD;
-		fallthrough;
-
-	case PHY_INTERFACE_MODE_REVRMII:
-	case PHY_INTERFACE_MODE_RMII:
-	case PHY_INTERFACE_MODE_SMII:
-	case PHY_INTERFACE_MODE_REVMII:
-	case PHY_INTERFACE_MODE_MII:
-		caps |= MAC_10HD | MAC_10FD;
-		fallthrough;
-
-	case PHY_INTERFACE_MODE_100BASEX:
-		caps |= MAC_100HD | MAC_100FD;
-		break;
-
-	case PHY_INTERFACE_MODE_TBI:
-	case PHY_INTERFACE_MODE_MOCA:
-	case PHY_INTERFACE_MODE_RTBI:
-	case PHY_INTERFACE_MODE_1000BASEX:
-		caps |= MAC_1000HD;
-		fallthrough;
-	case PHY_INTERFACE_MODE_1000BASEKX:
-	case PHY_INTERFACE_MODE_TRGMII:
-		caps |= MAC_1000FD;
-		break;
-
-	case PHY_INTERFACE_MODE_2500BASEX:
-		caps |= MAC_2500FD;
-		break;
-
-	case PHY_INTERFACE_MODE_5GBASER:
-		caps |= MAC_5000FD;
-		break;
-
-	case PHY_INTERFACE_MODE_XGMII:
-	case PHY_INTERFACE_MODE_RXAUI:
-	case PHY_INTERFACE_MODE_XAUI:
-	case PHY_INTERFACE_MODE_10GBASER:
-	case PHY_INTERFACE_MODE_10GKR:
-		caps |= MAC_10000FD;
-		break;
-
-	case PHY_INTERFACE_MODE_25GBASER:
-		caps |= MAC_25000FD;
-		break;
-
-	case PHY_INTERFACE_MODE_XLGMII:
-		caps |= MAC_40000FD;
-		break;
-
-	case PHY_INTERFACE_MODE_INTERNAL:
-		caps |= ~0;
-		break;
-
-	case PHY_INTERFACE_MODE_NA:
-	case PHY_INTERFACE_MODE_MAX:
-		break;
-	}
+	caps |= phylink_link_caps_to_mac_caps(link_caps);
 
 	switch (rate_matching) {
 	case RATE_MATCH_OPEN_LOOP:
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration
  2025-03-03  9:03 ` [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration Maxime Chevallier
@ 2025-03-04 14:43   ` Maxime Chevallier
  2025-03-06 12:48     ` Russell King (Oracle)
  2025-03-06  8:56   ` Paolo Abeni
  1 sibling, 1 reply; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-04 14:43 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: netdev, linux-kernel, thomas.petazzoni, linux-arm-kernel,
	Christophe Leroy, Herve Codina, Florian Fainelli, Vladimir Oltean,
	Köry Maincent, Oleksij Rempel, Simon Horman, Romain Gantois

Hi,

On Mon,  3 Mar 2025 10:03:15 +0100
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> When phylink creates a fixed-link configuration, it finds a matching
> linkmode to set as the advertised, lp_advertising and supported modes
> based on the speed and duplex of the fixed link.
> 
> Use the newly introduced phy_caps_lookup to get these modes instead of
> phy_lookup_settings(). This has the side effect that the matched
> settings and configured linkmodes may now contain several linkmodes (the
> intersection of supported linkmodes from the phylink settings and the
> linkmodes that match speed/duplex) instead of the one from
> phy_lookup_settings().
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---

Maybe before anything goes further with this patch, I'd like to get
some feedback from it on a particular point. This changes the linkmodes
that are reported on fixed-link interfaces. Instead of reporting one
single mode, we report all modes supported by the fixed-link' speed and
duplex settings.

The following example is a before/after of the "ethtool ethX" output on
a 1G fixed link :

Before this patch :

	Settings for eth0:
	Supported ports: [ MII ]
	Supported link modes:   1000baseT/Full 
	Supported pause frame use: Symmetric
	Supports auto-negotiation: Yes
	Supported FEC modes: Not reported
	Advertised link modes:  Not reported
	Advertised pause frame use: No
	Advertised auto-negotiation: No
	Advertised FEC modes: Not reported
	Speed: Unknown!
	Duplex: Half
	Port: MII
	PHYAD: 0
	Transceiver: internal
	Auto-negotiation: off
	Supports Wake-on: d
	Wake-on: d
	Link detected: no

After :

	Supported ports: [ MII ]
	Supported link modes:   1000baseT/Full 
	                        1000baseKX/Full 
	                        1000baseX/Full 
	                        1000baseT1/Full 
	Supported pause frame use: Symmetric
	Supports auto-negotiation: Yes
	Supported FEC modes: Not reported
	Advertised link modes:  Not reported
	Advertised pause frame use: No
	Advertised auto-negotiation: No
	Advertised FEC modes: Not reported
	Speed: Unknown!
	Duplex: Half
	Port: MII
	PHYAD: 0
	Transceiver: internal
	Auto-negotiation: off
	Supports Wake-on: d
	Wake-on: d
	Link detected: no

The fixed-link in question is for the CPU port of a DSA switch.

In my opinion, this is OK as the linkmodes expressed here don't match
physical linkmodes on an actual wire, but as this is a user visible
change, I'd like to make sure this is OK. Any comment here is more than
welcome.

Maxime

> V4: Remove unnecessary linklmode_zero in phylink_set_fixed_link(),
> follwing Russell's comment
> 
>  drivers/net/phy/phylink.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 6c67d5c9b787..0b9585cb508e 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -805,9 +805,10 @@ static int phylink_validate(struct phylink *pl, unsigned long *supported,
>  static int phylink_parse_fixedlink(struct phylink *pl,
>  				   const struct fwnode_handle *fwnode)
>  {
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(match) = { 0, };
>  	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +	const struct link_capabilities *c;
>  	struct fwnode_handle *fixed_node;
> -	const struct phy_setting *s;
>  	struct gpio_desc *desc;
>  	u32 speed;
>  	int ret;
> @@ -879,8 +880,10 @@ static int phylink_parse_fixedlink(struct phylink *pl,
>  	linkmode_copy(pl->link_config.advertising, pl->supported);
>  	phylink_validate(pl, pl->supported, &pl->link_config);
>  
> -	s = phy_lookup_setting(pl->link_config.speed, pl->link_config.duplex,
> -			       pl->supported, true);
> +	c = phy_caps_lookup(pl->link_config.speed, pl->link_config.duplex,
> +			    pl->supported, true);
> +	if (c)
> +		linkmode_and(match, pl->supported, c->linkmodes);
>  
>  	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, mask);
>  	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, mask);
> @@ -889,9 +892,10 @@ static int phylink_parse_fixedlink(struct phylink *pl,
>  
>  	phylink_set(pl->supported, MII);
>  
> -	if (s) {
> -		__set_bit(s->bit, pl->supported);
> -		__set_bit(s->bit, pl->link_config.lp_advertising);
> +	if (c) {
> +		linkmode_or(pl->supported, pl->supported, match);
> +		linkmode_or(pl->link_config.lp_advertising,
> +			    pl->link_config.lp_advertising, match);
>  	} else {
>  		phylink_warn(pl, "fixed link %s duplex %dMbps not recognised\n",
>  			     pl->link_config.duplex == DUPLEX_FULL ? "full" : "half",
> @@ -1879,21 +1883,20 @@ static int phylink_register_sfp(struct phylink *pl,
>  int phylink_set_fixed_link(struct phylink *pl,
>  			   const struct phylink_link_state *state)
>  {
> -	const struct phy_setting *s;
> +	const struct link_capabilities *c;
>  	unsigned long *adv;
>  
>  	if (pl->cfg_link_an_mode != MLO_AN_PHY || !state ||
>  	    !test_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state))
>  		return -EINVAL;
>  
> -	s = phy_lookup_setting(state->speed, state->duplex,
> -			       pl->supported, true);
> -	if (!s)
> +	c = phy_caps_lookup(state->speed, state->duplex,
> +			    pl->supported, true);
> +	if (!c)
>  		return -EINVAL;
>  
>  	adv = pl->link_config.advertising;
> -	linkmode_zero(adv);
> -	linkmode_set_bit(s->bit, adv);
> +	linkmode_and(adv, pl->supported, c->linkmodes);
>  	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, adv);
>  
>  	pl->link_config.speed = state->speed;


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH net-next v4 02/13] net: phy: Use an internal, searchable storage for the linkmodes
  2025-03-03  9:03 ` [PATCH net-next v4 02/13] net: phy: Use an internal, searchable storage for the linkmodes Maxime Chevallier
@ 2025-03-06  8:30   ` Paolo Abeni
  2025-03-06  8:57     ` Maxime Chevallier
  0 siblings, 1 reply; 23+ messages in thread
From: Paolo Abeni @ 2025-03-06  8:30 UTC (permalink / raw)
  To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
	Eric Dumazet, Russell King, Heiner Kallweit
  Cc: netdev, linux-kernel, thomas.petazzoni, linux-arm-kernel,
	Christophe Leroy, Herve Codina, Florian Fainelli, Vladimir Oltean,
	Köry Maincent, Oleksij Rempel, Simon Horman, Romain Gantois

On 3/3/25 10:03 AM, Maxime Chevallier wrote:
[...]
> +static int speed_duplex_to_capa(int speed, unsigned int duplex)
> +{
> +	if (duplex == DUPLEX_UNKNOWN ||
> +	    (speed > SPEED_1000 && duplex != DUPLEX_FULL))
> +		return -EINVAL;
> +
> +	switch (speed) {
> +	case SPEED_10: return duplex == DUPLEX_FULL ?
> +			      LINK_CAPA_10FD : LINK_CAPA_10HD;
> +	case SPEED_100: return duplex == DUPLEX_FULL ?
> +			       LINK_CAPA_100FD : LINK_CAPA_100HD;
> +	case SPEED_1000: return duplex == DUPLEX_FULL ?
> +				LINK_CAPA_1000FD : LINK_CAPA_1000HD;
> +	case SPEED_2500: return LINK_CAPA_2500FD;
> +	case SPEED_5000: return LINK_CAPA_5000FD;
> +	case SPEED_10000: return LINK_CAPA_10000FD;
> +	case SPEED_20000: return LINK_CAPA_20000FD;
> +	case SPEED_25000: return LINK_CAPA_25000FD;
> +	case SPEED_40000: return LINK_CAPA_40000FD;
> +	case SPEED_50000: return LINK_CAPA_50000FD;
> +	case SPEED_56000: return LINK_CAPA_56000FD;
> +	case SPEED_100000: return LINK_CAPA_100000FD;
> +	case SPEED_200000: return LINK_CAPA_200000FD;
> +	case SPEED_400000: return LINK_CAPA_400000FD;
> +	case SPEED_800000: return LINK_CAPA_800000FD;
> +	}
> +

What about adding some code here to help future patch updating LINK_CAPA
definition as needed?

Something alike:

	pr_err_once("Unknown speed %d, please update LINK_CAPS\n", speed);


> +	return -EINVAL;
> +}
> +
> +/**
> + * phy_caps_init() - Initializes the link_caps array from the link_mode_params.
> + */
> +void phy_caps_init(void)
> +{
> +	const struct link_mode_info *linkmode;
> +	int i, capa;
> +
> +	/* Fill the caps array from net/ethtool/common.c */
> +	for (i = 0; i < __ETHTOOL_LINK_MODE_MASK_NBITS; i++) {
> +		linkmode = &link_mode_params[i];
> +		capa = speed_duplex_to_capa(linkmode->speed, linkmode->duplex);
> +
> +		if (capa < 0)
> +			continue;

Or even error-out here.

/P


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration
  2025-03-03  9:03 ` [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration Maxime Chevallier
  2025-03-04 14:43   ` Maxime Chevallier
@ 2025-03-06  8:56   ` Paolo Abeni
  2025-03-06 10:12     ` Maxime Chevallier
  1 sibling, 1 reply; 23+ messages in thread
From: Paolo Abeni @ 2025-03-06  8:56 UTC (permalink / raw)
  To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
	Eric Dumazet, Russell King, Heiner Kallweit
  Cc: netdev, linux-kernel, thomas.petazzoni, linux-arm-kernel,
	Christophe Leroy, Herve Codina, Florian Fainelli, Vladimir Oltean,
	Köry Maincent, Oleksij Rempel, Simon Horman, Romain Gantois

On 3/3/25 10:03 AM, Maxime Chevallier wrote:
> @@ -879,8 +880,10 @@ static int phylink_parse_fixedlink(struct phylink *pl,
>  	linkmode_copy(pl->link_config.advertising, pl->supported);
>  	phylink_validate(pl, pl->supported, &pl->link_config);
>  
> -	s = phy_lookup_setting(pl->link_config.speed, pl->link_config.duplex,
> -			       pl->supported, true);
> +	c = phy_caps_lookup(pl->link_config.speed, pl->link_config.duplex,
> +			    pl->supported, true);
> +	if (c)
> +		linkmode_and(match, pl->supported, c->linkmodes);

How about using only the first bit from `c->linkmodes`, to avoid
behavior changes?

Thanks!

Paolo


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH net-next v4 02/13] net: phy: Use an internal, searchable storage for the linkmodes
  2025-03-06  8:30   ` Paolo Abeni
@ 2025-03-06  8:57     ` Maxime Chevallier
  2025-03-06 10:47       ` Paolo Abeni
  0 siblings, 1 reply; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-06  8:57 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Russell King,
	Heiner Kallweit, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

Hello Paolo,

On Thu, 6 Mar 2025 09:30:11 +0100
Paolo Abeni <pabeni@redhat.com> wrote:

> On 3/3/25 10:03 AM, Maxime Chevallier wrote:
> [...]
> > +static int speed_duplex_to_capa(int speed, unsigned int duplex)
> > +{
> > +	if (duplex == DUPLEX_UNKNOWN ||
> > +	    (speed > SPEED_1000 && duplex != DUPLEX_FULL))
> > +		return -EINVAL;
> > +
> > +	switch (speed) {
> > +	case SPEED_10: return duplex == DUPLEX_FULL ?
> > +			      LINK_CAPA_10FD : LINK_CAPA_10HD;
> > +	case SPEED_100: return duplex == DUPLEX_FULL ?
> > +			       LINK_CAPA_100FD : LINK_CAPA_100HD;
> > +	case SPEED_1000: return duplex == DUPLEX_FULL ?
> > +				LINK_CAPA_1000FD : LINK_CAPA_1000HD;
> > +	case SPEED_2500: return LINK_CAPA_2500FD;
> > +	case SPEED_5000: return LINK_CAPA_5000FD;
> > +	case SPEED_10000: return LINK_CAPA_10000FD;
> > +	case SPEED_20000: return LINK_CAPA_20000FD;
> > +	case SPEED_25000: return LINK_CAPA_25000FD;
> > +	case SPEED_40000: return LINK_CAPA_40000FD;
> > +	case SPEED_50000: return LINK_CAPA_50000FD;
> > +	case SPEED_56000: return LINK_CAPA_56000FD;
> > +	case SPEED_100000: return LINK_CAPA_100000FD;
> > +	case SPEED_200000: return LINK_CAPA_200000FD;
> > +	case SPEED_400000: return LINK_CAPA_400000FD;
> > +	case SPEED_800000: return LINK_CAPA_800000FD;
> > +	}
> > +  
> 
> What about adding some code here to help future patch updating LINK_CAPA
> definition as needed?
> 
> Something alike:
> 
> 	pr_err_once("Unknown speed %d, please update LINK_CAPS\n", speed);
> 
> 
> > +	return -EINVAL;
> > +}
> > +
> > +/**
> > + * phy_caps_init() - Initializes the link_caps array from the link_mode_params.
> > + */
> > +void phy_caps_init(void)
> > +{
> > +	const struct link_mode_info *linkmode;
> > +	int i, capa;
> > +
> > +	/* Fill the caps array from net/ethtool/common.c */
> > +	for (i = 0; i < __ETHTOOL_LINK_MODE_MASK_NBITS; i++) {
> > +		linkmode = &link_mode_params[i];
> > +		capa = speed_duplex_to_capa(linkmode->speed, linkmode->duplex);
> > +
> > +		if (capa < 0)
> > +			continue;  
> 
> Or even error-out here.

Good point yes indeed. Russell raised the point for the need of keeping
this in sync with new SPEED_XXX definitions, I'll add a check that
errors out.

I hope that's OK though, as higher speeds are introduced and used by
NICs that usually don't use phylib at all, so there's a good chance
that the developper introducing the new speed won't have CONFIG_PHYLIB
enabled.

Is that still good ?

Maxime

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration
  2025-03-06  8:56   ` Paolo Abeni
@ 2025-03-06 10:12     ` Maxime Chevallier
  2025-03-06 12:51       ` Russell King (Oracle)
  0 siblings, 1 reply; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-06 10:12 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Russell King,
	Heiner Kallweit, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

On Thu, 6 Mar 2025 09:56:32 +0100
Paolo Abeni <pabeni@redhat.com> wrote:

> On 3/3/25 10:03 AM, Maxime Chevallier wrote:
> > @@ -879,8 +880,10 @@ static int phylink_parse_fixedlink(struct phylink *pl,
> >  	linkmode_copy(pl->link_config.advertising, pl->supported);
> >  	phylink_validate(pl, pl->supported, &pl->link_config);
> >  
> > -	s = phy_lookup_setting(pl->link_config.speed, pl->link_config.duplex,
> > -			       pl->supported, true);
> > +	c = phy_caps_lookup(pl->link_config.speed, pl->link_config.duplex,
> > +			    pl->supported, true);
> > +	if (c)
> > +		linkmode_and(match, pl->supported, c->linkmodes);  
> 
> How about using only the first bit from `c->linkmodes`, to avoid
> behavior changes?

If what we want is to keep the exact same behaviour, then we need to
go one step further and make sure we keep the same one as before, and
it's not guaranteed that the first bit in c->linkmodes is this one.

We could however have a default supported mask for fixed-link in phylink
that contains all the linkmodes we allow for fixed links, then filter
with the lookup, something like :


-       linkmode_fill(pl->supported);
+       /* (in a dedicated helper) Linkmodes reported for fixed links below
+        * 10G */
+       linkmode_zero(pl->supported);
+
+       linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, pl->supported);
+       linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, pl->supported);
+       linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, pl->supported);
+       linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, pl->supported);
+       linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, pl->supported);
+       linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, pl->supported);
+       linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, pl->supported);
+       linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, pl->supported);
+       linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, pl->supported);
+
        linkmode_copy(pl->link_config.advertising, pl->supported);
        phylink_validate(pl, pl->supported, &pl->link_config);
 
-       s = phy_lookup_setting(pl->link_config.speed, pl->link_config.duplex,
-                              pl->supported, true);
+       c = phy_caps_lookup(pl->link_config.speed, pl->link_config.duplex,
+                           pl->supported, true);
+       if (c)
+               linkmode_and(match, pl->supported, c->linkmodes);

That way we should have a consistent behaviour with what we currently
have, and to me it's more explicit what will the  fixed-link linkmodes
be.

I'd like to make sure Russell is OK with that though :)

Thanks you for the review Paolo !

Maxime

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH net-next v4 02/13] net: phy: Use an internal, searchable storage for the linkmodes
  2025-03-06  8:57     ` Maxime Chevallier
@ 2025-03-06 10:47       ` Paolo Abeni
  0 siblings, 0 replies; 23+ messages in thread
From: Paolo Abeni @ 2025-03-06 10:47 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Russell King,
	Heiner Kallweit, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

On 3/6/25 9:57 AM, Maxime Chevallier wrote:
> On Thu, 6 Mar 2025 09:30:11 +0100
> Paolo Abeni <pabeni@redhat.com> wrote:
>> On 3/3/25 10:03 AM, Maxime Chevallier wrote:
[...]
>>> +/**
>>> + * phy_caps_init() - Initializes the link_caps array from the link_mode_params.
>>> + */
>>> +void phy_caps_init(void)
>>> +{
>>> +	const struct link_mode_info *linkmode;
>>> +	int i, capa;
>>> +
>>> +	/* Fill the caps array from net/ethtool/common.c */
>>> +	for (i = 0; i < __ETHTOOL_LINK_MODE_MASK_NBITS; i++) {
>>> +		linkmode = &link_mode_params[i];
>>> +		capa = speed_duplex_to_capa(linkmode->speed, linkmode->duplex);
>>> +
>>> +		if (capa < 0)
>>> +			continue;  
>>
>> Or even error-out here.
> 
> Good point yes indeed. Russell raised the point for the need of keeping
> this in sync with new SPEED_XXX definitions, I'll add a check that
> errors out.
> 
> I hope that's OK though, as higher speeds are introduced and used by
> NICs that usually don't use phylib at all, so there's a good chance
> that the developper introducing the new speed won't have CONFIG_PHYLIB
> enabled.
> 
> Is that still good ?

I think so. New linkmodes should be added on net-next. Booting the
kernel with phylib support on any device should catch the error. I think
even the CI should catch this.

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration
  2025-03-04 14:43   ` Maxime Chevallier
@ 2025-03-06 12:48     ` Russell King (Oracle)
  0 siblings, 0 replies; 23+ messages in thread
From: Russell King (Oracle) @ 2025-03-06 12:48 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Heiner Kallweit, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

On Tue, Mar 04, 2025 at 03:43:30PM +0100, Maxime Chevallier wrote:
> On Mon,  3 Mar 2025 10:03:15 +0100
> Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
> 
> > When phylink creates a fixed-link configuration, it finds a matching
> > linkmode to set as the advertised, lp_advertising and supported modes
> > based on the speed and duplex of the fixed link.
> > 
> > Use the newly introduced phy_caps_lookup to get these modes instead of
> > phy_lookup_settings(). This has the side effect that the matched
> > settings and configured linkmodes may now contain several linkmodes (the
> > intersection of supported linkmodes from the phylink settings and the
> > linkmodes that match speed/duplex) instead of the one from
> > phy_lookup_settings().
> > 
> > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > ---
> 
> Maybe before anything goes further with this patch, I'd like to get
> some feedback from it on a particular point. This changes the linkmodes
> that are reported on fixed-link interfaces. Instead of reporting one
> single mode, we report all modes supported by the fixed-link' speed and
> duplex settings.

This is a good question. We have historically only used the baseT link
modes because the software PHY implementation was based around clause
22 baseT PHYs (although that doesn't support >1G of course.)

The real question is... does it matter, to which I'd say I don't know.
One can argue that it shouldn't matter, and I think userspace would be
unlikely to break, but userspace tends to do weird stuff all the time
so there's never any guarantee.

> The fixed-link in question is for the CPU port of a DSA switch.
> 
> In my opinion, this is OK as the linkmodes expressed here don't match
> physical linkmodes on an actual wire, but as this is a user visible
> change, I'd like to make sure this is OK. Any comment here is more than
> welcome.

Maybe Andrew has an opinion, but I suspect like me, it's really a case
that "we don't know".

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration
  2025-03-06 10:12     ` Maxime Chevallier
@ 2025-03-06 12:51       ` Russell King (Oracle)
  2025-03-06 13:26         ` Maxime Chevallier
  0 siblings, 1 reply; 23+ messages in thread
From: Russell King (Oracle) @ 2025-03-06 12:51 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: Paolo Abeni, davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet,
	Heiner Kallweit, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

On Thu, Mar 06, 2025 at 11:12:20AM +0100, Maxime Chevallier wrote:
> On Thu, 6 Mar 2025 09:56:32 +0100
> Paolo Abeni <pabeni@redhat.com> wrote:
> 
> > On 3/3/25 10:03 AM, Maxime Chevallier wrote:
> > > @@ -879,8 +880,10 @@ static int phylink_parse_fixedlink(struct phylink *pl,
> > >  	linkmode_copy(pl->link_config.advertising, pl->supported);
> > >  	phylink_validate(pl, pl->supported, &pl->link_config);
> > >  
> > > -	s = phy_lookup_setting(pl->link_config.speed, pl->link_config.duplex,
> > > -			       pl->supported, true);
> > > +	c = phy_caps_lookup(pl->link_config.speed, pl->link_config.duplex,
> > > +			    pl->supported, true);
> > > +	if (c)
> > > +		linkmode_and(match, pl->supported, c->linkmodes);  
> > 
> > How about using only the first bit from `c->linkmodes`, to avoid
> > behavior changes?
> 
> If what we want is to keep the exact same behaviour, then we need to
> go one step further and make sure we keep the same one as before, and
> it's not guaranteed that the first bit in c->linkmodes is this one.
> 
> We could however have a default supported mask for fixed-link in phylink
> that contains all the linkmodes we allow for fixed links, then filter
> with the lookup, something like :
> 
> 
> -       linkmode_fill(pl->supported);
> +       /* (in a dedicated helper) Linkmodes reported for fixed links below
> +        * 10G */
> +       linkmode_zero(pl->supported);
> +
> +       linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, pl->supported);
> +       linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, pl->supported);
> +       linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, pl->supported);
> +       linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, pl->supported);
> +       linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, pl->supported);
> +       linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, pl->supported);
> +       linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, pl->supported);
> +       linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, pl->supported);
> +       linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, pl->supported);

Good idea, but do we have some way to automatically generate the baseT
link modes?

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration
  2025-03-06 12:51       ` Russell King (Oracle)
@ 2025-03-06 13:26         ` Maxime Chevallier
  0 siblings, 0 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-03-06 13:26 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Paolo Abeni, davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet,
	Heiner Kallweit, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

On Thu, 6 Mar 2025 12:51:16 +0000
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Thu, Mar 06, 2025 at 11:12:20AM +0100, Maxime Chevallier wrote:
> > On Thu, 6 Mar 2025 09:56:32 +0100
> > Paolo Abeni <pabeni@redhat.com> wrote:
> >   
> > > On 3/3/25 10:03 AM, Maxime Chevallier wrote:  
> > > > @@ -879,8 +880,10 @@ static int phylink_parse_fixedlink(struct phylink *pl,
> > > >  	linkmode_copy(pl->link_config.advertising, pl->supported);
> > > >  	phylink_validate(pl, pl->supported, &pl->link_config);
> > > >  
> > > > -	s = phy_lookup_setting(pl->link_config.speed, pl->link_config.duplex,
> > > > -			       pl->supported, true);
> > > > +	c = phy_caps_lookup(pl->link_config.speed, pl->link_config.duplex,
> > > > +			    pl->supported, true);
> > > > +	if (c)
> > > > +		linkmode_and(match, pl->supported, c->linkmodes);    
> > > 
> > > How about using only the first bit from `c->linkmodes`, to avoid
> > > behavior changes?  
> > 
> > If what we want is to keep the exact same behaviour, then we need to
> > go one step further and make sure we keep the same one as before, and
> > it's not guaranteed that the first bit in c->linkmodes is this one.
> > 
> > We could however have a default supported mask for fixed-link in phylink
> > that contains all the linkmodes we allow for fixed links, then filter
> > with the lookup, something like :
> > 
> > 
> > -       linkmode_fill(pl->supported);
> > +       /* (in a dedicated helper) Linkmodes reported for fixed links below
> > +        * 10G */
> > +       linkmode_zero(pl->supported);
> > +
> > +       linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, pl->supported);
> > +       linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, pl->supported);
> > +       linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, pl->supported);
> > +       linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, pl->supported);
> > +       linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, pl->supported);
> > +       linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, pl->supported);
> > +       linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, pl->supported);
> > +       linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, pl->supported);
> > +       linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, pl->supported);  
> 
> Good idea, but do we have some way to automatically generate the baseT
> link modes?

I think we could with some of the preliminary phy_port patches I had
sent before going into that phy_caps series :

https://lore.kernel.org/netdev/20250213101606.1154014-2-maxime.chevallier@bootlin.com/

It adds the information about medium, maybe we could adapt that, making
sure we filter out BaseT1 for example, but that would be a generic way
of generating that list indeed.

I don't necessarily mean to add this "mediums" thing into this series
right now, we could for now set that list of all BaseT modes in an
internal helper, then later on convert it to the mediums-based
linkmodes listing. I'll go back to phy_ports after phy_caps :)

Thanks,

Maxime


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2025-03-06 13:26 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-03  9:03 [PATCH net-next v4 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 01/13] net: ethtool: Export the link_mode_params definitions Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 02/13] net: phy: Use an internal, searchable storage for the linkmodes Maxime Chevallier
2025-03-06  8:30   ` Paolo Abeni
2025-03-06  8:57     ` Maxime Chevallier
2025-03-06 10:47       ` Paolo Abeni
2025-03-03  9:03 ` [PATCH net-next v4 03/13] net: phy: phy_caps: Move phy_speeds to phy_caps Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 04/13] net: phy: phy_caps: Move __set_linkmode_max_speed " Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 05/13] net: phy: phy_caps: Introduce phy_caps_valid Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 06/13] net: phy: phy_caps: Implement link_capabilities lookup by linkmode Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 07/13] net: phy: phy_caps: Allow looking-up link caps based on speed and duplex Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 08/13] net: phy: phy_device: Use link_capabilities lookup for PHY aneg config Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration Maxime Chevallier
2025-03-04 14:43   ` Maxime Chevallier
2025-03-06 12:48     ` Russell King (Oracle)
2025-03-06  8:56   ` Paolo Abeni
2025-03-06 10:12     ` Maxime Chevallier
2025-03-06 12:51       ` Russell King (Oracle)
2025-03-06 13:26         ` Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 10/13] net: phy: drop phy_settings and the associated lookup helpers Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 11/13] net: phylink: Add a mapping between MAC_CAPS and LINK_CAPS Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 12/13] net: phylink: Convert capabilities to linkmodes using phy_caps Maxime Chevallier
2025-03-03  9:03 ` [PATCH net-next v4 13/13] net: phy: phy_caps: Allow getting an phy_interface's capabilities Maxime Chevallier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).