[PATCH net-next 0/3] net: phy: dp83822: Add support for changing the MAC series termination

[PATCH net-next 0/3] net: phy: dp83822: Add support for changing the MAC series termination

[Dimitri Fedrau via B4 Relay]

The dp83822 provides the possibility to set the resistance value of the
the MAC series termination. Modifying the resistance to an appropriate
value can reduce signal reflections and therefore improve signal quality.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
Dimitri Fedrau (3):
      dt-bindings: net: ethernet-phy: add property mac-series-termination-ohms
      net: phy: Add helper for getting MAC series termination resistance
      net: phy: dp83822: Add support for changing the MAC series termination

 .../devicetree/bindings/net/ethernet-phy.yaml      |  5 +++
 drivers/net/phy/dp83822.c                          | 36 ++++++++++++++++++++++
 drivers/net/phy/phy_device.c                       | 15 +++++++++
 include/linux/phy.h                                |  3 ++
 4 files changed, 59 insertions(+)
---
base-commit: 865eddcf0afbcd54f79b81e6327ea40c997714c7
change-id: 20250305-dp83822-mac-impedance-840435b0c9e6

Best regards,
-- 
Dimitri Fedrau <dimitri.fedrau@liebherr.com>

[PATCH net-next 1/3] dt-bindings: net: ethernet-phy: add property mac-series-termination-ohms

[Dimitri Fedrau via B4 Relay]

From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

Add property mac-series-termination-ohms in the device tree bindings for
selecting the resistance value of the builtin series termination resistors
of the PHY. Changing the resistance to an appropriate value can reduce
signal reflections and therefore improve signal quality.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 Documentation/devicetree/bindings/net/ethernet-phy.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 824bbe4333b7ed95cc39737d3c334a20aa890f01..4a710315a83ccf15bfc210ae432ae988cf31e04c 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -238,6 +238,11 @@ properties:
       peak-to-peak specified in ANSI X3.263. When omitted, the PHYs default
       will be left as is.
 
+  mac-series-termination-ohms:
+    description:
+      The resistance value of the series termination resistor. When omitted, the
+      PHYs default will be left as is.
+
   leds:
     type: object
 

-- 
2.39.5

Re: [PATCH net-next 1/3] dt-bindings: net: ethernet-phy: add property mac-series-termination-ohms

[Rob Herring]

On Fri, Mar 07, 2025 at 11:30:01AM +0100, Dimitri Fedrau wrote:
> Add property mac-series-termination-ohms in the device tree bindings for
> selecting the resistance value of the builtin series termination resistors
> of the PHY. Changing the resistance to an appropriate value can reduce
> signal reflections and therefore improve signal quality.
> 
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> ---
>  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 824bbe4333b7ed95cc39737d3c334a20aa890f01..4a710315a83ccf15bfc210ae432ae988cf31e04c 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -238,6 +238,11 @@ properties:
>        peak-to-peak specified in ANSI X3.263. When omitted, the PHYs default
>        will be left as is.
>  
> +  mac-series-termination-ohms:

A property of the MAC (or associated with it) should be in the MAC's 
node. Also, sounds like either either end could have a property. We 
already have similar properties in other cases: 'termination-ohms' for 
example. That appears to be for series term as well given a value of 
120ohms. 

Rob

Re: [PATCH net-next 1/3] dt-bindings: net: ethernet-phy: add property mac-series-termination-ohms

[Andrew Lunn]

On Tue, Mar 11, 2025 at 12:33:44PM -0500, Rob Herring wrote:
> On Fri, Mar 07, 2025 at 11:30:01AM +0100, Dimitri Fedrau wrote:
> > Add property mac-series-termination-ohms in the device tree bindings for
> > selecting the resistance value of the builtin series termination resistors
> > of the PHY. Changing the resistance to an appropriate value can reduce
> > signal reflections and therefore improve signal quality.
> > 
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > ---
> >  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > index 824bbe4333b7ed95cc39737d3c334a20aa890f01..4a710315a83ccf15bfc210ae432ae988cf31e04c 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > @@ -238,6 +238,11 @@ properties:
> >        peak-to-peak specified in ANSI X3.263. When omitted, the PHYs default
> >        will be left as is.
> >  
> > +  mac-series-termination-ohms:
> 
> A property of the MAC (or associated with it) should be in the MAC's 
> node.

