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/