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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx
> > +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: signature.asc
Description: Digital signature


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux