netdev - Re: [PATCH 4/5] net: phy: marvell: Add LED accessors for Marvell 88E1510

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:   Wed, 20 Apr 2022 17:03:07 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Kai-Heng Feng <kai.heng.feng@...onical.com>
Cc:     hkallweit1@...il.com, linux@...linux.org.uk,
        peppe.cavallaro@...com, alexandre.torgue@...s.st.com,
        joabreu@...opsys.com, davem@...emloft.net, kuba@...nel.org,
        pabeni@...hat.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
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@...onical.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