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

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f18ef2e8-f3ac-43df-95d4-96cea73e72d4@lunn.ch>
Date: Tue, 10 Sep 2024 18:32:19 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>,
	Stefan Eichenberger <eichest@...il.com>
Cc: 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>,
	Stefan Eichenberger <eichest@...il.com>,
	Dimitri Fedrau <dima.fedrau@...il.com>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
	netdev@...r.kernel.org, linux-renesas-soc@...r.kernel.org
Subject: Re: [net-next 3/3] net: phy: marvell-88q2xxx: Enable auto
 negotiation for mv88q2110

On Fri, Sep 06, 2024 at 11:39:23PM +0200, Niklas Söderlund wrote:
> 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.

I'm just worried about reports of regressions. It could be you are
currently the only user of this driver, and you obviously don't care
about the change in behaviour, and can change the configuration of the
other end so that it also does autoneg.

But then again, Stefan Eichenberger <eichest@...il.com> added this
driver. Does autoneg by default, not forced, cause problems for his
systems?

Stefan?

	Andrew