Messages in this thread | | | From | Kai-Heng Feng <> | Date | Thu, 21 Apr 2022 11:11:55 +0800 | Subject | Re: [PATCH 4/5] net: phy: marvell: Add LED accessors for Marvell 88E1510 |
| |
On Wed, Apr 20, 2022 at 11:03 PM Andrew Lunn <andrew@lunn.ch> wrote: > > 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.
OK, will do.
> > And you need to make your API generic enough that the PHY driver can > save anywhere from 1 bit to 42 bytes of configuration.
OK, I guess I'll let the implementation stays within driver callback, so each driver can decide how many bits need to be saved.
> > 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.
This is not feasible. If BIOS can define a method and restore the LED by itself, it can put the method inside its S3 method and I don't have to work on this at the first place.
Kai-Heng
> > Andrew
| |