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

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

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.

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.

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!

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,

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

...

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,

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.

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

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,

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