diff mbox series

[net,v3] net: phy: disable eee due to errata on various KSZ switches

Message ID 20241007174211.3511506-1-tharvey@gateworks.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net,v3] net: phy: disable eee due to errata on various KSZ switches | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/apply fail Patch does not apply to net-0

Commit Message

Tim Harvey Oct. 7, 2024, 5:42 p.m. UTC 
The well-known errata regarding EEE not being functional on various KSZ
switches has been refactored a few times. Recently the refactoring has
excluded several switches that the errata should also apply to.

Disable EEE for additional switches with this errata.

The original workaround for the errata was applied with a register
write to manually disable the EEE feature in MMD 7:60 which was being
applied for KSZ9477/KSZ9897/KSZ9567 switch ID's.

Then came commit ("26dd2974c5b5 net: phy: micrel: Move KSZ9477 errata
fixes to PHY driver") and commit ("6068e6d7ba50 net: dsa: microchip:
remove KSZ9477 PHY errata handling") which moved the errata from the
switch driver to the PHY driver but only for PHY_ID_KSZ9477 (PHY ID)
however that PHY code was dead code because an entry was never added
for PHY_ID_KSZ9477 via MODULE_DEVICE_TABLE.

This was apparently realized much later and commit ("54a4e5c16382 net:
phy: micrel: add Microchip KSZ 9477 to the device table") added the
PHY_ID_KSZ9477 to the PHY driver but as the errata was only being
applied to PHY_ID_KSZ9477 it's not completely clear what switches
that relates to.

Later commit ("6149db4997f5 net: phy: micrel: fix KSZ9477 PHY issues
after suspend/resume") breaks this again for all but KSZ9897 by only
applying the errata for that PHY ID.

The most recent time this was affected was with commit ("08c6d8bae48c
net: phy: Provide Module 4 KSZ9477 errata (DS80000754C)") which
removes the blatant register write to MMD 7:60 and replaces it by
setting phydev->eee_broken_modes = -1 so that the generic phy-c45 code
disables EEE but this is only done for the KSZ9477_CHIP_ID (Switch ID).

Fixes: 08c6d8bae48c ("net: phy: Provide Module 4 KSZ9477 errata (DS80000754C)")
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
v3: added missing fixes tag
v2: added fixes tag and history of issue
---
 drivers/net/dsa/microchip/ksz_common.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Oleksij Rempel Oct. 7, 2024, 6:38 p.m. UTC | #1 
On Mon, Oct 07, 2024 at 10:42:11AM -0700, Tim Harvey wrote:
> The well-known errata regarding EEE not being functional on various KSZ
> switches has been refactored a few times. Recently the refactoring has
> excluded several switches that the errata should also apply to.
> 
> Disable EEE for additional switches with this errata.
> 
> The original workaround for the errata was applied with a register
> write to manually disable the EEE feature in MMD 7:60 which was being
> applied for KSZ9477/KSZ9897/KSZ9567 switch ID's.
> 
> Then came commit ("26dd2974c5b5 net: phy: micrel: Move KSZ9477 errata
> fixes to PHY driver") and commit ("6068e6d7ba50 net: dsa: microchip:
> remove KSZ9477 PHY errata handling") which moved the errata from the
> switch driver to the PHY driver but only for PHY_ID_KSZ9477 (PHY ID)
> however that PHY code was dead code because an entry was never added
> for PHY_ID_KSZ9477 via MODULE_DEVICE_TABLE.
> 
> This was apparently realized much later and commit ("54a4e5c16382 net:
> phy: micrel: add Microchip KSZ 9477 to the device table") added the
> PHY_ID_KSZ9477 to the PHY driver but as the errata was only being
> applied to PHY_ID_KSZ9477 it's not completely clear what switches
> that relates to.
> 
> Later commit ("6149db4997f5 net: phy: micrel: fix KSZ9477 PHY issues
> after suspend/resume") breaks this again for all but KSZ9897 by only
> applying the errata for that PHY ID.
> 
> The most recent time this was affected was with commit ("08c6d8bae48c
> net: phy: Provide Module 4 KSZ9477 errata (DS80000754C)") which
> removes the blatant register write to MMD 7:60 and replaces it by
> setting phydev->eee_broken_modes = -1 so that the generic phy-c45 code
> disables EEE but this is only done for the KSZ9477_CHIP_ID (Switch ID).
> 
> Fixes: 08c6d8bae48c ("net: phy: Provide Module 4 KSZ9477 errata (DS80000754C)")
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> v3: added missing fixes tag
> v2: added fixes tag and history of issue
> ---
>  drivers/net/dsa/microchip/ksz_common.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index b074b4bb0629..d2bd82a1067c 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -2578,11 +2578,19 @@ static u32 ksz_get_phy_flags(struct dsa_switch *ds, int port)
>  		if (!port)
>  			return MICREL_KSZ8_P1_ERRATA;
>  		break;
> +	case KSZ8795_CHIP_ID:
> +	case KSZ8794_CHIP_ID:
> +	case KSZ8765_CHIP_ID:
> +		/* KSZ87xx DS80000687C Module 2 */
> +	case KSZ9896_CHIP_ID:
> +		/* KSZ9896 Errata DS80000757A Module 2 */
> +	case KSZ9897_CHIP_ID:
> +		/* KSZ9897 Errata DS00002394C Module 4 */
> +	case KSZ9567_CHIP_ID:
> +		/* KSZ9567 Errata DS80000756A Module 4 */
>  	case KSZ9477_CHIP_ID:
> -		/* KSZ9477 Errata DS80000754C
> -		 *
> -		 * Module 4: Energy Efficient Ethernet (EEE) feature select must
> -		 * be manually disabled
> +		/* KSZ9477 Errata DS80000754C 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
> -- 

Similar fix is already present in net:
0411f73c13afc ("net: dsa: microchip: disable EEE for KSZ8567/KSZ9567/KSZ9896/KSZ9897.")

But your patch provides some quirks for KSZ87xx  and some extra comments
which are nice to have too. Can you please rebase your patch on top of
latest net.

Regards,
Oleksij
Tim Harvey Oct. 15, 2024, 9:04 p.m. UTC | #2 
On Mon, Oct 7, 2024 at 11:38 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> On Mon, Oct 07, 2024 at 10:42:11AM -0700, Tim Harvey wrote:
> > The well-known errata regarding EEE not being functional on various KSZ
> > switches has been refactored a few times. Recently the refactoring has
> > excluded several switches that the errata should also apply to.
> >
> > Disable EEE for additional switches with this errata.
> >
> > The original workaround for the errata was applied with a register
> > write to manually disable the EEE feature in MMD 7:60 which was being
> > applied for KSZ9477/KSZ9897/KSZ9567 switch ID's.
> >
> > Then came commit ("26dd2974c5b5 net: phy: micrel: Move KSZ9477 errata
> > fixes to PHY driver") and commit ("6068e6d7ba50 net: dsa: microchip:
> > remove KSZ9477 PHY errata handling") which moved the errata from the
> > switch driver to the PHY driver but only for PHY_ID_KSZ9477 (PHY ID)
> > however that PHY code was dead code because an entry was never added
> > for PHY_ID_KSZ9477 via MODULE_DEVICE_TABLE.
> >
> > This was apparently realized much later and commit ("54a4e5c16382 net:
> > phy: micrel: add Microchip KSZ 9477 to the device table") added the
> > PHY_ID_KSZ9477 to the PHY driver but as the errata was only being
> > applied to PHY_ID_KSZ9477 it's not completely clear what switches
> > that relates to.
> >
> > Later commit ("6149db4997f5 net: phy: micrel: fix KSZ9477 PHY issues
> > after suspend/resume") breaks this again for all but KSZ9897 by only
> > applying the errata for that PHY ID.
> >
> > The most recent time this was affected was with commit ("08c6d8bae48c
> > net: phy: Provide Module 4 KSZ9477 errata (DS80000754C)") which
> > removes the blatant register write to MMD 7:60 and replaces it by
> > setting phydev->eee_broken_modes = -1 so that the generic phy-c45 code
> > disables EEE but this is only done for the KSZ9477_CHIP_ID (Switch ID).
> >
> > Fixes: 08c6d8bae48c ("net: phy: Provide Module 4 KSZ9477 errata (DS80000754C)")
> > Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> > ---
> > v3: added missing fixes tag
> > v2: added fixes tag and history of issue
> > ---
> >  drivers/net/dsa/microchip/ksz_common.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> > index b074b4bb0629..d2bd82a1067c 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -2578,11 +2578,19 @@ static u32 ksz_get_phy_flags(struct dsa_switch *ds, int port)
> >               if (!port)
> >                       return MICREL_KSZ8_P1_ERRATA;
> >               break;
> > +     case KSZ8795_CHIP_ID:
> > +     case KSZ8794_CHIP_ID:
> > +     case KSZ8765_CHIP_ID:
> > +             /* KSZ87xx DS80000687C Module 2 */
> > +     case KSZ9896_CHIP_ID:
> > +             /* KSZ9896 Errata DS80000757A Module 2 */
> > +     case KSZ9897_CHIP_ID:
> > +             /* KSZ9897 Errata DS00002394C Module 4 */
> > +     case KSZ9567_CHIP_ID:
> > +             /* KSZ9567 Errata DS80000756A Module 4 */
> >       case KSZ9477_CHIP_ID:
> > -             /* KSZ9477 Errata DS80000754C
> > -              *
> > -              * Module 4: Energy Efficient Ethernet (EEE) feature select must
> > -              * be manually disabled
> > +             /* KSZ9477 Errata DS80000754C 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
> > --
>
> Similar fix is already present in net:
> 0411f73c13afc ("net: dsa: microchip: disable EEE for KSZ8567/KSZ9567/KSZ9896/KSZ9897.")
>
> But your patch provides some quirks for KSZ87xx  and some extra comments
> which are nice to have too. Can you please rebase your patch on top of
> latest net.
>

Hi Oleksij,

Yes, I can submit an update.

Best Regards,

Tim
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index b074b4bb0629..d2bd82a1067c 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2578,11 +2578,19 @@  static u32 ksz_get_phy_flags(struct dsa_switch *ds, int port)
 		if (!port)
 			return MICREL_KSZ8_P1_ERRATA;
 		break;
+	case KSZ8795_CHIP_ID:
+	case KSZ8794_CHIP_ID:
+	case KSZ8765_CHIP_ID:
+		/* KSZ87xx DS80000687C Module 2 */
+	case KSZ9896_CHIP_ID:
+		/* KSZ9896 Errata DS80000757A Module 2 */
+	case KSZ9897_CHIP_ID:
+		/* KSZ9897 Errata DS00002394C Module 4 */
+	case KSZ9567_CHIP_ID:
+		/* KSZ9567 Errata DS80000756A Module 4 */
 	case KSZ9477_CHIP_ID:
-		/* KSZ9477 Errata DS80000754C
-		 *
-		 * Module 4: Energy Efficient Ethernet (EEE) feature select must
-		 * be manually disabled
+		/* KSZ9477 Errata DS80000754C 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