Re: [PATCH v5 01/16] rust: pass module name to `Module::init`

Greg KH
2024-12-11T14:30:55Z

On Wed, Dec 11, 2024 at 03:21:53PM +0100, Danilo Krummrich wrote:
> > Really? You can't have something in a required "register()" type function?
> > Something for when the driver "instance" is created as part of
> > pci::Driver? You do that today in your sample driver for the id table:
> > const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
> >
> > Something else called DRIVER_NAME that you could then set:
> > const DRIVER_NAME: env!(KBUILD_MODNAME);
>
> Sure, that's possible. But that means that the driver has to set it explicitly
> -- even when e.g. module_pci_driver! is used.
>
> In C you don't have that, because there it's implicit within the
> pci_register_driver() macro. (Regardless of whether it's a single module for a
> single driver or multiple drivers per module.)
>
> Anyways, like I mentioned, given that we have `env!(KBUILD_MODNAME)` (which we
> still need to add), there are other options to make it work similarly, e.g. add
> a parameter to `pci::Adapter` and bake this into `module_pci_driver!`.

Ok, I'm all for that, just don't modify the module rust functions for it :)

> For this particular option, it would mean that for modules registering multiple
> drivers a corresponding name would need to be passed explicitly.

True.

> > Also, I think you will want this for when a single module registers
> > multiple drivers which I think can happen at times, so you could
> > manually override the DRIVER_NAME field.
>
> My proposal above would provide this option, but do we care? In C no one ever
> changes the name. There is zero users of __pci_register_driver(), everyone uses
> pci_register_driver() where the name is just KBUILD_MODNAME. Same for
> __platform_driver_register().

You are right. But I see other drivers messing with that field, oh no,
wait, that's just the EDAC layer doing really odd things (a known issue
as that layer does very many odd things.)

So nevermind, multiple drivers per module isn't going to be an issue, if
you all can move it to a macro (best yet, like what Alice pointed out),
that would be fine with me.

thanks,

greg k-h