Re: Marvell 88E1512 PHY LED2 mode mismatch with Elkhartlake pin mode

[Andrew Lunn <andrew@lunn.ch>]

On Tue, Jul 19, 2022 at 06:36:20PM +0200, Gilles BULOZ wrote:
> Dear developers,
> 
> On a custom Elkhartlake board based on the Intel CRB, it turns out I have
> the 88E1512 PHY configured in polled mode ("intel-eth-pci 0000:00:1e.4 eno1:
> PHY [stmmac-1:01] driver [Marvell 88E1510] (irq=POLL)" in dmesg) and the
> LED2/INT# pin is configured in LED2 mode by marvell_config_led() in
> drivers/net/phy/marvell.c (MII_88E1510_PHY_LED_DEF written to
> MII_PHY_LED_CTRL). This pin is connected as on the CRB to an Elkhartlake pin
> for a PHY interrupt but for some reason the interrupt is enabled on the
> Elkhartlake.
> So when I shutdown the system (S5), any activity on link makes LED2/INT# toggle and power the system back on.
> 
> I tried to force  phydev->dev_flag to use
> MII_88E1510_PHY_LED0_LINK_LED1_ACTIVE instead of MII_88E1510_PHY_LED_DEF but
> I've been unable to find how to force this flag. And I discovered that the
> value of MII_88E1510_PHY_LED0_LINK_LED1_ACTIVE = 0x1040 is not OK for me
> because LED2 is set to "link status" so if I use this value the system is
> back "on" on link change (better than on activity but still not OK).
> 
> As a final workaround I've patched drivers/net/phy/marvell.c at
> marvell_config_led() to have "LED0=link LED1=activity LED2=off" by writing
> 0x1840 to MII_PHY_LED_CTRL, but I know this is a ugly workaround.
> 
> So I'm wondering if PHY "irq=POLL" is the expected operating mode ?
> In this case what should disable the interrupt on the Elkhartlake pin ?
> Is wake on Lan supported if PHY is set to "irq=POLL" ?

This sounds a bit like:

https://lore.kernel.org/lkml/YelxMFOiqnfIVmyy@lunn.ch/T/

If you comment out marvell_config_led(), and read back the registers,
does it look like the firmware has setup the LEDs to something
sensible?

Is the IRQ described in ACPI? Maybe you could wire it up. Set
phydev->irq before connecting the PHY, and then phylib will use the
IRQ, not polling. That might also solve your wakeup problem, in that
when the interrupt is disabled at shutdown, it should disable it in
the PHY.

    Andrew