Re: [PATCH v7 0/3] PCIe Enclosure LED Management

[Bjorn Helgaas <helgaas@kernel.org>]

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
>