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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fbe66b6d-2517-4a6b-8bd2-ec6d94b8dc8e@raschen.org>
Date: Mon, 29 Sep 2025 21:30:27 +0200
From: Josef Raschen <josef@...chen.org>
To: Andrew Lunn <andrew@...n.ch>
Cc: Arun Ramadoss <arun.ramadoss@...rochip.com>,
 UNGLinuxDriver@...rochip.com, Heiner Kallweit <hkallweit1@...il.com>,
 Russell King <linux@...linux.org.uk>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: phy: microchip_t1: LAN887X: Fix device init issues.

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