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

[Josef Raschen]

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