[PATCH net-next RFC v2 0/6] net: phy: Introduce a port representation

[PATCH net-next RFC v2 0/6] net: phy: Introduce a port representation

[Maxime Chevallier]

Hello everyone,

This is a second RFC for the introduction of a front-facing interfaces
for ethernet devices.

The firts RFC[1] already got some reviews, focusing on the DT part of
that work. To better lay the ground for further discussions, this second
round includes a binding :)

Oleksij suggested some further possibilities for improving this binding,
as we could consider describing connectors in great details for
crossover detection, PoE ping mappings, etc. However, as this is
preliminary work, the included binding is still quite simple but can
certainly be extended.

This RFC V2 doesn't bring much compared to V1 :
 - A binding was introduced
 - A warning has been fixed in the dp83822 patch
 - The "lanes" property has been made optional

You'll find below more or less the same text as the cover for RFC V1.

First, a short disclaimer. This series is RFC, and there are quite a lot of
shortcomings :

 - The port representation is in a minimal form
 - No SFP support is included, but it will be required for that series to come
   out of RFC as we can't gracefully handle multi-port interfaces without it.

These shortcomigs come from timing constraints, but also because I'd like to
start discussing that topic with some code as a basis.

For now, the only representation we have about the physical ports of an interface
come from the 'port' field (PORT_FIBRE, PORT_TP, PORT_MII, etc.), the presence or
not of an SFP module and the linkmodes reported by the ethtol ksettings ops. This
isn't enough to get a good idea of what the actual interface with the outside world
looks like.

The end-goal of the work this series is a part of is to get support for multi-port
interfaces. My end use-case has 2 ports, each driven by a dedicated PHY, but this
also applies to dual-port PHYs.

The current series introduces the object "struct phy_port". The naming may be
improved, as I think we could consider supporting port representation without
depending on phylib (not the case in this RFC). For now, not only do we integrate
that work in phylib, but only PHY-driven ports are supported.

In some situations, having a good representation of the physical port in devicetree
proves to be quite useful. We're seeing vendor properties to address the lack of
port representation such as micrel,fiber-mode or ti,fiber-mode, but there are needs
for more (glitchy straps that detect fiber mode on a PHY connected to copper,
RJ45 ports connected with 2 lanes only, ...).

Example 1 : PHY with RJ45 connected with 2 lanes only

&mdio {

	ge_phy: ethernet-phy@0 {
		reg = <0>;

		mdi {
			port-0 {
				media = "BaseT",
				lanes = <2>;
			};
		};

	};
};

Example 2 : PHY with a 100BaseFX port, without SFP

&mdio {

	fiber-phy: ethernet-phy@0 {
		reg = <0>;

		mdi {
			port-0 {
				media = "BaseF",
				lanes = <1>;
			};
		};

	};
};

These ports may even be used to specify PSE-PD information for PoE ports that
are drivern by a dedicated PoE controller sitting in-between the PHY and the
connector :

&mdio {

	ge_phy: ethernet-phy@0 {
		reg = <0>;

		mdi {
			port-0 {
				media = "BaseT",
				lanes = <4>;
				pse = <&pse1>;
			};
		};

	};
};

The ports are initialized using the following sequence :

1: The PHY driver's probe() function indicated the max number of ports the device
can control

2: We parse the devicetree to find generic port representations

3: If we don't have at least one port from DT, we create one

4: We call the phy's .attach_port() for each port created so far. This allows
   the phy driver either to take action based on the generic port devicetree
   indications, or to populate the port information based on straps and
   vendor-specific DT properties (think micrel,fiber-mode and similar)

5: If the ports are still not initialized (no .attach_port, no generic DT), then
   we use snesible default value from what the PHY's supported modes.

6: We reconstruct the PHY's supported field in case there are limitations from the
   port (2 lanes on BaseT for example). This last step will need to be changed
   when SFP gets added.

So, the current work is only about init. The next steps for that work are :

 - Introduce phy_port_ops, including a .configure() and a .read_status() to get
 proper support for multi-port PHYs. This also means maintaining a list of
 advertising/lp_advertising modes for each port.

 - Add SFP support. It's a tricky part, the way I see that and have prototyped is
 by representing the SFP cage itself as a port, as well as the SFP module's port.
 ports therefore become chainable.

 - Add the ability to list the ports in userspace.

Prototype work for the above parts exist, but due to lack of time I couldn't get
them polished enough for inclusion in that RFC.

Let me know if you see this going in the right direction, I'm really all ears
for suggestions on this, it's quite difficult to make sure no current use-case
breaks and no heavy rework is needed in PHY drivers.

Patches 1, 2 and 3 are preparatory work for the mediums representation. Patch 4
introduces the phy_port and patch 5 shows an example of usage in the dp83822 phy.

Patch 6 is the new binding.

Thanks,

Maxime

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

Maxime Chevallier (6):
  net: ethtool: common: Make BaseT a 4-lanes mode
  net: ethtool: Introduce ETHTOOL_LINK_MEDIUM_* values
  net: ethtool: Export the link_mode_params definitions
  net: phy: Introduce PHY ports representation
  net: phy: dp83822: Add support for phy_port representation
  dt-bindings: net: Introduce the phy-port description

 .../devicetree/bindings/net/ethernet-phy.yaml |  18 ++
 .../bindings/net/ethernet-port.yaml           |  47 +++++
 drivers/net/phy/Makefile                      |   2 +-
 drivers/net/phy/dp83822.c                     |  63 +++---
 drivers/net/phy/phy_device.c                  | 167 +++++++++++++++
 drivers/net/phy/phy_port.c                    | 166 +++++++++++++++
 include/linux/ethtool.h                       |  73 +++++++
 include/linux/phy.h                           |  31 +++
 include/linux/phy_port.h                      |  69 ++++++
 net/ethtool/common.c                          | 197 ++++++++++--------
 net/ethtool/common.h                          |   7 -
 11 files changed, 716 insertions(+), 124 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/ethernet-port.yaml
 create mode 100644 drivers/net/phy/phy_port.c
 create mode 100644 include/linux/phy_port.h

-- 
2.48.1

[PATCH net-next RFC v2 1/6] net: ethtool: common: Make BaseT a 4-lanes mode

[Maxime Chevallier]

When referring to BaseT ethernet, we are most of the time thinking of
BaseT4 ethernet on Cat5/6/7 cables. This is therefore BaseT4, although
BaseT4 is also possible for 100BaseTX. This is even more true now that
we have a special __LINK_MODE_LANES_T1 mode especially for Single Pair
ethernet.

Mark BaseT as being a 4-lanes mode.

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

 net/ethtool/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 2bd77c94f9f1..8452d3216bce 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -244,7 +244,7 @@ static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
 #define __LINK_MODE_LANES_LR8_ER8_FR8	8
 #define __LINK_MODE_LANES_LRM		1
 #define __LINK_MODE_LANES_MLD2		2
-#define __LINK_MODE_LANES_T		1
+#define __LINK_MODE_LANES_T		4
 #define __LINK_MODE_LANES_T1		1
 #define __LINK_MODE_LANES_X		1
 #define __LINK_MODE_LANES_FX		1
-- 
2.48.1

[PATCH net-next RFC v2 2/6] net: ethtool: Introduce ETHTOOL_LINK_MEDIUM_* values

[Maxime Chevallier]

In an effort to have a better representation of Ethernet ports,
introduce enumeration values representing the various ethernet Mediums.

This is part of the 802.3 naming convention, for example :

1000 Base T 4
 |    |   | |
 |    |   | \_ lanes (4)
 |    |   \___ Medium (T == Twisted Copper Pairs)
 |    \_______ Baseband transmission
 \____________ Speed

 Other example :

10000 Base K X 4
           | | \_ lanes (4)
           | \___ encoding (BaseX is 8b/10b while BaseR is 66b/64b)
           \_____ Medium (K is backplane ethernet)

In the case of representing a physical port, only the medium and number
of lanes should be relevant. One exception would be 1000BaseX, which is
currently also used as a medium in what appears to be any of
1000BaseSX, 1000BaseFX, 1000BaseCX and 1000BaseLX.

These mediums are set in the net/ethtool/common.c lookup table that
maintains a list of all linkmodes with their number of lanes, medium,
encoding, speed and duplex.

One notable exception to this is 100M BaseT Ethernet. 100BaseTX is a 2-lanes
protocol but it will also work on 4-lanes cables, so the lookup table
contains 2 sets of lane numbers, indicating the min number of lanes for
a protocol to work and the "nominal" number of lanes as well.

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

 include/linux/ethtool.h |  48 ++++++++++
 net/ethtool/common.c    | 194 +++++++++++++++++++++-------------------
 net/ethtool/common.h    |   2 +
 3 files changed, 153 insertions(+), 91 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 870994cc3ef7..62b5a9d7efd5 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -210,6 +210,54 @@ static inline u8 *ethtool_rxfh_context_key(struct ethtool_rxfh_context *ctx)
 
 void ethtool_rxfh_context_lost(struct net_device *dev, u32 context_id);
 
