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

[PATCH net-next v2 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>
---
Changes in v2:
- Renamed "mac-series-termination-ohms" to "mac-termination-ohms"
- Added description for "mac-termination-ohms"
- Renamed "phy_get_mac_series_termination" to "phy_get_mac_termination"
- Dropped "mac_series_termination_modify" from dp83822_private
- Init mac_termination_index in dp8382x_probe
- Renamed "mac_series_termination" to "mac_termination"
- Link to v1: https://lore.kernel.org/r/20250307-dp83822-mac-impedance-v1-0-bdd85a759b45@liebherr.com

---
Dimitri Fedrau (3):
      dt-bindings: net: ethernet-phy: add property mac-termination-ohms
      net: phy: Add helper for getting MAC termination resistance
      net: phy: dp83822: Add support for changing the MAC termination

 .../devicetree/bindings/net/ethernet-phy.yaml      |  9 ++++++
 drivers/net/phy/dp83822.c                          | 33 ++++++++++++++++++++++
 drivers/net/phy/phy_device.c                       | 15 ++++++++++
 include/linux/phy.h                                |  3 ++
 4 files changed, 60 insertions(+)
---
base-commit: 61f96e684edd28ca40555ec49ea1555df31ba619
change-id: 20250305-dp83822-mac-impedance-840435b0c9e6

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

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

[Dimitri Fedrau via B4 Relay]

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

Add property mac-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 | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 824bbe4333b7ed95cc39737d3c334a20aa890f01..93574c2726e5b2bc81a2dfb2462eb73c7cbfe6a8 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -238,6 +238,15 @@ properties:
       peak-to-peak specified in ANSI X3.263. When omitted, the PHYs default
       will be left as is.
 
+  mac-termination-ohms:
+    description:
+      The xMII signals need series termination on the driver side to match both
+      the output driver impedance and the line characteristic impedance, to
+      prevent reflections and EMI problems. Select a resistance value which is
+      supported by the builtin resistors of the PHY, otherwise the resistors may
+      have to be placed on board. When omitted, the PHYs default will be left as
+      is.
+
   leds:
     type: object
 

-- 
2.39.5

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

[Dimitri Fedrau via B4 Relay]

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

Add helper which returns the MAC 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 675fbd225378796085cdc6c92ee2d7e9bff0b6f6..6bd888a7ad14f0e71670e43cf0dd7ecebc2b23b9 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2948,6 +2948,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_termination - stores MAC termination in @val
+ * @phydev: phy_device struct
+ * @dev: pointer to the devices device struct
+ * @val: MAC termination
+ *
+ * Returns: 0 on success, < 0 on failure
+ */
+int phy_get_mac_termination(struct phy_device *phydev, struct device *dev,
+			    u32 *val)
+{
+	return phy_get_u32_property(dev, "mac-termination-ohms", val);
+}
+EXPORT_SYMBOL_GPL(phy_get_mac_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 a2bfae80c44975db013760413867cebdf20c8216..63d237763f842e80d04bb534e32cd42110129e41 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -2046,6 +2046,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_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 v2 3/3] net: phy: dp83822: Add support for changing the MAC 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 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 | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 14f36154963841dff98be5af4dfbd2760325c13d..490c9f4e5d4e4dc866ef99f426f7497b5e1b49b4 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,7 @@ struct dp83822_private {
 	u32 gpio2_clk_out;
 	bool led_pin_enable[DP83822_MAX_LED_PINS];
 	int tx_amplitude_100base_tx_index;
+	int mac_termination_index;
 };
 
 static int dp83822_config_wol(struct phy_device *phydev,
@@ -533,6 +538,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_termination_index >= 0)
+		phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_IOCTRL,
+			       DP83822_IOCTRL_MAC_IMPEDANCE_CTRL,
+			       FIELD_PREP(DP83822_IOCTRL_MAC_IMPEDANCE_CTRL,
+					  dp83822->mac_termination_index));
+
 	err = dp83822_config_init_leds(phydev);
 	if (err)
 		return err;
@@ -736,6 +747,10 @@ static const u32 tx_amplitude_100base_tx_gain[] = {
 	93, 95, 97, 98, 100, 102, 103, 105,
 };
 
+static const u32 mac_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;
@@ -852,6 +867,23 @@ static int dp83822_of_init(struct phy_device *phydev)
 		}
 	}
 
+	ret = phy_get_mac_termination(phydev, dev, &val);
+	if (!ret) {
+		for (i = 0; i < ARRAY_SIZE(mac_termination); i++) {
+			if (mac_termination[i] == val) {
+				dp83822->mac_termination_index = i;
+				break;
+			}
+		}
+
+		if (dp83822->mac_termination_index < 0) {
+			phydev_err(phydev,
+				   "Invalid value for mac-termination-ohms property (%u)\n",
+				   val);
+			return -EINVAL;
+		}
+	}
+
 	return dp83822_of_init_leds(phydev);
 }
 
