lkml.org 
[lkml]   [2022]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [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

    \
     
     \ /
      Last update: 2022-04-21 05:13    [W:3.473 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site