Re: [PATCH] PCI: Introduce Cray ClusterStor E1000 NVMe slot LED driver

[Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>]

On Tue, 3 Sep 2024 17:18:20 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Tue, Aug 27, 2024 at 02:03:48PM -0700, Tony Hutter wrote:
> > Add driver to control the NVMe slot LEDs on the Cray ClusterStor E1000.
> > The driver provides hotplug attention status callbacks for the 24 NVMe
> > slots on the E1000.  This allows users to access the E1000's locate and
> > fault LEDs via the normal /sys/bus/pci/slots/<slot>/attention sysfs
> > entries.  This driver uses IPMI to communicate with the E1000 controller to
> > toggle the LEDs.  
> 
> I hope/assume the interface is the same as one of the others, i.e.,
> the existing one added for NVMe behind VMD by
> https://git.kernel.org/linus/576243b3f9ea ("PCI: pciehp: Allow
> exclusive userspace control of indicators") or the new one for NPEM
> and the _DSM at
> https://lore.kernel.org/linux-pci/20240814122900.13525-3-mariusz.tkaczyk@linux.intel.com/
> 
> I suppose we intend that the ledmon utility will be able to drive
> these LEDs?  Whatever the user, we should try to minimize the number
> of different interfaces for this functionality.

Ledmon won't support it, at least not in current form. Ledmon support for pciehp
attention is limited to VMD, i.e. first we must find VMD driver then we are
looking for slot/attention.
I'm not familiar with any attempt to add support for this in ledmon.

From the end user perspective, I don't like pciehp/attention because we are
refereeing to pciehp driver not pcieport and to determine proper slot we
need to do additional matching by slot/address. I would be simpler.
https://github.com/intel/ledmon/blob/main/src/lib/vmdssd.c#L100

Thanks,
Mariusz