archive mirror
 help / color / mirror / Atom feed
From: stuart hayes <>
To: Pavel Machek <>
Cc: Randy Dunlap <>,
	Bjorn Helgaas <>,, Keith Busch <>,,
	"" <>
Subject: Re: [PATCH v2] Add support for PCIe SSD status LED management
Date: Wed, 2 Jun 2021 10:36:03 -0500	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20210602090504.GA10900@amd>

On 6/2/2021 4:05 AM, Pavel Machek wrote:
> Hi!
>>>>>   [ok] [locate] failed rebuild pfa hotspare ica ifa invalid disabled
>>> So what does this do? Turns the LED on if driver is in "ok" or
>>> "locate" states?
>> This would cause the system to somehow show the user that that this drive
>> (or drive slot) is the one that it wants a person to be able to physically
>> locate (possibly by flashing a blue LED on/near the drive), and also that
>> the drive is OK.  It would presumably do that by lighting the LEDs on/near
>> the drive with certain colors and/or flashing patterns, though it could, in
>> theory, put "OK" in an LCD on the drive slot.  How the states are displayed
>> to the user is beyond the scope of the specs.
>> (The _DSM and NPEM specs provide for a way to control status LEDs on a drive
>> or drive slot.  Typically drives will have 2 or 3 LEDs that are illuminated
>> in different colors or flashing patterns to indicate various states (like
>> those listed in IBPI / SFF-8489), though the _DSM / NPEM specs do not
>> specify how the states are displayed.)
> Well, LED subsystem expects to know how many LEDs are there and what
> the LEDs are, and expects individual control over them.
> "2 or 3 LEDs with unknown patterns", or LCD display is outside of scope
> of LED subsystem, so this should be independend.
> Best regards,
> 								Pavel

Yes... I had originally submitted this without using the LED subsystem, 
and Greg K-H said I had to (see  Here's the 
relevant bit.

(Greg K-H:)
>> > And why isn't this code using the existing LED subsystem?  Don't create
>> > random new driver-specific sysfs files that do things the existing class
>> > drivers do please.
>> >
>> I did consider the LED subsystem, but I don't think it is a great match.
>> >> In spite of the name, this _DSM doesn't directly control individual 
>> sets the state(s) of the PCI port to which an SSD may be connected, so that
>> it may be conveyed to the user... a processor (or at least some logic) will
>> decide how to show which states are active, probably by setting combinations
>> of LEDs to certain colors or blink patterns, or possibly on some other type
>> of display.
(Greg K-H:)
> If you are allowing LEDs to be controlled by the user, then yes, you
> have to use the LED subsystem as you should never try to create a brand
> new driver-specific user/kernel API just for your tiny driver right?
> Please work with the subsystems we have, they are unified for a reason.

So I'm not sure what to do.

  reply	other threads:[~2021-06-02 15:37 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-01 20:38 [PATCH v2] Add support for PCIe SSD status LED management Stuart Hayes
2021-06-01 21:20 ` Randy Dunlap
2021-06-01 22:38   ` Pavel Machek
2021-06-02  3:18     ` stuart hayes
2021-06-02  9:05       ` Pavel Machek
2021-06-02 15:36         ` stuart hayes [this message]
2021-06-02 22:40           ` Pavel Machek
2021-06-04 20:13             ` stuart hayes
2021-06-02 16:33       ` Marek Behún
2021-06-01 23:39   ` Randy Dunlap
2021-06-01 23:48 ` kernel test robot
2021-06-02  7:05 ` Lukas Wunner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).