@@ -931,6 +963,7 @@ static int dp8382x_probe(struct phy_device *phydev)
 		return -ENOMEM;
 
 	dp83822->tx_amplitude_100base_tx_index = -1;
+	dp83822->mac_termination_index = -1;
 	phydev->priv = dp83822;
 
 	return 0;

-- 
2.39.5

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

[Russell King (Oracle)]

On Tue, Apr 08, 2025 at 09:45:34AM +0200, Dimitri Fedrau via B4 Relay wrote:
> From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> 
> The dp83822 provides the possibility to set the resistance value of the
> the MAC 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>

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

-- 
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 v2 2/3] net: phy: Add helper for getting MAC termination resistance

[Russell King (Oracle)]

On Tue, Apr 08, 2025 at 09:45:33AM +0200, Dimitri Fedrau via B4 Relay wrote:
> From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> 
> Add helper which returns the MAC 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>

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

-- 
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 v2 3/3] net: phy: dp83822: Add support for changing the MAC termination

[Andrew Lunn]

> +static const u32 mac_termination[] = {
> +	99, 91, 84, 78, 73, 69, 65, 61, 58, 55, 53, 50, 48, 46, 44, 43,

Please add this list to the binding.


    Andrew

---
pw-bot: cr

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

[Fedrau Dimitri (LED)]

-----Ursprüngliche Nachricht-----
Von: Andrew Lunn <andrew@lunn.ch> 
Gesendet: Dienstag, 8. April 2025 14:47
An: Fedrau Dimitri (LED) <dimitri.fedrau@liebherr.com>
Cc: Heiner Kallweit <hkallweit1@gmail.com>; Russell King <linux@armlinux.org.uk>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Dimitri Fedrau <dima.fedrau@gmail.com>
Betreff: Re: [PATCH net-next v2 3/3] net: phy: dp83822: Add support for changing the MAC termination

> > +static const u32 mac_termination[] = {
> > +	99, 91, 84, 78, 73, 69, 65, 61, 58, 55, 53, 50, 48, 46, 44, 43,
> 
> Please add this list to the binding.

Add this list to "ti,dp83822.yaml" ?

Best regards,
Dimitri Fedrau

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

[Andrew Lunn]

On Tue, Apr 08, 2025 at 01:01:17PM +0000, Fedrau Dimitri (LED) wrote:
> -----Ursprüngliche Nachricht-----
> Von: Andrew Lunn <andrew@lunn.ch> 
> Gesendet: Dienstag, 8. April 2025 14:47
> An: Fedrau Dimitri (LED) <dimitri.fedrau@liebherr.com>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>; Russell King <linux@armlinux.org.uk>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Dimitri Fedrau <dima.fedrau@gmail.com>
> Betreff: Re: [PATCH net-next v2 3/3] net: phy: dp83822: Add support for changing the MAC termination
> 
> > > +static const u32 mac_termination[] = {
> > > +	99, 91, 84, 78, 73, 69, 65, 61, 58, 55, 53, 50, 48, 46, 44, 43,
> > 
> > Please add this list to the binding.
> 
> Add this list to "ti,dp83822.yaml" ?

Yes please. Ideally we want the DT validation tools to pick up invalid
values before they reach the kernel.

    Andrew

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

[Dimitri Fedrau]

On Tue, Apr 08, 2025 at 03:28:32PM +0200, Andrew Lunn wrote:
> On Tue, Apr 08, 2025 at 01:01:17PM +0000, Fedrau Dimitri (LED) wrote:
> > -----Ursprüngliche Nachricht-----
> > Von: Andrew Lunn <andrew@lunn.ch> 
> > Gesendet: Dienstag, 8. April 2025 14:47
> > An: Fedrau Dimitri (LED) <dimitri.fedrau@liebherr.com>
> > Cc: Heiner Kallweit <hkallweit1@gmail.com>; Russell King <linux@armlinux.org.uk>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Dimitri Fedrau <dima.fedrau@gmail.com>
> > Betreff: Re: [PATCH net-next v2 3/3] net: phy: dp83822: Add support for changing the MAC termination
> > 
> > > > +static const u32 mac_termination[] = {
> > > > +	99, 91, 84, 78, 73, 69, 65, 61, 58, 55, 53, 50, 48, 46, 44, 43,
> > > 
> > > Please add this list to the binding.
> > 
> > Add this list to "ti,dp83822.yaml" ?
> 
> Yes please. Ideally we want the DT validation tools to pick up invalid
> values before they reach the kernel.
>
Ok, but then I would have to add "mac-termination-ohms" property to
"ti,dp83822.yaml" as well together with the allowed values ? Ending up in
some sort of duplication, because the property is already defined in
"ethernet-phy.yaml". Is this the right way to do it ?

Best regards,
Dimitri Fedrau

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

[Rob Herring]

On Tue, Apr 08, 2025 at 07:18:31PM +0200, Dimitri Fedrau wrote:
> On Tue, Apr 08, 2025 at 03:28:32PM +0200, Andrew Lunn wrote:
> > On Tue, Apr 08, 2025 at 01:01:17PM +0000, Fedrau Dimitri (LED) wrote:
> > > -----Ursprüngliche Nachricht-----
> > > Von: Andrew Lunn <andrew@lunn.ch> 
> > > Gesendet: Dienstag, 8. April 2025 14:47
> > > An: Fedrau Dimitri (LED) <dimitri.fedrau@liebherr.com>
> > > Cc: Heiner Kallweit <hkallweit1@gmail.com>; Russell King <linux@armlinux.org.uk>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Dimitri Fedrau <dima.fedrau@gmail.com>
> > > Betreff: Re: [PATCH net-next v2 3/3] net: phy: dp83822: Add support for changing the MAC termination
> > > 
> > > > > +static const u32 mac_termination[] = {
> > > > > +	99, 91, 84, 78, 73, 69, 65, 61, 58, 55, 53, 50, 48, 46, 44, 43,
> > > > 
> > > > Please add this list to the binding.
> > > 
> > > Add this list to "ti,dp83822.yaml" ?
> > 
> > Yes please. Ideally we want the DT validation tools to pick up invalid
> > values before they reach the kernel.
> >
> Ok, but then I would have to add "mac-termination-ohms" property to
> "ti,dp83822.yaml" as well together with the allowed values ? Ending up in
> some sort of duplication, because the property is already defined in
> "ethernet-phy.yaml". Is this the right way to do it ?

It is not duplication because you are adding constraints. I was thinking 
the definition in ethernet-phy.yaml should have some logical limit. Say
200 ohms as termination resistance is typically 10s of ohms. 

Seems like a long list to define to me, but if the driver is going to 
reject unknown values it makes sense. You could also make the driver 
just pick the closest match rather than require an exact match. 
Resistance values are nominal anyways.

Rob

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

[Dimitri Fedrau]

Am Fri, Apr 11, 2025 at 10:04:48AM -0500 schrieb Rob Herring:
> On Tue, Apr 08, 2025 at 07:18:31PM +0200, Dimitri Fedrau wrote:
> > On Tue, Apr 08, 2025 at 03:28:32PM +0200, Andrew Lunn wrote:
> > > On Tue, Apr 08, 2025 at 01:01:17PM +0000, Fedrau Dimitri (LED) wrote:
> > > > -----Ursprüngliche Nachricht-----
> > > > Von: Andrew Lunn <andrew@lunn.ch> 
> > > > Gesendet: Dienstag, 8. April 2025 14:47
> > > > An: Fedrau Dimitri (LED) <dimitri.fedrau@liebherr.com>
> > > > Cc: Heiner Kallweit <hkallweit1@gmail.com>; Russell King <linux@armlinux.org.uk>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Dimitri Fedrau <dima.fedrau@gmail.com>
> > > > Betreff: Re: [PATCH net-next v2 3/3] net: phy: dp83822: Add support for changing the MAC termination
> > > > 
> > > > > > +static const u32 mac_termination[] = {
> > > > > > +	99, 91, 84, 78, 73, 69, 65, 61, 58, 55, 53, 50, 48, 46, 44, 43,
> > > > > 
> > > > > Please add this list to the binding.
> > > > 
> > > > Add this list to "ti,dp83822.yaml" ?
> > > 
> > > Yes please. Ideally we want the DT validation tools to pick up invalid
> > > values before they reach the kernel.
> > >
> > Ok, but then I would have to add "mac-termination-ohms" property to
> > "ti,dp83822.yaml" as well together with the allowed values ? Ending up in
> > some sort of duplication, because the property is already defined in
> > "ethernet-phy.yaml". Is this the right way to do it ?
> 
> It is not duplication because you are adding constraints. I was thinking 
> the definition in ethernet-phy.yaml should have some logical limit. Say
> 200 ohms as termination resistance is typically 10s of ohms. 
> 
> Seems like a long list to define to me, but if the driver is going to 
> reject unknown values it makes sense. You could also make the driver 
> just pick the closest match rather than require an exact match. 
> Resistance values are nominal anyways.
>
Will add the logical limit into ethernet-phy.yaml and add the table with
the allowed values into ti,dp83822.yaml.