On Fri, 5 Jul 2024, Mariusz Tkaczyk wrote: > Device Specific Method PCIe SSD Status LED Management (_DSM) defined in > PCI Firmware Spec r3.3 sec 4.7 provides a way to manage LEDs via ACPI. > > The design is similar to NPEM defined in PCIe Base Specification r6.1 > sec 6.28: > - both standards are indication oriented, > - _DSM supported bits are corresponding to NPEM capability > register bits > - _DSM control bits are corresponding to NPEM control register > bits. > > _DSM does not support enclosure specific indications and special NPEM > commands NPEM_ENABLE and NPEM_RESET. > > _DSM is implemented as a second op in NPEM driver. The standard used > in background is not presented to user. The interface is accessed same > as NPEM. > > According to spec, _DSM has higher priority and availability of _DSM > in not limited to devices with NPEM support. It is followed in > implementation. > > Link: https://members.pcisig.com/wg/PCI-SIG/document/14025 > Link: https://members.pcisig.com/wg/PCI-SIG/document/15350 > Suggested-by: Lukas Wunner <lukas@xxxxxxxxx> > Signed-off-by: Stuart Hayes <stuart.w.hayes@xxxxxxxxx> > Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@xxxxxxxxxxxxxxx> > --- > drivers/pci/npem.c | 147 ++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 144 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/npem.c b/drivers/pci/npem.c > index fd3366bc3fb0..e3bc28d089d3 100644 > --- a/drivers/pci/npem.c > +++ b/drivers/pci/npem.c > @@ -11,6 +11,14 @@ > * PCIe Base Specification r6.1 sec 6.28 > * PCIe Base Specification r6.1 sec 7.9.19 > * > + * _DSM Definitions for PCIe SSD Status LED > + * PCI Firmware Specification, r3.3 sec 4.7 > + * > + * Two backends are supported to manipulate indications: Direct NPEM register > + * access (npem_ops) and indirect access through the ACPI _DSM (dsm_ops). > + * _DSM is used if supported, else NPEM. > + * > + * Copyright (c) 2021-2022 Dell Inc. > * Copyright (c) 2023-2024 Intel Corporation > * Mariusz Tkaczyk <mariusz.tkaczyk@xxxxxxxxxxxxxxx> > */ > @@ -55,6 +63,21 @@ static const struct indication npem_indications[] = { > {0, NULL} > }; > > +/* _DSM PCIe SSD LED States are corresponding to NPEM register values */ > +static const struct indication dsm_indications[] = { > + {PCI_NPEM_IND_OK, "enclosure:ok"}, > + {PCI_NPEM_IND_LOCATE, "enclosure:locate"}, > + {PCI_NPEM_IND_FAIL, "enclosure:fail"}, > + {PCI_NPEM_IND_REBUILD, "enclosure:rebuild"}, > + {PCI_NPEM_IND_PFA, "enclosure:pfa"}, > + {PCI_NPEM_IND_HOTSPARE, "enclosure:hotspare"}, > + {PCI_NPEM_IND_ICA, "enclosure:ica"}, > + {PCI_NPEM_IND_IFA, "enclosure:ifa"}, > + {PCI_NPEM_IND_IDT, "enclosure:idt"}, > + {PCI_NPEM_IND_DISABLED, "enclosure:disabled"}, > + {0, NULL} > +}; > + > #define for_each_indication(ind, inds) \ > for (ind = inds; ind->bit; ind++) > > @@ -250,6 +273,120 @@ static bool npem_has_dsm(struct pci_dev *pdev) > BIT(GET_STATE_DSM) | BIT(SET_STATE_DSM)); > } > > +struct dsm_output { > + u16 status; > + u8 function_specific_err; > + u8 vendor_specific_err; > + u32 state; > +} __packed; As mentioned by Christoph, __packed is not required here due to natural alignment (Using __packed will cause other effects beyond just preventing compiler from adding padding which is why it's good to avoid it where it's not needed). With that removed, Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> -- i.