[PATCH v2 1/2] net: phy: microchip: add downshift tunable support for LAN88xx

[PATCH v2 1/2] net: phy: microchip: add downshift tunable support for LAN88xx

[Nicolai Buchwitz]

Implement the standard ETHTOOL_PHY_DOWNSHIFT tunable for the LAN88xx
PHY. This allows runtime configuration of the auto-downshift feature
via ethtool:

  ethtool --set-phy-tunable eth0 downshift on count 3

The LAN88xx PHY supports downshifting from 1000BASE-T to 100BASE-TX
after 2-5 failed auto-negotiation attempts. Valid count values are
2, 3, 4 and 5.

This is based on an earlier downstream implementation by Phil Elwell.

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/phy/microchip.c  | 64 ++++++++++++++++++++++++++++++++++++
 include/linux/microchipphy.h |  5 +++
 2 files changed, 69 insertions(+)

diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c
index dc8634e7bcbe..bc293d2dd130 100644
--- a/drivers/net/phy/microchip.c
+++ b/drivers/net/phy/microchip.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (C) 2015 Microchip Technology
  */
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mii.h>
@@ -193,6 +194,67 @@ static void lan88xx_config_TR_regs(struct phy_device *phydev)
 		phydev_warn(phydev, "Failed to Set Register[0x1686]\n");
 }
 
