Messages in this thread | | | From | Dong Aisheng <> | Date | Mon, 29 Jun 2020 15:04:40 +0800 | Subject | Re: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module |
| |
On Thu, Jun 25, 2020 at 6:43 AM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Aisheng Dong (2020-06-23 19:59:09) > > > > > > - bool > > > > > > - def_bool ARCH_MXC > > > > > > + tristate "IMX clock" > > > > > > + depends on ARCH_MXC > > > > > > > > > > > > But user can still set MXC_CLK to be m, either via make menuconfig > > > > > > or > > > > > defconfig. > > > > > > > > > > Isn't that what we want? > > > > > > > > No, if user set MXC_CLK to m, the build will break for i.MX6&7. > > > > > > > > > Why does ARCH_MXC being enabled mandate that it is builtin? Is some > > > > > architecture level code calling into the clk driver? > > > > > > > > > > > > It's mainly because there's no Kconfig options for i.MX6 &7 clock drivers. > > > > It just reuses ARCH config CONFIG_SOC_XXX which can only be y. > > > > e.g. > > > > obj-$(CONFIG_SOC_IMX6Q) += clk-imx6q.o > > > > obj-$(CONFIG_SOC_IMX6SL) += clk-imx6sl.o > > > > obj-$(CONFIG_SOC_IMX7ULP) += clk-imx7ulp.o > > > > obj-$(CONFIG_SOC_VF610) += clk-vf610.o .. > > > > > > > > If setting MXC_CLK to m, the platform clock drivers will fail to build > > > > due to miss to find symbols defined in the common clock library by > > > > CONFIG_MXC_CLK. > > > > So we have to avoid users to be able to config MXC_CLK to m for i.MX6&7. > > > > Only depends on ARCH_MXC mean user still can set it to m. > > > > > > I think for i.MX6/7, although MXC_CLK is tristate, but it is selected by > > > ARCH_MXC which is always "y", so MXC_CLK can ONLY be "y" even it is explicitly > > > set to "m" in imx_v6_v7_defconfig file. So that means MXC_CLK can ONLY > > > support built-in for i.MX6/7 SoCs, and that is what we want? > > > > > > > Yes, I'm trying to explain to Stephen why we have to select MXC_CLK in ARCH_MXC > > And what issues we will met if not select it. > > > > Why aren't there options to enable clk-imx6q and clk-imx6sl in the > clk/imx/Kconfig file? Those can be bool or tristate depending on if the > SoC drivers use CLK_OF_DECLARE or not and depend on the mxc-clk library > and SoC config we have in the makefile today.
Yes, we can do that in clk/imx/Kconfig as follows theoretically. config CLK_IMX6Q bool def_bool ARCH_MXC && ARM select MXC_CLK
But we have totally 15 platforms that need to change. e.g. drivers/clk/imx/Makefile obj-$(CONFIG_SOC_IMX1) += clk-imx1.o obj-$(CONFIG_SOC_IMX21) += clk-imx21.o obj-$(CONFIG_SOC_IMX25) += clk-imx25.o obj-$(CONFIG_SOC_IMX27) += clk-imx27.o obj-$(CONFIG_SOC_IMX31) += clk-imx31.o obj-$(CONFIG_SOC_IMX35) += clk-imx35.o obj-$(CONFIG_SOC_IMX5) += clk-imx5.o obj-$(CONFIG_SOC_IMX6Q) += clk-imx6q.o obj-$(CONFIG_SOC_IMX6SL) += clk-imx6sl.o obj-$(CONFIG_SOC_IMX6SLL) += clk-imx6sll.o obj-$(CONFIG_SOC_IMX6SX) += clk-imx6sx.o obj-$(CONFIG_SOC_IMX6UL) += clk-imx6ul.o obj-$(CONFIG_SOC_IMX7D) += clk-imx7d.o obj-$(CONFIG_SOC_IMX7ULP) += clk-imx7ulp.o obj-$(CONFIG_SOC_VF610) += clk-vf610.o
Not sure if it's really worth to do that. The easiest way to address this issue is just select it in arch/arm/mach-imx/Kconfig, then no need to change those 15 clock config options or just builtin clk libraries.
Stephen, which one do you prefer?
Regards Aisheng
| |