[PATCH 1/2] net: phy: micrel_ksz90x1: disable asymmetric pause for KSZ9031

[Max Merchel]

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 <Markus.Niebel at ew.tq-group.com>
>>
>> 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 <Markus.Niebel at ew.tq-group.com>
>> Signed-off-by: Max Merchel <Max.Merchel at ew.tq-group.com>
>> ---
>>   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/