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  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:   Thu, 21 Apr 2022 14:57:37 +0200
From:   Andrew Lunn <>
To:     Kai-Heng Feng <>
Subject: Re: [PATCH 4/5] net: phy: marvell: Add LED accessors for Marvell

On Thu, Apr 21, 2022 at 08:24:00PM +0800, Kai-Heng Feng wrote:
> On Thu, Apr 21, 2022 at 7:51 PM Andrew Lunn <> wrote:
> >
> > > 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.
> >
> > So maybe just declare the BIOS as FUBAR and move on to the next issue
> > assigned to you.
> >
> > Do we really want the maintenance burden of this code for one machines
> > BIOS?
> Wasn't this the "set precedence" we discussed earlier for? Someone has
> to be the first, and more users will leverage the new property we
> added.

I both agree and disagree. I'm trying to make this feature generic,
unlike you who seem to be doing the minimal, only saving one of three
LED configuration registers. But on the other hand, i'm not sure there
will be more users. Do you have a list of machines where the BIOS is
FUBAR? Is it one machine? A range of machines from one vendor, or
multiple vendors with multiple machines. I would feel better about the
maintenance burden if i knew that this was going to be used a lot.
> > Maybe the better solution is to push back on the vendor and its
> > BIOS, tell them how they should of done this, if the BIOS wants to be
> > in control of the LEDs it needs to offer the methods to control the
> > LEDs. And then hopefully the next machine the vendor produces will
> > have working BIOS.
> The BIOS doesn't want to control the LED. It just provides a default
> LED setting suitable for this platform, so the driver can use this
> value over the hardcoded one in marvell phy driver.

Exactly, it wants to control the LED, and tell the OS not to touch it

> So this really has nothing to do with with any ACPI method.
> I believe the new property can be useful for DT world too.

DT generally never trusts the bootloader to do anything. So i doubt
such a DT property would ever be used. Also, DT is about describing
the hardware, not how to configure the hardware. So you could list
there is a PHY LED, what colour it is, etc. But in general, you would
not describe how it is configured, that something else is configuring
it and it should be left alone.

> > Your other option is to take part in the effort to add control of the
> > LEDs via the standard Linux LED subsystem. The Marvel PHY driver is
> > likely to be one of the first to gain support this for. So you can
> > then totally take control of the LED from the BIOS and put it in the
> > users hands. And such a solution will be applicable to many machines,
> > not just one.
> This series just wants to use the default value platform firmware provides.
> Create a sysfs to let user meddle with LED value doesn't really help
> the case here.

I would disagree. You can add a systemd service to configure it at
boot however you want. It opens up the possibility to implement
ethtool --identify in a generic way, etc. It is a much more powerful
and useful feature than saying 'don't touch', and also it justify the
maintenance burden.


Powered by blists - more mailing lists