mbox series

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

Message ID 20241220201506.2791940-1-maxime.chevallier@bootlin.com (mailing list archive)
Headers show
Series net: phy: Introduce a port representation | expand

Message

Maxime Chevallier Dec. 20, 2024, 8:14 p.m. UTC 
Hello everyone,

This is a long overdue series to kick-off the introduction of a better representation
of front-facing ports for ethernet interfaces.

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

 - There's no DT binding although this series adds a generic representation of
   ports
 - 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, ...).

As I said this RFC has no binding, sorry about that, but let's take a look at
the proposed DT syntax :

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.

Thanks,

Maxime

Maxime Chevallier (5):
  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

 drivers/net/phy/Makefile     |   2 +-
 drivers/net/phy/dp83822.c    |  60 +++++++----
 drivers/net/phy/phy_device.c | 167 +++++++++++++++++++++++++++++
 drivers/net/phy/phy_port.c   | 159 ++++++++++++++++++++++++++++
 include/linux/ethtool.h      |  62 +++++++++++
 include/linux/phy.h          |  31 ++++++
 include/linux/phy_port.h     |  69 ++++++++++++
 net/ethtool/common.c         | 197 +++++++++++++++++++----------------
 net/ethtool/common.h         |   7 --
 9 files changed, 633 insertions(+), 121 deletions(-)
 create mode 100644 drivers/net/phy/phy_port.c
 create mode 100644 include/linux/phy_port.h

Comments

Oleksij Rempel Dec. 22, 2024, 3:59 p.m. UTC | #1 
Hi Maxime,

On Fri, Dec 20, 2024 at 09:14:59PM +0100, Maxime Chevallier wrote:
> Hello everyone,
> 
> This is a long overdue series to kick-off the introduction of a better representation
> of front-facing ports for ethernet interfaces.
> 
> First, a short disclaimer. This series is RFC, and there are quite a lot of
> shortcomings :
> 
>  - There's no DT binding although this series adds a generic representation of
>    ports
>  - 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, ...).
> 
> As I said this RFC has no binding, sorry about that, but let's take a look at
> the proposed DT syntax :
> 
> 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.


I love the idea of introducing a port description. It’s a step in the right.
However, in the proposed schema, I see some weak points that could be improved
for better flexibility and usability.

### Weak Points Identified

1. **`lanes` Do Not Provide Enough Information**
   The concept of "lanes" can be ambiguous, as it is context-dependent. For
differential connections, a lane typically refers to a differential pair, but
the schema doesn't clearly define how the lanes map to physical wires or pins.
By describing explicit pin mappings and supported modes, the `lanes` property
becomes obsolete.

2. **`media` Lacks Sufficient Detail**
   The `media` property currently doesn’t add meaningful information for
describing the physical characteristics of a port. By defining the connector
type (e.g., RJ45, BNC, LC) and supported modes explicitly, the need for the
`media` property is removed.

### Proposed Port Description Schema

Here’s how I imagine the port description could look to address these issues:

