On Thu, Jun 20, 2024 at 04:19:48PM +0200, Greg KH wrote:
> On Wed, Jun 19, 2024 at 01:39:47AM +0200, Danilo Krummrich wrote:
> > In a subsequent patch we introduce the `Registration` abstraction used
> > to register driver structures. Some subsystems require the module name on
> > driver registration (e.g. PCI in __pci_register_driver()), hence pass
> > the module name to `Module::init`.
>
> I understand the need/want here, but it feels odd that you have to
> change anything to do it.
>
> >
> > Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> > ---
> > rust/kernel/lib.rs | 14 ++++++++++----
> > rust/kernel/net/phy.rs | 2 +-
> > rust/macros/module.rs | 3 ++-
> > samples/rust/rust_minimal.rs | 2 +-
> > samples/rust/rust_print.rs | 2 +-
> > 5 files changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index a791702b4fee..5af00e072a58 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -71,7 +71,7 @@ pub trait Module: Sized + Sync + Send {
> > /// should do.
> > ///
> > /// Equivalent to the `module_init` macro in the C API.
> > - fn init(module: &'static ThisModule) -> error::Result<Self>;
> > + fn init(name: &'static str::CStr, module: &'static ThisModule) -> error::Result<Self>;
>
> Why can't the name come directly from the build system? Why must it be
> passed into the init function of the module "class"? What is it going
> to do with it?
The name does come from the build system, that's where `Module::init` gets it
from.
>
> A PCI, or other bus, driver "knows" it's name already by virtue of the
> build system, so it can pass that string into whatever function needs
Let's take pci_register_driver() as example.
```
#define pci_register_driver(driver) \
__pci_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
```
In C drivers this works because (1) it's a macro and (2) it's called directly
from the driver code.
In Rust, for very good reasons, we have abstractions for C APIs, hence the
actual call to __pci_register_driver() does not come from code within the
module, but from the PCI Rust abstraction `Module::init` calls into instead.
Consequently, we have to pass things through. This also isn't new, please note
that the current code already does the same thing: `Module::init` (without this
patch) is already declared as
`fn init(module: &'static ThisModule) -> error::Result<Self>`
passing through `ThisModule` for the exact same reason.
Please also note that in the most common case (one driver per module) we don't
see any of this anyway.
Just like the C macro module_pci_driver(), Rust drivers can use the
`module_pci_driver!` macro.
Example from Nova:
```
kernel::module_pci_driver! {
// The driver type that implements the corresponding probe() and
// remove() driver callbacks.
type: NovaDriver,
name: "Nova",
author: "Danilo Krummrich",
description: "Nova GPU driver",
license: "GPL v2",
}
```
> that, but the module init function itself does NOT need that.
>
> So I fail to understand why we need to burden ALL module init functions
> with this, when only a very very very tiny subset of all drivers will
> ever need to know this, and even then, they don't need to know it at
> init module time, they know it at build time and it will be a static
> string at that point, it will not be coming in through an init call.
>
> thanks,
>
> greg k-h
>
