Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 31 Aug 2023 12:37:09 +0200 | From | Oleksij Rempel <> | Subject | Re: [PATCH v3] net: phy: Provide Module 4 KSZ9477 errata (DS80000754C) |
| |
On Thu, Aug 31, 2023 at 11:28:00AM +0100, Russell King (Oracle) wrote: > On Thu, Aug 31, 2023 at 09:25:27AM +0200, Lukasz Majewski wrote: > > diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h > > index 8bef1ab62bba..eed474fc7308 100644 > > --- a/include/linux/micrel_phy.h > > +++ b/include/linux/micrel_phy.h > > @@ -44,6 +44,7 @@ > > #define MICREL_PHY_50MHZ_CLK 0x00000001 > > #define MICREL_PHY_FXEN 0x00000002 > > #define MICREL_KSZ8_P1_ERRATA 0x00000003 > > +#define MICREL_NO_EEE 0x00000004 > > Erm... Maybe someone should clarify this... we have in the code the > following tests for these "flags": > > /* Support legacy board-file configuration */ > if (phydev->dev_flags & MICREL_PHY_50MHZ_CLK) { > priv->rmii_ref_clk_sel = true; > priv->rmii_ref_clk_sel_val = true; > } > > /* Skip auto-negotiation in fiber mode */ > if (phydev->dev_flags & MICREL_PHY_FXEN) { > phydev->speed = SPEED_100; > return 0; > } > > if (phydev->dev_flags & MICREL_KSZ8_P1_ERRATA) > return -EOPNOTSUPP; > > /* 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) > phydev->eee_broken_modes = -1; > > Is it intentional that setting MICREL_PHY_50MHZ_CLK on its own also > activates the MICREL_KSZ8_P1_ERRATA and vice versa? Is it intentional > that setting MICREL_PHY_FXEN also activates MICREL_KSZ8_P1_ERRATA and > vice versa? > > To me, this looks horribly broken, and this patch just perpetuates the > brokenness (but at least 0x4 doesn't overlap with the other flags.) > > If it is intentional, then MICREL_KSZ8_P1_ERRATA should be defined to > make it explicit - in other words, as > (MICREL_PHY_FXEN|MICREL_PHY_50MHZ_CLK). If not, all these flags should > be defined using (1 << n) or BIT() to make it explicit that they're a > bit, and not just a hex number that gets incremented when the next flag > is added.
Indeed, it is broken. I'll send a fix for this one. MICREL_KSZ8_P1_ERRATA should be BIT(3) and MICREL_NO_EEE BIT(4)
Thx!
Regards, Oleksij -- 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 |
| ![\](/images/icornerr.gif) |