Messages in this thread | | | Date | Wed, 20 Apr 2022 17:03:07 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH 4/5] net: phy: marvell: Add LED accessors for Marvell 88E1510 |
| |
On Wed, Apr 20, 2022 at 08:40:51PM +0800, Kai-Heng Feng wrote: > Implement get_led_config() and set_led_config() callbacks so phy core > can use firmware LED as platform requested. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/net/phy/marvell.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > index 2702faf7b0f60..c5f13e09b0692 100644 > --- a/drivers/net/phy/marvell.c > +++ b/drivers/net/phy/marvell.c > @@ -750,6 +750,30 @@ static int m88e1510_config_aneg(struct phy_device *phydev) > return err; > } > > +static int marvell_get_led_config(struct phy_device *phydev) > +{ > + int led; > + > + led = phy_read_paged(phydev, MII_MARVELL_LED_PAGE, MII_PHY_LED_CTRL); > + if (led < 0) { > + phydev_warn(phydev, "Fail to get marvell phy LED.\n"); > + led = 0; > + }
I've said this multiple times, there are three LED registers, The Function Control register, the Priority Control register and the Timer control register. It is the combination of all three that defines how the LEDs work. You need to save all of them.
And you need to make your API generic enough that the PHY driver can save anywhere from 1 bit to 42 bytes of configuration.
I don't know ACPI, but i'm pretty sure this is not the ACPI way of doing this. I think you should be defining an ACPI method, which phylib can call after initialising the hardware to allow the firmware to configure the LED.
Andrew
| |