But it is the PHY which uses the property, and the PHY which is
implementing the resistor.

Also, a PHY has two sides, one towards the MAC and a second media side
to the network peer via the Ethernet cable. Both sides need
termination resistors. So we need something in the name to make it
clear which side of the PHY we are talking about. So we might end up
with something like mac-termination-ohms and media-termination-ohms,
in the PHY node.

> Also, sounds like either either end could have a property.

True, the MAC could also need a similar property, since the outputs
from the MAC to the PHY needs termination resistors.  For the MAC,
termination-ohms is probably sufficient, or phy-termination-ohms to
indicate it is towards the PHY?

	Andrew

[PATCH net-next 2/3] net: phy: Add helper for getting MAC series termination resistance

[Dimitri Fedrau via B4 Relay]

From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

Add helper which returns the MAC series termination resistance value.
Modifying the resistance to an appropriate value can reduce signal
reflections and therefore improve signal quality.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 drivers/net/phy/phy_device.c | 15 +++++++++++++++
 include/linux/phy.h          |  3 +++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b2d32fbc8c8507f6280e4c3c671466f672c1cc8f..575583aa8b4d49bef796e96059e4f0dd56b351f9 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2975,6 +2975,21 @@ int phy_get_tx_amplitude_gain(struct phy_device *phydev, struct device *dev,
 }
 EXPORT_SYMBOL_GPL(phy_get_tx_amplitude_gain);
 
+/**
+ * phy_get_mac_series_termination - stores MAC series termination in @val
+ * @phydev: phy_device struct
+ * @dev: pointer to the devices device struct
+ * @val: MAC series termination
+ *
+ * Returns: 0 on success, < 0 on failure
+ */
+int phy_get_mac_series_termination(struct phy_device *phydev,
+				   struct device *dev, u32 *val)
+{
+	return phy_get_u32_property(dev, "mac-series-termination-ohms", val);
+}
+EXPORT_SYMBOL_GPL(phy_get_mac_series_termination);
+
 static int phy_led_set_brightness(struct led_classdev *led_cdev,
 				  enum led_brightness value)
 {
diff --git a/include/linux/phy.h b/include/linux/phy.h
index c4a6385faf41c3156d1a9086eaf5c746640cddbe..633899c4250c91a366849ee0c11d93feec48262f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -2052,6 +2052,9 @@ int phy_get_tx_amplitude_gain(struct phy_device *phydev, struct device *dev,
 			      enum ethtool_link_mode_bit_indices linkmode,
 			      u32 *val);
 
+int phy_get_mac_series_termination(struct phy_device *phydev,
+				   struct device *dev, u32 *val);
+
 void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv,
 		       bool *tx_pause, bool *rx_pause);
 

-- 
2.39.5

[PATCH net-next 3/3] net: phy: dp83822: Add support for changing the MAC series termination

[Dimitri Fedrau via B4 Relay]

From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

The dp83822 provides the possibility to set the resistance value of the
the MAC series termination. Modifying the resistance to an appropriate
value can reduce signal reflections and therefore improve signal quality.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 drivers/net/phy/dp83822.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 3662f3905d5ade8ad933608fcaeabb714a588418..bd8082cdc00e4abe165ee7c515b252eb32149d45 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -33,6 +33,7 @@
 #define MII_DP83822_MLEDCR	0x25
 #define MII_DP83822_LDCTRL	0x403
 #define MII_DP83822_LEDCFG1	0x460
+#define MII_DP83822_IOCTRL	0x461
 #define MII_DP83822_IOCTRL1	0x462
 #define MII_DP83822_IOCTRL2	0x463
 #define MII_DP83822_GENCFG	0x465
@@ -118,6 +119,9 @@
 #define DP83822_LEDCFG1_LED1_CTRL	GENMASK(11, 8)
 #define DP83822_LEDCFG1_LED3_CTRL	GENMASK(7, 4)
 
