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

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

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:
> > On Tue, Dec 07, 2021 at 03:54:37PM +0100, Andrew Lunn wrote:
> > > On Tue, Dec 07, 2021 at 10:59:19AM +0000, Russell King (Oracle) wrote:
> > > > 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 <martyn.welch@collabora.com>
> > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > 
> > > 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: <stable@vger.kernel.org> # 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!