Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

On 2/24/23 10:36, Oleksij Rempel wrote:
> On Fri, Feb 24, 2023 at 09:41:32AM -0800, Guenter Roeck wrote:
> > On Fri, Feb 24, 2023 at 05:52:13PM +0100, Oleksij Rempel wrote:
> > > On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote:
> > > > On 2/23/23 20:53, Oleksij Rempel wrote:
> > > > > Hallo Guenter,
> > > > >
> > > > > On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote:
> > > > > > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
> > > > > > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
> > > > > > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
> > > > > > > >
> > > > > > > > It should work as before except write operation to the EEE adv registers
> > > > > > > > will be done only if some EEE abilities was detected.
> > > > > > > >
> > > > > > > > If some driver will have a regression, related driver should provide own
> > > > > > > > .get_features callback. See micrel.c:ksz9477_get_features() as example.
> > > > > > > >
> > > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> > > > > > > > Reviewed-by: Andrew Lunn <andrew@xxxxxxx>
> > > > > > >
> > > > > > > This patch causes network interface failures with all my xtensa qemu
> > > > > > > emulations. Reverting it fixes the problem. Bisect log is attached
> > > > > > > for reference.
> > > > > >
> > > > > > Also affected are arm:cubieboard emulations, with same symptom.
> > > > > > arm:bletchley-bmc emulations crash. In both cases, reverting this patch
> > > > > > fixes the problem.
> > > > >
> > > > > Please test this fixes:
> > > > > https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@xxxxxxxxxx/
> > > >
> > > > Applied and tested
> > > >
> > > > 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes
> > > > 068a35a8d62c net: phy: do not force EEE support
> > > > 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function
> > > > ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation
> > > >
> > > > on top of
> > > >
> > > > d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > >
> > > > No change for xtensa and arm:cubieboard; network interfaces still fail.
> > >
> > > Huh, interesting.
> > >
> > > can you please send me the kernel logs.
> > There is nothing useful there, or at least I don't see anything useful.
> > The Ethernet interfaces (sun4i-emac for cubieboard and ethoc for xtensa)
> > just don't come up.
> >
> > Sample logs:
> >
> > cubieboard:
> >
> > https://kerneltests.org/builders/qemu-arm-v7-master/builds/531/steps/qemubuildcommand/logs/stdio
> >
> > xtensa:
> >
> > https://kerneltests.org/builders/qemu-xtensa-master/builds/2177/steps/qemubuildcommand/logs/stdio
> >
> > and, for completeness, bletchley-bmc:
> >
> > https://kerneltests.org/builders/qemu-arm-aspeed-master/builds/531/steps/qemubuildcommand/logs/stdio
> >
> > Those logs are without the above set of patches, but I don't see a
> > difference with the patches applied for cubieboard and xtensa. I
> > started a complete test run (for all emulations) with the patches
> > applied; that should take about an hour to complete.
> > I could also add some debug logging, but you'd have to give me
> > some hints about what to add and where.
>
> OK, interesting. These are emulated PHYs. QEMU seems to return 0 or
> 0xFFFF on unsupported registers. May be I'm wrong.
> All EEE read/write accesses depend on initial capability read
> genphy_c45_read_eee_cap1()
>
> Can you please add this trace:
>
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index f595acd0a895..67dac9f0e71d 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev)
>           * (Register 3.20)
>           */
>          val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
> +       printk("MDIO_PCS_EEE_ABLE = 0x%04x", val);
>          if (val < 0)
>                  return val;

For cubieboard:

MDIO_PCS_EEE_ABLE = 0x0000

qemu reports attempts to access unsupported registers.

I had a look at the Allwinner mdio driver. There is no indication suggesting
what the real hardware would return when trying to access unsupported registers,
and the Ethernet controller datasheet is not public.

For xtensa:

MDIO_PCS_EEE_ABLE = 0x0014

I didn't try to find out what that means.

qemu did not report attempts to access unsupported registers.

Guenter