+/* IOCTRL bits */
+#define DP83822_IOCTRL_MAC_IMPEDANCE_CTRL	GENMASK(4, 1)
+
 /* IOCTRL1 bits */
 #define DP83822_IOCTRL1_GPIO3_CTRL		GENMASK(10, 8)
 #define DP83822_IOCTRL1_GPIO3_CTRL_LED3		BIT(0)
@@ -202,6 +206,8 @@ struct dp83822_private {
 	u32 gpio2_clk_out;
 	bool led_pin_enable[DP83822_MAX_LED_PINS];
 	int tx_amplitude_100base_tx_index;
+	bool mac_series_termination_modify;
+	int mac_series_termination_index;
 };
 
 static int dp83822_config_wol(struct phy_device *phydev,
@@ -533,6 +539,12 @@ static int dp83822_config_init(struct phy_device *phydev)
 			       FIELD_PREP(DP83822_100BASE_TX_LINE_DRIVER_SWING,
 					  dp83822->tx_amplitude_100base_tx_index));
 
+	if (dp83822->mac_series_termination_modify)
+		phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_IOCTRL,
+			       DP83822_IOCTRL_MAC_IMPEDANCE_CTRL,
+			       FIELD_PREP(DP83822_IOCTRL_MAC_IMPEDANCE_CTRL,
+					  dp83822->mac_series_termination_index));
+
 	err = dp83822_config_init_leds(phydev);
 	if (err)
 		return err;
@@ -736,6 +748,10 @@ static const u32 tx_amplitude_100base_tx_gain[] = {
 	93, 95, 97, 98, 100, 102, 103, 105,
 };
 
+static const u32 mac_series_termination[] = {
+	99, 91, 84, 78, 73, 69, 65, 61, 58, 55, 53, 50, 48, 46, 44, 43,
+};
+
 static int dp83822_of_init_leds(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
@@ -853,6 +869,26 @@ static int dp83822_of_init(struct phy_device *phydev)
 		}
 	}
 
+	dp83822->mac_series_termination_index = -1;
+	ret = phy_get_mac_series_termination(phydev, dev, &val);
+	if (!ret) {
+		for (i = 0; i < ARRAY_SIZE(mac_series_termination); i++) {
+			if (mac_series_termination[i] == val) {
+				dp83822->mac_series_termination_index = i;
+				break;
+			}
+		}
+
+		if (dp83822->mac_series_termination_index < 0) {
+			phydev_err(phydev,
+				   "Invalid value for mac-series-termination-ohms property (%u)\n",
+				   val);
+			return -EINVAL;
+		}
+
+		dp83822->mac_series_termination_modify = true;
+	}
+
 	return dp83822_of_init_leds(phydev);
 }
 

-- 
2.39.5

Re: [PATCH net-next 0/3] net: phy: dp83822: Add support for changing the MAC series termination

[Andrew Lunn]

On Fri, Mar 07, 2025 at 11:30:00AM +0100, Dimitri Fedrau via B4 Relay wrote:
> The dp83822 provides the possibility to set the resistance value of the
> the MAC series termination. Modifying the resistance to an appropriate
> value can reduce signal reflections and therefore improve signal quality.

I have some dumb questions....

By referring to MAC from the perspective of the PHY, do you mean the
termination of the bus between the MAC and the PHY? The SGMII SERDES,
or RGMII?

I'm assuming the terminology is direct from the datasheet of the PHY?
But since this is a bit of a niche area, no other PHY driver currently
supports anythings like this, the terminology is not well known. So it
would be good to expand the description, to make it really clear what
you are talking about, so if anybody else wants to add the same
feature, they make use of the property, not add a new property.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next 0/3] net: phy: dp83822: Add support for changing the MAC series termination

[Dimitri Fedrau]

Am Fri, Mar 07, 2025 at 02:39:32PM +0100 schrieb Andrew Lunn:
> On Fri, Mar 07, 2025 at 11:30:00AM +0100, Dimitri Fedrau via B4 Relay wrote:
> > The dp83822 provides the possibility to set the resistance value of the
> > the MAC series termination. Modifying the resistance to an appropriate
> > value can reduce signal reflections and therefore improve signal quality.
> 
> I have some dumb questions....
> 
> By referring to MAC from the perspective of the PHY, do you mean the
> termination of the bus between the MAC and the PHY? The SGMII SERDES,
> or RGMII?
>
- Yes, the perspective is from the PHY.
- Yes, but only the outputs towards the MAC. Resistors can be saved on
  the PCB when they are integrated into the PHY.
