Messages in this thread | | | Date | Fri, 21 Jan 2022 14:22:15 +0100 | From | Andrew Lunn <> | Subject | Re: [PATCH v2] net: phy: marvell: Honor phy LED set by system firmware on a Dell hardware |
| |
On Fri, Jan 21, 2022 at 12:01:35PM +0800, Kai-Heng Feng wrote: > On Thu, Jan 20, 2022 at 10:26 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > On Thu, Jan 20, 2022 at 01:19:29PM +0800, Kai-Heng Feng wrote: > > > BIOS on Dell Edge Gateway 3200 already makes its own phy LED setting, so > > > instead of setting another value, keep it untouched and restore the saved > > > value on system resume. > > > > > > Introduce config_led() callback in phy_driver() to make the implemtation > > > generic. > > > > I'm also wondering if we need to take a step back here and get the > > ACPI guys involved. I don't know much about ACPI, but shouldn't it > > provide a control method to configure the PHYs LEDs? > > > > We already have the basics for defining a PHY in ACPI. See: > > > > https://www.kernel.org/doc/html/latest/firmware-guide/acpi/dsd/phy.html > > These properties seem to come from device-tree.
They are similar to what DT has, but expressed in an ACPI way. DT has been used with PHY drivers for a long time, but ACPI is new. The ACPI standard also says nothing about PHYs. So Linux has defined its own properties, which we expect all ACPI machine to use. According to the ACPI maintainers, this is within the ACPI standard. Maybe at some point somebody will submit the current definitions to the standards body for approval, or maybe the standard will do something completely different, but for the moment, this is what we have, and what you should use.
> > so you could extend this to include a method to configure the LEDs for > > a specific PHY. > > How to add new properties? Is it required to add new properties to > both DT and ACPI?
Since all you are adding is a boolean, 'Don't touch the PHY LED configuration', it should be easy to do for both.
What is interesting for Marvell PHYs is WoL, which is part of LED configuration. I've not checked, but i guess there are other PHYs which reuse LED output for a WoL interrupt. So it needs to be clearly defined if we expect the BIOS to also correctly configure WoL, or if Linux is responsible for configuring WoL, even though it means changing the LED configuration.
Andrew
| |