Re: [PATCH v2] arm64: dts: renesas: white-hawk: ethernet: Describe avb1 and avb2

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Geert,

Thanks for your report.

On 2024-03-26 16:54:49 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Sat, Mar 9, 2024 at 4:56 PM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > Describe the two Marvel 88Q2110/QFN40 PHYs available on the R-Car V4H
> > White Hawk RAVB/Ethernet(1000Base-T1) sub-board. The two PHYs are wired
> > up on the board by default, there is no need to move any resistors which
> > are needed to access other PHYs available on this sub-board.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> When accidentally booting a kernel without CONFIG_MARVELL_88Q2XXX_PHY=y,
> I am greeted with the following warning splat (same for the second PHY):

I can reproduce this, but I'm not really sure how to deal with it and 
it's unrelated to Renesas AVB Ethernet driver.

What happens when when running without CONFIG_MARVELL_88Q2XXX_PHY=y is 
that the core fallback to use the generic Generic Clause 45 PHY driver.  
And the generic driver don't work correctly for the PHY on V4H (88Q2110).

The splat happens when the generic driver tries to enable auto 
negotiation. This is known to be broken for 88Q2110 and the 
CONFIG_MARVELL_88Q2XXX_PHY=y driver disables this. From 
marvell-88q2xxx.c

        /* The PHY signalizes it supports autonegotiation. Unfortunately, so
         * far it was not possible to get a link even when following the init
         * sequence provided by Marvell. Disable it for now until a proper
         * workaround is found or a new PHY revision is released.
         */
        if (phydev->drv->phy_id == MARVELL_PHY_ID_88Q2110)
                linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
                                   phydev->supported);

If I hack the generic PHY to also not try auto negotiation the splat go 
away and the PHY binds using the generic driver. I did not test using it 
further.

I can't try to submit such a hack as that would break all other PHY 
where auto negotiation works. So I'm not sure we can do anything about 
this other than make sure we have the correct PHY drivers around, or use 
PHYs that works with the Clause 45 specification.

> 
> -mv88q2110 e6810000.ethernet-ffffffff:00: attached PHY driver
> (mii_bus:phy_addr=e6810000.ethernet-ffffffff:00, irq=POLL)
> +Generic Clause 45 PHY e6810000.ethernet-ffffffff:00: attached PHY
> driver (mii_bus:phy_addr=e6810000.ethernet-ffffffff:00, irq=POLL)
> +rcar-du feb00000.display: adding to PM domain always-on
> -mv88q2110 e6820000.ethernet-ffffffff:00: attached PHY driver
> (mii_bus:phy_addr=e6820000.ethernet-ffffffff:00, irq=POLL)
> +rcar-du feb00000.display: removing from PM domain always-on
> +------------[ cut here ]------------
> +_phy_start_aneg+0x0/0xa8: returned: -22
> +WARNING: CPU: 2 PID: 55 at drivers/net/phy/phy.c:1262
> _phy_state_machine+0x120/0x198
> +Modules linked in:
> +CPU: 2 PID: 55 Comm: kworker/2:1 Not tainted
> 6.9.0-rc1-white-hawk-02587-g577b6a49a6d4 #235
> +Hardware name: Renesas White Hawk CPU and Breakout boards based on
> r8a779g0 (DT)
> +Workqueue: events_power_efficient phy_state_machine
> +pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> +pc : _phy_state_machine+0x120/0x198
> +lr : _phy_state_machine+0x120/0x198
> +sp : ffffffc082dd3d10
> +x29: ffffffc082dd3d10 x28: ffffff8440089c05 x27: ffffffc081090000
> +x26: ffffffc080e03008 x25: 0000000000000000 x24: ffffffc0815603d0
> +x23: ffffffc080e03008 x22: ffffff86bef98100 x21: 0000000000000004
> +x20: 0000000000000001 x19: ffffff84435b3000 x18: 0000000000000000
> +x17: 0000000000000000 x16: 0000000000000000 x15: 0720072007320732
> +x14: 072d0720073a0764 x13: 0720072007320732 x12: 072d0720073a0764
> +x11: 000000000000033a x10: ffffffc0810b9ac8 x9 : ffffffc081379ca8
> +x8 : ffffffc082dd3a18 x7 : ffffffc082dd3a20 x6 : 00000000ffff7fff
> +x5 : c0000000ffff7fff x4 : 0000000000000000 x3 : 0000000000000001
> +x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffff8440a98000
> +Call trace:
> + _phy_state_machine+0x120/0x198
> + phy_state_machine+0x2c/0x5c
> + process_scheduled_works+0x314/0x4d4
> + worker_thread+0x1b8/0x20c
> + kthread+0xd8/0xe8
> + ret_from_fork+0x10/0x20
> +irq event stamp: 16
> +hardirqs last  enabled at (15): [<ffffffc080913144>]
> _raw_spin_unlock_irq+0x2c/0x40
> +hardirqs last disabled at (16): [<ffffffc08090d434>] __schedule+0x1cc/0x870
> +softirqs last  enabled at (0): [<ffffffc0800800f8>] copy_process+0x698/0x1924
> +softirqs last disabled at (0): [<0000000000000000>] 0x0
> +---[ end trace 0000000000000000 ]---
> 
> Is that expected behavior?
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Kind Regards,
Niklas Söderlund