Re: [PATCH RFC net] net: dsa: mv88e6xxx: allow use of PHYs on CPU and DSA ports

On Tue, Dec 07, 2021 at 04:07:26PM +0100, Andrew Lunn wrote:

On Tue, Dec 07, 2021 at 02:58:26PM +0000, Russell King (Oracle) wrote:

quoted

On Tue, Dec 07, 2021 at 03:54:37PM +0100, Andrew Lunn wrote:

quoted

On Tue, Dec 07, 2021 at 10:59:19AM +0000, Russell King (Oracle) wrote:

quoted

Martyn Welch reports that his CPU port is unable to link where it has
been necessary to use one of the switch ports with an internal PHY for
the CPU port. The reason behind this is the port control register is
left forcing the link down, preventing traffic flow.

This occurs because during initialisation, phylink expects the link to
be down, and DSA forces the link down by synthesising a call to the
DSA drivers phylink_mac_link_down() method, but we don't touch the
forced-link state when we later reconfigure the port.

Resolve this by also unforcing the link state when we are operating in
PHY mode and the PPU is set to poll the PHY to retrieve link status
information.

Reported-by: Martyn Welch <redacted>
Signed-off-by: Russell King (Oracle) <redacted>

Hi Russell

It would be good to have a Fixes: tag here, to help with back porting.

Oh, I thought this was a new development, not a regression. Do you have
a pointer to the earlier bits of the thread please, e.g. the message ID
of the original report.

This all seems to be part of:

b98043f66e8c6f1fd75d11af7b28c55018c58d79.camel@collabora.com

It looks like 5.15-rc3 has issues, but i suspect it goes back further.
I'm also assuming it is a regression, not that it never worked in the
first place. Maybe i'm wrong?

Thanks.

It looks like DT support for this was added in e26dead44268, which is
in v4.16-rc1.

The introduction of the phylink_mac_link_down() in net/dsa/port.c was
in 3be98b2d5fbc, v5.7-rc2, as was the addition of MLO_AN_FIXED in
mv88e6xxx DSA link down/up functions in 34b5e6a33c1a (these are
consecutive commits.)

Thankfully, the addition of mv88e6xxx_port_ppu_updates was in v5.7-rc1.

Now, this patch can't simply be backported without the update to
mv88e6xxx_port_ppu_updates(), so I feel we need to invent a Requires:
or Depends: tag so stable people don't backport this without the other
patch - my recent experience with stable is that patches get picked up
quite randomly.

I think I'd be tempted to go with:

Fixes: 3be98b2d5fbc ("net: dsa: Down cpu/dsa ports phylink will control")

I think we also need:

Cc: <redacted> # v5.7-rc2: xxxxx: net: dsa: mv88e6xxx: fix "don't use PHY_DETECT on internal PHY's"

which seems to be a thing according to stable-kernel-rules.rst...
with the xxxxx replaced with the proper sha1 ID once the dependent
patch has been applied to the net tree.

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