linux-kernel - Re: [PATCH v2] net: phy: marvell: Honor phy LED set by system firmware on a Dell hardware

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:   Fri, 21 Jan 2022 14:04:59 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Kai-Heng Feng <kai.heng.feng@...onical.com>
Cc:     hkallweit1@...il.com, linux@...linux.org.uk,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] net: phy: marvell: Honor phy LED set by system
 firmware on a Dell hardware

> > Since you talked about suspend/resume, does this machine support WoL?
> > Is the BIOS configuring LED2 to be used as an interrupt when WoL is
> > enabled in the BIOS? Do you need to save/restore that configuration
> > over suspend/review? And prevent the driver from changing the
> > configuration?
> 
> This NIC on the machine doesn't support WoL.

I'm surprised about that. Are you really sure?

What are you doing for resume? pressing the power button?

> > > +static const struct dmi_system_id platform_flags[] = {
> > > +     {
> > > +             .matches = {
> > > +                     DMI_MATCH(DMI_SYS_VENDOR, "Dell EMC"),
> > > +                     DMI_MATCH(DMI_PRODUCT_NAME, "Edge Gateway 3200"),
> > > +             },
> > > +             .driver_data = (void *)PHY_USE_FIRMWARE_LED,
> > > +     },
> >
> > This needs a big fat warning, that it will affect all LEDs for PHYs
> > which linux is driving, on that machine. So PHYs on USB dongles, PHYs
> > in SFPs, PHYs on plugin PCIe card etc.
> >
> > Have you talked with Dells Product Manager and do they understand the
> > implications of this?
> 
> Right, that's why the original approach is passing the flag from the MAC driver.
> That approach can be more specific and doesn't touch unrelated PHYs.

More specific, but still will go wrong at some point, A PCEe card
using that MAC etc. And this is general infrastructure you are adding
here, it can be used by any machine, any combination of MAC and PHY
etc. So you need to clearly document its limits so others are not
surprised.

	Andrew