linux-kernel - Re: [PATCH v6] Introduce Cray ClusterStor E1000 NVMe slot LED driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6198fe41-6f21-4bf1-8dac-c904c062ab83@llnl.gov>
Date: Mon, 8 Dec 2025 17:31:44 -0800
From: Tony Hutter <hutter2@...l.gov>
To: Lukas Wunner <lukas@...ner.de>
Cc: Bjorn Helgaas <helgaas@...nel.org>, corey@...yard.net,
        alok.a.tiwari@...cle.com, mariusz.tkaczyk@...ux.intel.com,
        minyard@....org, linux-pci@...r.kernel.org,
        openipmi-developer@...ts.sourceforge.net,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6] Introduce Cray ClusterStor E1000 NVMe slot LED driver

> You may want to consider making this driver modular and auto-load it
> using a MODULE_DEVICE_TABLE(dmi, ...) declaration. 

Thanks for the suggestion.  One of the issues of making it modular is that
craye1k has a dependency on IPMI.  So craye1k could get loaded first and
create the /sys/bus/pci/slots/<slot>/attention files, but none of those files
would be usable until IPMI loads.  If you force it to be built-in, then you
pretty much guarantee that if the 'attention' file exist, it's usable.

On 11/23/25 08:35, Lukas Wunner wrote:
> On Thu, Oct 30, 2025 at 10:13:57AM +0100, Lukas Wunner wrote:
>> On Wed, Oct 08, 2025 at 04:48:22PM -0700, Tony Hutter wrote:
>>> @@ -376,8 +383,16 @@ int __init pcie_hp_init(void)
>>>  
>>>  	retval = pcie_port_service_register(&hpdriver_portdrv);
>>>  	pr_debug("pcie_port_service_register = %d\n", retval);
>>> -	if (retval)
>>> +	if (retval) {
>>>  		pr_debug("Failure to register service\n");
>>> +		return retval;
>>> +	}
>>> +
>>> +#ifdef CONFIG_HOTPLUG_PCI_PCIE_CRAY_E1000
>>> +	retval = craye1k_init();
>>> +	if (retval)
>>> +		pr_debug("Failure to register Cray E1000 extensions");
>>> +#endif
>>
>> You also need to annotate craye1k_init() with __init.
> 
> Sorry, I've realized that I made the following note during review
> of your patch but forgot to include it in the reply above:
> 
> You may want to consider making this driver modular and auto-load it
> using a MODULE_DEVICE_TABLE(dmi, ...) declaration.  craye1k_init()
> would then become the module_init() call and you could tear down
> everything in a module_exit() call.  If you want to run craye1k_init()
> earlier if built-in, use e.g. an arch_initcall() instead of a
> module_init() call.  There's precedent in the tree for combinations
> of arch_initcall() + module_exit().
> 
> Thanks,
> 
> Lukas