+enum ethtool_link_medium {
+	ETHTOOL_LINK_MEDIUM_BASET = 0,
+	ETHTOOL_LINK_MEDIUM_BASEK,
+	ETHTOOL_LINK_MEDIUM_BASES,
+	ETHTOOL_LINK_MEDIUM_BASEC,
+	ETHTOOL_LINK_MEDIUM_BASEL,
+	ETHTOOL_LINK_MEDIUM_BASED,
+	ETHTOOL_LINK_MEDIUM_BASEE,
+	ETHTOOL_LINK_MEDIUM_BASEF,
+	ETHTOOL_LINK_MEDIUM_BASEV,
+	ETHTOOL_LINK_MEDIUM_BASEMLD, /* I don't know what that is */
+	ETHTOOL_LINK_MEDIUM_BASEX, /* Not a true medium, but abused... */
+	ETHTOOL_LINK_MEDIUM_NONE,
+
+	__ETHTOOL_LINK_MEDIUM_LAST,
+};
+
+static inline const char *phy_mediums(enum ethtool_link_medium medium)
+{
+	switch (medium) {
+	case ETHTOOL_LINK_MEDIUM_BASET:
+		return "BaseT";
+	case ETHTOOL_LINK_MEDIUM_BASEK:
+		return "BaseK";
+	case ETHTOOL_LINK_MEDIUM_BASES:
+		return "BaseS";
+	case ETHTOOL_LINK_MEDIUM_BASEC:
+		return "BaseC";
+	case ETHTOOL_LINK_MEDIUM_BASEL:
+		return "BaseL";
+	case ETHTOOL_LINK_MEDIUM_BASED:
+		return "BaseD";
+	case ETHTOOL_LINK_MEDIUM_BASEE:
+		return "BaseE";
+	case ETHTOOL_LINK_MEDIUM_BASEF:
+		return "BaseF";
+	case ETHTOOL_LINK_MEDIUM_BASEV:
+		return "BaseV";
+	case ETHTOOL_LINK_MEDIUM_BASEMLD:
+		return "BaseMLD";
+	case ETHTOOL_LINK_MEDIUM_BASEX:
+		return "BaseX";
+	case ETHTOOL_LINK_MEDIUM_NONE:
+		return "None";
+	default: return "unknown";
+	}
+}
+
 /* 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 8452d3216bce..487731945224 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -255,11 +255,22 @@ static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
 #define __LINK_MODE_LANES_DR8_2		8
 #define __LINK_MODE_LANES_T1BRR		1
 
-#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex)	\
+#define __DEFINE_LINK_MODE_PARAMS_LANES(_speed, _type, _min_lanes, _lanes, _duplex, _medium) \
 	[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = {		\
 		.speed  = SPEED_ ## _speed, \
+		.min_lanes  = _min_lanes, \
+		.lanes  = _lanes, \
+		.duplex	= __DUPLEX_ ## _duplex, \
+		.medium = ETHTOOL_LINK_MEDIUM_BASE ## _medium \
+	}
+
+#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex, _medium)	\
+	[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = {		\
+		.speed  = SPEED_ ## _speed, \
+		.min_lanes  = __LINK_MODE_LANES_ ## _type, \
 		.lanes  = __LINK_MODE_LANES_ ## _type, \
-		.duplex	= __DUPLEX_ ## _duplex \
+		.duplex	= __DUPLEX_ ## _duplex, \
+		.medium = ETHTOOL_LINK_MEDIUM_BASE ## _medium \
 	}
 #define __DUPLEX_Half DUPLEX_HALF
 #define __DUPLEX_Full DUPLEX_FULL
@@ -268,116 +279,117 @@ static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
 		.speed	= SPEED_UNKNOWN, \
 		.lanes	= 0, \
 		.duplex	= DUPLEX_UNKNOWN, \
+		.medium	= ETHTOOL_LINK_MEDIUM_NONE, \
 	}
 
 const struct link_mode_info link_mode_params[] = {
-	__DEFINE_LINK_MODE_PARAMS(10, T, Half),
-	__DEFINE_LINK_MODE_PARAMS(10, T, Full),
-	__DEFINE_LINK_MODE_PARAMS(100, T, Half),
-	__DEFINE_LINK_MODE_PARAMS(100, T, Full),
-	__DEFINE_LINK_MODE_PARAMS(1000, T, Half),
-	__DEFINE_LINK_MODE_PARAMS(1000, T, Full),
+	__DEFINE_LINK_MODE_PARAMS_LANES(10, T, 2, 4, Half, T),
+	__DEFINE_LINK_MODE_PARAMS_LANES(10, T, 2, 4, Full, T),
+	__DEFINE_LINK_MODE_PARAMS_LANES(100, T, 2, 4, Half, T),
+	__DEFINE_LINK_MODE_PARAMS_LANES(100, T, 2, 4, Full, T),
+	__DEFINE_LINK_MODE_PARAMS(1000, T, Half, T),
+	__DEFINE_LINK_MODE_PARAMS(1000, T, Full, T),
 	__DEFINE_SPECIAL_MODE_PARAMS(Autoneg),
 	__DEFINE_SPECIAL_MODE_PARAMS(TP),
 	__DEFINE_SPECIAL_MODE_PARAMS(AUI),
 	__DEFINE_SPECIAL_MODE_PARAMS(MII),
 	__DEFINE_SPECIAL_MODE_PARAMS(FIBRE),
 	__DEFINE_SPECIAL_MODE_PARAMS(BNC),
-	__DEFINE_LINK_MODE_PARAMS(10000, T, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, T, Full, T),
 	__DEFINE_SPECIAL_MODE_PARAMS(Pause),
 	__DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
-	__DEFINE_LINK_MODE_PARAMS(2500, X, Full),
+	__DEFINE_LINK_MODE_PARAMS(2500, X, Full, X),
 	__DEFINE_SPECIAL_MODE_PARAMS(Backplane),
-	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
+	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full, K),
 	[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = {
 		.speed	= SPEED_10000,
 		.lanes	= 1,
 		.duplex = DUPLEX_FULL,
 	},
-	__DEFINE_LINK_MODE_PARAMS(20000, MLD2, Full),
-	__DEFINE_LINK_MODE_PARAMS(20000, KR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(40000, KR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(40000, CR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(40000, SR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(40000, LR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(56000, KR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(56000, CR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(56000, SR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(56000, LR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(25000, CR, Full),
-	__DEFINE_LINK_MODE_PARAMS(25000, KR, Full),
-	__DEFINE_LINK_MODE_PARAMS(25000, SR, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, CR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, KR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, KR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, SR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, CR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, SR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(1000, X, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, CR, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, SR, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, LR, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, LRM, Full),
-	__DEFINE_LINK_MODE_PARAMS(10000, ER, Full),
-	__DEFINE_LINK_MODE_PARAMS(2500, T, Full),
-	__DEFINE_LINK_MODE_PARAMS(5000, T, Full),
+	__DEFINE_LINK_MODE_PARAMS(20000, MLD2, Full, MLD),
+	__DEFINE_LINK_MODE_PARAMS(20000, KR2, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(40000, KR4, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(40000, CR4, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(40000, SR4, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(40000, LR4, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(56000, KR4, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(56000, CR4, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(56000, SR4, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(56000, LR4, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(25000, CR, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(25000, KR, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(25000, SR, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(50000, CR2, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(50000, KR2, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(100000, KR4, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(100000, SR4, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(100000, CR4, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(50000, SR2, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(1000, X, Full, X), /* :( */
+	__DEFINE_LINK_MODE_PARAMS(10000, CR, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(10000, SR, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(10000, LR, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(10000, LRM, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(10000, ER, Full, E),
+	__DEFINE_LINK_MODE_PARAMS(2500, T, Full, T),
+	__DEFINE_LINK_MODE_PARAMS(5000, T, Full, T),
 	__DEFINE_SPECIAL_MODE_PARAMS(FEC_NONE),
 	__DEFINE_SPECIAL_MODE_PARAMS(FEC_RS),
 	__DEFINE_SPECIAL_MODE_PARAMS(FEC_BASER),
-	__DEFINE_LINK_MODE_PARAMS(50000, KR, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, SR, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, CR, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, Full),
-	__DEFINE_LINK_MODE_PARAMS(50000, DR, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, KR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, SR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, CR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, DR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, KR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, SR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, DR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, CR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(100, T1, Full),
-	__DEFINE_LINK_MODE_PARAMS(1000, T1, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, KR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, SR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, DR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, CR8, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, KR, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(50000, SR, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(50000, CR, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, Full, L), /* I guess ? */
+	__DEFINE_LINK_MODE_PARAMS(50000, DR, Full, D),
+	__DEFINE_LINK_MODE_PARAMS(100000, KR2, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(100000, SR2, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(100000, CR2, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(100000, DR2, Full, D),
+	__DEFINE_LINK_MODE_PARAMS(200000, KR4, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(200000, SR4, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(200000, DR4, Full, D),
+	__DEFINE_LINK_MODE_PARAMS(200000, CR4, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(100, T1, Full, T),
+	__DEFINE_LINK_MODE_PARAMS(1000, T1, Full, T),
+	__DEFINE_LINK_MODE_PARAMS(400000, KR8, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(400000, SR8, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(400000, DR8, Full, D),
+	__DEFINE_LINK_MODE_PARAMS(400000, CR8, Full, C),
 	__DEFINE_SPECIAL_MODE_PARAMS(FEC_LLRS),
-	__DEFINE_LINK_MODE_PARAMS(100000, KR, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, SR, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, DR, Full),
-	__DEFINE_LINK_MODE_PARAMS(100000, CR, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, KR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, SR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, DR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(200000, CR2, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, KR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, SR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, DR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(400000, CR4, Full),
-	__DEFINE_LINK_MODE_PARAMS(100, FX, Half),
-	__DEFINE_LINK_MODE_PARAMS(100, FX, Full),
-	__DEFINE_LINK_MODE_PARAMS(10, T1L, Full),
-	__DEFINE_LINK_MODE_PARAMS(800000, CR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(800000, KR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(800000, DR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(800000, DR8_2, Full),
-	__DEFINE_LINK_MODE_PARAMS(800000, SR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(800000, VR8, Full),
-	__DEFINE_LINK_MODE_PARAMS(10, T1S, Full),
-	__DEFINE_LINK_MODE_PARAMS(10, T1S, Half),
-	__DEFINE_LINK_MODE_PARAMS(10, T1S_P2MP, Half),
-	__DEFINE_LINK_MODE_PARAMS(10, T1BRR, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, KR, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(100000, SR, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(100000, DR, Full, D),
+	__DEFINE_LINK_MODE_PARAMS(100000, CR, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(200000, KR2, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(200000, SR2, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(200000, DR2, Full, D),
+	__DEFINE_LINK_MODE_PARAMS(200000, CR2, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(400000, KR4, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(400000, SR4, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, Full, L),
+	__DEFINE_LINK_MODE_PARAMS(400000, DR4, Full, D),
+	__DEFINE_LINK_MODE_PARAMS(400000, CR4, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(100, FX, Half, F),
+	__DEFINE_LINK_MODE_PARAMS(100, FX, Full, F),
+	__DEFINE_LINK_MODE_PARAMS(10, T1L, Full, T),
+	__DEFINE_LINK_MODE_PARAMS(800000, CR8, Full, C),
+	__DEFINE_LINK_MODE_PARAMS(800000, KR8, Full, K),
+	__DEFINE_LINK_MODE_PARAMS(800000, DR8, Full, D),
+	__DEFINE_LINK_MODE_PARAMS(800000, DR8_2, Full, D),
+	__DEFINE_LINK_MODE_PARAMS(800000, SR8, Full, S),
+	__DEFINE_LINK_MODE_PARAMS(800000, VR8, Full, V),
+	__DEFINE_LINK_MODE_PARAMS(10, T1S, Full, T),
+	__DEFINE_LINK_MODE_PARAMS(10, T1S, Half, T),
+	__DEFINE_LINK_MODE_PARAMS(10, T1S_P2MP, Half, T),
+	__DEFINE_LINK_MODE_PARAMS(10, T1BRR, Full, T),
 };
 static_assert(ARRAY_SIZE(link_mode_params) == __ETHTOOL_LINK_MODE_MASK_NBITS);
 
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index 850eadde4bfc..b25011a05fea 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -16,8 +16,10 @@
 
 struct link_mode_info {
 	int				speed;
+	u8				min_lanes;
 	u8				lanes;
 	u8				duplex;
+	enum ethtool_link_medium	medium;
 };
 
 struct genl_info;
-- 
2.48.1

[PATCH net-next RFC v2 3/6] net: ethtool: Export the link_mode_params definitions

[Maxime Chevallier]

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>
---
RFC V2: No changes

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

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 62b5a9d7efd5..519a90ce24d3 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -258,6 +258,16 @@ static inline const char *phy_mediums(enum ethtool_link_medium medium)
 	}
 }
 
+struct link_mode_info {
+	int                             speed;
+	u8                              min_lanes;
+	u8                              lanes;
+	u8                              duplex;
+	enum ethtool_link_medium        medium;
+};
+
+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 487731945224..43b6135c8f0a 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -392,6 +392,7 @@ const struct link_mode_info link_mode_params[] = {
 	__DEFINE_LINK_MODE_PARAMS(10, T1BRR, Full, T),
 };
 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 b25011a05fea..ee250a5c5364 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -14,14 +14,6 @@
 
 #define __SOF_TIMESTAMPING_CNT (const_ilog2(SOF_TIMESTAMPING_LAST) + 1)
 
-struct link_mode_info {
-	int				speed;
-	u8				min_lanes;
-	u8				lanes;
-	u8				duplex;
-	enum ethtool_link_medium	medium;
-};
-
 struct genl_info;
 struct hwtstamp_provider_desc;
 
@@ -34,7 +26,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

[PATCH net-next RFC v2 4/6] net: phy: Introduce PHY ports representation

[Maxime Chevallier]

Ethernet provides a wide variety of layer 1 protocols and standards for
data transmission. The front-facing ports of an interface have their own
complexity and configurability.

Introduce a representation of these front-facing ports. The current code
is minimalistic and only support ports controlled by PHY devices, but
the plan is to extend that to SFP as well as raw Ethernet MACs that
don't use PHY devices.

This minimal port representation allows describing the media and number
of lanes of a port. From that information, we can derive the linkmodes
usable on the port, which can be used to limit the capabilities of an
interface.

For now, the port lanes and medium is derived from devicetree, defined
by the PHY driver, or populated with default values (as we assume that
all PHYs expose at least one port).

The typical example is 100M ethernet. 100BaseT can work using only 2
lanes on a Cat 5 cables. However, in the situation where a 10/100/1000
capable PHY is wired to its RJ45 port through 2 lanes only, we have no
way of detecting that. The "max-speed" DT property can be used, but a
more accurate representation can be used :

mdi {
	port-0 {
		media = "BaseT";
		lanes = <2>;
	};
};

From that information, we can derive the max speed reachable on the
port.

Another benefit of having that is to avoid vendor-specific DT properties
(micrel,fiber-mode or ti,fiber-mode).

This basic representation is meant to be expanded, by the introduction
of port ops, userspace listing of ports, and support for multi-port
devices.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
RFC V2: Made lanes optional, and added a helper to get default lanes
values

 drivers/net/phy/Makefile     |   2 +-
 drivers/net/phy/phy_device.c | 167 +++++++++++++++++++++++++++++++++++
 drivers/net/phy/phy_port.c   | 166 ++++++++++++++++++++++++++++++++++
 include/linux/ethtool.h      |  15 ++++
 include/linux/phy.h          |  31 +++++++
 include/linux/phy_port.h     |  69 +++++++++++++++
 6 files changed, 449 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/phy/phy_port.c
 create mode 100644 include/linux/phy_port.h

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index c8dac6e92278..de1415a46629 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_port.o
 mdio-bus-y			+= mdio_bus.o mdio_device.o
 
 ifdef CONFIG_MDIO_DEVICE
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 46713d27412b..f385b8fc70d1 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -656,6 +656,13 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
 
 	dev->state = PHY_DOWN;
 	INIT_LIST_HEAD(&dev->leds);
+	INIT_LIST_HEAD(&dev->ports);
+
+	/* The driver's probe function must change that to the real number
+	 * of ports possible on the PHY. We assume by default we are dealing
+	 * with a single-port PHY
+	 */
+	dev->max_n_ports = 1;
 
 	mutex_init(&dev->lock);
 	INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
@@ -3405,6 +3412,159 @@ static int of_phy_leds(struct phy_device *phydev)
 	return 0;
 }
 
+static int phy_add_port(struct phy_device *phydev, struct phy_port *port)
+{
+	int ret = 0;
+
+	if (phydev->n_ports == phydev->max_n_ports)
+		return -EBUSY;
+
+	/* We set all ports as active by default, PHY drivers may deactivate
+	 * them (when unused)
+	 */
+	port->active = true;
+
+	if (phydev->drv && phydev->drv->attach_port)
+		ret = phydev->drv->attach_port(phydev, port);
+
+	if (ret)
+		return ret;
+
+	/* The PHY driver might have added, removed or set medium/lanes info,
+	 * so update the port supported accordingly.
+	 */
+	phy_port_update_supported(port);
+
+	list_add(&port->head, &phydev->ports);
+
+	phydev->n_ports++;
+
+	return 0;
+}
+
+static void phy_del_port(struct phy_device *phydev, struct phy_port *port)
+{
+	if (!phydev->n_ports)
+		return;
+
+	list_del(&port->head);
+
+	phydev->n_ports--;
+}
+
+static void phy_cleanup_ports(struct phy_device *phydev)
+{
+	struct phy_port *tmp, *port;
+
+	list_for_each_entry_safe(port, tmp, &phydev->ports, head) {
+		phy_del_port(phydev, port);
+		phy_port_destroy(port);
+	}
+}
+
+static int phy_default_setup_single_port(struct phy_device *phydev)
+{
+	struct phy_port *port = phy_port_alloc();
+
+	if (!port)
+		return -ENOMEM;
+
+	port->parent_type = PHY_PORT_PHY;
+	port->phy = phydev;
+	linkmode_copy(port->supported, phydev->supported);
+
+	/* default medium is copper */
+	if (!port->mediums)
+		port->mediums |= BIT(ETHTOOL_LINK_MEDIUM_BASET);
+
+	phy_add_port(phydev, port);
+
+	return 0;
+}
+
+static int of_phy_ports(struct phy_device *phydev)
+{
+	struct device_node *node = phydev->mdio.dev.of_node;
+	struct device_node *mdi;
+	struct phy_port *port;
+	int err;
+
+	if (!IS_ENABLED(CONFIG_OF_MDIO))
+		return 0;
+
+	if (!node)
+		return 0;
+
+	mdi = of_get_child_by_name(node, "mdi");
+	if (!mdi)
+		return 0;
+
+	for_each_available_child_of_node_scoped(mdi, port_node) {
+		port = phy_of_parse_port(port_node);
+		if (IS_ERR(port)) {
+			err = PTR_ERR(port);
+			goto out_err;
+		}
+
+		port->parent_type = PHY_PORT_PHY;
+		port->phy = phydev;
+		err = phy_add_port(phydev, port);
+		if (err)
+			goto out_err;
+	}
+	of_node_put(mdi);
+
+	return 0;
+
+out_err:
+	phy_cleanup_ports(phydev);
+	of_node_put(mdi);
+	return err;
+}
+
+static int phy_setup_ports(struct phy_device *phydev)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(ports_supported);
+	struct phy_port *port;
+	int ret;
+
+	ret = of_phy_ports(phydev);
+	if (ret)
+		return ret;
+
+	if (phydev->n_ports < phydev->max_n_ports) {
+		ret = phy_default_setup_single_port(phydev);
+		if (ret)
+			goto out;
+	}
+
+	linkmode_zero(ports_supported);
+
+	/* Aggregate the supported modes, which are made-up of :
+	 *  - What the PHY itself supports
+	 *  - What the sum of all ports support
+	 */
+	list_for_each_entry(port, &phydev->ports, head)
+		if (port->active)
+			linkmode_or(ports_supported, ports_supported,
+				    port->supported);
+
+	if (!linkmode_empty(ports_supported))
+		linkmode_and(phydev->supported, phydev->supported,
+			     ports_supported);
+
+	/* For now, the phy->port field is set as the first active port's type */
+	list_for_each_entry(port, &phydev->ports, head)
+		if (port->active)
+			phydev->port = phy_port_get_type(port);
+
+	return 0;
+
+out:
+	phy_cleanup_ports(phydev);
+	return ret;
+}
+
 /**
  * fwnode_mdio_find_device - Given a fwnode, find the mdio_device
  * @fwnode: pointer to the mdio_device's fwnode
@@ -3554,6 +3714,11 @@ static int phy_probe(struct device *dev)
 		phydev->is_gigabit_capable = 1;
 
 	of_set_phy_supported(phydev);
+
+	err = phy_setup_ports(phydev);
+	if (err)
+		goto out;
+
 	phy_advertise_supported(phydev);
 
 	/* Get PHY default EEE advertising modes and handle them as potentially
@@ -3630,6 +3795,8 @@ static int phy_remove(struct device *dev)
 
 	phydev->state = PHY_DOWN;
 
+	phy_cleanup_ports(phydev);
+
 	sfp_bus_del_upstream(phydev->sfp_bus);
 	phydev->sfp_bus = NULL;
 
diff --git a/drivers/net/phy/phy_port.c b/drivers/net/phy/phy_port.c
new file mode 100644
index 000000000000..3a7bdc44b556
--- /dev/null
+++ b/drivers/net/phy/phy_port.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Framework to drive Ethernet ports
+ *
+ * Copyright (c) 2024 Maxime Chevallier <maxime.chevallier@bootlin.com>
+ */
+
+#include <linux/linkmode.h>
+#include <linux/of.h>
+#include <linux/phy_port.h>
+
+/**
+ * phy_port_alloc: Allocate a new phy_port
+ *
+ * Returns a newly allocated struct phy_port, or NULL.
+ */
+struct phy_port *phy_port_alloc(void)
+{
+	struct phy_port *port;
+
+	port = kzalloc(sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return NULL;
+
+	linkmode_zero(port->supported);
+	INIT_LIST_HEAD(&port->head);
+
+	return port;
+}
+EXPORT_SYMBOL_GPL(phy_port_alloc);
+
+/**
+ * phy_port_destroy: Free a struct phy_port
+ */
+void phy_port_destroy(struct phy_port *port)
+{
+	kfree(port);
+}
+EXPORT_SYMBOL_GPL(phy_port_destroy);
+
+static void ethtool_medium_get_supported(unsigned long *supported,
+					 enum ethtool_link_medium medium,
+					 int lanes)
+{
+	int i;
+
+	for (i = 0; i < __ETHTOOL_LINK_MODE_MASK_NBITS; i++) {
+		/* Special bits such as Autoneg, Pause, Asym_pause, etc. are
+		 * set and will be masked away by the port parent.
+		 */
+		if (link_mode_params[i].medium == ETHTOOL_LINK_MEDIUM_NONE) {
+			linkmode_set_bit(i, supported);
+			continue;
+		}
+
+		/* For most cases, min_lanes == lanes, except for 10/100BaseT that work
+		 * on 2 lanes but are compatible with 4 lanes mediums
+		 */
+		if (link_mode_params[i].medium == medium &&
+		    link_mode_params[i].lanes >= lanes &&
+		    link_mode_params[i].min_lanes <= lanes) {
+			linkmode_set_bit(i, supported);
+		}
+	}
+}
+
+static enum ethtool_link_medium ethtool_str_to_medium(const char *str)
+{
+	int i;
+
+	for (i = 0; i < __ETHTOOL_LINK_MEDIUM_LAST; i++)
+		if (!strcmp(phy_mediums(i), str))
+			return i;
+
+	return ETHTOOL_LINK_MEDIUM_NONE;
+}
+
+/**
+ * phy_of_parse_port: Create a phy_port from a firmware representation
+ *
+ * Returns a newly allocated and initialized phy_port pointer, or an ERR_PTR.
+ */
+struct phy_port *phy_of_parse_port(struct device_node *dn)
+{
+	struct fwnode_handle *fwnode = of_fwnode_handle(dn);
+	enum ethtool_link_medium medium;
+	struct phy_port *port;
+	struct property *prop;
+	const char *med_str;
+	u32 lanes, mediums = 0;
+	int ret;
+
+	ret = fwnode_property_read_u32(fwnode, "lanes", &lanes);
+	if (ret)
+		lanes = 0;
+
+	ret = fwnode_property_read_string(fwnode, "media", &med_str);
+	if (ret)
+		return ERR_PTR(ret);
+
+	of_property_for_each_string(to_of_node(fwnode), "media", prop, med_str) {
+		medium = ethtool_str_to_medium(med_str);
+		if (medium == ETHTOOL_LINK_MEDIUM_NONE)
+			return ERR_PTR(-EINVAL);
+
+		mediums |= BIT(medium);
+	}
+
+	if (!mediums)
+		return ERR_PTR(-EINVAL);
+
+	port = phy_port_alloc();
+	if (!port)
+		return ERR_PTR(-ENOMEM);
+
+	port->lanes = lanes;
+	port->mediums = mediums;
+
+	return port;
+}
+EXPORT_SYMBOL_GPL(phy_of_parse_port);
+
+/**
+ * phy_port_update_supported: Setup the port->supported field
+ * port: the port to update
+ *
+ * Once the port's medium list and number of lanes has been configured based
+ * on firmware, straps and vendor-specific properties, this function may be
+ * called to update the port's supported linkmodes list.
+ *
+ * Any mode that was manually set in the port's supported list remains set.
+ */
+void phy_port_update_supported(struct phy_port *port)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+	int i, lanes = 1;
+
+	/* If there's no lanes specified, we grab the default number of
+	 * lanes as the max of the default lanes for each medium
+	 */
+	if (!port->lanes)
+		for_each_set_bit(i, &port->mediums, __ETHTOOL_LINK_MEDIUM_LAST)
+			lanes = max_t(int, lanes, phy_medium_default_lanes(i));
+
+	for_each_set_bit(i, &port->mediums, __ETHTOOL_LINK_MEDIUM_LAST) {
+		linkmode_zero(supported);
+		ethtool_medium_get_supported(supported, i, port->lanes);
+		linkmode_or(port->supported, port->supported, supported);
+	}
+}
+EXPORT_SYMBOL_GPL(phy_port_update_supported);
+
+/**
+ * phy_port_get_type: get the PORT_* attribut for that port.
+ */
+int phy_port_get_type(struct phy_port *port)
+{
+	if (port->mediums & ETHTOOL_LINK_MEDIUM_BASET)
+		return PORT_TP;
+
+	if (phy_port_is_fiber(port) ||
+	    (port->mediums & BIT(ETHTOOL_LINK_MEDIUM_BASEX)))
+		return PORT_FIBRE;
+
+	return PORT_OTHER;
+}
+EXPORT_SYMBOL_GPL(phy_port_get_type);
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 519a90ce24d3..32fe062b715c 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -227,6 +227,10 @@ enum ethtool_link_medium {
 	__ETHTOOL_LINK_MEDIUM_LAST,
 };
 
+#define ETHTOOL_MEDIUM_FIBER_BITS (BIT(ETHTOOL_LINK_MEDIUM_BASES) | \
+				   BIT(ETHTOOL_LINK_MEDIUM_BASEL) | \
+				   BIT(ETHTOOL_LINK_MEDIUM_BASEF))
+
 static inline const char *phy_mediums(enum ethtool_link_medium medium)
 {
 	switch (medium) {
@@ -258,6 +262,17 @@ static inline const char *phy_mediums(enum ethtool_link_medium medium)
 	}
 }
 
+static inline int phy_medium_default_lanes(enum ethtool_link_medium medium)
+{
+	/* Let's consider that the default BaseT ethernet is BaseT4, i.e.
+	 * Gigabit Ethernet.
+	 */
+	if (medium == ETHTOOL_LINK_MEDIUM_BASET)
+		return 4;
+
+	return 1;
+}
+
 struct link_mode_info {
 	int                             speed;
 	u8                              min_lanes;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 19f076a71f94..5d536b601e3f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -21,6 +21,7 @@
 #include <linux/mii.h>
 #include <linux/mii_timestamper.h>
 #include <linux/module.h>
+#include <linux/phy_port.h>
 #include <linux/timer.h>
 #include <linux/workqueue.h>
 #include <linux/mod_devicetable.h>
@@ -642,6 +643,9 @@ struct macsec_ops;
  * @master_slave_state: Current master/slave configuration
  * @mii_ts: Pointer to time stamper callbacks
  * @psec: Pointer to Power Sourcing Equipment control struct
+ * @ports: List of PHY ports structures
+ * n_ports: Number of ports currently attached to the PHY
+ * @max_n_ports: Max number of ports this PHY can expose
  * @lock:  Mutex for serialization access to PHY
  * @state_queue: Work queue for state machine
  * @link_down_events: Number of times link was lost
@@ -734,6 +738,7 @@ struct phy_device {
 
 	/* Host supported PHY interface types. Should be ignored if empty. */
 	DECLARE_PHY_INTERFACE_MASK(host_interfaces);
+	DECLARE_PHY_INTERFACE_MASK(sfp_bus_interfaces);
 
 #ifdef CONFIG_LED_TRIGGER_PHY
 	struct phy_led_trigger *phy_led_triggers;
@@ -776,6 +781,10 @@ struct phy_device {
 	struct mii_timestamper *mii_ts;
 	struct pse_control *psec;
 
+	struct list_head ports;
+	int n_ports;
+	int max_n_ports;
+
 	u8 mdix;
 	u8 mdix_ctrl;
 
@@ -1273,6 +1282,27 @@ struct phy_driver {
 	 */
 	int (*led_polarity_set)(struct phy_device *dev, int index,
 				unsigned long modes);
+
+	/**
+	 * @attach_port: Indicates to the PHY driver that a port is detected
+	 * @dev: PHY device to notify
+	 * @port: The port being added
+	 *
+	 * Called when a port that needs to be driven by the PHY is found. The
+	 * number of time this will be called depends on phydev->max_n_ports,
+	 * which the driver can change in .probe().
+	 *
+	 * The port that is being passed may or may not be initialized. If it is
+	 * already initialized, it is by the generic port representation from
+	 * devicetree, which superseeds any strapping or vendor-specific
+	 * properties.
+	 *
+	 * If the port isn't initialized, the port->mediums and port->lanes
+	 * fields must be set, possibly according to stapping information.
+	 *
+	 * Returns 0, or an error code.
+	 */
+	int (*attach_port)(struct phy_device *dev, struct phy_port *port);
 };
 #define to_phy_driver(d) container_of_const(to_mdio_common_driver(d),		\
 				      struct phy_driver, mdiodrv)
@@ -2083,6 +2113,7 @@ void phy_trigger_machine(struct phy_device *phydev);
 void phy_mac_interrupt(struct phy_device *phydev);
 void phy_start_machine(struct phy_device *phydev);
 void phy_stop_machine(struct phy_device *phydev);
+
 void phy_ethtool_ksettings_get(struct phy_device *phydev,
 			       struct ethtool_link_ksettings *cmd);
 int phy_ethtool_ksettings_set(struct phy_device *phydev,
diff --git a/include/linux/phy_port.h b/include/linux/phy_port.h
new file mode 100644
index 000000000000..b34c15523adc
--- /dev/null
+++ b/include/linux/phy_port.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <linux/ethtool.h>
+#include <linux/types.h>
+
+#ifndef __PHY_PORT_H
+#define __PHY_PORT_H
+
+struct phy_port;
+
+/**
+ * enum phy_port_parent - The device this port is attached to
+ *
+ * @PHY_PORT_PHY: Indicates that the port is driven by a PHY device
+ */
+enum phy_port_parent {
+	PHY_PORT_PHY,
+};
+
+/**
+ * struct phy_port - A representation of a network device physical interface
+ *
+ * @head: Used by the port's parent to list ports
+ * @parent_type: The type of device this port is directly connected to
+ * @phy: If the parent is PHY_PORT_PHYDEV, the PHY controlling that port
+ * @lanes: The number of lanes (diff pairs) this port has, 0 if not applicable
+ * @medium: The physical medium this port provides access to
+ * @supported: The link modes this port can expose
+ * @active: Indicates if the port is currently part of the active link.
+ */
+struct phy_port {
+	struct list_head head;
+	enum phy_port_parent parent_type;
+	union {
+		struct phy_device *phy;
+	};
+
+	int lanes;
+	unsigned long mediums;
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+
+	bool active;
+};
+
+struct phy_port *phy_port_alloc(void);
+void phy_port_destroy(struct phy_port *port);
+
+static inline struct phy_device *port_phydev(struct phy_port *port)
+{
+	return port->phy;
+}
+
+struct phy_port *phy_of_parse_port(struct device_node *dn);
+
+static inline bool phy_port_is_copper(struct phy_port *port)
+{
+	return port->mediums == BIT(ETHTOOL_LINK_MEDIUM_BASET);
+}
+
+static inline bool phy_port_is_fiber(struct phy_port *port)
+{
+	return !!(port->mediums & ETHTOOL_MEDIUM_FIBER_BITS);
+}
+
+void phy_port_update_supported(struct phy_port *port);
+
+int phy_port_get_type(struct phy_port *port);
+
+#endif
-- 
2.48.1

[PATCH net-next RFC v2 5/6] net: phy: dp83822: Add support for phy_port representation

[Maxime Chevallier]

With the phy_port representation intrduced, we can use .attach_port to
populate the port information based on either the straps or the
ti,fiber-mode property. This allows simplifying the probe function and
allow users to override the strapping configuration.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
RFC V2: Addressd a warning for an unused variable

 drivers/net/phy/dp83822.c | 63 ++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 24 deletions(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 6599feca1967..a1ddb7791452 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -781,17 +781,6 @@ static int dp83822_of_init(struct phy_device *phydev)
 	struct device *dev = &phydev->mdio.dev;
 	const char *of_val;
 
-	/* Signal detection for the PHY is only enabled if the FX_EN and the
-	 * SD_EN pins are strapped. Signal detection can only enabled if FX_EN
-	 * is strapped otherwise signal detection is disabled for the PHY.
-	 */
-	if (dp83822->fx_enabled && dp83822->fx_sd_enable)
-		dp83822->fx_signal_det_low = device_property_present(dev,
-								     "ti,link-loss-low");
-	if (!dp83822->fx_enabled)
-		dp83822->fx_enabled = device_property_present(dev,
-							      "ti,fiber-mode");
-
 	if (!device_property_read_string(dev, "ti,gpio2-clk-out", &of_val)) {
 		if (strcmp(of_val, "mac-if") == 0) {
 			dp83822->gpio2_clk_out = DP83822_CLK_SRC_MAC_IF;
@@ -884,6 +873,43 @@ static int dp83822_read_straps(struct phy_device *phydev)
 	return 0;
 }
 
+static int dp83822_attach_port(struct phy_device *phydev, struct phy_port *port)
+{
+	struct dp83822_private *dp83822 = phydev->priv;
+	int ret;
+
+	if (port->mediums) {
+		if (phy_port_is_fiber(port) ||
+		    port->mediums & BIT(ETHTOOL_LINK_MEDIUM_BASEX))
+			dp83822->fx_enabled = true;
+	} else {
+		ret = dp83822_read_straps(phydev);
+		if (ret)
+			return ret;
+
+#ifdef CONFIG_OF_MDIO
+		if (dp83822->fx_enabled && dp83822->fx_sd_enable)
+			dp83822->fx_signal_det_low =
+				device_property_present(dev, "ti,link-loss-low");
+		if (!dp83822->fx_enabled)
+			dp83822->fx_enabled =
+				device_property_present(dev, "ti,fiber-mode");
+#endif
+
+		if (dp83822->fx_enabled) {
+			port->lanes = 1;
+			port->mediums = BIT(ETHTOOL_LINK_MEDIUM_BASEF) |
+					BIT(ETHTOOL_LINK_MEDIUM_BASEX);
+		} else {
+			/* This PHY can only to 100BaseTX max, so on 2 lanes */
+			port->lanes = 2;
+			port->mediums = BIT(ETHTOOL_LINK_MEDIUM_BASET);
+		}
+	}
+
+	return 0;
+}
+
 static int dp8382x_probe(struct phy_device *phydev)
 {
 	struct dp83822_private *dp83822;
@@ -900,25 +926,13 @@ static int dp8382x_probe(struct phy_device *phydev)
 
 static int dp83822_probe(struct phy_device *phydev)
 {
-	struct dp83822_private *dp83822;
 	int ret;
 
 	ret = dp8382x_probe(phydev);
 	if (ret)
 		return ret;
 
-	dp83822 = phydev->priv;
-
-	ret = dp83822_read_straps(phydev);
-	if (ret)
-		return ret;
-
-	ret = dp83822_of_init(phydev);
-	if (ret)
-		return ret;
-
-	if (dp83822->fx_enabled)
-		phydev->port = PORT_FIBRE;
+	dp83822_of_init(phydev);
 
 	return 0;
 }
@@ -1104,6 +1118,7 @@ static int dp83822_led_hw_control_get(struct phy_device *phydev, u8 index,
 		.led_hw_is_supported = dp83822_led_hw_is_supported,	\
 		.led_hw_control_set = dp83822_led_hw_control_set,	\
 		.led_hw_control_get = dp83822_led_hw_control_get,	\
+		.attach_port = dp83822_attach_port		\
 	}
 
 #define DP83825_PHY_DRIVER(_id, _name)				\
-- 
2.48.1

[PATCH net-next RFC v2 6/6] dt-bindings: net: Introduce the phy-port description

[Maxime Chevallier]

The ability to describe the physical ports of Ethernet devices is useful
to describe multi-port devices, as well as to remove any ambiguity with
regard to the nature of the port.

Moreover, describing ports allows for a better description of features
that are tied to connectors, such as PoE through the PSE-PD devices.

Introduce a binding to allow describing the ports, for now with 2
attributes :

 - The number of lanes, which is a quite generic property that allows
   differentating between multiple similar technologies such as BaseT1
   and "regular" BaseT (which usually means BaseT4).

 - The media that can be used on that port, such as BaseT for Twisted
   Copper, BaseC for coax copper, BaseS/L for Fiber, BaseK for backplane
   ethernet, etc. This allows defining the nature of the port, and
   therefore avoids the need for vendor-specific properties such as
   "micrel,fiber-mode" or "ti,fiber-mode".

The port description lives in its own file, as it is intended in the
future to allow describing the ports for phy-less devices.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
RFC V2: New patch

 .../devicetree/bindings/net/ethernet-phy.yaml | 18 +++++++
 .../bindings/net/ethernet-port.yaml           | 47 +++++++++++++++++++
 2 files changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ethernet-port.yaml

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 2c71454ae8e3..950fdacfd27d 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -261,6 +261,17 @@ properties:
 
     additionalProperties: false
 
+  mdi:
+    type: object
+
+    patternProperties:
+      '^port-[a-f0-9]+$':
+        $ref: /schemas/net/ethernet-port.yaml#
+
+        unevaluatedProperties: false
+
+    additionalProperties: false
+
 required:
   - reg
 
@@ -297,5 +308,12 @@ examples:
                     default-state = "keep";
                 };
             };
+
+            mdi {
+              port-0 {
+                lanes = <2>;
+                media = "BaseT";
+              };
+            };
         };
     };
diff --git a/Documentation/devicetree/bindings/net/ethernet-port.yaml b/Documentation/devicetree/bindings/net/ethernet-port.yaml
new file mode 100644
index 000000000000..bf0f64f1b0aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ethernet-port.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/ethernet-port.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic Ethernet Port
+
+maintainers:
+  - Maxime Chevallier <maxime.chevallier@bootlin.com>
+
+description:
+  An Ethernet port represents an output, such as a connector, of a network
+  component such as a PHY, an Ethernet controller with no PHY, or an SFP module.
+
+properties:
+
+  lanes:
+    description:
+      Defines the number of lanes on the port, that is the number of physical
+      channels used to convey the data with the link partner.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  media:
+    description:
+      The mediums, as defined in 802.3, that can be used on the port.
+    items:
+      enum:
+        - BaseT
+        - BaseK
+        - BaseS
+        - BaseC
+        - BaseL
+        - BaseD
+        - BaseE
+        - BaseF
+        - BaseV
+        - BaseMLD
+        - BaseX
+
+required:
+  - lanes
+  - media
+
+additionalProperties: true
+
+...
-- 
2.48.1

Re: [PATCH net-next RFC v2 1/6] net: ethtool: common: Make BaseT a 4-lanes mode

[Russell King (Oracle)]

On Wed, Jan 22, 2025 at 06:42:46PM +0100, Maxime Chevallier wrote:
> When referring to BaseT ethernet, we are most of the time thinking of
> BaseT4 ethernet on Cat5/6/7 cables. This is therefore BaseT4, although
> BaseT4 is also possible for 100BaseTX. This is even more true now that
> we have a special __LINK_MODE_LANES_T1 mode especially for Single Pair
> ethernet.
> 
> Mark BaseT as being a 4-lanes mode.

This is a problem:

1.4.50 10BASE-T: IEEE 802.3 Physical Layer specification for a 10 Mb/s
CSMA/CD local area network over two pairs of twisted-pair telephone
wire. (See IEEE Std 802.3, Clause 14.)

Then we have the 100BASE-T* family, which can be T1, T2, T4 or TX.
T1 is over a single balanced twisted pair. T2 is over two pairs of
Cat 3 or better. T4 is over four pairs of Cat3/4/5.

The common 100BASE-T* type is TX, which is over two pairs of Cat5.
This is sadly what the ethtool 100baseT link modes are used to refer
to.

We do have a separate link mode for 100baseT1, but not 100baseT4.

So, these ethtool modes that are of the form baseT so far are
describing generally two pairs, one pair in each direction. (T1 is
a single pair that is bidirectional.)

It's only once we get to 1000BASE-T (1000baseT) that we get to an
ethtool link mode that has four lanes in a bidirectional fashion.

So, simply redefining this ends up changing 10baseT and 100baseT from
a single lane in each direction to four lanes (and is a "lane" here
defined as the total number of pairs used for communication in both
directions, or the total number of lanes used in either direction.

Hence, I'm not sure this makes sense.

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

Re: [PATCH net-next RFC v2 1/6] net: ethtool: common: Make BaseT a 4-lanes mode

[Russell King (Oracle)]

On Wed, Jan 22, 2025 at 06:55:17PM +0000, Russell King (Oracle) wrote:
> On Wed, Jan 22, 2025 at 06:42:46PM +0100, Maxime Chevallier wrote:
> > When referring to BaseT ethernet, we are most of the time thinking of
> > BaseT4 ethernet on Cat5/6/7 cables. This is therefore BaseT4, although
> > BaseT4 is also possible for 100BaseTX. This is even more true now that
> > we have a special __LINK_MODE_LANES_T1 mode especially for Single Pair
> > ethernet.
> > 
> > Mark BaseT as being a 4-lanes mode.
> 
> This is a problem:
> 
> 1.4.50 10BASE-T: IEEE 802.3 Physical Layer specification for a 10 Mb/s
> CSMA/CD local area network over two pairs of twisted-pair telephone
> wire. (See IEEE Std 802.3, Clause 14.)
> 
> Then we have the 100BASE-T* family, which can be T1, T2, T4 or TX.
> T1 is over a single balanced twisted pair. T2 is over two pairs of
> Cat 3 or better. T4 is over four pairs of Cat3/4/5.
> 
> The common 100BASE-T* type is TX, which is over two pairs of Cat5.
> This is sadly what the ethtool 100baseT link modes are used to refer
> to.
> 
> We do have a separate link mode for 100baseT1, but not 100baseT4.
> 
> So, these ethtool modes that are of the form baseT so far are
> describing generally two pairs, one pair in each direction. (T1 is
> a single pair that is bidirectional.)
> 
> It's only once we get to 1000BASE-T (1000baseT) that we get to an
> ethtool link mode that has four lanes in a bidirectional fashion.
> 
> So, simply redefining this ends up changing 10baseT and 100baseT from
> a single lane in each direction to four lanes (and is a "lane" here
> defined as the total number of pairs used for communication in both
> directions, or the total number of lanes used in either direction.
> 
> Hence, I'm not sure this makes sense.

Looking at patch 2, I don't see why you need patch 1. It's not really
improving the situation. Before the patch, the number of lanes for
some BASE-T is wrong. After the patch, the number of lanes for some
BASE-T is also wrong - just a different subset.

I think you should drop this patch and just have patch 2.

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

Re: [PATCH net-next RFC v2 0/6] net: phy: Introduce a port representation

[Russell King (Oracle)]

On Wed, Jan 22, 2025 at 06:42:45PM +0100, Maxime Chevallier wrote:
> First, a short disclaimer. This series is RFC, and there are quite a lot of
> shortcomings :
> 
>  - The port representation is in a minimal form
>  - No SFP support is included, but it will be required for that series to come
>    out of RFC as we can't gracefully handle multi-port interfaces without it.

Agreed - because I think SFP brings with it a whole host of issues that
describing the media facing port would be error prone - and to do it
accurately, we'd need a table of quirks to fix lots of broken modules.

For example, many SFP modules with RJ45 connectors describe themselves
as having a LC connector!

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

Re: [PATCH net-next RFC v2 2/6] net: ethtool: Introduce ETHTOOL_LINK_MEDIUM_* values

[Kory Maincent]

On Wed, 22 Jan 2025 18:42:47 +0100
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> In an effort to have a better representation of Ethernet ports,
> introduce enumeration values representing the various ethernet Mediums.
> 
> This is part of the 802.3 naming convention, for example :
> 
> 1000 Base T 4
>  |    |   | |
>  |    |   | \_ lanes (4)
>  |    |   \___ Medium (T == Twisted Copper Pairs)
>  |    \_______ Baseband transmission
>  \____________ Speed
> 
>  Other example :
> 
> 10000 Base K X 4
>            | | \_ lanes (4)
>            | \___ encoding (BaseX is 8b/10b while BaseR is 66b/64b)
>            \_____ Medium (K is backplane ethernet)
> 
> In the case of representing a physical port, only the medium and number
> of lanes should be relevant. One exception would be 1000BaseX, which is
> currently also used as a medium in what appears to be any of
> 1000BaseSX, 1000BaseFX, 1000BaseCX and 1000BaseLX.



> -	__DEFINE_LINK_MODE_PARAMS(100, T, Half),
> -	__DEFINE_LINK_MODE_PARAMS(100, T, Full),
> -	__DEFINE_LINK_MODE_PARAMS(1000, T, Half),
> -	__DEFINE_LINK_MODE_PARAMS(1000, T, Full),
> +	__DEFINE_LINK_MODE_PARAMS_LANES(10, T, 2, 4, Half, T),
> +	__DEFINE_LINK_MODE_PARAMS_LANES(10, T, 2, 4, Full, T),
> +	__DEFINE_LINK_MODE_PARAMS_LANES(100, T, 2, 4, Half, T),
> +	__DEFINE_LINK_MODE_PARAMS_LANES(100, T, 2, 4, Full, T),


> -	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full),
> -	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full),
> -	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
> +	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full, K),
> +	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full, K),
> +	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full, K),

The medium information is used twice.
Maybe we could redefine the __DEFINE_LINK_MODE_PARAMS like this to avoid
redundant information:
#define __DEFINE_LINK_MODE_PARAMS(_speed, _medium, _encoding, _lanes, _duplex)

And something like this when the lanes are not a fix number:
#define __DEFINE_LINK_MODE_PARAMS_LANES_RANGE(_speed, _medium, _encoding,
_min_lanes, _max_lanes, _duplex)

Then we can remove all the __LINK_MODE_LANES_XX defines which may be
wrong as you have spotted in patch 1.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next RFC v2 4/6] net: phy: Introduce PHY ports representation

[Simon Horman]

On Wed, Jan 22, 2025 at 06:42:49PM +0100, Maxime Chevallier wrote:
> Ethernet provides a wide variety of layer 1 protocols and standards for
> data transmission. The front-facing ports of an interface have their own
> complexity and configurability.
> 
> Introduce a representation of these front-facing ports. The current code
> is minimalistic and only support ports controlled by PHY devices, but
> the plan is to extend that to SFP as well as raw Ethernet MACs that
> don't use PHY devices.
> 
> This minimal port representation allows describing the media and number
> of lanes of a port. From that information, we can derive the linkmodes
> usable on the port, which can be used to limit the capabilities of an
> interface.
> 
> For now, the port lanes and medium is derived from devicetree, defined
> by the PHY driver, or populated with default values (as we assume that
> all PHYs expose at least one port).
> 
> The typical example is 100M ethernet. 100BaseT can work using only 2
> lanes on a Cat 5 cables. However, in the situation where a 10/100/1000
> capable PHY is wired to its RJ45 port through 2 lanes only, we have no
> way of detecting that. The "max-speed" DT property can be used, but a
> more accurate representation can be used :
> 
> mdi {
> 	port-0 {
> 		media = "BaseT";
> 		lanes = <2>;
> 	};
> };
> 
> >From that information, we can derive the max speed reachable on the
> port.
> 
> Another benefit of having that is to avoid vendor-specific DT properties
> (micrel,fiber-mode or ti,fiber-mode).
> 
> This basic representation is meant to be expanded, by the introduction
> of port ops, userspace listing of ports, and support for multi-port
> devices.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

...

> diff --git a/drivers/net/phy/phy_port.c b/drivers/net/phy/phy_port.c

...

> +/**
> + * phy_port_destroy: Free a struct phy_port
> + */
> +void phy_port_destroy(struct phy_port *port)

nit: The Kernel doc for this function should include documentation of
     the port parameter.

     Flagged, along with several other Kernel doc issues,
     by ./scripts/kernel-doc -none

...

Re: [PATCH net-next RFC v2 0/6] net: phy: Introduce a port representation

[Kory Maincent]

On Wed, 22 Jan 2025 18:42:45 +0100
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> Hello everyone,
> 
> This is a second RFC for the introduction of a front-facing interfaces
> for ethernet devices.
> 
> The firts RFC[1] already got some reviews, focusing on the DT part of
> that work. To better lay the ground for further discussions, this second
> round includes a binding :)
> 
> Oleksij suggested some further possibilities for improving this binding,
> as we could consider describing connectors in great details for
> crossover detection, PoE ping mappings, etc. However, as this is
> preliminary work, the included binding is still quite simple but can
> certainly be extended.
> 
> This RFC V2 doesn't bring much compared to V1 :
>  - A binding was introduced
>  - A warning has been fixed in the dp83822 patch
>  - The "lanes" property has been made optional

Small question, I know you want to begin with something simple but would it be
possible to consider how to support the port representation in NIT and
switch drivers? Maybe it is out of your scope but it would be nice if you
consider how NIT and switches can support it in your development.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next RFC v2 0/6] net: phy: Introduce a port representation

[Kory Maincent]

On Thu, 23 Jan 2025 11:31:21 +0100
Kory Maincent <kory.maincent@bootlin.com> wrote:

> On Wed, 22 Jan 2025 18:42:45 +0100
> Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
> 
> > Hello everyone,
> > 
> > This is a second RFC for the introduction of a front-facing interfaces
> > for ethernet devices.
> > 
> > The firts RFC[1] already got some reviews, focusing on the DT part of
> > that work. To better lay the ground for further discussions, this second
> > round includes a binding :)
> > 
> > Oleksij suggested some further possibilities for improving this binding,
> > as we could consider describing connectors in great details for
> > crossover detection, PoE ping mappings, etc. However, as this is
> > preliminary work, the included binding is still quite simple but can
> > certainly be extended.
> > 
> > This RFC V2 doesn't bring much compared to V1 :
> >  - A binding was introduced
> >  - A warning has been fixed in the dp83822 patch
> >  - The "lanes" property has been made optional  
> 
> Small question, I know you want to begin with something simple but would it be
> possible to consider how to support the port representation in NIT and
> switch drivers? Maybe it is out of your scope but it would be nice if you
> consider how NIT and switches can support it in your development.

s/NIT/NIC/

Sorry.
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next RFC v2 1/6] net: ethtool: common: Make BaseT a 4-lanes mode

[Maxime Chevallier]

Hello Russell,

On Wed, 22 Jan 2025 18:55:17 +0000
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Jan 22, 2025 at 06:42:46PM +0100, Maxime Chevallier wrote:
> > When referring to BaseT ethernet, we are most of the time thinking of
> > BaseT4 ethernet on Cat5/6/7 cables. This is therefore BaseT4, although
> > BaseT4 is also possible for 100BaseTX. This is even more true now that
> > we have a special __LINK_MODE_LANES_T1 mode especially for Single Pair
> > ethernet.
> > 
> > Mark BaseT as being a 4-lanes mode.  
> 
> This is a problem:
> 
> 1.4.50 10BASE-T: IEEE 802.3 Physical Layer specification for a 10 Mb/s
> CSMA/CD local area network over two pairs of twisted-pair telephone
> wire. (See IEEE Std 802.3, Clause 14.)
> 
> Then we have the 100BASE-T* family, which can be T1, T2, T4 or TX.
> T1 is over a single balanced twisted pair. T2 is over two pairs of
> Cat 3 or better. T4 is over four pairs of Cat3/4/5.
> 
> The common 100BASE-T* type is TX, which is over two pairs of Cat5.
> This is sadly what the ethtool 100baseT link modes are used to refer
> to.
> 
> We do have a separate link mode for 100baseT1, but not 100baseT4.
> 
> So, these ethtool modes that are of the form baseT so far are
> describing generally two pairs, one pair in each direction. (T1 is
> a single pair that is bidirectional.)
> 
> It's only once we get to 1000BASE-T (1000baseT) that we get to an
> ethtool link mode that has four lanes in a bidirectional fashion.
> 
> So, simply redefining this ends up changing 10baseT and 100baseT from
> a single lane in each direction to four lanes (and is a "lane" here
> defined as the total number of pairs used for communication in both
> directions, or the total number of lanes used in either direction.
> 
> Hence, I'm not sure this makes sense.
> 

I'm fine with your justification, so let's simplify and drop that
patch then. That should also avoid the lanes/pairs confusion as well.

Thanks for the feedback !

Maxime

Re: [PATCH net-next RFC v2 6/6] dt-bindings: net: Introduce the phy-port description

[Rob Herring]

On Wed, Jan 22, 2025 at 06:42:51PM +0100, Maxime Chevallier wrote:
> The ability to describe the physical ports of Ethernet devices is useful
> to describe multi-port devices, as well as to remove any ambiguity with
> regard to the nature of the port.
> 
> Moreover, describing ports allows for a better description of features
> that are tied to connectors, such as PoE through the PSE-PD devices.

Seems like we need a connector binding like we've ended up needing in 
other cases.

> 
> Introduce a binding to allow describing the ports, for now with 2
> attributes :
> 
>  - The number of lanes, which is a quite generic property that allows
>    differentating between multiple similar technologies such as BaseT1
>    and "regular" BaseT (which usually means BaseT4).
> 
>  - The media that can be used on that port, such as BaseT for Twisted
>    Copper, BaseC for coax copper, BaseS/L for Fiber, BaseK for backplane
>    ethernet, etc. This allows defining the nature of the port, and
>    therefore avoids the need for vendor-specific properties such as
>    "micrel,fiber-mode" or "ti,fiber-mode".
> 
> The port description lives in its own file, as it is intended in the
> future to allow describing the ports for phy-less devices.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> RFC V2: New patch
> 
>  .../devicetree/bindings/net/ethernet-phy.yaml | 18 +++++++
>  .../bindings/net/ethernet-port.yaml           | 47 +++++++++++++++++++
>  2 files changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/ethernet-port.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 2c71454ae8e3..950fdacfd27d 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -261,6 +261,17 @@ properties:
>  
>      additionalProperties: false
>  
> +  mdi:
> +    type: object
> +
> +    patternProperties:
> +      '^port-[a-f0-9]+$':

'port' is already a node name for graphs. It's also the deprecated name 
for 'ethernet-port' in the switch/DSA bindings.

> +        $ref: /schemas/net/ethernet-port.yaml#

A confusing name considering we already have 'ethernet-port'.

Rob

Re: [PATCH net-next RFC v2 2/6] net: ethtool: Introduce ETHTOOL_LINK_MEDIUM_* values

[Maxime Chevallier]

Hi Köry,

On Thu, 23 Jan 2025 10:35:34 +0100
Kory Maincent <kory.maincent@bootlin.com> wrote:

> On Wed, 22 Jan 2025 18:42:47 +0100
> Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
> 
> > In an effort to have a better representation of Ethernet ports,
> > introduce enumeration values representing the various ethernet Mediums.
> > 
> > This is part of the 802.3 naming convention, for example :
> > 
> > 1000 Base T 4
> >  |    |   | |
> >  |    |   | \_ lanes (4)
> >  |    |   \___ Medium (T == Twisted Copper Pairs)
> >  |    \_______ Baseband transmission
> >  \____________ Speed
> > 
> >  Other example :
> > 
> > 10000 Base K X 4
> >            | | \_ lanes (4)
> >            | \___ encoding (BaseX is 8b/10b while BaseR is 66b/64b)
> >            \_____ Medium (K is backplane ethernet)
> > 
> > In the case of representing a physical port, only the medium and number
> > of lanes should be relevant. One exception would be 1000BaseX, which is
> > currently also used as a medium in what appears to be any of
> > 1000BaseSX, 1000BaseFX, 1000BaseCX and 1000BaseLX.  
> 
> 
> 
> > -	__DEFINE_LINK_MODE_PARAMS(100, T, Half),
> > -	__DEFINE_LINK_MODE_PARAMS(100, T, Full),
> > -	__DEFINE_LINK_MODE_PARAMS(1000, T, Half),
> > -	__DEFINE_LINK_MODE_PARAMS(1000, T, Full),
> > +	__DEFINE_LINK_MODE_PARAMS_LANES(10, T, 2, 4, Half, T),
> > +	__DEFINE_LINK_MODE_PARAMS_LANES(10, T, 2, 4, Full, T),
> > +	__DEFINE_LINK_MODE_PARAMS_LANES(100, T, 2, 4, Half, T),
> > +	__DEFINE_LINK_MODE_PARAMS_LANES(100, T, 2, 4, Full, T),  
> 
> 
> > -	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full),
> > -	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full),
> > -	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
> > +	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full, K),
> > +	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full, K),
> > +	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full, K),  
> 
> The medium information is used twice.
> Maybe we could redefine the __DEFINE_LINK_MODE_PARAMS like this to avoid
> redundant information:
> #define __DEFINE_LINK_MODE_PARAMS(_speed, _medium, _encoding, _lanes, _duplex)
> 
> And something like this when the lanes are not a fix number:
> #define __DEFINE_LINK_MODE_PARAMS_LANES_RANGE(_speed, _medium, _encoding,
> _min_lanes, _max_lanes, _duplex)
> 
> Then we can remove all the __LINK_MODE_LANES_XX defines which may be
> wrong as you have spotted in patch 1.

My apologies, I missed your review and didn't address it in the new
iteration :(

I will give this a try, see hw this looks, so that we can separate the
encoding info from the medium info.

Thanks !

Maxime

> Regards,

Re: [PATCH net-next RFC v2 2/6] net: ethtool: Introduce ETHTOOL_LINK_MEDIUM_* values

[Maxime Chevallier]

Hi again Köry,

> Hi Köry,
> 
> On Thu, 23 Jan 2025 10:35:34 +0100
> Kory Maincent <kory.maincent@bootlin.com> wrote:
> 
> > On Wed, 22 Jan 2025 18:42:47 +0100
> > Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
> >   
> > > In an effort to have a better representation of Ethernet ports,
> > > introduce enumeration values representing the various ethernet Mediums.
> > > 
> > > This is part of the 802.3 naming convention, for example :
> > > 
> > > 1000 Base T 4
> > >  |    |   | |
> > >  |    |   | \_ lanes (4)
> > >  |    |   \___ Medium (T == Twisted Copper Pairs)
> > >  |    \_______ Baseband transmission
> > >  \____________ Speed
> > > 
> > >  Other example :
> > > 
> > > 10000 Base K X 4
> > >            | | \_ lanes (4)
> > >            | \___ encoding (BaseX is 8b/10b while BaseR is 66b/64b)
> > >            \_____ Medium (K is backplane ethernet)
> > > 
> > > In the case of representing a physical port, only the medium and number
> > > of lanes should be relevant. One exception would be 1000BaseX, which is
> > > currently also used as a medium in what appears to be any of
> > > 1000BaseSX, 1000BaseFX, 1000BaseCX and 1000BaseLX.    
> > 
> > 
> >   
> > > -	__DEFINE_LINK_MODE_PARAMS(100, T, Half),
> > > -	__DEFINE_LINK_MODE_PARAMS(100, T, Full),
> > > -	__DEFINE_LINK_MODE_PARAMS(1000, T, Half),
> > > -	__DEFINE_LINK_MODE_PARAMS(1000, T, Full),
> > > +	__DEFINE_LINK_MODE_PARAMS_LANES(10, T, 2, 4, Half, T),
> > > +	__DEFINE_LINK_MODE_PARAMS_LANES(10, T, 2, 4, Full, T),
> > > +	__DEFINE_LINK_MODE_PARAMS_LANES(100, T, 2, 4, Half, T),
> > > +	__DEFINE_LINK_MODE_PARAMS_LANES(100, T, 2, 4, Full, T),    
> > 
> >   
> > > -	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full),
> > > -	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full),
> > > -	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
> > > +	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full, K),
> > > +	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full, K),
> > > +	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full, K),    
> > 
> > The medium information is used twice.
> > Maybe we could redefine the __DEFINE_LINK_MODE_PARAMS like this to avoid
> > redundant information:
> > #define __DEFINE_LINK_MODE_PARAMS(_speed, _medium, _encoding, _lanes, _duplex)
> > 
> > And something like this when the lanes are not a fix number:
> > #define __DEFINE_LINK_MODE_PARAMS_LANES_RANGE(_speed, _medium, _encoding,
> > _min_lanes, _max_lanes, _duplex)
> > 
> > Then we can remove all the __LINK_MODE_LANES_XX defines which may be
> > wrong as you have spotted in patch 1.  
> 
> I will give this a try, see hw this looks, so that we can separate the
> encoding info from the medium info.

So I did give it a go, but it turns out to be much more complex than
expected... The _type information from definitions like :

__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full, K),

(here _type is KX4) can't really be split into the individual
attributes <Medium, Encoding, Lanes> without having some complex macro
logic. This type is used to convert to actual linkmodes that already
exist in the kernel :

#define ETHTOOL_LINK_MODE(speed, type, duplex) \
	ETHTOOL_LINK_MODE_ ## speed ## base ## type ## _ ## duplex ## _BIT

Say we want to generate the _type from <Medium, Encoding, Lanes> with a
macro, we have to cover all the weird cases :

1000BaseT => No encoding, no lanes
10000BaseKX4 => K medium, X encoding, 4 lanes
10000BeseKX => K medium, X encding, no lanes (which means 1 lane)
1000BaseX => Just encoding
100000BaseLR4_ER4 => One link mode that applies for 2 mediums ?

While doable, this will probably end-up more complex and hard to
maintain than re-specifying the medium :(

Maxime