| Message ID | cover.1568239378.git.amit.kucheria@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Cleanup arm64 driver dependencies | expand |
On Thu, Sep 12, 2019 at 12:18 AM Amit Kucheria <amit.kucheria@linaro.org> wrote: > > I was using initcall_debugging on a QCOM platform and ran across a bunch of > driver initcalls that are enabled even if their SoC support is disabled. > > Here are some fixups for a subset of them. The idea seems reasonable, disabling a platform may just turn off all the drivers that are not useful elsewhere, but there are mistakes in a lot of your changes, so I'm certainly not applying these for 5.4. Generally speaking, the way that works best is config SUBSYS_DRIVER_FOO tristate "SUBSYS support for FOO platform" depends on ARCH_FOO || COMPILE_TEST depends on SUBSYS default "m" if ARCH_FOO This means it's enabled as a loadable module by default (use default "y" instead where necessary) as long as the platform is enabled, but an x86 allmodconfig build also includes it because of COMPILE_TEST, while any configuration without ARCH_FOO that is not compile-testing cannot enable it. Arnd
On Thu, Sep 12, 2019 at 03:48:44AM +0530, Amit Kucheria wrote: > I was using initcall_debugging on a QCOM platform and ran across a bunch of > driver initcalls that are enabled even if their SoC support is disabled. What exactly is the problem you're trying to fix here? For the drivers I looked at these were bog standard register the driver with the subsystem type initcalls on optional drivers so not doing anything particularly disruptive or anything like that. For any given system that's going to be an issue for the overwhelming majority of drivers on the tree, including those that aren't associated with any particular architecture.
Hi Arnd, On Thu, Sep 12, 2019 at 2:59 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Sep 12, 2019 at 12:18 AM Amit Kucheria <amit.kucheria@linaro.org> wrote: > > > > I was using initcall_debugging on a QCOM platform and ran across a bunch of > > driver initcalls that are enabled even if their SoC support is disabled. > > > > Here are some fixups for a subset of them. > > The idea seems reasonable, disabling a platform may just turn off > all the drivers that are not useful elsewhere, but there are mistakes > in a lot of your changes, so I'm certainly not applying these for 5.4. OK, thanks for confirming that you have no objections to such changes, per-se. I'll spend some more time ensuring COMPILE_TEST coverage for these cleanups. I only focused on quickly cleaning up my initcall_debug output for now. > Generally speaking, the way that works best is > > config SUBSYS_DRIVER_FOO > tristate "SUBSYS support for FOO platform" > depends on ARCH_FOO || COMPILE_TEST > depends on SUBSYS > default "m" if ARCH_FOO > > This means it's enabled as a loadable module by default (use > default "y" instead where necessary) as long as the platform > is enabled, but an x86 allmodconfig build also includes it > because of COMPILE_TEST, while any configuration without > ARCH_FOO that is not compile-testing cannot enable it. How would you like to handle defconfigs which list a driver explicitly? Should we add ARCH_FOO to those defconfigs or remove DRIVER_FOO from them? Regards, Amit
On Thu, Sep 12, 2019 at 11:29:00AM +0200, Arnd Bergmann wrote: > Generally speaking, the way that works best is > config SUBSYS_DRIVER_FOO > tristate "SUBSYS support for FOO platform" > depends on ARCH_FOO || COMPILE_TEST > depends on SUBSYS > default "m" if ARCH_FOO > This means it's enabled as a loadable module by default (use > default "y" instead where necessary) as long as the platform > is enabled, but an x86 allmodconfig build also includes it > because of COMPILE_TEST, while any configuration without > ARCH_FOO that is not compile-testing cannot enable it. Indeed, though we shouldn't be adding any default m/y to things that don't already default on.
On Thu, Sep 12, 2019 at 3:17 PM Mark Brown <broonie@kernel.org> wrote: > > On Thu, Sep 12, 2019 at 03:48:44AM +0530, Amit Kucheria wrote: > > > I was using initcall_debugging on a QCOM platform and ran across a bunch of > > driver initcalls that are enabled even if their SoC support is disabled. > > What exactly is the problem you're trying to fix here? For the > drivers I looked at these were bog standard register the driver > with the subsystem type initcalls on optional drivers so not > doing anything particularly disruptive or anything like that. I was trying to prune the defconfig only to drivers that make sense on the SoC. e.g. Why should I see a brcmstb_soc_device_early_init() call on a QCOM system when I've disabled ARCH_BRCMSTB? I came across this while trying to figure out how to make thermal and cpufreq frameworks initialise as early as possible. > For any given system that's going to be an issue for the > overwhelming majority of drivers on the tree, including those > that aren't associated with any particular architecture. Indeed. From a quick check, MFD and GPIO has a bunch of 'generic' drivers that aren't SoC-specific. I'm sure there are several such drivers in regulator framework too. They don't need to be 'fixed'. I was just trying to ring-fence obvious SoC-specific drivers behind a ARCH_FOO dependency since they seemed like low-hanging fruit. Let me know if it isn't a good use of everyone's time. Regards, Amit
On Thu, Sep 12, 2019 at 03:33:20PM +0530, Amit Kucheria wrote: > On Thu, Sep 12, 2019 at 3:17 PM Mark Brown <broonie@kernel.org> wrote: > > > I was using initcall_debugging on a QCOM platform and ran across a bunch of > > > driver initcalls that are enabled even if their SoC support is disabled. > > What exactly is the problem you're trying to fix here? For the > > drivers I looked at these were bog standard register the driver > > with the subsystem type initcalls on optional drivers so not > > doing anything particularly disruptive or anything like that. > I was trying to prune the defconfig only to drivers that make sense on > the SoC. e.g. Why should I see a brcmstb_soc_device_early_init() call > on a QCOM system when I've disabled ARCH_BRCMSTB? So this is really just the standard make Kconfig easier to use by filtering out noise thing. It'd be clearer if you said that in the changelog, and like the review comments have been saying you need to leave in an || COMPILE_TEST in there otherwise it's actively harmful. > I came across this while trying to figure out how to make thermal and > cpufreq frameworks initialise as early as possible. AFAICT you'd also have been happy if you just built these drivers modular?