Hi Petr, Thanks for the feedback!
On Fri, May 22, 2026 at 03:06:04PM +0200, Petr Pavlu wrote: > On 5/18/26 12:19 PM, Shashank Balaji wrote: > > Commit "driver core: platform: set mod_name in driver registration" will set > > struct device_driver's mod_name member for platform driver registration. > > For a > > driver to be registered with its mod_name set, module_kset needs to be > > initialized, which currently happens in a subsys_initcall in > > param_sysfs_init(). > > The tegra cbb drivers register themselves before module_kset init, in a > > core_initcall. This works currently because > > lookup_or_create_module_kobject(), > > which dereferences module_kset via kset_find_obj(), is not called if > > mod_name > > is not set, which is the case now. > > > > So in preparation for the commit "driver core: platform: set mod_name in > > driver registration", > > move module_kset init to pure_initcall level, ensuring it happens before > > tegra > > cbb driver registration. > > > > Suggested-by: Gary Guo <[email protected]> > > Co-developed-by: Rahul Bukte <[email protected]> > > Signed-off-by: Rahul Bukte <[email protected]> > > Signed-off-by: Shashank Balaji <[email protected]> > > --- > > Patch 4 depends on this patch > > --- > > kernel/params.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/params.c b/kernel/params.c > > index 74d620bc2521..ac088d4b09a9 100644 > > --- a/kernel/params.c > > +++ b/kernel/params.c > > @@ -957,7 +957,7 @@ static int __init param_sysfs_init(void) > > > > return 0; > > } > > -subsys_initcall(param_sysfs_init); > > +pure_initcall(param_sysfs_init); > > > > /* > > * param_sysfs_builtin_init - add sysfs version and parameter > > > > The change looks ok to me functionality-wise. Sysfs is initialized > earlier in do_basic_setup() and other code, such as classes_init(), > calls kset_create_and_add() similarly early. > > One minor issue is that pure_initcall() was originally intended for > static variable initialization. The file include/linux/init.h says: > > | /* > | * A "pure" initcall has no dependencies on anything else, and purely > | * initializes variables that couldn't be statically initialized. > | * > | * This only exists for built-in code, not for modules. > | * Keep main.c:initcall_level_names[] in sync. > | */ > | #define pure_initcall(fn) __define_initcall(fn, 0) > > The patch stretches the intended use of pure_initcall() somewhat in this > regard. However, other code already appears to do the same, so I guess > this is ok. Ah yeah, I thought of this too, but it seems like everyone else is doing it. Linus introduced pure_initcall in b3438f8266cb ("Add "pure_initcall" for static variable initialization") with the comment, and he introduced another user of it in 140d0b2108fa ("Do 'shm_init_ns()' in an early pure_initcall") which already stretches the intended use as per the comment. Given that it's just being used for "run me before core_initcall; early_initcall is too early for me" without any "pureness" requirements, I suppose the comment is due for a revision? > Additionally, I think it would be good to update the comment preceding > param_sysfs_init(). It currently says: > > | /* > | * param_sysfs_init - create "module" kset > | * > | * This must be done before the initramfs is unpacked and > | * request_module() thus becomes possible, because otherwise the > | * module load would fail in mod_sysfs_init. > | */ > > I suggest changing it to something like follows: > > This must be done before any driver registration so that when a driver comes > from a built-in module, the driver core can add the module under /sys/module > and create the associated driver symlinks. Thanks for catching this! I'll add it in the next revision. Thanks, Shashank
