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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJ+vNU0BGaLco2g9mTf4eDyY4-u9P0HWeK-TUzsb8JPsZs3Ymg@mail.gmail.com>
Date: Tue, 15 Oct 2024 14:04:41 -0700
From: Tim Harvey <tharvey@...eworks.com>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: Woojung Huh <woojung.huh@...rochip.com>, UNGLinuxDriver@...rochip.com, 
	Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, 
	Vladimir Oltean <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Lukasz Majewski <lukma@...x.de>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v3] net: phy: disable eee due to errata on various KSZ switches

On Mon, Oct 7, 2024 at 11:38 AM Oleksij Rempel <o.rempel@...gutronix.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@...eworks.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