Messages in this thread Patch in this message | | | From | Chunyan Zhang <> | Date | Fri, 10 Apr 2020 11:45:16 +0800 | Subject | Re: [PATCH] [RFC] clk: sprd: fix compile-testing |
| |
On Fri, 10 Apr 2020 at 04:17, Sandeep Patil <sspatil@android.com> wrote: > > > > On Wed, Apr 8, 2020 at 11:09 PM Chunyan Zhang <zhang.lyra@gmail.com> wrote: >> >> Hi Arnd, >> >> Thanks for finding out this and fixing it, but we have a minor concern >> for changing ARCH_APRD back to bool. >> >> On Thu, Apr 9, 2020 at 2:57 AM Arnd Bergmann <arnd@arndb.de> wrote: >> > >> > I got a build failure with CONFIG_ARCH_SPRD=m when the >> > main portion of the clock driver failed to get linked into >> > the kernel: >> > >> > ERROR: modpost: "sprd_pll_sc_gate_ops" [drivers/clk/sprd/sc9863a-clk.ko] undefined! >> > ERROR: modpost: "sprd_pll_ops" [drivers/clk/sprd/sc9863a-clk.ko] undefined! >> > ERROR: modpost: "sprd_div_ops" [drivers/clk/sprd/sc9863a-clk.ko] undefined! >> > ERROR: modpost: "sprd_comp_ops" [drivers/clk/sprd/sc9863a-clk.ko] undefined! >> > ERROR: modpost: "sprd_mux_ops" [drivers/clk/sprd/sc9863a-clk.ko] undefined! >> > ERROR: modpost: "sprd_gate_ops" [drivers/clk/sprd/sc9863a-clk.ko] undefined! >> > ERROR: modpost: "sprd_sc_gate_ops" [drivers/clk/sprd/sc9863a-clk.ko] undefined! >> > ERROR: modpost: "sprd_clk_probe" [drivers/clk/sprd/sc9863a-clk.ko] undefined! >> > ERROR: modpost: "sprd_clk_regmap_init" [drivers/clk/sprd/sc9863a-clk.ko] undefined! >> > ERROR: modpost: "sprd_pll_ops" [drivers/clk/sprd/sc9860-clk.ko] undefined! >> > ERROR: modpost: "sprd_div_ops" [drivers/clk/sprd/sc9860-clk.ko] undefined! >> > ERROR: modpost: "sprd_mux_ops" [drivers/clk/sprd/sc9860-clk.ko] undefined! >> > >> > This is a combination of two trivial bugs: >> > >> > - A platform should not be 'tristate', it should be a 'bool' symbol >> > like the other platforms, if only for consistency, and to avoid >> > surprises like this one. >> >> After a discussion, we decided to change ARCH_SPRD to tristate, the >> idea was that we hope we can simply switch all sprd drivers' configs >> (whose default is ARCH_SPRD) to 'm' by setting ARCH_SPRD=m, or switch >> all them to 'y' by setting ARCH_SPRD=y, instead of changing them one >> by one. This requirement originally came from that Google GKI project >> asks all vendor drivers to be built as modules. > > > > Unfortunately, even if ARCH_SPRD can be tristate, we found out (like Ard did here) that none of the other platform symbols can be tristate :(. > > So, we are going to enable all CONFIG_ARCH_XXXX in our defconfig[1]. Chunyan, Please feel free to submit that patch to AOSP for that. > > This does present us with a problem. We found that a bunch of drivers are 'default y if ARCH_XXX'. A lot of them have no symbol dependencies on the code that gets compiled with ARCH_XXX. They depend on it only because "the driver is only needed for the XXX SoC or the family". > > For example, enabling CONFIG_ARCH_MEDIATEK, will end up building almost all drivers in drivers/pinctrl/mediatek as far as I can see. > > This does add up. It increases the size of the kernel considerably. I have plans to send out the comparison in the future (later this year) once we are done collecting all def configs and see how bad that is. > > The only sane way I can see that can be resolved (if people agree that's a problem), is to make the ARCH_XXX code tristate-able and make the ARCH_XXX Kconfig select every driver it needs, instead of the other way round.
If we making the ARCH_XXX Kconfig select all drivers it needs, we will not have chance to custom the kernel Image for debug purpose. For example we can bringup a minimum system with only serial driver on sprd platforms.
> > All that being said, It is obviously not ok to have the allmodconfig broken like this without adding explicit dependencies as suggested above, or revert CONFIG_ARCH_SPRD to be a 'bool'.
We see this broken because I shouldn't leave clk Makefile a tristate compile [1] after changing ARCH_SPRD to be tristate.
If we will make ARCH_SPRD tristate-able in the future and you all aggree that, I would like to do it now, and pay more attention to Makefiles and dependencies.
I can also make a change like below:
diff --git a/drivers/clk/sprd/Kconfig b/drivers/clk/sprd/Kconfig index e18c80fbe804..9f7d9d8899a5 100644 --- a/drivers/clk/sprd/Kconfig +++ b/drivers/clk/sprd/Kconfig @@ -2,6 +2,7 @@ config SPRD_COMMON_CLK tristate "Clock support for Spreadtrum SoCs" depends on ARCH_SPRD || COMPILE_TEST + depends on m || ARCH_SPRD != m default ARCH_SPRD select REGMAP_MMIO
Arnd, Stephen, Sandeep, what do you think? Does that make sense? Thanks, Chunyan
[1] https://elixir.bootlin.com/linux/v5.6.3/source/drivers/clk/Makefile#L108
> > So fwiw, > > Acked-by: Sandeep Patil <sspatil@android.com> > > - ssp > > 1. https://android.googlesource.com/kernel/common/+/refs/heads/android-5.4/arch/arm64/configs/gki_defconfig#45
| |