From: Pavel Machek <pavel@ucw.cz>
To: Randy Dunlap <rdunlap@infradead.org>
Cc: Stuart Hayes <stuart.w.hayes@gmail.com>,
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: Wed, 2 Jun 2021 00:38:12 +0200 [thread overview]
Message-ID: <20210601223812.GA5128@amd> (raw)
In-Reply-To: <3d1272b8-4edc-f2b1-85ea-f5cea65b4871@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]
Hi!
> > # echo "ok locate" >/sys/class/leds/0000:88:00.0::drive_status/states
> > # cat /sys/class/leds/0000:88:00.0::drive_status/states
This has two problems: ":" already has special meaning in LED name,
and we'd prefer not to have new "states" interface unless absolutely
neccessary.
> > [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?
> > +Date: April 2021
> > +Contact: linux-pci@vger.kernel.org
> > +Description:
> > + This attribute indicates the status states supported by a drive
> > + or drive slot's LEDs, as defined in the "_DSM additions for PCIe
> > + SSD Status LED Management" ECN to the PCI Firmware Specification
> > + Revision 3.2, dated 12 February 2020, and in "Native PCIe
> > + Enclosure Management", section 6.29 of the PCIe Base Spec 5.0.
> > +
> > + Only supported states will be shown, and the currently active
> > + states are shown in brackets. The active state(s) can be written
> > + by echoing a space or comma separated string of states to this
> > + attribute. For example:
> > +
> > + # echo "ok locate" >/sys/class/leds/0000:88:00.0::drive_status/states
> > + # cat /sys/class/leds/0000:88:00.0::drive_status/states
> > + [ok] [locate] failed rebuild pfa hotspare ica ifa
This goes against "one value per file", really needs better
description, and probably needs different interface.
> > +config PCIE_SSD_LEDS
> > + tristate "PCIe SSD status LED support"
> > + depends on ACPI && NEW_LEDS
>
> I expect that should be LEDS_CLASS instead of NEW_LEDS.
> Did you test it with NEW_LEDS=y and LEDS_CLASS not set?
>
>
> [adding Pavel and linux-leds m.l. for other review]
Thank you!
Pavel
--
http://www.livejournal.com/~pavelmachek
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
next prev parent reply other threads:[~2021-06-01 22:38 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 [this message]
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
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:
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=20210601223812.GA5128@amd \
--to=pavel@ucw.cz \
--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=rdunlap@infradead.org \
--cc=stuart.w.hayes@gmail.com \
/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 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).