From: stuart hayes <stuart.w.hayes@gmail.com>
To: Pavel Machek <pavel@ucw.cz>
Cc: Randy Dunlap <rdunlap@infradead.org>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, Keith Busch <kbusch@kernel.org>,
kw@linux.com,
"linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>
Subject: Re: [PATCH v2] Add support for PCIe SSD status LED management
Date: Fri, 4 Jun 2021 15:13:46 -0500 [thread overview]
Message-ID: <09f731e7-40c8-9087-adb1-e996e08871a2@gmail.com> (raw)
In-Reply-To: <20210602224059.GA19673@duo.ucw.cz>
On 6/2/2021 5:40 PM, Pavel Machek wrote:
> Hi!
>
>>>> 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.
>
>>
>> Yes... I had originally submitted this without using the LED subsystem, and
>> Greg K-H said I had to (see
>> https://www.spinics.net/lists/linux-pci/msg102013.html). Here's the
>> relevant bit.
>
> ...
>
>> So I'm not sure what to do.
>
> Explain to Greg that you don't know how particular system implements
> the indication. It can be small display, 2 LEDs, 3 LEDs, etc... LED
> subsystem expects direct LED control.
>
> Best regards,
> Pavel
>
Is there any chance you would accept drive status LEDs using the LED
subsystem, if the driver was rewritten to represent each possible drive
state as a single led_classdev device, each with no special attributes,
just brightness of 0 or 1?
The drive states are logically each displaying one bit of information to
the user, and each could actually be physically represented by a
physical LED (though probably not). I only combined them into a single
led_classdev device with an extra attribute because I thought it made
more sense to do so, but it sounds like I might have made a bad choice
in doing so.
In support of using the LED subsystem for this--there are other LEDs in
linux which aren't necessarily always represented by single physical
LEDs, such as "caps lock" for keyboards. Virtual consoles display
keyboard indicators as text in a window rather than an LED--and someone
_could_ make a weird keyboard that displays the 3 bits of
caps/scroll/number lock info as a decimal number on a display. My point
is that it seems to me that the LED subsystem is already being used for
things that logically display a bit of information to the user, which
may or may not be represented by a single LED. (Ethernet port LEDs are
another possible example--they might combine link present, link speed,
and activity all on one LED, by using different colors and flashing
patterns.)
next prev parent reply other threads:[~2021-06-04 20:13 UTC|newest]
Thread overview: 13+ 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
2021-06-02 22:40 ` Pavel Machek
2021-06-04 20:13 ` stuart hayes [this message]
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-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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=09f731e7-40c8-9087-adb1-e996e08871a2@gmail.com \
--to=stuart.w.hayes@gmail.com \
--cc=bhelgaas@google.com \
--cc=kbusch@kernel.org \
--cc=kw@linux.com \
--cc=linux-leds@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=rdunlap@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.