Am 24.11.25 um 17:31 schrieb Quentin Schulz:
Hi Max, Markus,

On 11/21/25 9:18 AM, Max Merchel wrote:
From: Markus Niebel <[email protected]>

Disable the support due to chip errata and call genphy_config_aneg
instead of genphy_config. For a complete describtion look at the
errata sheets: DS80000691D or DS80000692D.


Linux counterpart seems to be 3aed3e2a143c ("net: phy: micrel: add Asym Pause workaround"). It does seem to set an additional bit (ADVERTISE_PAUSE_CAP in U-Boot) but the explanation doesn't seem to apply to U-Boot.

I checked ADVERTISE_PAUSE_CAP in the code and ran tests on TQMa6-MBa6. Setting ADVERTISE_PAUSE_CAP isn't necessary for the workaround.


Though another interesting commit is in the Linux tree as well, 407d8098cb1a ("net: phy: micrel: add Asym Pause workaround for KSZ9021"). I assume we need a similar fix in ksz9021_config()?

I will integrate the workaround for KSZ9021 into the commit in V2 and add references to both Linux commits.

Signed-off-by: Markus Niebel <[email protected]>
Signed-off-by: Max Merchel <[email protected]>
---
  drivers/net/phy/micrel_ksz90x1.c | 18 +++++++++++++++++-
  1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel_ksz90x1.c b/drivers/net/phy/ micrel_ksz90x1.c
index a02dbe900b8..bc42506c65e 100644
--- a/drivers/net/phy/micrel_ksz90x1.c
+++ b/drivers/net/phy/micrel_ksz90x1.c
@@ -336,6 +336,7 @@ static int ksz9031_phy_extwrite(struct phy_device *phydev, int addr,
  static int ksz9031_config(struct phy_device *phydev)
  {
+    unsigned int features = phydev->drv->features;

It's a u32 member, so we can simply use a u32 for the local variable.

I've changed it in V2.


This will be overshadowed by unsigned features in the if block, just remove the overshadowing variable and features = phydev->drv->features; in the if block since you'll already have set it.

      int ret;
      ret = ksz9031_of_config(phydev);
@@ -371,7 +372,22 @@ static int ksz9031_config(struct phy_device *phydev)
          return 0;
      }
-    return genphy_config(phydev);
+    /* Silicon Errata Sheet (DS80000691D or DS80000692D):
+     * Whenever the device's Asymmetric Pause capability is set to 1,
+     * link-up may fail after a link-up to link-down transition.
+     *
+     * Workaround:
+     * Do not enable the Asymmetric Pause capability bit.
+     */
+    features &= ~ADVERTISE_PAUSE_ASYM;
+    /* update feature support and forward to advertised features */
+    phydev->supported = features;
+    phydev->advertising = phydev->supported;
+
+    /* genphy_restart_aneg called from genphy_config_aneg */
+    return genphy_config_aneg(phydev);
+
+    return 0;

Don't need the return 0 here as it won't be reached.

I remove it.>
Is this errata only applicable if 1000BASE-T is supported? I couldn't find that in the doc but maybe I misread. In which case, we need the exact same logic in the if (env_get("disable_giga")) section I think?

I checked. It doesn't only apply to 1000BASE-T.
To use the workaround for KSZ9021 as well, it's implemented as a function and called before `if (env_get("disable_giga"))` for KSZ9031.


Cheers,
Quentin


I send V2

--
Best regards,
Max

TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Reply via email to