[PATCH net-next 0/2] microchip/micrel switch: replace MICREL_NO_EEE workaround

[PATCH net-next 0/2] microchip/micrel switch: replace MICREL_NO_EEE workaround

[Heiner Kallweit]

On several supported switches the integrated PHY's have buggy EEE.
On the GBit-capable ones it's always the same type of PHY with PHY ID
0x00221631, so I think we can assume that all PHY's with this id
have this EEE issue. Let's simplify the erratum handling by simply
clearing phydev->supported_eee for this PHY type.

Heiner Kallweit (2):
  net: phy: micrel: disable EEE on KSZ9477-type PHY
  net: dsa: microchip: remove MICREL_NO_EEE workaround

 drivers/net/dsa/microchip/ksz_common.c | 25 -------------------------
 drivers/net/phy/micrel.c               | 12 ++++++------
 include/linux/micrel_phy.h             |  1 -
 3 files changed, 6 insertions(+), 32 deletions(-)

-- 
2.47.1

[PATCH net-next 1/2] net: phy: micrel: disable EEE on KSZ9477-type PHY

[Heiner Kallweit]

On several supported switches the integrated PHY's have buggy EEE.
On the GBit-capable ones it's always the same type of PHY with PHY ID
0x00221631. So we can simplify the erratum handling by simply clearing
phydev->supported_eee for this PHY type.

Note: The KSZ9477 PHY driver also covers e.g. the internal PHY of
      KSZ9563 (ID: 0x00221637), which is unaffected by the EEE issue.
      Therefore check for the exact PHY ID.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/micrel.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 3ef508840..ece6d026e 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1522,6 +1522,12 @@ static int ksz9477_get_features(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
+	/* See KSZ9477 Errata DS80000754C Module 4 */
+	if (phydev->phy_id == PHY_ID_KSZ9477) {
+		linkmode_zero(phydev->supported_eee);
+		return 0;
+	}
+
 	/* The "EEE control and capability 1" (Register 3.20) seems to be
 	 * influenced by the "EEE advertisement 1" (Register 7.60). Changes
 	 * on the 7.60 will affect 3.20. So, we need to construct our own list
@@ -2000,12 +2006,6 @@ static int ksz9477_config_init(struct phy_device *phydev)
 			return err;
 	}
 
-	/* According to KSZ9477 Errata DS80000754C (Module 4) all EEE modes
-	 * in this switch shall be regarded as broken.
-	 */
-	if (phydev->dev_flags & MICREL_NO_EEE)
-		linkmode_fill(phydev->eee_broken_modes);
-
 	return kszphy_config_init(phydev);
 }
 
-- 
2.47.1

[PATCH net-next 2/2] net: dsa: microchip: remove MICREL_NO_EEE workaround

[Heiner Kallweit]

The integrated PHY's on all these switch types have the same PHY ID.
So we can assume that the issue is related to the PHY type, not the
switch type. After having disabled EEE for this PHY type, we can remove
the workarouund code here.

Note: On the fast ethernet models listed here the integrated PHY has
      PHY ID 0x00221550, which is handled by PHY driver
      "Micrel KSZ87XX Switch". This PHY driver doesn't handle flag
      MICREL_NO_EEE, therefore setting the flag for these models
      results in a no-op.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 25 -------------------------
 include/linux/micrel_phy.h             |  1 -
 2 files changed, 26 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index df314724e..c29dc1c9c 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2990,31 +2990,6 @@ static u32 ksz_get_phy_flags(struct dsa_switch *ds, int port)
 		if (!port)
 			return MICREL_KSZ8_P1_ERRATA;
 		break;
