Re: [PATCH v2 2/2] net: phy: microchip: enable downshift by default on LAN88xx

[Nicolai Buchwitz <nb@tipi-net.de>]

On 31.3.2026 13:32, Russell King (Oracle) wrote:
> On Tue, Mar 31, 2026 at 12:46:27AM +0200, Nicolai Buchwitz wrote:
>> Enable auto-downshift from 1000BASE-T to 100BASE-TX after 2 failed
>> auto-negotiation attempts by default. This ensures that links with
>> faulty or missing cable pairs (C and D) fall back to 100Mbps without
>> requiring userspace configuration.
>> 
>> Users can override or disable downshift at runtime:
>> 
>>   ethtool --set-phy-tunable eth0 downshift off
>> 
>> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
> I'm slightly concerned by this commit. ->config_init() is called when
> the netdev attaches the PHY, and also during the resume path - and it's
> the second one which I believe is a problem here.
> 
> If the user has configured the downshift, it is reasonable for the user
> to expect the setting to be preserved over a suspend/resume. However,
> by placing this code in ->config_init(), you will overwrite the user's
> setting when the system resumes.

You have a valid point. Looking at other drivers, Marvell has the
same issue: m88e1112_config_init() unconditionally sets downshift to 3
on every config_init call.

I see two options:

1. Save the user's setting in the driver's priv struct and restore it
in config_init instead of blindly applying the default.

2. Handle it generically in the PHY core, saving/restoring tunable
state across suspend/resume for all drivers.

I'd lean towards (1) to keep this series simple. (2) could be a
follow-up that fixes Marvell and others too. What do you think?

Thanks

Nicolai