Re: [PATCH v4 2/3] PCI/NPEM: Add Native PCIe Enclosure Management support

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

 



On Wed, Jul 31, 2024 at 01:51:17PM +0200, Mariusz Tkaczyk wrote:
> On Fri, 26 Jul 2024 09:29:36 +0200
> Marek Behún <kabel@xxxxxxxxxx> wrote:
> 
> > On Thu, Jul 11, 2024 at 10:30:08AM +0200, Mariusz Tkaczyk wrote:
> > > Native PCIe Enclosure Management (NPEM, PCIe r6.1 sec 6.28) allows
> > > managing LED in storage enclosures. NPEM is indication oriented
> > > and it does not give direct access to LED. Although each of
> > > the indications *could* represent an individual LED, multiple
> > > indications could also be represented as a single,
> > > multi-color LED or a single LED blinking in a specific interval.
> > > The specification leaves that open.  
> > 
> > The specification leaves it open, but isn't there a way to determine
> > how it is implemented? In ACPI, maybe?
> 
> 
> What would be a point of that? There are blinking patterns standards for 2-LED
> systems and 3-LED systems but NPEM is projected to not be limited to
> the led system you have. I mean that we shouldn't try to determine what hardware
> does - it belongs to hardware. Kernel task is just to read what NPEM registers
> are presenting and trust it.

Hi.

My point is about what a LED class device in kernel means, and what the brightness
attribute means (in terms of intended behavior).
One LED class device should represent one hardware LED (possibly multicolor), and
nothing else.
Setting the brightness attribute to the value of max_brightness should light the LED
on, and setting it to 0 should light it off.
So if on some device doing
  echo 1 >brightness
makes the LED for example blink, it is wrong.

That's why I am asking whether it is possible to determine what the hardware is
doing from some description, like ACPI or device-tree.

If setting brightness to 1 makes some LED blink (without a LED trigger), than
the device does not behave according to the LED class expectations.

> I can realize NPEM with separate LED for each indication. Who knows, maybe in
> the future it would become real.
> 
> > 
> > > Each enabled indication (capability register bit on) is represented as a
> > > ledclass_dev which can be controlled through sysfs. For every ledclass
> > > device only 2 brightness states are allowed: LED_ON (1) or LED_OFF (0).
> > > It is corresponding to NPEM control register (Indication bit on/off).
> > > 
> > > Ledclass devices appear in sysfs as child devices (subdirectory) of PCI
> > > device which has an NPEM Extended Capability and indication is enabled
> > > in NPEM capability register. For example, these are leds created for
> > > pcieport "10000:02:05.0" on my setup:
> > > 
> > > leds/
> > > ├── 10000:02:05.0:enclosure:fail
> > > ├── 10000:02:05.0:enclosure:locate
> > > ├── 10000:02:05.0:enclosure:ok
> > > └── 10000:02:05.0:enclosure:rebuild
> > > 
> > > They can be also found in "/sys/class/leds" directory. Parent PCIe device
> > > bdf is used to guarantee uniqueness across leds subsystem.
> > > 
> > > To enable/disable fail indication "brightness" file can be edited:
> > > echo 1 > ./leds/10000:02:05.0:enclosure:fail/brightness
> > > echo 0 > ./leds/10000:02:05.0:enclosure:fail/brightness  
> > 
> > Have you considered implemtening this via a led trigger?
> > 
> > Something like:
> >   echo pcie-enclosure > /sys/class/leds/<LED>/trigger
> >   echo 1 >/sys/class/leds/<LED>/fail
> > but properly thought up.
> > 
> 
> No I didn't. I understand the triggers as an actions that may involve led
> change we can configure. I thought, it should be cross driver reference (for
> example, change LED if keyboard capslock is pressed) and triggers are optional.
> 
> For that reasons I did not consider it. Please explain this concept in details.
> 
> I think that forcing one and only trigger you can use may we even worse because
> it seems to be definitely design incompatible (triggers are optional) but I'm
> not an expert.

Look for example at the netdev trigger. Originally it was software only, and you
could set it up so that it would for example blink on rx/tx activity of a network
interface.

But recently it gained support to offload this blinking to hardware: some LEDs
are wired from ethenret PHYs, and on those PHYs you can set it in hardware that
the LED will blink on rx/tx activity (or something else).

So now if the netdev trigger determines that the LED is connected to the PHY of
a network interface, and that network interface is set to be triggering the LED,
it will offload the blinking to hardware.

Marek




[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