-	case KSZ8567_CHIP_ID:
-		/* KSZ8567R Errata DS80000752C Module 4 */
-	case KSZ8765_CHIP_ID:
-	case KSZ8794_CHIP_ID:
-	case KSZ8795_CHIP_ID:
-		/* KSZ879x/KSZ877x/KSZ876x Errata DS80000687C Module 2 */
-	case KSZ9477_CHIP_ID:
-		/* KSZ9477S Errata DS80000754A Module 4 */
-	case KSZ9567_CHIP_ID:
-		/* KSZ9567S Errata DS80000756A Module 4 */
-	case KSZ9896_CHIP_ID:
-		/* KSZ9896C Errata DS80000757A Module 3 */
-	case KSZ9897_CHIP_ID:
-	case LAN9646_CHIP_ID:
-		/* KSZ9897R Errata DS80000758C Module 4 */
-		/* Energy Efficient Ethernet (EEE) feature select must be manually disabled
-		 *   The EEE feature is enabled by default, but it is not fully
-		 *   operational. It must be manually disabled through register
-		 *   controls. If not disabled, the PHY ports can auto-negotiate
-		 *   to enable EEE, and this feature can cause link drops when
-		 *   linked to another device supporting EEE.
-		 *
-		 * The same item appears in the errata for all switches above.
-		 */
-		return MICREL_NO_EEE;
 	}
 
 	return 0;
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 591bf5b5e..9af01bdd8 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -44,7 +44,6 @@
 #define MICREL_PHY_50MHZ_CLK	BIT(0)
 #define MICREL_PHY_FXEN		BIT(1)
 #define MICREL_KSZ8_P1_ERRATA	BIT(2)
-#define MICREL_NO_EEE		BIT(3)
 
 #define MICREL_KSZ9021_EXTREG_CTRL	0xB
 #define MICREL_KSZ9021_EXTREG_DATA_WRITE	0xC
-- 
2.47.1

Re: [PATCH net-next 1/2] net: phy: micrel: disable EEE on KSZ9477-type PHY

[Oleksij Rempel]

On Fri, Dec 20, 2024 at 02:51:32PM +0100, Heiner Kallweit wrote:
> On several supported switches the integrated PHY's have buggy EEE.
> On the GBit-capable ones it's always the same type of PHY with PHY ID
> 0x00221631. So we can simplify the erratum handling by simply clearing
> phydev->supported_eee for this PHY type.
> 
> Note: The KSZ9477 PHY driver also covers e.g. the internal PHY of
>       KSZ9563 (ID: 0x00221637), which is unaffected by the EEE issue.
>       Therefore check for the exact PHY ID.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/phy/micrel.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 3ef508840..ece6d026e 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -1522,6 +1522,12 @@ static int ksz9477_get_features(struct phy_device *phydev)
>  	if (ret)
>  		return ret;
>  
> +	/* See KSZ9477 Errata DS80000754C Module 4 */
> +	if (phydev->phy_id == PHY_ID_KSZ9477) {
> +		linkmode_zero(phydev->supported_eee);
> +		return 0;
> +	}

Hm.. with this change, we won't be able to disable EEE. Zeroed
supported_eee will avoid writing to the EEE advertisement register.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next 1/2] net: phy: micrel: disable EEE on KSZ9477-type PHY

[Heiner Kallweit]

On 20.12.2024 15:40, Oleksij Rempel wrote:
> On Fri, Dec 20, 2024 at 02:51:32PM +0100, Heiner Kallweit wrote:
>> On several supported switches the integrated PHY's have buggy EEE.
>> On the GBit-capable ones it's always the same type of PHY with PHY ID
>> 0x00221631. So we can simplify the erratum handling by simply clearing
>> phydev->supported_eee for this PHY type.
>>
>> Note: The KSZ9477 PHY driver also covers e.g. the internal PHY of
>>       KSZ9563 (ID: 0x00221637), which is unaffected by the EEE issue.
>>       Therefore check for the exact PHY ID.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/net/phy/micrel.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>> index 3ef508840..ece6d026e 100644
>> --- a/drivers/net/phy/micrel.c
>> +++ b/drivers/net/phy/micrel.c
>> @@ -1522,6 +1522,12 @@ static int ksz9477_get_features(struct phy_device *phydev)
>>  	if (ret)
>>  		return ret;
>>  
>> +	/* See KSZ9477 Errata DS80000754C Module 4 */
>> +	if (phydev->phy_id == PHY_ID_KSZ9477) {
>> +		linkmode_zero(phydev->supported_eee);
>> +		return 0;
>> +	}
> 
> Hm.. with this change, we won't be able to disable EEE. Zeroed
> supported_eee will avoid writing to the EEE advertisement register.
> 
Indeed, genphy_c45_write_eee_adv() would become a no-op.
We have to do it differently.