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

[Lukas Wunner <lukas@wunner.de>]

On Tue, Jun 18, 2024 at 10:56:53AM +0200, Mariusz Tkaczyk wrote:
> On Sat, 15 Jun 2024 12:33:45 +0200 Lukas Wunner <lukas@wunner.de> wrote:
>> > On Fri, Jun 14, 2024 at 04:06:14PM -0500, stuart hayes wrote:
> > > Failing pci_npem_init() if this ops->get_active_indications() fails
> > > will keep this from working on most (all?) Dell servers, because the
> > > _DSM get/set functions use an IPMI operation region to get/set the
> > > active LEDs, and this is getting run before the IPMI drivers and
> > > acpi_ipmi module (which provides ACPI access to IPMI operation
> > > regions) get loaded.  (GET_SUPPORTED_STATES works without IPMI.)  
> > 
> > CONFIG_ACPI_IPMI is tristate.  Even if it's built-in, the
> > module_initcall() becomes a device_initcall().
> > 
> > PCI enumeration happens from a subsys_initcall(), way earlier
> > than device_initcall().
> > 
> > If you set CONFIG_ACPI_IPMI=y and change the module_initcall() in
> > drivers/acpi/acpi_ipmi.c to arch_initcall(), does the issue go away?
> 
> That seems to be the best option. Please test Lukas proposal and let me know.
> Shouldn't I make a dependency to ACPI_IPMI in Kconfig (with optional comment
> about initcall)?
> 
> +config PCI_NPEM
> +	bool "Native PCIe Enclosure Management"
> +	depends on LEDS_CLASS=y
> +	depends on ACPI_IPMI=y

This would effectively disallow NPEM on non-ACPI systems.

I think what you want instead is to allow either ACPI_IPMI=y or
ACPI_IPMI=n, but not ACPI_IPMI=m, so:

	depends on ACPI_IPMI!=m

Thanks,

Lukas