Re: [PATCH v2] Add support for PCIe SSD status LED management

[Randy Dunlap <rdunlap@infradead.org>]

Hi,

On 6/1/21 1:38 PM, Stuart Hayes wrote:
> This patch adds support for the PCIe SSD Status LED Management
> interface, as described in the "_DSM Additions for PCIe SSD Status LED
> Management" ECN to the PCI Firmware Specification revision 3.2.
> 
> It will add a single (led_classdev) LED for any PCIe device that has the
> relevant _DSM--presumably only drives or drive slots will have this. The
> available and active status states are exposed using attribute "states"
> under the LED device. Reading this attribute will show the states supported
> by the interface, and those states which are currently active are shown
> in brackets, like this:
> 
>  # 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 invalid disabled
> 
> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> ---
> V2:
> 	* Simplified interface to a single "states" attribute under the LED
> 	  classdev using only state names
> 	* Reworked driver to separate _DSM specific code, so support for
> 	  NPEM (or other methods) could be easily be added
> 	* Use BIT macro
> 
>  .../sysfs-class-led-driver-pcie-ssd-leds      |  18 +
>  drivers/pci/Kconfig                           |  12 +
>  drivers/pci/Makefile                          |   1 +
>  drivers/pci/pcie-ssd-leds.c                   | 457 ++++++++++++++++++
>  4 files changed, 488 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-pcie-ssd-leds
>  create mode 100644 drivers/pci/pcie-ssd-leds.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-pcie-ssd-leds b/Documentation/ABI/testing/sysfs-class-led-driver-pcie-ssd-leds
> new file mode 100644
> index 000000000000..1f07733b6f35
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-led-driver-pcie-ssd-leds
> @@ -0,0 +1,18 @@
> +What:		/sys/class/leds/<led>/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 invalid disabled
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 0c473d75e625..f4acf1ad0fb5 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -190,6 +190,18 @@ config PCI_HYPERV
>  	  The PCI device frontend driver allows the kernel to import arbitrary
>  	  PCI devices from a PCI backend to support PCI driver domains.
>  
> +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]

> +	help
> +	  Driver for PCIe SSD status LED management as described in a PCI
> +	  Firmware Specification, Revision 3.2 ECN.
> +
> +	  When enabled, an LED interface will be created for each PCIe device
> +	  that has the ACPI method described in the referenced specification,
> +	  to allow the device status LEDs for that PCIe device (presumably a
> +	  solid state storage device or its slot) to be seen and controlled.
> +
>  choice
>  	prompt "PCI Express hierarchy optimization setting"
>  	default PCIE_BUS_DEFAULT

thanks.
-- 
~Randy