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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 26 Jul 2022 15:59:49 +0200
From:   Gilles BULOZ <gilles.buloz@...tron.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     netdev@...r.kernel.org
Subject: Re: Marvell 88E1512 PHY LED2 mode mismatch with Elkhartlake pin mode

Le 19/07/2022 à 23:35, Andrew Lunn a écrit :
> 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://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2FYelxMFOiqnfIVmyy%40lunn.ch%2FT%2F&amp;data=05%7C01%7Cgilles.buloz%40kontron.com%7C3c6eef4b24214d5c7a8408da69ce9e4b%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637938633398700195%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=%2FIM7eVVfMfpjNCDpJaMZTXejNWENIHZWKExGmSTz%2FFQ%3D&amp;reserved=0
>
> 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?
The value programmed by the BIOS to MII_PHY_LED_CTRL is 0x0030 meaning LED2=link, LED1=activity, and LED0=link (and reserved bit 12 
is set to 0 instead of keeping it to its default 1). So this is also not something OK if the interrupt is enabled on the Elkartlake 
side for LED2/INT#
>
> Is the IRQ described in ACPI?
OK, I'm going to check for it
> Maybe you could wire it up. Set
> phydev->irq before connecting the PHY,
OK do you do that (set phydev->irq) ?
> 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.
Is the PHY interrupt needed to support WakeOnLan ?
And is the PHY POLL mode what we have on the EHL CRB (I don't have the CRB here so I can't check that) ?
Thanks
Gilles
>
>      Andrew
> .