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

[Lukas Wunner <lukas@wunner.de>]

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