Re: [PATCH] net: phy: microchip_t1: LAN887X: Fix device init issues.

[Josef Raschen <josef@raschen.org>]

On 9/26/25 23:46, Andrew Lunn wrote:
> On Fri, Sep 26, 2025 at 11:24:56PM +0200, Josef Raschen wrote:
>> Hello Andrew,
>>
>> Thanks for your feedback.
>>
>> On 9/26/25 00:00, Andrew Lunn wrote:
>>> On Thu, Sep 25, 2025 at 10:52:22PM +0200, Josef Raschen wrote:
>>>> Currently, for a LAN8870 phy, before link up, a call to ethtool to set
>>>> master-slave fails with 'operation not supported'. Reason: speed, duplex
>>>> and master/slave are not properly initialized.
>>>>
>>>> This change sets proper initial states for speed and duplex and publishes
>>>> master-slave states. A default link up for speed 1000, full duplex and
>>>> slave mode then works without having to call ethtool.
>>>
>>> Hi Josef
>>>
>>> What you are missing from the commit message is an explanation why the
>>> LAN8870 is special, it needs to do something no other PHY does. Is
>>> there something broken with this PHY? Some register not following
>>> 802.3?
>>>
>>>       Andrew
>>>
>>> ---
>>> pw-bot: cr
>>>
>>
>> Special about the LAN8870 might be that it is a dual speed T1 phy.
>> As most other T1 pyhs have only one possible configuration the unknown
>> speed configuration was not a problem so far. But here, when
>> calling link up without manually setting the speed before, it seems to
>> pick speed 100 in phy_sanitize_settings(). I assume that this is not the
>> desired behavior?
> 
> What speeds does the PHY say it supports?
> 
> phy_sanitize_settings() should pick the highest speed the PHY supports
> as the default. So if it is picking 100, it suggests the PHY is not
> reporting it supports 1000? Or phy_sanitize_settings() is broken for
> 1000Base-T1? Please try understand why it is picking 100.

It should pick 1000 then for this phy. I checked already that the device 
is actually reporting being 100Base-T1 and 1000Base-T1 capable. I will 
have a look, why it doesn't pick SPEED_1000 then. I did not know that 
phy_sanitize_settings() is supposed to pick the highest speed already.

>> The second problem is that ethtool initially does not allow to set
>> master-slave at all. You first have to call ethtool without the
>> master-slave argument, then again with it. This is fixed by reading
>> the master slave configuration from the device which seems to be missing
>> in the .config_init and .config_aneg functions. I took this solution from
>> net/phy/dp83tg720.c.
> 
> How does this work with a regular T4 or T2 PHY? Ideally, A T1 should
> be no different. And ideally, we want a solution for all T1 PHYs,
> assuming it is not something which is special for this PHY.

I agree, a generic solution would be best. I will read a bit into the 
code to see if there is a better solution.

Thanks,
Josef