#### **Device Tree Example**
/* Ports should be in the root of DT */
ports {
    /* Twisted-Pair Example */
    port0: ethernet-port@0 {
        reg = <0>; /* Port index */
        label = "ETH0"; /* Physical label on the device */
        connector = "RJ45"; /* Connector type */
        supported-modes = <10BaseT 100BaseTX>; /* Supported modes */

        pairs {
            pair@0 {
                name = "A"; /* Pair A */
                pins = <1 2>; /* Connector pins */
                phy-mapping = <PHY_TX0_P PHY_TX0_N>; /* PHY pin mapping */
                pse-mapping = <PSE_OUT0_P PSE_OUT0_N>; /* PSE pin mapping */
            };
            pair@1 {
                name = "B"; /* Pair B */
                pins = <3 6>;
                phy-mapping = <PHY_RX0_P PHY_RX0_N>;
                pse-mapping = <PSE_OUT1_P PSE_OUT1_N>;
            };
        };

        pse = <&pse1>; /* Reference to attached PSE controller */

        leds {
            link = <&led0>; /* Link status LED */
            activity = <&led1>; /* Activity LED */
        };
    };

    /* Single-Pair Ethernet (SPE) Example */
    port1: ethernet-port@1 {
        reg = <1>;
        label = "SPE1";
        connector = "H-MTD"; /* Single-pair connector */
        supported-modes = <1000BaseT1>; /* Supported SPE modes */

        pairs {
            pair@0 {
                name = "A"; /* Single pair for SPE */
                pins = <1 2>;
                phy-mapping = <PHY_TXRX0_P PHY_TXRX0_N>;
                pse-mapping = <PSE_OUT0_P PSE_OUT0_N>;
            };
        };

        pse = <&pse2>; /* Reference to a different PSE controller */

        leds {
            link = <&led3>;
            activity = <&led4>;
        };
    };

    /* Coaxial Example */
    port2: ethernet-port@2 {
        reg = <2>;
        label = "COAX0"; /* Physical label for coaxial port */
        connector = "BNC"; /* Connector type for coaxial */
        supported-modes = <10Base2>; /* Supported coaxial modes */

        coaxial {
            pins = <1>; /* Signal pin for coaxial port */
            phy-mapping = <PHY_SIG>; /* Single-ended PHY signal mapping */
            pse-mapping = <PSE_OUT>; /* PSE mapping (if applicable) */
        };
    };

    /* Fiber Example */
    port3: ethernet-port@3 {
        reg = <3>;
        label = "FIBER0"; /* Physical label for fiber port */
        connector = "LC"; /* Connector type for fiber */
        supported-modes = <100BaseFX>; /* Supported fiber modes */

        fiber {
            tx-pins = <1 2>; /* TX signal pins for fiber */
            rx-pins = <3 4>; /* RX signal pins for fiber */
            phy-mapping = <PHY_TX_P PHY_TX_N PHY_RX_P PHY_RX_N>; /* TX/RX PHY mappings */
        };
    };

    /* SFP Port Example */
    port4: ethernet-port@4 {
        reg = <4>; /* Port index */
        label = "SFP0"; /* Physical label for the SFP port */
        connector = "SFP"; /* Connector type for SFP */
        supported-modes = <1000BaseX 10GBaseSR>; /* Supported SFP modes */

        sfp {
            tx-pins = <1 2>; /* TX differential pair for SFP module */
            rx-pins = <3 4>; /* RX differential pair for SFP module */
            phy-mapping = <PHY_TX_P PHY_TX_N PHY_RX_P PHY_RX_N>; /* TX/RX PHY mappings */

            i2c = <&i2c0>; /* Reference to the I2C bus for SFP module management */
            mod-def0 = <&gpio1 5 GPIO_ACTIVE_HIGH>; /* GPIO for MOD-DEF0 */
            tx-disable = <&gpio1 6 GPIO_ACTIVE_HIGH>; /* GPIO for TX_DISABLE */
            los = <&gpio1 7 GPIO_ACTIVE_HIGH>; /* GPIO for Loss of Signal (LOS) */
            presence = <&gpio1 8 GPIO_ACTIVE_HIGH>; /* GPIO for module presence */
        };

        leds {
            link = <&led5>; /* Link status LED */
            activity = <&led6>; /* Activity LED */
        };
    };
};

### How This Schema Addresses the Problems

1. **Precise Pair and Pin Mapping** 
   Each pair is explicitly described with its physical connector pins and
corresponding mappings to PHY and PSE pins. This eliminates ambiguity around
lane definitions and allows the PHY to handle pair-specific configurations like
polarity inversion or swapped pairs.

2. **Simplified Media Representation**
   By specifying `connector` (e.g., RJ45, LC, BNC), the schema provides
practical information about the physical interface without requiring an
abstract `media` property.

3. **Support for LEDs and Power Delivery**
   The schema integrates additional hardware elements like LED assignments and
PSE references, making it easier to configure ports for diagnostics and power
delivery (PoE/PoDL).

I hope this feedback helps refine the port description schema. Let me know what
you think, and I’m happy to discuss this further!