- The PHY is able to operate with RGMII, RMII and MII.

Should I rename then "mac-series-termination-ohms" to
"output-mac-series-termination-ohms" or similar ?

> I'm assuming the terminology is direct from the datasheet of the PHY?
> But since this is a bit of a niche area, no other PHY driver currently
> supports anythings like this, the terminology is not well known. So it
> would be good to expand the description, to make it really clear what
> you are talking about, so if anybody else wants to add the same
> feature, they make use of the property, not add a new property.

The datasheet calls it MAC impedance control, that is what someone might
want to achieve when selecting the proper resistance value. I named it
"mac-series-termination-ohms" instead to make sure what is done to
achieve impedance matching.

Should I add the proper description in the bindings ? Description of the
properties are somehow short. However will expand the description.

Best regards,
Dimitri Fedrau

Re: [PATCH net-next 0/3] net: phy: dp83822: Add support for changing the MAC series termination

[Andrew Lunn]

> Should I add the proper description in the bindings ? Description of the
> properties are somehow short. However will expand the description.

Yes, please expand the description. For well known concepts, we can
keep the binding description short. But i would not consider this a
well known concept, so we need to spell out in detail what it is.

My knowledge of transmission lines and termination is not so good....

So this configures the resistor on the PHY outputs. Do PHY inputs also
need termination resistors? Could there be PHYs which also allow such
resistors to be configured? Are there use cases where you need
asymmetric termination resistors?

My questions are trying to lead to an answer to your question:

> Should I rename then "mac-series-termination-ohms" to
> "output-mac-series-termination-ohms" or similar ?

We should think about this from the general case, not one specific
PHY, and ideally from thinking about the physics of termination.

https://electronics.stackexchange.com/questions/524620/impedance-termination-of-marvell-phy

This seems to suggest RGMII only has termination resistors at the
outputs. So "mac-series-termination-ohms" would be O.K.

	Andrew

Re: [PATCH net-next 0/3] net: phy: dp83822: Add support for changing the MAC series termination

[Dimitri Fedrau]

Am Fri, Mar 07, 2025 at 04:34:59PM +0100 schrieb Andrew Lunn:
> > Should I add the proper description in the bindings ? Description of the
> > properties are somehow short. However will expand the description.
> 
> Yes, please expand the description. For well known concepts, we can
> keep the binding description short. But i would not consider this a
> well known concept, so we need to spell out in detail what it is.
>
Ok.

> My knowledge of transmission lines and termination is not so good....
> 
> So this configures the resistor on the PHY outputs. Do PHY inputs also
> need termination resistors? Could there be PHYs which also allow such
> resistors to be configured? Are there use cases where you need
> asymmetric termination resistors?
>
I think they are also needed for the PHY inputs, but termination
resistors should be placed near the driver. In case the MAC doesn't have
them integrated they should be placed near the MAC outputs. From PHY
perspective we just care about the PHY outputs.

https://resources.pcb.cadence.com/blog/termination-resistors-in-pcb-design

I don't know if there is a general rule that states wether to use series
termination or not. I think it depends on the PCB design and if there are
issues with signal quality. Maybe this one helps, there is a small
chapter regarding series termination:

https://resources.altium.com/p/gigabit-ethernet-101-basics-implementation

Found it reading this:

https://community.nxp.com/t5/i-MX-Processors/Why-doesn-t-NXP-use-termination-resistors-for-MII-data-lines/td-p/1360873

> My questions are trying to lead to an answer to your question:
> 
> > Should I rename then "mac-series-termination-ohms" to
> > "output-mac-series-termination-ohms" or similar ?
> 
> We should think about this from the general case, not one specific
> PHY, and ideally from thinking about the physics of termination.
> 
> https://electronics.stackexchange.com/questions/524620/impedance-termination-of-marvell-phy
> 
> This seems to suggest RGMII only has termination resistors at the
> outputs. So "mac-series-termination-ohms" would be O.K.
>
Ok.

Best regards,
Dimitri Fedrau