+static int lan88xx_get_downshift(struct phy_device *phydev, u8 *data)
+{
+	int val;
+
+	val = phy_read_paged(phydev, 1, LAN78XX_PHY_CTRL3);
+	if (val < 0)
+		return val;
+
+	if (!(val & LAN78XX_PHY_CTRL3_AUTO_DOWNSHIFT)) {
+		*data = DOWNSHIFT_DEV_DISABLE;
+		return 0;
+	}
+
+	*data = FIELD_GET(LAN78XX_PHY_CTRL3_DOWNSHIFT_CTRL_MASK, val) + 2;
+
+	return 0;
+}
+
+static int lan88xx_set_downshift(struct phy_device *phydev, u8 cnt)
+{
+	u32 mask = LAN78XX_PHY_CTRL3_DOWNSHIFT_CTRL_MASK |
+		   LAN78XX_PHY_CTRL3_AUTO_DOWNSHIFT;
+
+	if (cnt == DOWNSHIFT_DEV_DISABLE)
+		return phy_modify_paged(phydev, 1, LAN78XX_PHY_CTRL3,
+					LAN78XX_PHY_CTRL3_AUTO_DOWNSHIFT, 0);
+
+	if (cnt == DOWNSHIFT_DEV_DEFAULT_COUNT)
+		cnt = 2;
+
+	if (cnt < 2 || cnt > 5)
+		return -EINVAL;
+
+	return phy_modify_paged(phydev, 1, LAN78XX_PHY_CTRL3, mask,
+				FIELD_PREP(LAN78XX_PHY_CTRL3_DOWNSHIFT_CTRL_MASK,
+					   cnt - 2) |
+				LAN78XX_PHY_CTRL3_AUTO_DOWNSHIFT);
+}
+
+static int lan88xx_get_tunable(struct phy_device *phydev,
+			       struct ethtool_tunable *tuna, void *data)
+{
+	switch (tuna->id) {
+	case ETHTOOL_PHY_DOWNSHIFT:
+		return lan88xx_get_downshift(phydev, data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int lan88xx_set_tunable(struct phy_device *phydev,
+			       struct ethtool_tunable *tuna, const void *data)
+{
+	switch (tuna->id) {
+	case ETHTOOL_PHY_DOWNSHIFT:
+		return lan88xx_set_downshift(phydev, *(const u8 *)data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int lan88xx_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
@@ -499,6 +561,8 @@ static struct phy_driver microchip_phy_driver[] = {
 	.set_wol	= lan88xx_set_wol,
 	.read_page	= lan88xx_read_page,
 	.write_page	= lan88xx_write_page,
+	.get_tunable	= lan88xx_get_tunable,
+	.set_tunable	= lan88xx_set_tunable,
 },
 {
 	PHY_ID_MATCH_MODEL(PHY_ID_LAN937X_TX),
diff --git a/include/linux/microchipphy.h b/include/linux/microchipphy.h
index 517288da19fd..7da956c666a0 100644
--- a/include/linux/microchipphy.h
+++ b/include/linux/microchipphy.h
@@ -61,6 +61,11 @@
 /* Registers specific to the LAN7800/LAN7850 embedded phy */
 #define LAN78XX_PHY_LED_MODE_SELECT		(0x1D)
 
+/* PHY Control 3 register (page 1) */
+#define LAN78XX_PHY_CTRL3			(0x14)
+#define  LAN78XX_PHY_CTRL3_AUTO_DOWNSHIFT	BIT(4)
+#define  LAN78XX_PHY_CTRL3_DOWNSHIFT_CTRL_MASK	GENMASK(3, 2)
+
 /* DSP registers */
 #define PHY_ARDENNES_MMD_DEV_3_PHY_CFG		(0x806A)
 #define PHY_ARDENNES_MMD_DEV_3_PHY_CFG_ZD_DLY_EN_	(0x2000)
-- 
2.51.0

Re: [PATCH v2 1/2] net: phy: microchip: add downshift tunable support for LAN88xx

[Andrew Lunn]

On Tue, Mar 31, 2026 at 12:46:26AM +0200, Nicolai Buchwitz wrote:
> Implement the standard ETHTOOL_PHY_DOWNSHIFT tunable for the LAN88xx
> PHY. This allows runtime configuration of the auto-downshift feature
> via ethtool:
> 
>   ethtool --set-phy-tunable eth0 downshift on count 3
> 
> The LAN88xx PHY supports downshifting from 1000BASE-T to 100BASE-TX
> after 2-5 failed auto-negotiation attempts. Valid count values are
> 2, 3, 4 and 5.
> 
> This is based on an earlier downstream implementation by Phil Elwell.
> 
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v2 1/2] net: phy: microchip: add downshift tunable support for LAN88xx

[Russell King (Oracle)]

On Tue, Mar 31, 2026 at 12:46:26AM +0200, Nicolai Buchwitz wrote:
> Implement the standard ETHTOOL_PHY_DOWNSHIFT tunable for the LAN88xx
> PHY. This allows runtime configuration of the auto-downshift feature
> via ethtool:
> 
>   ethtool --set-phy-tunable eth0 downshift on count 3
> 
> The LAN88xx PHY supports downshifting from 1000BASE-T to 100BASE-TX
> after 2-5 failed auto-negotiation attempts. Valid count values are
> 2, 3, 4 and 5.
> 
> This is based on an earlier downstream implementation by Phil Elwell.
> 
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>

I didn't see a cover message. Please always include a cover message with
patch series.

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 v2 1/2] net: phy: microchip: add downshift tunable support for LAN88xx

[Nicolai Buchwitz]

On 31.3.2026 13:29, Russell King (Oracle) wrote:
> On Tue, Mar 31, 2026 at 12:46:26AM +0200, Nicolai Buchwitz wrote:
>> Implement the standard ETHTOOL_PHY_DOWNSHIFT tunable for the LAN88xx
>> PHY. This allows runtime configuration of the auto-downshift feature
>> via ethtool:
>> 
>>   ethtool --set-phy-tunable eth0 downshift on count 3
>> 
>> The LAN88xx PHY supports downshifting from 1000BASE-T to 100BASE-TX
>> after 2-5 failed auto-negotiation attempts. Valid count values are
>> 2, 3, 4 and 5.
>> 
>> This is based on an earlier downstream implementation by Phil Elwell.
>> 
>> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> 
> I didn't see a cover message. Please always include a cover message 
> with
> patch series.

Apparently the cover message [1] was sent with slightly different 
recipients (although it was a single git send-email with 
--cc-cmd='scripts/get_maintainer.pl --norolestats').

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

Thanks, I will add it to the patch.

Nicolai

> Thanks!

[1] 
https://lore.kernel.org/netdev/20260330224630.579937-1-nb@tipi-net.de/

[PATCH v2 2/2] net: phy: microchip: enable downshift by default on LAN88xx

[Nicolai Buchwitz]

Enable auto-downshift from 1000BASE-T to 100BASE-TX after 2 failed
auto-negotiation attempts by default. This ensures that links with
faulty or missing cable pairs (C and D) fall back to 100Mbps without
requiring userspace configuration.

Users can override or disable downshift at runtime:

  ethtool --set-phy-tunable eth0 downshift off

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/microchip.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c
index bc293d2dd130..802b8a6e54e6 100644
--- a/drivers/net/phy/microchip.c
+++ b/drivers/net/phy/microchip.c
@@ -346,7 +346,7 @@ static void lan88xx_set_mdix(struct phy_device *phydev)
 
 static int lan88xx_config_init(struct phy_device *phydev)
 {
-	int val;
+	int val, err;
 
 	/*Zerodetect delay enable */
 	val = phy_read_mmd(phydev, MDIO_MMD_PCS,
@@ -359,6 +359,11 @@ static int lan88xx_config_init(struct phy_device *phydev)
 	/* Config DSP registers */
 	lan88xx_config_TR_regs(phydev);
 
+	/* Enable downshift after 2 failed attempts by default */
+	err = lan88xx_set_downshift(phydev, 2);
+	if (err < 0)
+		return err;
+
 	return 0;
 }
 
-- 
2.51.0

Re: [PATCH v2 2/2] net: phy: microchip: enable downshift by default on LAN88xx

[Russell King (Oracle)]

On Tue, Mar 31, 2026 at 12:46:27AM +0200, Nicolai Buchwitz wrote:
> Enable auto-downshift from 1000BASE-T to 100BASE-TX after 2 failed
> auto-negotiation attempts by default. This ensures that links with
> faulty or missing cable pairs (C and D) fall back to 100Mbps without
> requiring userspace configuration.
> 
> Users can override or disable downshift at runtime:
> 
>   ethtool --set-phy-tunable eth0 downshift off
> 
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

I'm slightly concerned by this commit. ->config_init() is called when
the netdev attaches the PHY, and also during the resume path - and it's
the second one which I believe is a problem here.

If the user has configured the downshift, it is reasonable for the user
to expect the setting to be preserved over a suspend/resume. However,
by placing this code in ->config_init(), you will overwrite the user's
setting when the system resumes.

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

Re: [PATCH v2 2/2] net: phy: microchip: enable downshift by default on LAN88xx

[Nicolai Buchwitz]

On 31.3.2026 13:32, Russell King (Oracle) wrote:
> On Tue, Mar 31, 2026 at 12:46:27AM +0200, Nicolai Buchwitz wrote:
>> Enable auto-downshift from 1000BASE-T to 100BASE-TX after 2 failed
>> auto-negotiation attempts by default. This ensures that links with
>> faulty or missing cable pairs (C and D) fall back to 100Mbps without
>> requiring userspace configuration.
>> 
>> Users can override or disable downshift at runtime:
>> 
>>   ethtool --set-phy-tunable eth0 downshift off
>> 
>> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
> I'm slightly concerned by this commit. ->config_init() is called when
> the netdev attaches the PHY, and also during the resume path - and it's
> the second one which I believe is a problem here.
> 
> If the user has configured the downshift, it is reasonable for the user
> to expect the setting to be preserved over a suspend/resume. However,
> by placing this code in ->config_init(), you will overwrite the user's
> setting when the system resumes.

You have a valid point. Looking at other drivers, Marvell has the
same issue: m88e1112_config_init() unconditionally sets downshift to 3
on every config_init call.

I see two options:

1. Save the user's setting in the driver's priv struct and restore it
in config_init instead of blindly applying the default.

2. Handle it generically in the PHY core, saving/restoring tunable
state across suspend/resume for all drivers.

I'd lean towards (1) to keep this series simple. (2) could be a
follow-up that fixes Marvell and others too. What do you think?

Thanks

Nicolai

Re: [PATCH v2 2/2] net: phy: microchip: enable downshift by default on LAN88xx

[Russell King (Oracle)]

On Tue, Mar 31, 2026 at 01:58:55PM +0200, Nicolai Buchwitz wrote:
> On 31.3.2026 13:32, Russell King (Oracle) wrote:
> > On Tue, Mar 31, 2026 at 12:46:27AM +0200, Nicolai Buchwitz wrote:
> > > Enable auto-downshift from 1000BASE-T to 100BASE-TX after 2 failed
> > > auto-negotiation attempts by default. This ensures that links with
> > > faulty or missing cable pairs (C and D) fall back to 100Mbps without
> > > requiring userspace configuration.
> > > 
> > > Users can override or disable downshift at runtime:
> > > 
> > >   ethtool --set-phy-tunable eth0 downshift off
> > > 
> > > Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > 
> > I'm slightly concerned by this commit. ->config_init() is called when
> > the netdev attaches the PHY, and also during the resume path - and it's
> > the second one which I believe is a problem here.
> > 
> > If the user has configured the downshift, it is reasonable for the user
> > to expect the setting to be preserved over a suspend/resume. However,
> > by placing this code in ->config_init(), you will overwrite the user's
> > setting when the system resumes.
> 
> You have a valid point. Looking at other drivers, Marvell has the
> same issue: m88e1112_config_init() unconditionally sets downshift to 3
> on every config_init call.
> 
> I see two options:
> 
> 1. Save the user's setting in the driver's priv struct and restore it
> in config_init instead of blindly applying the default.
> 
> 2. Handle it generically in the PHY core, saving/restoring tunable
> state across suspend/resume for all drivers.
> 
> I'd lean towards (1) to keep this series simple. (2) could be a
> follow-up that fixes Marvell and others too. What do you think?

Or (3) configure the default it in the probe function?

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

Re: [PATCH v2 2/2] net: phy: microchip: enable downshift by default on LAN88xx

[Nicolai Buchwitz]

On 31.3.2026 14:01, Russell King (Oracle) wrote:
> On Tue, Mar 31, 2026 at 01:58:55PM +0200, Nicolai Buchwitz wrote:
>> On 31.3.2026 13:32, Russell King (Oracle) wrote:
>> > On Tue, Mar 31, 2026 at 12:46:27AM +0200, Nicolai Buchwitz wrote:
>> > > Enable auto-downshift from 1000BASE-T to 100BASE-TX after 2 failed
>> > > auto-negotiation attempts by default. This ensures that links with
>> > > faulty or missing cable pairs (C and D) fall back to 100Mbps without
>> > > requiring userspace configuration.
>> > >
>> > > Users can override or disable downshift at runtime:
>> > >
>> > >   ethtool --set-phy-tunable eth0 downshift off
>> > >
>> > > Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
>> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>> >
>> > I'm slightly concerned by this commit. ->config_init() is called when
>> > the netdev attaches the PHY, and also during the resume path - and it's
>> > the second one which I believe is a problem here.
>> >
>> > If the user has configured the downshift, it is reasonable for the user
>> > to expect the setting to be preserved over a suspend/resume. However,
>> > by placing this code in ->config_init(), you will overwrite the user's
>> > setting when the system resumes.
>> 
>> You have a valid point. Looking at other drivers, Marvell has the
>> same issue: m88e1112_config_init() unconditionally sets downshift to 3
>> on every config_init call.
>> 
>> I see two options:
>> 
>> 1. Save the user's setting in the driver's priv struct and restore it
>> in config_init instead of blindly applying the default.
>> 
>> 2. Handle it generically in the PHY core, saving/restoring tunable
>> state across suspend/resume for all drivers.
>> 
>> I'd lean towards (1) to keep this series simple. (2) could be a
>> follow-up that fixes Marvell and others too. What do you think?
> 
> Or (3) configure the default it in the probe function?

Unfortunately the driver has .soft_reset = genphy_soft_reset which
runs before config_init on resume and would clear the setting? So
config_init still needs to set it.

Nicolai