[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240814202751.GA359905@bhelgaas>
Date: Wed, 14 Aug 2024 15:27:51 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Mariusz Tkaczyk <mariusz.tkaczyk@...ux.intel.com>
Cc: linux-pci@...r.kernel.org, Lukas Wunner <lukas@...ner.de>,
Christoph Hellwig <hch@....de>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Stuart Hayes <stuart.w.hayes@...il.com>,
Arnd Bergmann <arnd@...db.de>, Bjorn Helgaas <bhelgaas@...gle.com>,
Dan Williams <dan.j.williams@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Keith Busch <kbusch@...nel.org>, Marek Behun <marek.behun@....cz>,
Pavel Machek <pavel@....cz>, Randy Dunlap <rdunlap@...radead.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Lee Jones <lee@...nel.org>, linux-leds@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 0/3] PCIe Enclosure LED Management
[+cc Lee, linux-leds for comment on using the LED subsystem as
described in patch 2/3; would be nice to have a reviewed-by or ack for
this. Thread at
https://lore.kernel.org/r/20240814122900.13525-4-mariusz.tkaczyk@linux.intel.com]
On Wed, Aug 14, 2024 at 02:28:57PM +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.
v1 at https://lore.kernel.org/r/20240215142345.6073-1-mariusz.tkaczyk@linux.intel.com
> Changes from v1:
> - Renamed "pattern" to indication.
> - DSM support added.
> - fixed nits reported by Bjorn.
v2 at https://lore.kernel.org/r/20240528131940.16924-1-mariusz.tkaczyk@linux.intel.com
> 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.
v3 at https://lore.kernel.org/r/20240705125436.26057-1-mariusz.tkaczyk@linux.intel.com
> 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.
v4 at https://lore.kernel.org/r/20240711083009.5580-1-mariusz.tkaczyk@linux.intel.com
> Changes from v4:
> - Use 0 / 1 instead of LED_OFF/LED_ON, suggested by Marek.
> - Documentation added, suggested by Bjorn.
v5 at https://lore.kernel.org/r/20240813113024.17938-1-mariusz.tkaczyk@linux.intel.com
> Change from v5:
> - Remove unnecessary _packed, reported by Christoph.
> - Changed "led" to "LED" and other typos suggested by Randy.
>
> Suggested-by: Lukas Wunner <lukas@...ner.de>
> Reviewed-by: Christoph Hellwig <hch@....de>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> Tested-by: Stuart Hayes <stuart.w.hayes@...il.com>
Evidently you intend these tags to be applied to each patch, but b4
doesn't distribute tags from the cover letter across each individual
patch.
You included Christoph's Reviewed-by directly in patches 1 and 2, but
not Ilpo's. I didn't dig through the previous postings to verify all
this.
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: Dan Williams <dan.j.williams@...el.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Ilpo Jarvinen <ilpo.jarvinen@...ux.intel.com>
> Cc: Lukas Wunner <lukas@...ner.de>
> Cc: Keith Busch <kbusch@...nel.org>
> Cc: Marek Behun <marek.behun@....cz>
> Cc: Pavel Machek <pavel@....cz>
> Cc: Randy Dunlap <rdunlap@...radead.org>
> Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Cc: Stuart Hayes <stuart.w.hayes@...il.com>
> Link: https://lore.kernel.org/linux-pci/20240813113024.17938-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 | 590 ++++++++++++++++++++++++
> 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, 725 insertions(+), 1 deletion(-)
> create mode 100644 drivers/pci/npem.c
>
> --
> 2.35.3
>
Powered by blists - more mailing lists