| Message ID | 20240904104848.23480-1-mariusz.tkaczyk@linux.intel.com |
|---|---|
| Headers | show |
| Series | PCIe Enclosure LED Management | expand |
Device Specific Method PCIe SSD Status LED Management (_DSM) defined in
PCI Firmware Spec r3.3 sec 4.7 provides a way to manage LEDs via ACPI.
The design is similar to NPEM defined in PCIe Base Specification r6.1
sec 6.28:
- both standards are indication oriented,
- _DSM supported bits are corresponding to NPEM capability
register bits
- _DSM control bits are corresponding to NPEM control register
bits.
_DSM does not support enclosure specific indications and special NPEM
commands NPEM_ENABLE and NPEM_RESET.
_DSM is implemented as a second op in NPEM driver. The standard used
in background is logged with info priority. The interface is accessed
same as NPEM.
According to spec, _DSM has higher priority and availability of _DSM
in not limited to devices with NPEM support. It is followed in
implementation.
DELL implementation of DSM is using acpi ipmi, which is not available
imediatelly (in fact it tooks up to 10s for this interface
to be avialable). It can determine if DSM is supported
(GET_SUPPORTED_STATES_DSM is working) but it cannot serve GET_STATE_DSM
or SET_STATE_DSM commands in this time.
On Wed, Sep 04, 2024 at 12:48:45PM +0200, Mariusz Tkaczyk wrote: > Patchset is named as PCIe Enclosure LED Management because it adds two > features: > - Native PCIe Enclosure Management (NPEM) > - PCIe SSD Status LED Management (DSM) > > Both are pattern oriented standards, they tell which "indication" > should blink. It doesn't control physical LED or pattern visualization. > > Overall, driver is simple but it was not simple to fit it into interfaces > we have in kernel (We considered leds and enclosure interfaces). It reuses > leds interface, this approach seems to be the best because: > - leds are actively maintained, no new interface added. > - leds do not require any extensions, enclosure needs to be adjusted first. > > There are trade-offs: > - "brightness" is the name of sysfs file to control led. It is not > natural to use brightness to set patterns, that is why multiple led > devices are created (one per indication); > - Update of one led may affect other leds, led triggers may not work > as expected. > > Changes from v1: > - Renamed "pattern" to indication. > - DSM support added. > - Fixed nits reported by Bjorn. > > Changes from v2: > - Introduce lazy loading to allow DELL _DSM quirks to work, reported by > Stuart. > - leds class initcall moved up in Makefile, proposed by Dan. > - fix other nits reported by Dan and Iipo. > > Changes from v3: > - Remove unnecessary packed attr. > - Fix doc issue reported by lkp. > - Fix read_poll_timeout() error handling reported by Iipo. > - Minor fixes reported by Christoph. > > Changes from v4: > - Use 0 / 1 instead of LED_OFF/LED_ON, suggested by Marek. > - Documentation added, suggested by Bjorn. > > Change from v5: > - Remove unnecessary _packed, reported by Christoph. > - Changed "led" to "LED" and other typos suggested by Randy. > > Change from v6: > - Removed links, suggested by Bjorn. > - npem->active_inds_initialized:1 moved to DSM commit, suggested by Bjorn. > - Improve justification for active_inds_initialized, suggested by Bjorn. > - Chosen backed logging added, suggested by Bjorn. > > Suggested-by: Lukas Wunner <lukas@wunner.de> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > Tested-by: Stuart Hayes <stuart.w.hayes@gmail.com> Very nice. Applied to pci/npem for v6.12, thank you! I noticed that b4 didn't pick up Stuart's Tested-by from the cover letter. I assume it covers the whole series, so I added it to each patch. Let me know if that's not what you intended. > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Ilpo Jarvinen <ilpo.jarvinen@linux.intel.com> > Cc: Lukas Wunner <lukas@wunner.de> > Cc: Lee Jones <lee@kernel.org> > Cc: Keith Busch <kbusch@kernel.org> > Cc: Marek Behun <marek.behun@nic.cz> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Stuart Hayes <stuart.w.hayes@gmail.com> > Link: https://lore.kernel.org/linux-pci/20240814122900.13525-1-mariusz.tkaczyk@linux.intel.com/ > > Mariusz Tkaczyk (3): > leds: Init leds class earlier > PCI/NPEM: Add Native PCIe Enclosure Management support > PCI/NPEM: Add _DSM PCIe SSD status LED management > > Documentation/ABI/testing/sysfs-bus-pci | 72 +++ > drivers/Makefile | 4 +- > drivers/pci/Kconfig | 9 + > drivers/pci/Makefile | 1 + > drivers/pci/npem.c | 597 ++++++++++++++++++++++++ > drivers/pci/pci.h | 8 + > drivers/pci/probe.c | 2 + > drivers/pci/remove.c | 2 + > include/linux/pci.h | 3 + > include/uapi/linux/pci_regs.h | 35 ++ > 10 files changed, 732 insertions(+), 1 deletion(-) > create mode 100644 drivers/pci/npem.c > > -- > 2.35.3 >
On Wed, Sep 04, 2024 at 05:27:32PM -0500, Bjorn Helgaas wrote: > On Wed, Sep 04, 2024 at 12:48:45PM +0200, Mariusz Tkaczyk wrote: > I noticed that b4 didn't pick up Stuart's Tested-by from the cover > letter. I assume it covers the whole series, so I added it to each > patch. Let me know if that's not what you intended. You forgot to add -t IIRC which does spread the tags against cover letter to all of the patches.
Hello, Very nice series! [...] > +static const struct npem_ops dsm_ops = { > + .get_active_indications = dsm_get_active_indications, > + .set_active_indications = dsm_set_active_indications, > + .name = "_DSM PCIe SSD Status LED Management", A small nit: extra space. Bjorn, we can mend this on the branch, if are OK with that. Krzysztof