On 2017-03-09 01:32, Paul Gortmaker wrote: *snip* *snip* > If the mux subsystem isn't board specific (and at a glance, it seems > like it is not) then it might be the former and not the latter. But I > leave that call up to you
The mux subsystem may be needed for some boards and not for others, and boards not needing it could benefit from the de-bloating. So, a modular core is probably desired, methinks... >>> Oh, and speaking of EXPORT_SYMBOL, I should have added <linux/export.h> >>> to your file with the patch I sent, since it does do exports. >> >> Ok, I assume that it should be included by the individual drivers as well? > > Not really -- drivers, as leaf nodes in the overall system rarely do any > EXPORT_SYMBOL operations. It is the exporters and not the consumers who > should explicitly include that header. Yes, of course. I was confused for a bit. Possibly because at some previous revisions the mux core did depend on one of the drivers (mux-gpio) and that driver did export a hook function that the core used. That never felt clean and in the end it was dropped. At that time, the mux-gpio driver was also made non-modular to prevent the circular dependency. I suppose the same thing could have been accomplished with a third "library" module that both the core and the driver could have accessed... > Per the above use case (minimal vmlinux) the unload isn't so important, > and in many cases it may be racy or simply not make sense. If that is > the case, you don't need to provide a module_exit, and in the past I > think we used to bump the module use count at a successful load to > prevent unloading ; there might be a more elegant method now; google is > your friend here. Also, I don't think name reuse/confusion is an issue. Ok, I thought it all boiled down to making the mux-core Kconfig a tristate option and changing subsys_initcall(...) to module_init(...). But if I do that, I cannot be sure that the mux-core has been initialized before drivers and clients start to use it in the non-modular case (if things are modules, the dependencies should ensure that the mux-core is loaded/initialized before any users are loaded). But there is no topological sorting going on for ordering init calls in the non-modular case. In short, I get: WARNING: CPU: 0 PID: 1 at drivers/base/class.c:438: class_find_device+0xac/0xb8 class_find_device called for class 'mux' before it was initialized CPU:0 PID: 1 Comm: swapper Not tainted 4.11-rc1+ #1243 Hardware name: Atmel SAMA5 [<c010d24c>] (unwind_backtrace) from [<c010b1ec>] (show_stack+0x10/0x14) [<c010b1ec>] (show_stack) from [<c0115290>] (__warn+0xe0/0xf8) [<c0115290>] (__warn) from [<c01152e0>] (warn_slowpath_fmt+0x38/0x48) [<c01152e0>] (warn_slowpath_fmt) from [<c034715c>] (class_find_device+0xac/0xb8) [<c034715c>] (class_find_device) from [<c0434230>] (mux_control_get+0xa0/0x1bc) [<c0434230>] (mux_control_get) from [<c0434394>] (devm_mux_control_get+0x3c/0x78) [<c0434394>] (devm_mux_control_get) from [<c04060f0>] (i2c_mux_probe+0x44/0x294) [<c04060f0>] (i2c_mux_probe) from [<c03475ac>] (platform_drv_probe+0x4c/0xb0) [<c03475ac>] (platform_drv_probe) from [<c0345e30>] (driver_probe_device+0x26c/0x464) [<c0345e30>] (driver_probe_device) from [<c0346128>] (__driver_attach+0x100/0x11c) [<c0346128>] (__driver_attach) from [<c034403c>] (bus_for_each_dev+0x6c/0xa0) [<c034403c>] (bus_for_each_dev) from [<c0344a94>] (bus_add_driver+0x1c8/0x260) [<c0344a94>] (bus_add_driver) from [<c0346a7c>] (driver_register+0x78/0xf8) [<c0346a7c>] (driver_register) from [<c0800d50>] (do_one_initcall+0xb0/0x15c) [<c0800d50>] (do_one_initcall) from [<c0800f38>] (kernel_init_freeable+0x13c/0x1d8) [<c0800f38>] (kernel_init_freeable) from [<c057ab60>] (kernel_init+0x8/0x10c) [<c057ab60>] (kernel_init) from [<c0107fb8>] (ret_from_fork+0x14/0x3c) ---[ end trace d97274a16af7ef1c ]--- So, it appears that I also have to determine if the core has been initialized in all its entry points and return -EPROBE_DEFER (or something) when not. I suppose I could instead initialize on-demand, but that seems more difficult the do without races... Am I missing something? >> Hmmm, there's also the ".owner = THIS_MODULE" line in mux-core.c that >> seems related, is that line valid for non-modular "units"? > > That is largely a legacy bit of (repeatedly copied) boiler plate. If I > recall correctly, in a non-module it becomes ".owner = NULL". And even > if you are a proper module, I think the higher level wrappers like > module_platform_driver() do the right thing with .owner so that drivers > don't have to duplicate such mundane boilerplate. If you search git > history for THIS_MODULE, I think you will find batch removals from > drivers for where the per-driver boilerplate is simply redundant. Ah, but in this case it's the owner of the mux class, not the mux-core itself. Cheers, peda