Best regards,  
Oleksij
Oleksij Rempel Dec. 22, 2024, 6:54 p.m. UTC | #2 
On Sun, Dec 22, 2024 at 04:59:43PM +0100, Oleksij Rempel wrote:
> ### Proposed Port Description Schema
> 
> Here’s how I imagine the port description could look to address these issues:
> 
> #### **Device Tree Example**
> /* Ports should be in the root of DT */
> ports {
>     /* Twisted-Pair Example */
>     port0: ethernet-port@0 {
>         reg = <0>; /* Port index */
>         label = "ETH0"; /* Physical label on the device */
>         connector = "RJ45"; /* Connector type */
>         supported-modes = <10BaseT 100BaseTX>; /* Supported modes */
> 
>         pairs {
>             pair@0 {
>                 name = "A"; /* Pair A */
>                 pins = <1 2>; /* Connector pins */
>                 phy-mapping = <PHY_TX0_P PHY_TX0_N>; /* PHY pin mapping */
>                 pse-mapping = <PSE_OUT0_P PSE_OUT0_N>; /* PSE pin mapping */
>             };
>             pair@1 {
>                 name = "B"; /* Pair B */
>                 pins = <3 6>;
>                 phy-mapping = <PHY_RX0_P PHY_RX0_N>;
>                 pse-mapping = <PSE_OUT1_P PSE_OUT1_N>;
>             };
>         };
> 
>         pse = <&pse1>; /* Reference to attached PSE controller */
> 
>         leds {
>             link = <&led0>; /* Link status LED */
>             activity = <&led1>; /* Activity LED */
>         };
>     };

Here is updated version:

ports {
    /* 1000BaseT Port with Ethernet and simple PoE */
    port0: ethernet-port@0 {
        reg = <0>; /* Port index */
        label = "ETH0"; /* Physical label on the device */
        connector = "RJ45"; /* Connector type */
        supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */

        transformer {
            model = "ABC123"; /* Transformer model number */
            manufacturer = "TransformerCo"; /* Manufacturer name */

            pairs {
                pair@0 {
                    name = "A"; /* Pair A */
                    pins = <1 2>; /* Connector pins */
                    phy-mapping = <PHY_TX0_P PHY_TX0_N>; /* PHY pin mapping */
                    center-tap = "CT0"; /* Central tap identifier */
                    pse-negative = <PSE_GND>; /* CT0 connected to GND */
                };
                pair@1 {
                    name = "B"; /* Pair B */
                    pins = <3 6>; /* Connector pins */
                    phy-mapping = <PHY_RX0_P PHY_RX0_N>;
                    center-tap = "CT1"; /* Central tap identifier */
                    pse-positive = <PSE_OUT0>; /* CT1 connected to PSE_OUT0 */
                };
                pair@2 {
                    name = "C"; /* Pair C */
                    pins = <4 5>; /* Connector pins */
                    phy-mapping = <PHY_TXRX1_P PHY_TXRX1_N>; /* PHY connection only */
                    center-tap = "CT2"; /* Central tap identifier */
                    /* No power connection to CT2 */
                };
                pair@3 {
                    name = "D"; /* Pair D */
                    pins = <7 8>; /* Connector pins */
                    phy-mapping = <PHY_TXRX2_P PHY_TXRX2_N>; /* PHY connection only */
                    center-tap = "CT3"; /* Central tap identifier */
                    /* No power connection to CT3 */
                };
            };
        };

        pse = <&pse1>; /* Reference to the attached PSE controller */

        leds {
            ethernet-leds {
                link = <&eth_led0>; /* Link status LED */
                activity = <&eth_led1>; /* Activity LED */
                speed = <&eth_led2>; /* Speed indication LED */
            };

            poe-leds {
                power = <&poe_led0>; /* PoE power status LED */
                fault = <&poe_led1>; /* PoE fault indication LED */
                budget = <&poe_led2>; /* PoE budget usage LED */
            };
        };
    };
};

A port with fully configurable PoE support:

ports {
    /* 1000BaseT Port with Fully Configurable PoE */
    port0: ethernet-port@0 {
        reg = <0>; /* Port index */
        label = "ETH0"; /* Physical label on the device */
        connector = "RJ45"; /* Connector type */
        supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */
        shielding = "grounded"; /* Indicates the connector is shielded */
	/* 
	  grounded: Shield is connected to chassis or earth ground.
	  floating: Shield is not electrically connected.
	  capacitive: Shield is connected to ground via a capacitor.
	  signal: Shield is connected to the signal ground.
         */


        transformer {
            model = "ABC123"; /* Transformer model number */
            manufacturer = "TransformerCo"; /* Manufacturer name */

            pairs {
                pair@0 {
                    name = "A"; /* Pair A */
                    pins = <1 2>; /* Connector pins */
                    phy-mapping = <PHY_TX0_P PHY_TX0_N>; /* PHY pin mapping */
                    center-tap = "CT0"; /* Central tap identifier */
                    /* if pse-positive and pse-negative are present - polarity is configurable */
                    pse-positive = <PSE_OUT0_0>; /* PSE-controlled positive pin -> CT0 */
                    pse-negative = <PSE_OUT0_1>; /* PSE-controlled negative pin -> CT0 */
                };
                pair@1 {
                    name = "B"; /* Pair B */
                    pins = <3 6>; /* Connector pins */
                    phy-mapping = <PHY_RX0_P PHY_RX0_N>;
                    center-tap = "CT1"; /* Central tap identifier */
                    pse-positive = <PSE_OUT1_0>;
                    pse-negative = <PSE_OUT1_1>;
                };
                pair@2 {
                    name = "C"; /* Pair C */
                    pins = <4 5>; /* Connector pins */
                    phy-mapping = <PHY_TXRX1_P PHY_TXRX1_N>; /* PHY connection only */
                    center-tap = "CT2"; /* Central tap identifier */
                    pse-positive = <PSE_OUT2_0>;
                    pse-negative = <PSE_OUT2_1>;
                };
                pair@3 {
                    name = "D"; /* Pair D */
                    pins = <7 8>; /* Connector pins */
                    phy-mapping = <PHY_TXRX2_P PHY_TXRX2_N>; /* PHY connection only */
                    center-tap = "CT3"; /* Central tap identifier */
                    pse-positive = <PSE_OUT3_0>;
                    pse-negative = <PSE_OUT3_1>;
                };
            };
        };

        pse = <&pse1>; /* Reference to the attached PSE controller */

        thermal {
            temp-sensor = <&tsensor0>; /* Reference to temperature sensor */
            /* or */
            thermal-zone = <&thermal_zone0>; /* Reference to thermal zone */
        }

	fuses {
	    overcurrent-fuse {
	        type = "resettable"; /* Resettable polyfuse */
	        max-current = <1000>; /* Maximum current in milliamps */
	        location = "data-pairs"; /* Fuse protects data pairs */
	    };

	    overvoltage-fuse {
	        type = "tvs-diode"; /* TVS diode for surge protection */
	        clamp-voltage = <60>; /* Clamping voltage in volts */
	        location = "poe-pairs"; /* Fuse protects PoE pairs */
	    };
	};

        leds {
            ethernet-leds {
                link = <&eth_led0>; /* Link status LED */
                activity = <&eth_led1>; /* Activity LED */
                speed = <&eth_led2>; /* Speed indication LED */
            };

            poe-leds {
                power = <&poe_led0>; /* PoE power status LED */
                fault = <&poe_led1>; /* PoE fault indication LED */
                budget = <&poe_led2>; /* PoE budget usage LED */
            };
        };
    };
};

In case of PoDL, we will have something like this:

pair@0 {
    name = "A"; /* Single pair for 10BaseT1L */
    pins = <1 2>; /* Connector pins */
    phy-mapping = <PHY_TXRX0_P PHY_TXRX0_N>; /* PHY pin mapping */
    podl-mapping = <PODL_OUT0_P PODL_OUT0_N>; /* PoDL mapping: Positive and negative outputs */
};