Hi Andrew and Niklas, On Tue, 10 Sept 2024 at 17:32, Andrew Lunn <andrew@xxxxxxx> wrote: > > 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@xxxxxxxxx> added this > driver. Does autoneg by default, not forced, cause problems for his > systems? Sorry I didn't reply before, I'm currently travelling. That's also why I couldn't test the changes yet. For our use case the proposed change shouldn't be an issue. We anyway need to configure the speed from userspace. So I think the proposed change is fine from a user's perspective. It will require user space to configure the interface correctly before it starts the interface or it will fallback to autoneg. Regards, Stefan
