Re: [PATCH] net: phy: disable eee due to errata on various KSZ switches

From: Tim Harvey
Date: Mon Oct 07 2024 - 12:40:00 EST


On Sat, Oct 5, 2024 at 9:46 AM Andrew Lunn <andrew@xxxxxxx> wrote:
>
> On Fri, Oct 04, 2024 at 02:32:35PM -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.
>
> Does the commit message say why?
>
> Does this need a Fixes: tag?
>

Hi Andrew,

Good question. I couldn't really figure out what fixes tag would be
appropriate as this code has changed a few times and broken in strange
ways. Here's a history as best I can tell:

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. So even if we add a
'Fixes: 6068e6d7ba50' it would not be fixed.

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. I believe the code was proper at
this point.

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)
so its still broken at this point for the other switches that have
this errata.

So at this point, the only commit that my patch would apply over is
the most recent 08c6d8bae48c but that wouldn't fix any of the previous
issues and it would be unclear what switch was broken at what point in
time.

Best Regards,

Tim