Subject: Re: [net-next 3/3] net: phy: marvell-88q2xxx: Enable auto negotiation for mv88q2110

From: Niklas Söderlund <[email-masked]>
Date: Fri, 6 Sep 2024 23:39:23 +0200

On 2024-09-06 22:36:49 +0200, Andrew Lunn wrote:
> On Fri, Sep 06, 2024 at 03:39:51PM +0200, Niklas Söderlund wrote:
> > The initial marvell-88q2xxx driver only supported the Marvell 88Q2110
> > PHY without auto negotiation support. The reason documented states that
> > the provided initialization sequence did not to work. Now a method to
> > enable auto negotiation have been found by comparing the initialization
> > of other supported devices and an out-of-tree PHY driver.
> > 
> > Perform the minimal needed initialization of the PHY to get auto
> > negotiation working and remove the limitation that disables the auto
> > negotiation feature for the mv88q2110 device.
> > 
> > With this change a 1000Mbps full duplex link is able to be negotiated
> > between two mv88q2110 and the link works perfectly. The other side also
> > reflects the manually configure settings of the master device.
> > 
> >     # ethtool eth0
> >     Settings for eth0:
> >             Supported ports: [  ]
> >             Supported link modes:   100baseT1/Full
> >                                     1000baseT1/Full
> >             Supported pause frame use: Symmetric Receive-only
> >             Supports auto-negotiation: Yes
> 
> My understanding is that most automotive applications using T1 don't
> actually want auto-neg, because it is slow. Given the static use case,
> everything can be statically configured.
> 
> Is there a danger this change is going to cause regressions? There are
> users who are happy for it to use 100BaseT1 without negotiation, and
> the link partner is not offering any sort of negotiation. But with
> this change, autoneg is now the default, and the link fails to come
> up?

I'm not sure how the generic use-case looks like. All I can say all 
other devices supported by this driver support autoneg by default and 
the initial commit adds some of the autoneg features but disables it 
with a comment that they could not get it to work.

> 
> To me, this actually seems like a generic problem for automotive. We
> want to indicate the device does support autoneg, but we don't want it
> on by default? I don't know if we can express that at the moment?
> 
> 	Andrew

-- 
Kind Regards,
Niklas Söderlund