Subject: Re: riscv defconfig CONFIG_PM/macb/generic PHY regression in v5.18-rc1

From: "Russell King (Oracle)" <[email-masked]>
Date: Tue, 5 Apr 2022 19:55:42 +0100

On Tue, Apr 05, 2022 at 05:58:50PM +0100, Conor Dooley wrote:
> On 05/04/2022 16:53, Russell King (Oracle) wrote:
> > On Tue, Apr 05, 2022 at 01:05:12PM +0000, [email-masked] wrote:
> > > Hey,
> > > I seem to have come across a regression in the default riscv defconfig
> > > between riscv-for-linus-5.18-mw0 (bbde015227e8) & v5.18-rc1, exposed by
> > > c5179ef1ca0c ("RISC-V: Enable RISC-V SBI CPU Idle driver for QEMU virt
> > > machine") which causes the ethernet phy to not come up on my Icicle kit:
> > > [ 3.179864] macb 20112000.ethernet eth0: validation of sgmii with support 0000000,00000000,00006280 and advertisement 0000000,00000000,00004280 failed: -EINVAL
> > > [ 3.194490] macb 20112000.ethernet eth0: Could not attach PHY (-22)
> > 
> > I don't think that would be related to the idle driver. This looks like
> > the PHY hasn't filled in the supported mask at probe time - do you have
> > the driver for the PHY built-in or the PHY driver module loaded?
> 
> Hey Russel,
> The idle stuff enabled CONFIG_PM=y though in the default riscv
> defconfig, so it is not confined to just QEMU.
> 
> I am not sure what the symbol for the generic phy & I am not at work
> to properly check, so I hope this is the relevant part of the config:
> 
> CONFIG_PHYLINK=y
> CONFIG_PHYLIB=y
> CONFIG_SWPHY=y
> CONFIG_FIXED_PHY=y

The generic PHY is part of phylib, and will be used whenever phylib
wants to drive a PHY but no specific PHY driver is found at the point
the PHY device is attached in software to a network device.

For reference, that is a very important point to understand:

1) if the PHY driver is a module sitting on the root filesystem, but
the network driver attaches the PHY during boot before the root
filesystem is mounted, then the generic PHY driver will be used.

2) if the PHY driver is a module sitting on the root filesystem, and
the network driver attaches the PHY when the interface is brought up,
that is fine as long as the root filesystem is not network based.

> If you look at my response to Andrew [1] you'll see that my problems
> are not isolated to just the Generic PHY driver as a builtin Vitesse
> driver has issues too (although validation appears to have passed).

I've been catching up with email from the last three and a bit weeks,
so I haven't been reading all the threads before replying... there
will be some duplication between what Andrew and myself have said.

The right thing is certainly to use the Vitesse driver, and get to
the bottom of why the link won't come up when that driver is used.
I think from what I've been reading that it feels like a timing
issue - when cpu idle is enabled, then something affects the PHY
meaning that link never comes up.

The way this works with phylink in SGMII and in-band mode is that we
expect to read the link up/down, speed and duplex parameters from the
MAC/PCS end of the link, and the pause and link parameters from the
PHY. phylink will only report link up in this mode when the PHY and
the MAC/PCS both report that the link is up.

phylink should already contain sufficient debugging to work that out -
it prints at debug level whenever phylib reports a change to the link
parameters, and it also reports when the MAC/PCS triggers a change in
link state. That should be just about enough to work out which end of
the link is failing.

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