Re: [PATCH v2 net 3/4] net: dsa: mv88e6xxx: Never force link on in-band managed MACs

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Thu, Dec 19, 2024 at 01:30:42PM +0100, Tobias Waldekranz wrote:
> NOTE: This issue was addressed in the referenced commit, but a
> conservative approach was chosen, where only 6095, 6097 and 6185 got
> the fix.
> 
> Before the referenced commit, in the following setup, when the PHY
> detected loss of link on the MDI, mv88e6xxx would force the MAC
> down. If the MDI-side link was then re-established later on, there was
> no longer any MII link over which the PHY could communicate that
> information back to the MAC.
> 
>         .-SGMII/USXGMII
>         |
> .-----. v .-----.   .--------------.
> | MAC +---+ PHY +---+ MDI (Cu/SFP) |
> '-----'   '-----'   '--------------'
> 
> Since this a generic problem on all MACs connected to a SERDES - which
> is the only time when in-band-status is used - move all chips to a
> common mv88e6xxx_port_sync_link() implementation which avoids forcing
> links on _all_ in-band managed ports.
> 
> Fixes: 4efe76629036 ("net: dsa: mv88e6xxx: Don't force link when using in-band-status")
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>

I'm feeling uneasy about this change.

The history of the patch you refer to is - original v1:

https://lore.kernel.org/r/20201013021858.20530-2-chris.packham@alliedtelesis.co.nz

When v3 was submitted, it was unchanged:

https://lore.kernel.org/r/20201020034558.19438-2-chris.packham@alliedtelesis.co.nz

Both of these applied the in-band-status thing to all Marvell DSA
switches, but as Marek states here:

https://lore.kernel.org/r/20201020165115.3ecfd601@nic.cz

doing so breaks last least one Marvell DSA switch (88E6390). Hence why
this approach is taken, rather than not forcing the link status on all
DSA switches.

Your patch appears to be reverting us back to what was effectively in
Chris' v1 patch from back then, so I don't think we can accept this
change. Sorry.

If you have a switch that needs your change, then I think you also need
to adopt the "conservative" approach and only fix the switch you're
working with - short of someone going through all the Marvell DSA
switch datasheets or testing on real hardware, I don't think it's a
good idea to change the behaviour of all Marvell switches like this.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!