[PATCH V2 0/9] Support building i.MX8 SoCs clock driver as module

[PATCH V2 0/9] Support building i.MX8 SoCs clock driver as module

[Anson Huang]

Nowdays, there are more and more requirements of building SoC specific drivers
as modules, such as Android GKI (generic kernel image), this patch set supports
building i.MX8 SoCs clock drivers as modules, including i.MX8MQ/MM/MN/MP/QXP,
the common clock modules are: mxc-clk.ko for i.MX8MQ/MM/MN/MP, mxc-clk-scu.ko
for i.MX8QXP and later SoCs with SCU inside, normally, each platform can ONLY
insmod 1 common i.MX clock driver and its own SoC clock driver.

Since i.MX common clk driver will support module build and no longer selected
by default, so for i.MX ARMv7 platforms, need to manually select it to make build pass.

Changes since V1:
	- Fix build error for x86_64-allyesconfig by adding dependency for MXC_CLK_SCU;
	- Move lpcg clock driver change to SCU patch, this is incorrect in V1.

Anson Huang (9):
  clk: composite: Export clk_hw_register_composite()
  ARM: imx: Select MXC_CLK for ARCH_MXC
  clk: imx: Support building SCU clock driver as module
  clk: imx: Support building i.MX common clock driver as module
  clk: imx8mm: Support module build
  clk: imx8mn: Support module build
  clk: imx8mp: Support module build
  clk: imx8mq: Support module build
  clk: imx8qxp: Support module build

 arch/arm/mach-imx/Kconfig          |  1 +
 drivers/clk/clk-composite.c        |  1 +
 drivers/clk/imx/Kconfig            | 22 +++++++++++++---------
 drivers/clk/imx/Makefile           | 30 +++++++-----------------------
 drivers/clk/imx/clk-composite-8m.c |  1 +
 drivers/clk/imx/clk-cpu.c          |  1 +
 drivers/clk/imx/clk-frac-pll.c     |  1 +
 drivers/clk/imx/clk-gate2.c        |  1 +
 drivers/clk/imx/clk-imx8mm.c       |  1 +
 drivers/clk/imx/clk-imx8mn.c       |  1 +
 drivers/clk/imx/clk-imx8mp.c       |  1 +
 drivers/clk/imx/clk-imx8mq.c       |  1 +
 drivers/clk/imx/clk-imx8qxp-lpcg.c |  1 +
 drivers/clk/imx/clk-imx8qxp.c      |  1 +
 drivers/clk/imx/clk-lpcg-scu.c     |  1 +
 drivers/clk/imx/clk-pll14xx.c      |  4 ++++
 drivers/clk/imx/clk-scu.c          |  5 +++++
 drivers/clk/imx/clk-sscg-pll.c     |  1 +
 drivers/clk/imx/clk.c              | 28 ++++++++++++++++++++++------
 19 files changed, 65 insertions(+), 38 deletions(-)

-- 
2.7.4

[PATCH V2 1/9] clk: composite: Export clk_hw_register_composite()

[Anson Huang]

Export clk_hw_register_composite() to support user built as module.

ERROR: modpost: "clk_hw_register_composite" [drivers/clk/imx/mxc-clk.ko] undefined!
scripts/Makefile.modpost:111: recipe for target 'Module.symvers' failed
make[1]: *** [Module.symvers] Error 1
make[1]: *** Deleting file 'Module.symvers'
Makefile:1384: recipe for target 'modules' failed
make: *** [modules] Error 2

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/clk/clk-composite.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 7376f57..2ddb54f 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -328,6 +328,7 @@ struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name,
 					   rate_hw, rate_ops, gate_hw,
 					   gate_ops, flags);
 }
+EXPORT_SYMBOL_GPL(clk_hw_register_composite);
 
 struct clk_hw *clk_hw_register_composite_pdata(struct device *dev,
 			const char *name,
-- 
2.7.4

[PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC

[Anson Huang]

i.MX common clock drivers may support module build, so it is NOT
selected by default, for ARCH_MXC ARMv7 platforms, need to select
it manually to make build pass.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 arch/arm/mach-imx/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index e7d7b90..47b10d2 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -4,6 +4,7 @@ menuconfig ARCH_MXC
 	depends on ARCH_MULTI_V4_V5 || ARCH_MULTI_V6_V7 || ARM_SINGLE_ARMV7M
 	select ARCH_SUPPORTS_BIG_ENDIAN
 	select CLKSRC_IMX_GPT
+	select MXC_CLK
 	select GENERIC_IRQ_CHIP
 	select GPIOLIB
 	select PINCTRL
-- 
2.7.4

[PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Anson Huang]

There are more and more requirements of building SoC specific drivers
as modules, add support for building SCU clock driver as module to meet
the requirement.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V1:
	- add ARCH_MXC to build dependency to avoid build fail on x86 arch;
	- move clk-lpcg-scu.c change in to this patch.
---
 drivers/clk/imx/Kconfig        | 4 ++--
 drivers/clk/imx/Makefile       | 5 ++---
 drivers/clk/imx/clk-lpcg-scu.c | 1 +
 drivers/clk/imx/clk-scu.c      | 5 +++++
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index db0253f..ded0643 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -5,8 +5,8 @@ config MXC_CLK
 	def_bool ARCH_MXC
 
 config MXC_CLK_SCU
-	bool
-	depends on IMX_SCU
+	tristate "IMX SCU clock"
+	depends on ARCH_MXC && IMX_SCU
 
 config CLK_IMX8MM
 	bool "IMX8MM CCM Clock Driver"
diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 928f874..1af8cff 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -21,9 +21,8 @@ obj-$(CONFIG_MXC_CLK) += \
 	clk-sscg-pll.o \
 	clk-pll14xx.o
 
-obj-$(CONFIG_MXC_CLK_SCU) += \
-	clk-scu.o \
-	clk-lpcg-scu.o
+mxc-clk-scu-objs += clk-scu.o clk-lpcg-scu.o
+obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
 
 obj-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o
 obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o
diff --git a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c
index a73a799..8177f0e 100644
--- a/drivers/clk/imx/clk-lpcg-scu.c
+++ b/drivers/clk/imx/clk-lpcg-scu.c
@@ -114,3 +114,4 @@ struct clk_hw *imx_clk_lpcg_scu(const char *name, const char *parent_name,
 
 	return hw;
 }
+EXPORT_SYMBOL_GPL(imx_clk_lpcg_scu);
diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index b8b2072..9688981 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -8,6 +8,7 @@
 #include <linux/arm-smccc.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
+#include <linux/module.h>
 #include <linux/slab.h>
 
 #include "clk-scu.h"
@@ -132,6 +133,7 @@ int imx_clk_scu_init(void)
 {
 	return imx_scu_get_handle(&ccm_ipc_handle);
 }
+EXPORT_SYMBOL_GPL(imx_clk_scu_init);
 
 /*
  * clk_scu_recalc_rate - Get clock rate for a SCU clock
@@ -387,3 +389,6 @@ struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
 
 	return hw;
 }
+EXPORT_SYMBOL_GPL(__imx_clk_scu);
+
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V2 4/9] clk: imx: Support building i.MX common clock driver as module

[Anson Huang]

There are more and more requirements of building SoC specific drivers
as modules, add support for building i.MX common clock driver as module
to meet the requirement.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V1:
	- move scu lpcg driver change to SCU patch.
---
 drivers/clk/imx/Kconfig            |  8 ++++++--
 drivers/clk/imx/Makefile           | 25 +++++--------------------
 drivers/clk/imx/clk-composite-8m.c |  1 +
 drivers/clk/imx/clk-cpu.c          |  1 +
 drivers/clk/imx/clk-frac-pll.c     |  1 +
 drivers/clk/imx/clk-gate2.c        |  1 +
 drivers/clk/imx/clk-pll14xx.c      |  4 ++++
 drivers/clk/imx/clk-sscg-pll.c     |  1 +
 drivers/clk/imx/clk.c              | 28 ++++++++++++++++++++++------
 9 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index ded0643..678113b 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -1,8 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # common clock support for NXP i.MX SoC family.
 config MXC_CLK
-	bool
-	def_bool ARCH_MXC
+	tristate "IMX clock"
+	depends on ARCH_MXC
 
 config MXC_CLK_SCU
 	tristate "IMX SCU clock"
@@ -11,24 +11,28 @@ config MXC_CLK_SCU
 config CLK_IMX8MM
 	bool "IMX8MM CCM Clock Driver"
 	depends on ARCH_MXC
+	select MXC_CLK
 	help
 	    Build the driver for i.MX8MM CCM Clock Driver
 
 config CLK_IMX8MN
 	bool "IMX8MN CCM Clock Driver"
 	depends on ARCH_MXC
+	select MXC_CLK
 	help
 	    Build the driver for i.MX8MN CCM Clock Driver
 
 config CLK_IMX8MP
 	bool "IMX8MP CCM Clock Driver"
 	depends on ARCH_MXC
+	select MXC_CLK
 	help
 	    Build the driver for i.MX8MP CCM Clock Driver
 
 config CLK_IMX8MQ
 	bool "IMX8MQ CCM Clock Driver"
 	depends on ARCH_MXC
+	select MXC_CLK
 	help
 	    Build the driver for i.MX8MQ CCM Clock Driver
 
diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 1af8cff..1291f9b 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -1,25 +1,10 @@
 # SPDX-License-Identifier: GPL-2.0
 
-obj-$(CONFIG_MXC_CLK) += \
-	clk.o \
-	clk-busy.o \
-	clk-composite-8m.o \
-	clk-cpu.o \
-	clk-composite-7ulp.o \
-	clk-divider-gate.o \
-	clk-fixup-div.o \
-	clk-fixup-mux.o \
-	clk-frac-pll.o \
-	clk-gate-exclusive.o \
-	clk-gate2.o \
-	clk-pfd.o \
-	clk-pfdv2.o \
-	clk-pllv1.o \
-	clk-pllv2.o \
-	clk-pllv3.o \
-	clk-pllv4.o \
-	clk-sscg-pll.o \
-	clk-pll14xx.o
+mxc-clk-objs += clk.o clk-busy.o clk-composite-8m.o clk-cpu.o clk-composite-7ulp.o \
+		clk-divider-gate.o clk-fixup-div.o clk-fixup-mux.o clk-frac-pll.o \
+		clk-gate-exclusive.o clk-gate2.o clk-pfd.o clk-pfdv2.o clk-pllv1.o \
+		clk-pllv2.o clk-pllv3.o clk-pllv4.o clk-sscg-pll.o clk-pll14xx.o
+obj-$(CONFIG_MXC_CLK) += mxc-clk.o
 
 mxc-clk-scu-objs += clk-scu.o clk-lpcg-scu.o
 obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
index d2b5af8..73e064b 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -243,3 +243,4 @@ struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
 	kfree(mux);
 	return ERR_CAST(hw);
 }
+EXPORT_SYMBOL_GPL(imx8m_clk_hw_composite_flags);
diff --git a/drivers/clk/imx/clk-cpu.c b/drivers/clk/imx/clk-cpu.c
index cb182be..07dc88b 100644
--- a/drivers/clk/imx/clk-cpu.c
+++ b/drivers/clk/imx/clk-cpu.c
@@ -104,3 +104,4 @@ struct clk_hw *imx_clk_hw_cpu(const char *name, const char *parent_name,
 
 	return hw;
 }
+EXPORT_SYMBOL_GPL(imx_clk_hw_cpu);
diff --git a/drivers/clk/imx/clk-frac-pll.c b/drivers/clk/imx/clk-frac-pll.c
index 101e0a3..aa6aebf 100644
--- a/drivers/clk/imx/clk-frac-pll.c
+++ b/drivers/clk/imx/clk-frac-pll.c
@@ -233,3 +233,4 @@ struct clk_hw *imx_clk_hw_frac_pll(const char *name,
 
 	return hw;
 }
+EXPORT_SYMBOL_GPL(imx_clk_hw_frac_pll);
diff --git a/drivers/clk/imx/clk-gate2.c b/drivers/clk/imx/clk-gate2.c
index b87ab3c..b1b93ea 100644
--- a/drivers/clk/imx/clk-gate2.c
+++ b/drivers/clk/imx/clk-gate2.c
@@ -177,3 +177,4 @@ struct clk_hw *clk_hw_register_gate2(struct device *dev, const char *name,
 
 	return hw;
 }
+EXPORT_SYMBOL_GPL(clk_hw_register_gate2);
diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index f9eb189..5482775 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -68,6 +68,7 @@ struct imx_pll14xx_clk imx_1443x_pll = {
 	.rate_table = imx_pll1443x_tbl,
 	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
 };
+EXPORT_SYMBOL_GPL(imx_1443x_pll);
 
 struct imx_pll14xx_clk imx_1443x_dram_pll = {
 	.type = PLL_1443X,
@@ -75,12 +76,14 @@ struct imx_pll14xx_clk imx_1443x_dram_pll = {
 	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
 	.flags = CLK_GET_RATE_NOCACHE,
 };
+EXPORT_SYMBOL_GPL(imx_1443x_dram_pll);
 
 struct imx_pll14xx_clk imx_1416x_pll = {
 	.type = PLL_1416X,
 	.rate_table = imx_pll1416x_tbl,
 	.rate_count = ARRAY_SIZE(imx_pll1416x_tbl),
 };
+EXPORT_SYMBOL_GPL(imx_1416x_pll);
 
 static const struct imx_pll14xx_rate_table *imx_get_pll_settings(
 		struct clk_pll14xx *pll, unsigned long rate)
@@ -436,3 +439,4 @@ struct clk_hw *imx_dev_clk_hw_pll14xx(struct device *dev, const char *name,
 
 	return hw;
 }
+EXPORT_SYMBOL_GPL(imx_dev_clk_hw_pll14xx);
diff --git a/drivers/clk/imx/clk-sscg-pll.c b/drivers/clk/imx/clk-sscg-pll.c
index 773d8a5..2f6a0a0 100644
--- a/drivers/clk/imx/clk-sscg-pll.c
+++ b/drivers/clk/imx/clk-sscg-pll.c
@@ -537,3 +537,4 @@ struct clk_hw *imx_clk_hw_sscg_pll(const char *name,
 
 	return hw;
 }
+EXPORT_SYMBOL_GPL(imx_clk_hw_sscg_pll);
diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c
index 87ab8db..cc894b5 100644
--- a/drivers/clk/imx/clk.c
+++ b/drivers/clk/imx/clk.c
@@ -3,6 +3,7 @@
 #include <linux/clk-provider.h>
 #include <linux/err.h>
 #include <linux/io.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
@@ -13,6 +14,7 @@
 #define CCDR_MMDC_CH1_MASK		BIT(16)
 
 DEFINE_SPINLOCK(imx_ccm_lock);
+EXPORT_SYMBOL_GPL(imx_ccm_lock);
 
 void imx_unregister_clocks(struct clk *clks[], unsigned int count)
 {
@@ -29,8 +31,9 @@ void imx_unregister_hw_clocks(struct clk_hw *hws[], unsigned int count)
 	for (i = 0; i < count; i++)
 		clk_hw_unregister(hws[i]);
 }
+EXPORT_SYMBOL_GPL(imx_unregister_hw_clocks);
 
-void __init imx_mmdc_mask_handshake(void __iomem *ccm_base,
+void imx_mmdc_mask_handshake(void __iomem *ccm_base,
 				    unsigned int chn)
 {
 	unsigned int reg;
@@ -59,8 +62,9 @@ void imx_check_clk_hws(struct clk_hw *clks[], unsigned int count)
 			pr_err("i.MX clk %u: register failed with %ld\n",
 			       i, PTR_ERR(clks[i]));
 }
+EXPORT_SYMBOL_GPL(imx_check_clk_hws);
 
-static struct clk * __init imx_obtain_fixed_clock_from_dt(const char *name)
+static struct clk *imx_obtain_fixed_clock_from_dt(const char *name)
 {
 	struct of_phandle_args phandle;
 	struct clk *clk = ERR_PTR(-ENODEV);
@@ -80,7 +84,7 @@ static struct clk * __init imx_obtain_fixed_clock_from_dt(const char *name)
 	return clk;
 }
 
-struct clk * __init imx_obtain_fixed_clock(
+struct clk *imx_obtain_fixed_clock(
 			const char *name, unsigned long rate)
 {
 	struct clk *clk;
@@ -91,7 +95,7 @@ struct clk * __init imx_obtain_fixed_clock(
 	return clk;
 }
 
-struct clk_hw * __init imx_obtain_fixed_clock_hw(
+struct clk_hw *imx_obtain_fixed_clock_hw(
 			const char *name, unsigned long rate)
 {
 	struct clk *clk;
@@ -113,6 +117,7 @@ struct clk_hw * imx_obtain_fixed_clk_hw(struct device_node *np,
 
 	return __clk_get_hw(clk);
 }
+EXPORT_SYMBOL_GPL(imx_obtain_fixed_clk_hw);
 
 /*
  * This fixups the register CCM_CSCMR1 write value.
@@ -143,16 +148,24 @@ void imx_cscmr1_fixup(u32 *val)
 static int imx_keep_uart_clocks;
 static struct clk ** const *imx_uart_clocks;
 
-static int __init imx_keep_uart_clocks_param(char *str)
+static int __maybe_unused imx_keep_uart_clocks_param(char *str)
 {
 	imx_keep_uart_clocks = 1;
 
 	return 0;
 }
+
+#ifdef MODULE
+__setup_param("earlycon", imx_keep_uart_earlycon,
+	      imx_keep_uart_clocks_param);
+__setup_param("earlyprintk", imx_keep_uart_earlyprintk,
+	      imx_keep_uart_clocks_param);
+#else
 __setup_param("earlycon", imx_keep_uart_earlycon,
 	      imx_keep_uart_clocks_param, 0);
 __setup_param("earlyprintk", imx_keep_uart_earlyprintk,
 	      imx_keep_uart_clocks_param, 0);
+#endif
 
 void imx_register_uart_clocks(struct clk ** const clks[])
 {
@@ -164,8 +177,9 @@ void imx_register_uart_clocks(struct clk ** const clks[])
 			clk_prepare_enable(*imx_uart_clocks[i]);
 	}
 }
+EXPORT_SYMBOL_GPL(imx_register_uart_clocks);
 
-static int __init imx_clk_disable_uart(void)
+static int imx_clk_disable_uart(void)
 {
 	if (imx_keep_uart_clocks && imx_uart_clocks) {
 		int i;
@@ -177,3 +191,5 @@ static int __init imx_clk_disable_uart(void)
 	return 0;
 }
 late_initcall_sync(imx_clk_disable_uart);
+
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V2 5/9] clk: imx8mm: Support module build

[Anson Huang]

Support building i.MX8MM clock driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/clk/imx/Kconfig      | 2 +-
 drivers/clk/imx/clk-imx8mm.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index 678113b..97d86a3 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -9,7 +9,7 @@ config MXC_CLK_SCU
 	depends on ARCH_MXC && IMX_SCU
 
 config CLK_IMX8MM
-	bool "IMX8MM CCM Clock Driver"
+	tristate "IMX8MM CCM Clock Driver"
 	depends on ARCH_MXC
 	select MXC_CLK
 	help
diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index b793264..1e8242d 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -657,3 +657,4 @@ static struct platform_driver imx8mm_clk_driver = {
 	},
 };
 module_platform_driver(imx8mm_clk_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V2 6/9] clk: imx8mn: Support module build

[Anson Huang]

Support building i.MX8MN clock driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/clk/imx/Kconfig      | 2 +-
 drivers/clk/imx/clk-imx8mn.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index 97d86a3..5f537c3 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -16,7 +16,7 @@ config CLK_IMX8MM
 	    Build the driver for i.MX8MM CCM Clock Driver
 
 config CLK_IMX8MN
-	bool "IMX8MN CCM Clock Driver"
+	tristate "IMX8MN CCM Clock Driver"
 	depends on ARCH_MXC
 	select MXC_CLK
 	help
diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index 213cc37..ba930a7 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -608,3 +608,4 @@ static struct platform_driver imx8mn_clk_driver = {
 	},
 };
 module_platform_driver(imx8mn_clk_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V2 7/9] clk: imx8mp: Support module build

[Anson Huang]

Support building i.MX8MP clock driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/clk/imx/Kconfig      | 2 +-
 drivers/clk/imx/clk-imx8mp.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index 5f537c3..0811bed 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -23,7 +23,7 @@ config CLK_IMX8MN
 	    Build the driver for i.MX8MN CCM Clock Driver
 
 config CLK_IMX8MP
-	bool "IMX8MP CCM Clock Driver"
+	tristate "IMX8MP CCM Clock Driver"
 	depends on ARCH_MXC
 	select MXC_CLK
 	help
diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index b4d9db9..8517837 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -772,3 +772,4 @@ static struct platform_driver imx8mp_clk_driver = {
 	},
 };
 module_platform_driver(imx8mp_clk_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V2 8/9] clk: imx8mq: Support module build

[Anson Huang]

Support building i.MX8MQ clock driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/clk/imx/Kconfig      | 2 +-
 drivers/clk/imx/clk-imx8mq.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index 0811bed..060f9c3 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -30,7 +30,7 @@ config CLK_IMX8MP
 	    Build the driver for i.MX8MP CCM Clock Driver
 
 config CLK_IMX8MQ
-	bool "IMX8MQ CCM Clock Driver"
+	tristate "IMX8MQ CCM Clock Driver"
 	depends on ARCH_MXC
 	select MXC_CLK
 	help
diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index a64aace..2861c31 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -643,3 +643,4 @@ static struct platform_driver imx8mq_clk_driver = {
 	},
 };
 module_platform_driver(imx8mq_clk_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V2 9/9] clk: imx8qxp: Support module build

[Anson Huang]

Support building i.MX8QXP clock driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/clk/imx/Kconfig            | 2 +-
 drivers/clk/imx/clk-imx8qxp-lpcg.c | 1 +
 drivers/clk/imx/clk-imx8qxp.c      | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index 060f9c3..26cedbf 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -37,7 +37,7 @@ config CLK_IMX8MQ
 	    Build the driver for i.MX8MQ CCM Clock Driver
 
 config CLK_IMX8QXP
-	bool "IMX8QXP SCU Clock"
+	tristate "IMX8QXP SCU Clock"
 	depends on ARCH_MXC && IMX_SCU && ARM64
 	select MXC_CLK_SCU
 	help
diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
index 04c8ee3..8afaefc 100644
--- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
+++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
@@ -232,3 +232,4 @@ static struct platform_driver imx8qxp_lpcg_clk_driver = {
 };
 
 builtin_platform_driver(imx8qxp_lpcg_clk_driver);
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
index 5e2903e..a34c7c5 100644
--- a/drivers/clk/imx/clk-imx8qxp.c
+++ b/drivers/clk/imx/clk-imx8qxp.c
@@ -152,3 +152,4 @@ static struct platform_driver imx8qxp_clk_driver = {
 	.probe = imx8qxp_clk_probe,
 };
 builtin_platform_driver(imx8qxp_clk_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

RE: [PATCH V2 1/9] clk: composite: Export clk_hw_register_composite()

[Aisheng Dong]

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Tuesday, June 9, 2020 3:32 PM
> 
> Export clk_hw_register_composite() to support user built as module.
> 
> ERROR: modpost: "clk_hw_register_composite" [drivers/clk/imx/mxc-clk.ko]
> undefined!
> scripts/Makefile.modpost:111: recipe for target 'Module.symvers' failed
> make[1]: *** [Module.symvers] Error 1
> make[1]: *** Deleting file 'Module.symvers'
> Makefile:1384: recipe for target 'modules' failed
> make: *** [modules] Error 2
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> No change.
> ---
>  drivers/clk/clk-composite.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c index
> 7376f57..2ddb54f 100644
> --- a/drivers/clk/clk-composite.c
> +++ b/drivers/clk/clk-composite.c
> @@ -328,6 +328,7 @@ struct clk_hw *clk_hw_register_composite(struct
> device *dev, const char *name,
>  					   rate_hw, rate_ops, gate_hw,
>  					   gate_ops, flags);
>  }
> +EXPORT_SYMBOL_GPL(clk_hw_register_composite);
> 
>  struct clk_hw *clk_hw_register_composite_pdata(struct device *dev,
>  			const char *name,

I guess you'd better add this one as well.

Regards
Aisheng

> --
> 2.7.4

RE: [PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC

[Aisheng Dong]

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Tuesday, June 9, 2020 3:32 PM
> 
> i.MX common clock drivers may support module build, so it is NOT selected by
> default, for ARCH_MXC ARMv7 platforms, need to select it manually to make
> build pass.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Can't the original def_xxx work?

config MXC_CLK
        tristate
        def_tristate ARCH_MXC

Regards
Aisheng

> ---
> No change.
> ---
>  arch/arm/mach-imx/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig index
> e7d7b90..47b10d2 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -4,6 +4,7 @@ menuconfig ARCH_MXC
>  	depends on ARCH_MULTI_V4_V5 || ARCH_MULTI_V6_V7 ||
> ARM_SINGLE_ARMV7M
>  	select ARCH_SUPPORTS_BIG_ENDIAN
>  	select CLKSRC_IMX_GPT
> +	select MXC_CLK
>  	select GENERIC_IRQ_CHIP
>  	select GPIOLIB
>  	select PINCTRL
> --
> 2.7.4

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Aisheng Dong]

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Tuesday, June 9, 2020 3:32 PM
> 
> There are more and more requirements of building SoC specific drivers as
> modules, add support for building SCU clock driver as module to meet the
> requirement.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V1:
> 	- add ARCH_MXC to build dependency to avoid build fail on x86 arch;
> 	- move clk-lpcg-scu.c change in to this patch.
> ---
>  drivers/clk/imx/Kconfig        | 4 ++--
>  drivers/clk/imx/Makefile       | 5 ++---
>  drivers/clk/imx/clk-lpcg-scu.c | 1 +
>  drivers/clk/imx/clk-scu.c      | 5 +++++
>  4 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> db0253f..ded0643 100644
> --- a/drivers/clk/imx/Kconfig
> +++ b/drivers/clk/imx/Kconfig
> @@ -5,8 +5,8 @@ config MXC_CLK
>  	def_bool ARCH_MXC
> 
>  config MXC_CLK_SCU
> -	bool
> -	depends on IMX_SCU

Keep this line as it is

> +	tristate "IMX SCU clock"

i.MX SCU Clock core driver

> +	depends on ARCH_MXC && IMX_SCU
> 
>  config CLK_IMX8MM
>  	bool "IMX8MM CCM Clock Driver"
> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index
> 928f874..1af8cff 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -21,9 +21,8 @@ obj-$(CONFIG_MXC_CLK) += \
>  	clk-sscg-pll.o \
>  	clk-pll14xx.o
> 
> -obj-$(CONFIG_MXC_CLK_SCU) += \
> -	clk-scu.o \
> -	clk-lpcg-scu.o
> +mxc-clk-scu-objs += clk-scu.o clk-lpcg-scu.o
> +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o

Like i.MX pinctrl, I'm not sure if it's really necessary to build core libraries
as modules. Probably the simplest way is only building platform drivers part
as module. And leave those core libraries built in kernel.
This may make the code a bit cleaner.

Regards
Aisheng

> 
>  obj-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o
>  obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o diff --git
> a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c index
> a73a799..8177f0e 100644
> --- a/drivers/clk/imx/clk-lpcg-scu.c
> +++ b/drivers/clk/imx/clk-lpcg-scu.c
> @@ -114,3 +114,4 @@ struct clk_hw *imx_clk_lpcg_scu(const char *name,
> const char *parent_name,
> 
>  	return hw;
>  }
> +EXPORT_SYMBOL_GPL(imx_clk_lpcg_scu);
> diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c index
> b8b2072..9688981 100644
> --- a/drivers/clk/imx/clk-scu.c
> +++ b/drivers/clk/imx/clk-scu.c
> @@ -8,6 +8,7 @@
>  #include <linux/arm-smccc.h>
>  #include <linux/clk-provider.h>
>  #include <linux/err.h>
> +#include <linux/module.h>
>  #include <linux/slab.h>
> 
>  #include "clk-scu.h"
> @@ -132,6 +133,7 @@ int imx_clk_scu_init(void)  {
>  	return imx_scu_get_handle(&ccm_ipc_handle);
>  }
> +EXPORT_SYMBOL_GPL(imx_clk_scu_init);
> 
>  /*
>   * clk_scu_recalc_rate - Get clock rate for a SCU clock @@ -387,3 +389,6 @@
> struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
> 
>  	return hw;
>  }
> +EXPORT_SYMBOL_GPL(__imx_clk_scu);
> +
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Anson Huang]

> Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as
> module
> 
> > From: Anson Huang <Anson.Huang@nxp.com>
> > Sent: Tuesday, June 9, 2020 3:32 PM
> >
> > There are more and more requirements of building SoC specific drivers
> > as modules, add support for building SCU clock driver as module to
> > meet the requirement.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V1:
> > 	- add ARCH_MXC to build dependency to avoid build fail on x86 arch;
> > 	- move clk-lpcg-scu.c change in to this patch.
> > ---
> >  drivers/clk/imx/Kconfig        | 4 ++--
> >  drivers/clk/imx/Makefile       | 5 ++---
> >  drivers/clk/imx/clk-lpcg-scu.c | 1 +
> >  drivers/clk/imx/clk-scu.c      | 5 +++++
> >  4 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> > db0253f..ded0643 100644
> > --- a/drivers/clk/imx/Kconfig
> > +++ b/drivers/clk/imx/Kconfig
> > @@ -5,8 +5,8 @@ config MXC_CLK
> >  	def_bool ARCH_MXC
> >
> >  config MXC_CLK_SCU
> > -	bool
> > -	depends on IMX_SCU
> 
> Keep this line as it is
> 
> > +	tristate "IMX SCU clock"
> 
> i.MX SCU Clock core driver
> 
> > +	depends on ARCH_MXC && IMX_SCU
> >
> >  config CLK_IMX8MM
> >  	bool "IMX8MM CCM Clock Driver"
> > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index
> > 928f874..1af8cff 100644
> > --- a/drivers/clk/imx/Makefile
> > +++ b/drivers/clk/imx/Makefile
> > @@ -21,9 +21,8 @@ obj-$(CONFIG_MXC_CLK) += \
> >  	clk-sscg-pll.o \
> >  	clk-pll14xx.o
> >
> > -obj-$(CONFIG_MXC_CLK_SCU) += \
> > -	clk-scu.o \
> > -	clk-lpcg-scu.o
> > +mxc-clk-scu-objs += clk-scu.o clk-lpcg-scu.o
> > +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> 
> Like i.MX pinctrl, I'm not sure if it's really necessary to build core libraries as
> modules. Probably the simplest way is only building platform drivers part as
> module. And leave those core libraries built in kernel.
> This may make the code a bit cleaner.
> 

Will discuss this with Linaro guys about it, previous requirement I received is
all SoC specific modules need to be built as module.

Anson

RE: [PATCH V2 1/9] clk: composite: Export clk_hw_register_composite()

[Anson Huang]

> Subject: RE: [PATCH V2 1/9] clk: composite: Export
> clk_hw_register_composite()
> 
> > From: Anson Huang <Anson.Huang@nxp.com>
> > Sent: Tuesday, June 9, 2020 3:32 PM
> >
> > Export clk_hw_register_composite() to support user built as module.
> >
> > ERROR: modpost: "clk_hw_register_composite"
> > [drivers/clk/imx/mxc-clk.ko] undefined!
> > scripts/Makefile.modpost:111: recipe for target 'Module.symvers'
> > failed
> > make[1]: *** [Module.symvers] Error 1
> > make[1]: *** Deleting file 'Module.symvers'
> > Makefile:1384: recipe for target 'modules' failed
> > make: *** [modules] Error 2
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > No change.
> > ---
> >  drivers/clk/clk-composite.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> > index 7376f57..2ddb54f 100644
> > --- a/drivers/clk/clk-composite.c
> > +++ b/drivers/clk/clk-composite.c
> > @@ -328,6 +328,7 @@ struct clk_hw *clk_hw_register_composite(struct
> > device *dev, const char *name,
> >  					   rate_hw, rate_ops, gate_hw,
> >  					   gate_ops, flags);
> >  }
> > +EXPORT_SYMBOL_GPL(clk_hw_register_composite);
> >
> >  struct clk_hw *clk_hw_register_composite_pdata(struct device *dev,
> >  			const char *name,
> 
> I guess you'd better add this one as well.

I did NOT see this is used in upstream i.MX clock driver, from my test, it is not necessary so far.

Anson

RE: [PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC

[Anson Huang]

> Subject: RE: [PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC
> 
> > From: Anson Huang <Anson.Huang@nxp.com>
> > Sent: Tuesday, June 9, 2020 3:32 PM
> >
> > i.MX common clock drivers may support module build, so it is NOT
> > selected by default, for ARCH_MXC ARMv7 platforms, need to select it
> > manually to make build pass.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 
> Can't the original def_xxx work?
> 
> config MXC_CLK
>         tristate
>         def_tristate ARCH_MXC

Such change will make MXC_CLK=y even all i.MX8 SoCs clock drivers are selected as module,
so it does NOT meet the requirement of module support. Below is from .config when all
i.MX8 SoCs clock drivers are configured to module.

 CONFIG_MXC_CLK=y
 CONFIG_MXC_CLK_SCU=m
 CONFIG_CLK_IMX8MM=m
 CONFIG_CLK_IMX8MN=m
 CONFIG_CLK_IMX8MP=m
 CONFIG_CLK_IMX8MQ=m
 CONFIG_CLK_IMX8QXP=m

Anson

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Aisheng Dong]

> From: Anson Huang <anson.huang@nxp.com>
> Sent: Wednesday, June 17, 2020 8:27 PM
> 
> 
> > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock
> > driver as module
> >
> > > From: Anson Huang <Anson.Huang@nxp.com>
> > > Sent: Tuesday, June 9, 2020 3:32 PM
> > >
> > > There are more and more requirements of building SoC specific
> > > drivers as modules, add support for building SCU clock driver as
> > > module to meet the requirement.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > > Changes since V1:
> > > 	- add ARCH_MXC to build dependency to avoid build fail on x86 arch;
> > > 	- move clk-lpcg-scu.c change in to this patch.
> > > ---
> > >  drivers/clk/imx/Kconfig        | 4 ++--
> > >  drivers/clk/imx/Makefile       | 5 ++---
> > >  drivers/clk/imx/clk-lpcg-scu.c | 1 +
> > >  drivers/clk/imx/clk-scu.c      | 5 +++++
> > >  4 files changed, 10 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> > > db0253f..ded0643 100644
> > > --- a/drivers/clk/imx/Kconfig
> > > +++ b/drivers/clk/imx/Kconfig
> > > @@ -5,8 +5,8 @@ config MXC_CLK
> > >  	def_bool ARCH_MXC
> > >
> > >  config MXC_CLK_SCU
> > > -	bool
> > > -	depends on IMX_SCU
> >
> > Keep this line as it is
> >
> > > +	tristate "IMX SCU clock"
> >
> > i.MX SCU Clock core driver
> >
> > > +	depends on ARCH_MXC && IMX_SCU
> > >
> > >  config CLK_IMX8MM
> > >  	bool "IMX8MM CCM Clock Driver"
> > > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
> > > index 928f874..1af8cff 100644
> > > --- a/drivers/clk/imx/Makefile
> > > +++ b/drivers/clk/imx/Makefile
> > > @@ -21,9 +21,8 @@ obj-$(CONFIG_MXC_CLK) += \
> > >  	clk-sscg-pll.o \
> > >  	clk-pll14xx.o
> > >
> > > -obj-$(CONFIG_MXC_CLK_SCU) += \
> > > -	clk-scu.o \
> > > -	clk-lpcg-scu.o
> > > +mxc-clk-scu-objs += clk-scu.o clk-lpcg-scu.o
> > > +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> >
> > Like i.MX pinctrl, I'm not sure if it's really necessary to build core
> > libraries as modules. Probably the simplest way is only building
> > platform drivers part as module. And leave those core libraries built in kernel.
> > This may make the code a bit cleaner.
> >
> 
> Will discuss this with Linaro guys about it, previous requirement I received is all
> SoC specific modules need to be built as module.
> 

Okay. AFAIK it's not conflict.
You still make drivers into modules.
Only difference is for those common libraries part, we don't convert them into module
Which is less meaningless.
 
Regards
Aisheng

> Anson

RE: [PATCH V2 1/9] clk: composite: Export clk_hw_register_composite()

[Aisheng Dong]

> From: Anson Huang <anson.huang@nxp.com>
> Sent: Wednesday, June 17, 2020 8:32 PM
> 
> > Subject: RE: [PATCH V2 1/9] clk: composite: Export
> > clk_hw_register_composite()
> >
> > > From: Anson Huang <Anson.Huang@nxp.com>
> > > Sent: Tuesday, June 9, 2020 3:32 PM
> > >
> > > Export clk_hw_register_composite() to support user built as module.
> > >
> > > ERROR: modpost: "clk_hw_register_composite"
> > > [drivers/clk/imx/mxc-clk.ko] undefined!
> > > scripts/Makefile.modpost:111: recipe for target 'Module.symvers'
> > > failed
> > > make[1]: *** [Module.symvers] Error 1
> > > make[1]: *** Deleting file 'Module.symvers'
> > > Makefile:1384: recipe for target 'modules' failed
> > > make: *** [modules] Error 2
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > > No change.
> > > ---
> > >  drivers/clk/clk-composite.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/clk/clk-composite.c
> > > b/drivers/clk/clk-composite.c index 7376f57..2ddb54f 100644
> > > --- a/drivers/clk/clk-composite.c
> > > +++ b/drivers/clk/clk-composite.c
> > > @@ -328,6 +328,7 @@ struct clk_hw *clk_hw_register_composite(struct
> > > device *dev, const char *name,
> > >  					   rate_hw, rate_ops, gate_hw,
> > >  					   gate_ops, flags);
> > >  }
> > > +EXPORT_SYMBOL_GPL(clk_hw_register_composite);
> > >
> > >  struct clk_hw *clk_hw_register_composite_pdata(struct device *dev,
> > >  			const char *name,
> >
> > I guess you'd better add this one as well.
> 
> I did NOT see this is used in upstream i.MX clock driver, from my test, it is not
> necessary so far.
> 

Normally we could export them together in case it will be used in the future assuming your patch
is making clk-composite be able to be used by modules.

Anyway, it depends on Stephen's preference.

Regards
Aisheng
 
> Anson

RE: [PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC

[Aisheng Dong]

> From: Anson Huang <anson.huang@nxp.com>
> Sent: Wednesday, June 17, 2020 8:36 PM
> 
> > Subject: RE: [PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC
> >
> > > From: Anson Huang <Anson.Huang@nxp.com>
> > > Sent: Tuesday, June 9, 2020 3:32 PM
> > >
> > > i.MX common clock drivers may support module build, so it is NOT
> > > selected by default, for ARCH_MXC ARMv7 platforms, need to select it
> > > manually to make build pass.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> >
> > Can't the original def_xxx work?
> >
> > config MXC_CLK
> >         tristate
> >         def_tristate ARCH_MXC
> 
> Such change will make MXC_CLK=y even all i.MX8 SoCs clock drivers are selected
> as module, so it does NOT meet the requirement of module support. Below is
> from .config when all
> i.MX8 SoCs clock drivers are configured to module.
> 
>  CONFIG_MXC_CLK=y
>  CONFIG_MXC_CLK_SCU=m
>  CONFIG_CLK_IMX8MM=m
>  CONFIG_CLK_IMX8MN=m
>  CONFIG_CLK_IMX8MP=m
>  CONFIG_CLK_IMX8MQ=m
>  CONFIG_CLK_IMX8QXP=m
> 

It works at my side.
Below is my changes based on your patchset:
$ git diff
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 47b10d20f411..e7d7b90e2cf8 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -4,7 +4,6 @@ menuconfig ARCH_MXC
        depends on ARCH_MULTI_V4_V5 || ARCH_MULTI_V6_V7 || ARM_SINGLE_ARMV7M
        select ARCH_SUPPORTS_BIG_ENDIAN
        select CLKSRC_IMX_GPT
-       select MXC_CLK
        select GENERIC_IRQ_CHIP
        select GPIOLIB
        select PINCTRL
diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index 26cedbfe386c..f7b3e3a2cb9f 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -3,6 +3,7 @@
 config MXC_CLK
        tristate "IMX clock"
        depends on ARCH_MXC
+       def_tristate ARCH_MXC
 
 config MXC_CLK_SCU
        tristate "IMX SCU clock"

Regards
Aisheng

> Anson

RE: [PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC

[Anson Huang]

> -----Original Message-----
> From: Aisheng Dong <aisheng.dong@nxp.com>
> Sent: 2020年6月18日 11:09
> To: Anson Huang <anson.huang@nxp.com>; linux@armlinux.org.uk;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; mturquette@baylibre.com; sboyd@kernel.org;
> oleksandr.suvorov@toradex.com; Stefan Agner <stefan.agner@toradex.com>;
> arnd@arndb.de; Abel Vesa <abel.vesa@nxp.com>; Peng Fan
> <peng.fan@nxp.com>; tglx@linutronix.de; allison@lohutok.net;
> gregkh@linuxfoundation.org; info@metux.net; Leonard Crestez
> <leonard.crestez@nxp.com>; Andy Duan <fugang.duan@nxp.com>; Daniel
> Baluta <daniel.baluta@nxp.com>; yuehaibing@huawei.com;
> sfr@canb.auug.org.au; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; linux-clk@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC
> 
> > From: Anson Huang <anson.huang@nxp.com>
> > Sent: Wednesday, June 17, 2020 8:36 PM
> >
> > > Subject: RE: [PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC
> > >
> > > > From: Anson Huang <Anson.Huang@nxp.com>
> > > > Sent: Tuesday, June 9, 2020 3:32 PM
> > > >
> > > > i.MX common clock drivers may support module build, so it is NOT
> > > > selected by default, for ARCH_MXC ARMv7 platforms, need to select
> > > > it manually to make build pass.
> > > >
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > >
> > > Can't the original def_xxx work?
> > >
> > > config MXC_CLK
> > >         tristate
> > >         def_tristate ARCH_MXC
> >
> > Such change will make MXC_CLK=y even all i.MX8 SoCs clock drivers are
> > selected as module, so it does NOT meet the requirement of module
> > support. Below is from .config when all
> > i.MX8 SoCs clock drivers are configured to module.
> >
> >  CONFIG_MXC_CLK=y
> >  CONFIG_MXC_CLK_SCU=m
> >  CONFIG_CLK_IMX8MM=m
> >  CONFIG_CLK_IMX8MN=m
> >  CONFIG_CLK_IMX8MP=m
> >  CONFIG_CLK_IMX8MQ=m
> >  CONFIG_CLK_IMX8QXP=m
> >
> 
> It works at my side.
> Below is my changes based on your patchset:
> $ git diff
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig index
> 47b10d20f411..e7d7b90e2cf8 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -4,7 +4,6 @@ menuconfig ARCH_MXC
>         depends on ARCH_MULTI_V4_V5 || ARCH_MULTI_V6_V7 ||
> ARM_SINGLE_ARMV7M
>         select ARCH_SUPPORTS_BIG_ENDIAN
>         select CLKSRC_IMX_GPT
> -       select MXC_CLK
>         select GENERIC_IRQ_CHIP
>         select GPIOLIB
>         select PINCTRL
> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> 26cedbfe386c..f7b3e3a2cb9f 100644
> --- a/drivers/clk/imx/Kconfig
> +++ b/drivers/clk/imx/Kconfig
> @@ -3,6 +3,7 @@
>  config MXC_CLK
>         tristate "IMX clock"
>         depends on ARCH_MXC
> +       def_tristate ARCH_MXC
> 
>  config MXC_CLK_SCU
>         tristate "IMX SCU clock"
> 

I guess you tried imx_v6_v7_defconfig? It does NOT work for ARM64 defconfig when we try to make
CONFIG_MXC_CLK=m, Below are my change, you can see in .config, even all i.MX8 SoCs clock drivers
are configured to module, the CONFIG_MXC_CLK is still =y, but the expected result is =m.

BTW, all i.MX8 SoCs select MXC_CLK in their kconfig, this patch just does the same thing for i.MX6/7
in common place.

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 47b10d2..e7d7b90 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -4,7 +4,6 @@ menuconfig ARCH_MXC
        depends on ARCH_MULTI_V4_V5 || ARCH_MULTI_V6_V7 || ARM_SINGLE_ARMV7M
        select ARCH_SUPPORTS_BIG_ENDIAN
        select CLKSRC_IMX_GPT
-       select MXC_CLK
        select GENERIC_IRQ_CHIP
        select GPIOLIB
        select PINCTRL
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 8222e4b..21e2dbb 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -799,11 +799,11 @@ CONFIG_COMMON_CLK_S2MPS11=y
 CONFIG_COMMON_CLK_PWM=y
 CONFIG_COMMON_CLK_VC5=y
 CONFIG_CLK_RASPBERRYPI=m
-CONFIG_CLK_IMX8MM=y
-CONFIG_CLK_IMX8MN=y
-CONFIG_CLK_IMX8MP=y
-CONFIG_CLK_IMX8MQ=y
-CONFIG_CLK_IMX8QXP=y
+CONFIG_CLK_IMX8MM=m
+CONFIG_CLK_IMX8MN=m
+CONFIG_CLK_IMX8MP=m
+CONFIG_CLK_IMX8MQ=m
+CONFIG_CLK_IMX8QXP=m
 CONFIG_TI_SCI_CLK=y
 CONFIG_COMMON_CLK_QCOM=y
 CONFIG_QCOM_A53PLL=y
diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index 26cedbf..f7b3e3a 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -3,6 +3,7 @@
 config MXC_CLK
        tristate "IMX clock"
        depends on ARCH_MXC
+       def_tristate ARCH_MXC

 config MXC_CLK_SCU
        tristate "IMX SCU clock"

.config:
 CONFIG_MXC_CLK=y
 CONFIG_MXC_CLK_SCU=m
 CONFIG_CLK_IMX8MM=m
 CONFIG_CLK_IMX8MN=m
 CONFIG_CLK_IMX8MP=m
 CONFIG_CLK_IMX8MQ=m
 CONFIG_CLK_IMX8QXP=m

Anson

RE: [PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC

[Aisheng Dong]

> From: Anson Huang <anson.huang@nxp.com>
> Sent: Thursday, June 18, 2020 11:18 AM
> 
> > From: Aisheng Dong <aisheng.dong@nxp.com>
> > Sent: 2020年6月18日 11:09
> >
> > > From: Anson Huang <anson.huang@nxp.com>
> > > Sent: Wednesday, June 17, 2020 8:36 PM
> > >
> > > > Subject: RE: [PATCH V2 2/9] ARM: imx: Select MXC_CLK for ARCH_MXC
> > > >
> > > > > From: Anson Huang <Anson.Huang@nxp.com>
> > > > > Sent: Tuesday, June 9, 2020 3:32 PM
> > > > >
> > > > > i.MX common clock drivers may support module build, so it is NOT
> > > > > selected by default, for ARCH_MXC ARMv7 platforms, need to
> > > > > select it manually to make build pass.
> > > > >
> > > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > >
> > > > Can't the original def_xxx work?
> > > >
> > > > config MXC_CLK
> > > >         tristate
> > > >         def_tristate ARCH_MXC
> > >
> > > Such change will make MXC_CLK=y even all i.MX8 SoCs clock drivers
> > > are selected as module, so it does NOT meet the requirement of
> > > module support. Below is from .config when all
> > > i.MX8 SoCs clock drivers are configured to module.
> > >
> > >  CONFIG_MXC_CLK=y
> > >  CONFIG_MXC_CLK_SCU=m
> > >  CONFIG_CLK_IMX8MM=m
> > >  CONFIG_CLK_IMX8MN=m
> > >  CONFIG_CLK_IMX8MP=m
> > >  CONFIG_CLK_IMX8MQ=m
> > >  CONFIG_CLK_IMX8QXP=m
> > >
> >
> > It works at my side.
> > Below is my changes based on your patchset:
> > $ git diff
> > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> > index
> > 47b10d20f411..e7d7b90e2cf8 100644
> > --- a/arch/arm/mach-imx/Kconfig
> > +++ b/arch/arm/mach-imx/Kconfig
> > @@ -4,7 +4,6 @@ menuconfig ARCH_MXC
> >         depends on ARCH_MULTI_V4_V5 || ARCH_MULTI_V6_V7 ||
> > ARM_SINGLE_ARMV7M
> >         select ARCH_SUPPORTS_BIG_ENDIAN
> >         select CLKSRC_IMX_GPT
> > -       select MXC_CLK
> >         select GENERIC_IRQ_CHIP
> >         select GPIOLIB
> >         select PINCTRL
> > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> > 26cedbfe386c..f7b3e3a2cb9f 100644
> > --- a/drivers/clk/imx/Kconfig
> > +++ b/drivers/clk/imx/Kconfig
> > @@ -3,6 +3,7 @@
> >  config MXC_CLK
> >         tristate "IMX clock"
> >         depends on ARCH_MXC
> > +       def_tristate ARCH_MXC
> >
> >  config MXC_CLK_SCU
> >         tristate "IMX SCU clock"
> >
> 
> I guess you tried imx_v6_v7_defconfig? It does NOT work for ARM64 defconfig
> when we try to make CONFIG_MXC_CLK=m, Below are my change, you can see
> in .config, even all i.MX8 SoCs clock drivers are configured to module, the
> CONFIG_MXC_CLK is still =y, but the expected result is =m.
> 

It works at my side because it can manually selected as m or add CONFIG_MXC_CLK=m
In defconfig.
But now I got your point you want CONFIG_MXC_CLK default to m once define
CONFIG_CLK_IMX8X=m in defconfig.

> BTW, all i.MX8 SoCs select MXC_CLK in their kconfig, this patch just does the
> same thing for i.MX6/7 in common place.
> 

I just noticed for MX6&7, actually CONFIG_MXC_CLK can't be m otherwise we will meet build break.
That means CONFIG_MXC_CLK cannot be configurable by users for non-ARM64 platforms.
Thus can't use def_tristate and we still need select it under ARCH_MXC.
This lightly lose a bit meaning to make this core library as module.

IMHO I'd still prefer to builtin those core libraries rather than convert to module.

Regards
Aisheng

> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig index
> 47b10d2..e7d7b90 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -4,7 +4,6 @@ menuconfig ARCH_MXC
>         depends on ARCH_MULTI_V4_V5 || ARCH_MULTI_V6_V7 ||
> ARM_SINGLE_ARMV7M
>         select ARCH_SUPPORTS_BIG_ENDIAN
>         select CLKSRC_IMX_GPT
> -       select MXC_CLK
>         select GENERIC_IRQ_CHIP
>         select GPIOLIB
>         select PINCTRL
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index
> 8222e4b..21e2dbb 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -799,11 +799,11 @@ CONFIG_COMMON_CLK_S2MPS11=y
> CONFIG_COMMON_CLK_PWM=y  CONFIG_COMMON_CLK_VC5=y
> CONFIG_CLK_RASPBERRYPI=m -CONFIG_CLK_IMX8MM=y
> -CONFIG_CLK_IMX8MN=y -CONFIG_CLK_IMX8MP=y -CONFIG_CLK_IMX8MQ=y
> -CONFIG_CLK_IMX8QXP=y
> +CONFIG_CLK_IMX8MM=m
> +CONFIG_CLK_IMX8MN=m
> +CONFIG_CLK_IMX8MP=m
> +CONFIG_CLK_IMX8MQ=m
> +CONFIG_CLK_IMX8QXP=m
>  CONFIG_TI_SCI_CLK=y
>  CONFIG_COMMON_CLK_QCOM=y
>  CONFIG_QCOM_A53PLL=y
> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> 26cedbf..f7b3e3a 100644
> --- a/drivers/clk/imx/Kconfig
> +++ b/drivers/clk/imx/Kconfig
> @@ -3,6 +3,7 @@
>  config MXC_CLK
>         tristate "IMX clock"
>         depends on ARCH_MXC
> +       def_tristate ARCH_MXC
> 
>  config MXC_CLK_SCU
>         tristate "IMX SCU clock"
> 
> .config:
>  CONFIG_MXC_CLK=y
>  CONFIG_MXC_CLK_SCU=m
>  CONFIG_CLK_IMX8MM=m
>  CONFIG_CLK_IMX8MN=m
>  CONFIG_CLK_IMX8MP=m
>  CONFIG_CLK_IMX8MQ=m
>  CONFIG_CLK_IMX8QXP=m
> 
> Anson

Re: [PATCH V2 1/9] clk: composite: Export clk_hw_register_composite()

[Stephen Boyd]

Quoting Anson Huang (2020-06-09 00:32:05)
> Export clk_hw_register_composite() to support user built as module.
> 
> ERROR: modpost: "clk_hw_register_composite" [drivers/clk/imx/mxc-clk.ko] undefined!

Get rid of these messages below. We don't care that make modules failed.

> scripts/Makefile.modpost:111: recipe for target 'Module.symvers' failed
> make[1]: *** [Module.symvers] Error 1
> make[1]: *** Deleting file 'Module.symvers'
> Makefile:1384: recipe for target 'modules' failed
> make: *** [modules] Error 2
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Otherwise

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Stephen Boyd]

Quoting Aisheng Dong (2020-06-17 18:58:51)
> > From: Anson Huang <anson.huang@nxp.com>
> > > > +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> > >
> > > Like i.MX pinctrl, I'm not sure if it's really necessary to build core
> > > libraries as modules. Probably the simplest way is only building
> > > platform drivers part as module. And leave those core libraries built in kernel.
> > > This may make the code a bit cleaner.
> > >
> > 
> > Will discuss this with Linaro guys about it, previous requirement I received is all
> > SoC specific modules need to be built as module.
> > 
> 
> Okay. AFAIK it's not conflict.
> You still make drivers into modules.
> Only difference is for those common libraries part, we don't convert them into module
> Which is less meaningless.
>  

What is the benefit of making the core part of the SoC driver not a
module? From the module perspective it should be perfectly fine to make
it a module as well, and then depmod will sort out loading modules in
the right order.

This is for android right?

Re: [PATCH V2 4/9] clk: imx: Support building i.MX common clock driver as module

[Stephen Boyd]

Quoting Anson Huang (2020-06-09 00:32:08)
> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
> index 1af8cff..1291f9b 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -1,25 +1,10 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
> -obj-$(CONFIG_MXC_CLK) += \
> -       clk.o \
> -       clk-busy.o \
> -       clk-composite-8m.o \
> -       clk-cpu.o \
> -       clk-composite-7ulp.o \
> -       clk-divider-gate.o \
> -       clk-fixup-div.o \
> -       clk-fixup-mux.o \
> -       clk-frac-pll.o \
> -       clk-gate-exclusive.o \
> -       clk-gate2.o \
> -       clk-pfd.o \
> -       clk-pfdv2.o \
> -       clk-pllv1.o \
> -       clk-pllv2.o \
> -       clk-pllv3.o \
> -       clk-pllv4.o \
> -       clk-sscg-pll.o \
> -       clk-pll14xx.o
> +mxc-clk-objs += clk.o clk-busy.o clk-composite-8m.o clk-cpu.o clk-composite-7ulp.o \
> +               clk-divider-gate.o clk-fixup-div.o clk-fixup-mux.o clk-frac-pll.o \
> +               clk-gate-exclusive.o clk-gate2.o clk-pfd.o clk-pfdv2.o clk-pllv1.o \
> +               clk-pllv2.o clk-pllv3.o clk-pllv4.o clk-sscg-pll.o clk-pll14xx.o
> +obj-$(CONFIG_MXC_CLK) += mxc-clk.o

I enjoyed it when the files were all on their own line. Can you keep it
like that and just add mxc-clk-objs += to all the lines and remove the
trailing slash?

>  
>  mxc-clk-scu-objs += clk-scu.o clk-lpcg-scu.o
>  obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
> index d2b5af8..73e064b 100644
> --- a/drivers/clk/imx/clk-composite-8m.c
> +++ b/drivers/clk/imx/clk-composite-8m.c
> @@ -243,3 +243,4 @@ struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
>         kfree(mux);
>         return ERR_CAST(hw);
>  }
> +EXPORT_SYMBOL_GPL(imx8m_clk_hw_composite_flags);

Are all these files including <linux/export.h>? Because they should
unless they're using something from <linux/module.h> like
MODULE_LICENSE().

> diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c
> index 87ab8db..cc894b5 100644
> --- a/drivers/clk/imx/clk.c
> +++ b/drivers/clk/imx/clk.c
> @@ -143,16 +148,24 @@ void imx_cscmr1_fixup(u32 *val)
>  static int imx_keep_uart_clocks;
>  static struct clk ** const *imx_uart_clocks;
>  
> -static int __init imx_keep_uart_clocks_param(char *str)
> +static int __maybe_unused imx_keep_uart_clocks_param(char *str)
>  {
>         imx_keep_uart_clocks = 1;
>  
>         return 0;
>  }
> +
> +#ifdef MODULE
> +__setup_param("earlycon", imx_keep_uart_earlycon,
> +             imx_keep_uart_clocks_param);
> +__setup_param("earlyprintk", imx_keep_uart_earlyprintk,
> +             imx_keep_uart_clocks_param);

Why not fix __setup_param() to take the same number of arguments in
either case? That macro looks broken.

> +#else
>  __setup_param("earlycon", imx_keep_uart_earlycon,
>               imx_keep_uart_clocks_param, 0);
>  __setup_param("earlyprintk", imx_keep_uart_earlyprintk,
>               imx_keep_uart_clocks_param, 0);
> +#endif
>  
>  void imx_register_uart_clocks(struct clk ** const clks[])
>  {

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Aisheng Dong]

> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Saturday, June 20, 2020 11:28 AM
> Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as
> module
> 
> Quoting Aisheng Dong (2020-06-17 18:58:51)
> > > From: Anson Huang <anson.huang@nxp.com>
> > > > > +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> > > >
> > > > Like i.MX pinctrl, I'm not sure if it's really necessary to build
> > > > core libraries as modules. Probably the simplest way is only
> > > > building platform drivers part as module. And leave those core libraries
> built in kernel.
> > > > This may make the code a bit cleaner.
> > > >
> > >
> > > Will discuss this with Linaro guys about it, previous requirement I
> > > received is all SoC specific modules need to be built as module.
> > >
> >
> > Okay. AFAIK it's not conflict.
> > You still make drivers into modules.
> > Only difference is for those common libraries part, we don't convert
> > them into module Which is less meaningless.
> >
> 
> What is the benefit of making the core part of the SoC driver not a module?

Usually we could try to build it as module if it's not hard.

One question is sometimes those core part are shared with some platforms which can't built as module.
For i.MX case, it's mainly patch 4:
[V2,4/9] clk: imx: Support building i.MX common clock driver as module
https://patchwork.kernel.org/patch/11594801/

Those libraries are also used by i.MX6&7 which can't build as module.
So we need an extra workaround patch to forcely 'select' it under arch/arm/mach-imx/Kconfig
[V2,2/9] ARM: imx: Select MXC_CLK for ARCH_MXC
https://patchwork.kernel.org/patch/11594793/
Then the users can't configure it as module in order to not break build.

If build-in those common libraries, the implementation could be a bit easier and cleaner.
So I'm not sure if we still have to build them as module.
How would you suggest for such case?

> From the module perspective it should be perfectly fine to make it a module as
> well, and then depmod will sort out loading modules in the right order.
> 
> This is for android right?

Yes, for Android GKI support.

Regards
Aisheng

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Stephen Boyd]

Quoting Aisheng Dong (2020-06-22 20:42:19)
> > From: Stephen Boyd <sboyd@kernel.org>
> > Sent: Saturday, June 20, 2020 11:28 AM
> > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as
> > module
> > 
> > Quoting Aisheng Dong (2020-06-17 18:58:51)
> > > > From: Anson Huang <anson.huang@nxp.com>
> > > > > > +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> > > > >
> > > > > Like i.MX pinctrl, I'm not sure if it's really necessary to build
> > > > > core libraries as modules. Probably the simplest way is only
> > > > > building platform drivers part as module. And leave those core libraries
> > built in kernel.
> > > > > This may make the code a bit cleaner.
> > > > >
> > > >
> > > > Will discuss this with Linaro guys about it, previous requirement I
> > > > received is all SoC specific modules need to be built as module.
> > > >
> > >
> > > Okay. AFAIK it's not conflict.
> > > You still make drivers into modules.
> > > Only difference is for those common libraries part, we don't convert
> > > them into module Which is less meaningless.
> > >
> > 
> > What is the benefit of making the core part of the SoC driver not a module?
> 
> Usually we could try to build it as module if it's not hard.
> 
> One question is sometimes those core part are shared with some platforms which can't built as module.
> For i.MX case, it's mainly patch 4:
> [V2,4/9] clk: imx: Support building i.MX common clock driver as module
> https://patchwork.kernel.org/patch/11594801/
> 
> Those libraries are also used by i.MX6&7 which can't build as module.
> So we need an extra workaround patch to forcely 'select' it under arch/arm/mach-imx/Kconfig
> [V2,2/9] ARM: imx: Select MXC_CLK for ARCH_MXC
> https://patchwork.kernel.org/patch/11594793/
> Then the users can't configure it as module in order to not break build.
> 
> If build-in those common libraries, the implementation could be a bit easier and cleaner.
> So I'm not sure if we still have to build them as module.
> How would you suggest for such case?

Stop using 'select MXC_CLK' when requiring the core library code?
Instead, make it a 'depends' and then that will make depending modules
(i.e. the SoC files) that want to be builtin force the core module to be
builtin too. Other modular configs that depend on the core will still be
modular. 

I don't know why an architecture is selecting the clk code at all to be
honest. That can be moved to the defconfig instead of in the
architecture Kconfig and then you don't get a working system unless you
select the MXC_CLK config from the configurator tool (menuconfig,
nconfig, etc.) So ARCH_MXC shouldn't be in this discussion and the core
module should be selectable by the configurator and that should be
tristate and all SoC modules should depend on that core library module
and be selectable too and those Kconfigs can be tristate or bool.

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Aisheng Dong]

> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Tuesday, June 23, 2020 4:34 PM
> Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as
> module
> 
> Quoting Aisheng Dong (2020-06-22 20:42:19)
> > > From: Stephen Boyd <sboyd@kernel.org>
> > > Sent: Saturday, June 20, 2020 11:28 AM
> > > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock
> > > driver as module
> > >
> > > Quoting Aisheng Dong (2020-06-17 18:58:51)
> > > > > From: Anson Huang <anson.huang@nxp.com>
> > > > > > > +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> > > > > >
> > > > > > Like i.MX pinctrl, I'm not sure if it's really necessary to
> > > > > > build core libraries as modules. Probably the simplest way is
> > > > > > only building platform drivers part as module. And leave those
> > > > > > core libraries
> > > built in kernel.
> > > > > > This may make the code a bit cleaner.
> > > > > >
> > > > >
> > > > > Will discuss this with Linaro guys about it, previous
> > > > > requirement I received is all SoC specific modules need to be built as
> module.
> > > > >
> > > >
> > > > Okay. AFAIK it's not conflict.
> > > > You still make drivers into modules.
> > > > Only difference is for those common libraries part, we don't
> > > > convert them into module Which is less meaningless.
> > > >
> > >
> > > What is the benefit of making the core part of the SoC driver not a module?
> >
> > Usually we could try to build it as module if it's not hard.
> >
> > One question is sometimes those core part are shared with some platforms
> which can't built as module.
> > For i.MX case, it's mainly patch 4:
> > [V2,4/9] clk: imx: Support building i.MX common clock driver as module
> >
> >
> > Those libraries are also used by i.MX6&7 which can't build as module.
> > So we need an extra workaround patch to forcely 'select' it under
> > arch/arm/mach-imx/Kconfig [V2,2/9] ARM: imx: Select MXC_CLK for
> > ARCH_MXC
> > Then the users can't configure it as module in order to not break build.
> >
> > If build-in those common libraries, the implementation could be a bit easier
> and cleaner.
> > So I'm not sure if we still have to build them as module.
> > How would you suggest for such case?
> 
> Stop using 'select MXC_CLK' when requiring the core library code?
> Instead, make it a 'depends' and then that will make depending modules (i.e. the
> SoC files) that want to be builtin force the core module to be builtin too. Other
> modular configs that depend on the core will still be modular.
> 

It seems not work.
Patch 4 already changes it to depend on ARCH_MXC which can only be 'Y'.
[V2,4/9] clk: imx: Support building i.MX common clock driver as module
diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index ded0643..678113b 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -1,8 +1,8 @@ 
 # SPDX-License-Identifier: GPL-2.0
 # common clock support for NXP i.MX SoC family.
 config MXC_CLK
-	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.

Looks like only select it in arch Kconfig in Patch 2, there will be no issue.
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index e7d7b90..47b10d2 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -4,6 +4,7 @@  menuconfig ARCH_MXC
 	depends on ARCH_MULTI_V4_V5 || ARCH_MULTI_V6_V7 || ARM_SINGLE_ARMV7M
 	select ARCH_SUPPORTS_BIG_ENDIAN
 	select CLKSRC_IMX_GPT
+	select MXC_CLK
 	select GENERIC_IRQ_CHIP
 	select GPIOLIB
 	select PINCTRL

> I don't know why an architecture is selecting the clk code at all to be honest.
> That can be moved to the defconfig instead of in the architecture Kconfig and
> then you don't get a working system unless you select the MXC_CLK config from
> the configurator tool (menuconfig, nconfig, etc.) So ARCH_MXC shouldn't be in
> this discussion and the core module should be selectable by the configurator
> and that should be tristate and all SoC modules should depend on that core
> library module and be selectable too and those Kconfigs can be tristate or bool.

I guess it is because we didn't have requirements to make minimum booting required
drivers to be built as module before.

Regards
Aisheng

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Stephen Boyd]

Quoting Aisheng Dong (2020-06-23 02:00:47)
> > From: Stephen Boyd <sboyd@kernel.org>
> > Sent: Tuesday, June 23, 2020 4:34 PM
> > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as
> > module
> > 
> > Quoting Aisheng Dong (2020-06-22 20:42:19)
> > > > From: Stephen Boyd <sboyd@kernel.org>
> > > > Sent: Saturday, June 20, 2020 11:28 AM
> > > > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock
> > > > driver as module
> > > >
> > > > Quoting Aisheng Dong (2020-06-17 18:58:51)
> > > > > > From: Anson Huang <anson.huang@nxp.com>
> > > > > > > > +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> > > > > > >
> > > > > > > Like i.MX pinctrl, I'm not sure if it's really necessary to
> > > > > > > build core libraries as modules. Probably the simplest way is
> > > > > > > only building platform drivers part as module. And leave those
> > > > > > > core libraries
> > > > built in kernel.
> > > > > > > This may make the code a bit cleaner.
> > > > > > >
> > > > > >
> > > > > > Will discuss this with Linaro guys about it, previous
> > > > > > requirement I received is all SoC specific modules need to be built as
> > module.
> > > > > >
> > > > >
> > > > > Okay. AFAIK it's not conflict.
> > > > > You still make drivers into modules.
> > > > > Only difference is for those common libraries part, we don't
> > > > > convert them into module Which is less meaningless.
> > > > >
> > > >
> > > > What is the benefit of making the core part of the SoC driver not a module?
> > >
> > > Usually we could try to build it as module if it's not hard.
> > >
> > > One question is sometimes those core part are shared with some platforms
> > which can't built as module.
> > > For i.MX case, it's mainly patch 4:
> > > [V2,4/9] clk: imx: Support building i.MX common clock driver as module
> > >
> > >
> > > Those libraries are also used by i.MX6&7 which can't build as module.
> > > So we need an extra workaround patch to forcely 'select' it under
> > > arch/arm/mach-imx/Kconfig [V2,2/9] ARM: imx: Select MXC_CLK for
> > > ARCH_MXC
> > > Then the users can't configure it as module in order to not break build.
> > >
> > > If build-in those common libraries, the implementation could be a bit easier
> > and cleaner.
> > > So I'm not sure if we still have to build them as module.
> > > How would you suggest for such case?
> > 
> > Stop using 'select MXC_CLK' when requiring the core library code?
> > Instead, make it a 'depends' and then that will make depending modules (i.e. the
> > SoC files) that want to be builtin force the core module to be builtin too. Other
> > modular configs that depend on the core will still be modular.
> > 
> 
> It seems not work.
> Patch 4 already changes it to depend on ARCH_MXC which can only be 'Y'.
> [V2,4/9] clk: imx: Support building i.MX common clock driver as module
> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
> index ded0643..678113b 100644
> --- a/drivers/clk/imx/Kconfig
> +++ b/drivers/clk/imx/Kconfig
> @@ -1,8 +1,8 @@ 
>  # SPDX-License-Identifier: GPL-2.0
>  # common clock support for NXP i.MX SoC family.
>  config MXC_CLK
> -       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? Why does ARCH_MXC being enabled mandate that it
is builtin? Is some architecture level code calling into the clk
driver?

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Aisheng Dong]

> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Wednesday, June 24, 2020 8:58 AM
> Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as
> module
> 
> Quoting Aisheng Dong (2020-06-23 02:00:47)
> > > From: Stephen Boyd <sboyd@kernel.org>
> > > Sent: Tuesday, June 23, 2020 4:34 PM
> > > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock
> > > driver as module
> > >
> > > Quoting Aisheng Dong (2020-06-22 20:42:19)
> > > > > From: Stephen Boyd <sboyd@kernel.org>
> > > > > Sent: Saturday, June 20, 2020 11:28 AM
> > > > > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock
> > > > > driver as module
> > > > >
> > > > > Quoting Aisheng Dong (2020-06-17 18:58:51)
> > > > > > > From: Anson Huang <anson.huang@nxp.com>
> > > > > > > > > +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> > > > > > > >
> > > > > > > > Like i.MX pinctrl, I'm not sure if it's really necessary
> > > > > > > > to build core libraries as modules. Probably the simplest
> > > > > > > > way is only building platform drivers part as module. And
> > > > > > > > leave those core libraries
> > > > > built in kernel.
> > > > > > > > This may make the code a bit cleaner.
> > > > > > > >
> > > > > > >
> > > > > > > Will discuss this with Linaro guys about it, previous
> > > > > > > requirement I received is all SoC specific modules need to
> > > > > > > be built as
> > > module.
> > > > > > >
> > > > > >
> > > > > > Okay. AFAIK it's not conflict.
> > > > > > You still make drivers into modules.
> > > > > > Only difference is for those common libraries part, we don't
> > > > > > convert them into module Which is less meaningless.
> > > > > >
> > > > >
> > > > > What is the benefit of making the core part of the SoC driver not a
> module?
> > > >
> > > > Usually we could try to build it as module if it's not hard.
> > > >
> > > > One question is sometimes those core part are shared with some
> > > > platforms
> > > which can't built as module.
> > > > For i.MX case, it's mainly patch 4:
> > > > [V2,4/9] clk: imx: Support building i.MX common clock driver as
> > > > module
> > > >
> > > >
> > > > Those libraries are also used by i.MX6&7 which can't build as module.
> > > > So we need an extra workaround patch to forcely 'select' it under
> > > > arch/arm/mach-imx/Kconfig [V2,2/9] ARM: imx: Select MXC_CLK for
> > > > ARCH_MXC Then the users can't configure it as module in order to
> > > > not break build.
> > > >
> > > > If build-in those common libraries, the implementation could be a
> > > > bit easier
> > > and cleaner.
> > > > So I'm not sure if we still have to build them as module.
> > > > How would you suggest for such case?
> > >
> > > Stop using 'select MXC_CLK' when requiring the core library code?
> > > Instead, make it a 'depends' and then that will make depending
> > > modules (i.e. the SoC files) that want to be builtin force the core
> > > module to be builtin too. Other modular configs that depend on the core will
> still be modular.
> > >
> >
> > It seems not work.
> > Patch 4 already changes it to depend on ARCH_MXC which can only be 'Y'.
> > [V2,4/9] clk: imx: Support building i.MX common clock driver as module
> > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> > ded0643..678113b 100644
> > --- a/drivers/clk/imx/Kconfig
> > +++ b/drivers/clk/imx/Kconfig
> > @@ -1,8 +1,8 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  # common clock support for NXP i.MX SoC family.
> >  config MXC_CLK
> > -       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.

Regards
Aisheng

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Anson Huang]

> Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as
> module
> 
> > From: Stephen Boyd <sboyd@kernel.org>
> > Sent: Wednesday, June 24, 2020 8:58 AM
> > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock
> > driver as module
> >
> > Quoting Aisheng Dong (2020-06-23 02:00:47)
> > > > From: Stephen Boyd <sboyd@kernel.org>
> > > > Sent: Tuesday, June 23, 2020 4:34 PM
> > > > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock
> > > > driver as module
> > > >
> > > > Quoting Aisheng Dong (2020-06-22 20:42:19)
> > > > > > From: Stephen Boyd <sboyd@kernel.org>
> > > > > > Sent: Saturday, June 20, 2020 11:28 AM
> > > > > > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU
> > > > > > clock driver as module
> > > > > >
> > > > > > Quoting Aisheng Dong (2020-06-17 18:58:51)
> > > > > > > > From: Anson Huang <anson.huang@nxp.com>
> > > > > > > > > > +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> > > > > > > > >
> > > > > > > > > Like i.MX pinctrl, I'm not sure if it's really necessary
> > > > > > > > > to build core libraries as modules. Probably the
> > > > > > > > > simplest way is only building platform drivers part as
> > > > > > > > > module. And leave those core libraries
> > > > > > built in kernel.
> > > > > > > > > This may make the code a bit cleaner.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Will discuss this with Linaro guys about it, previous
> > > > > > > > requirement I received is all SoC specific modules need to
> > > > > > > > be built as
> > > > module.
> > > > > > > >
> > > > > > >
> > > > > > > Okay. AFAIK it's not conflict.
> > > > > > > You still make drivers into modules.
> > > > > > > Only difference is for those common libraries part, we don't
> > > > > > > convert them into module Which is less meaningless.
> > > > > > >
> > > > > >
> > > > > > What is the benefit of making the core part of the SoC driver
> > > > > > not a
> > module?
> > > > >
> > > > > Usually we could try to build it as module if it's not hard.
> > > > >
> > > > > One question is sometimes those core part are shared with some
> > > > > platforms
> > > > which can't built as module.
> > > > > For i.MX case, it's mainly patch 4:
> > > > > [V2,4/9] clk: imx: Support building i.MX common clock driver as
> > > > > module
> > > > >
> > > > >
> > > > > Those libraries are also used by i.MX6&7 which can't build as module.
> > > > > So we need an extra workaround patch to forcely 'select' it
> > > > > under arch/arm/mach-imx/Kconfig [V2,2/9] ARM: imx: Select
> > > > > MXC_CLK for ARCH_MXC Then the users can't configure it as module
> > > > > in order to not break build.
> > > > >
> > > > > If build-in those common libraries, the implementation could be
> > > > > a bit easier
> > > > and cleaner.
> > > > > So I'm not sure if we still have to build them as module.
> > > > > How would you suggest for such case?
> > > >
> > > > Stop using 'select MXC_CLK' when requiring the core library code?
> > > > Instead, make it a 'depends' and then that will make depending
> > > > modules (i.e. the SoC files) that want to be builtin force the
> > > > core module to be builtin too. Other modular configs that depend
> > > > on the core will
> > still be modular.
> > > >
> > >
> > > It seems not work.
> > > Patch 4 already changes it to depend on ARCH_MXC which can only be 'Y'.
> > > [V2,4/9] clk: imx: Support building i.MX common clock driver as
> > > module diff --git a/drivers/clk/imx/Kconfig
> > > b/drivers/clk/imx/Kconfig index ded0643..678113b 100644
> > > --- a/drivers/clk/imx/Kconfig
> > > +++ b/drivers/clk/imx/Kconfig
> > > @@ -1,8 +1,8 @@
> > >  # SPDX-License-Identifier: GPL-2.0
> > >  # common clock support for NXP i.MX SoC family.
> > >  config MXC_CLK
> > > -       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?

Anson

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Aisheng Dong]

> From: Anson Huang <anson.huang@nxp.com>
> Sent: Wednesday, June 24, 2020 10:36 AM
> 
> > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock
> > driver as module
> >
> > > From: Stephen Boyd <sboyd@kernel.org>
> > > Sent: Wednesday, June 24, 2020 8:58 AM
> > > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock
> > > driver as module
> > >
> > > Quoting Aisheng Dong (2020-06-23 02:00:47)
> > > > > From: Stephen Boyd <sboyd@kernel.org>
> > > > > Sent: Tuesday, June 23, 2020 4:34 PM
> > > > > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock
> > > > > driver as module
> > > > >
> > > > > Quoting Aisheng Dong (2020-06-22 20:42:19)
> > > > > > > From: Stephen Boyd <sboyd@kernel.org>
> > > > > > > Sent: Saturday, June 20, 2020 11:28 AM
> > > > > > > Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU
> > > > > > > clock driver as module
> > > > > > >
> > > > > > > Quoting Aisheng Dong (2020-06-17 18:58:51)
> > > > > > > > > From: Anson Huang <anson.huang@nxp.com>
> > > > > > > > > > > +obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o
> > > > > > > > > >
> > > > > > > > > > Like i.MX pinctrl, I'm not sure if it's really
> > > > > > > > > > necessary to build core libraries as modules. Probably
> > > > > > > > > > the simplest way is only building platform drivers
> > > > > > > > > > part as module. And leave those core libraries
> > > > > > > built in kernel.
> > > > > > > > > > This may make the code a bit cleaner.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Will discuss this with Linaro guys about it, previous
> > > > > > > > > requirement I received is all SoC specific modules need
> > > > > > > > > to be built as
> > > > > module.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Okay. AFAIK it's not conflict.
> > > > > > > > You still make drivers into modules.
> > > > > > > > Only difference is for those common libraries part, we
> > > > > > > > don't convert them into module Which is less meaningless.
> > > > > > > >
> > > > > > >
> > > > > > > What is the benefit of making the core part of the SoC
> > > > > > > driver not a
> > > module?
> > > > > >
> > > > > > Usually we could try to build it as module if it's not hard.
> > > > > >
> > > > > > One question is sometimes those core part are shared with some
> > > > > > platforms
> > > > > which can't built as module.
> > > > > > For i.MX case, it's mainly patch 4:
> > > > > > [V2,4/9] clk: imx: Support building i.MX common clock driver
> > > > > > as module
> > > > > >
> > > > > >
> > > > > > Those libraries are also used by i.MX6&7 which can't build as module.
> > > > > > So we need an extra workaround patch to forcely 'select' it
> > > > > > under arch/arm/mach-imx/Kconfig [V2,2/9] ARM: imx: Select
> > > > > > MXC_CLK for ARCH_MXC Then the users can't configure it as
> > > > > > module in order to not break build.
> > > > > >
> > > > > > If build-in those common libraries, the implementation could
> > > > > > be a bit easier
> > > > > and cleaner.
> > > > > > So I'm not sure if we still have to build them as module.
> > > > > > How would you suggest for such case?
> > > > >
> > > > > Stop using 'select MXC_CLK' when requiring the core library code?
> > > > > Instead, make it a 'depends' and then that will make depending
> > > > > modules (i.e. the SoC files) that want to be builtin force the
> > > > > core module to be builtin too. Other modular configs that depend
> > > > > on the core will
> > > still be modular.
> > > > >
> > > >
> > > > It seems not work.
> > > > Patch 4 already changes it to depend on ARCH_MXC which can only be 'Y'.
> > > > [V2,4/9] clk: imx: Support building i.MX common clock driver as
> > > > module diff --git a/drivers/clk/imx/Kconfig
> > > > b/drivers/clk/imx/Kconfig index ded0643..678113b 100644
> > > > --- a/drivers/clk/imx/Kconfig
> > > > +++ b/drivers/clk/imx/Kconfig
> > > > @@ -1,8 +1,8 @@
> > > >  # SPDX-License-Identifier: GPL-2.0  # common clock support for
> > > > NXP i.MX SoC family.
> > > >  config MXC_CLK
> > > > -       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.

Regards
Aisheng

> Anson

Re: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Arnd Bergmann]

On Wed, Jun 24, 2020 at 4:19 AM Aisheng Dong <aisheng.dong@nxp.com> wrote:
> > 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.

The link error can be easily avoided by building all the clk support into
a single loadable module like below.

Hower this only works if all drivers that have a runtime dependency
on the clk driver support deferred probing or are built as loadable
modules as well and get loaded after the clk driver.

     Arnd

8<---
diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 928f874c73d2..638bc00f5731 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0

-obj-$(CONFIG_MXC_CLK) += \
+obj-$(CONFIG_MXC_CLK) := clk-imx.ko
+
+clk-imx-y += \
        clk.o \
        clk-busy.o \
        clk-composite-8m.o \
@@ -25,24 +27,24 @@ obj-$(CONFIG_MXC_CLK_SCU) += \
        clk-scu.o \
        clk-lpcg-scu.o

-obj-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o
-obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o
-obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o
-obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o
-obj-$(CONFIG_CLK_IMX8QXP) += clk-imx8qxp.o clk-imx8qxp-lpcg.o
+clk-imx-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o
+clk-imx-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o
+clk-imx-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o
+clk-imx-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o
+clk-imx-$(CONFIG_CLK_IMX8QXP) += clk-imx8qxp.o clk-imx8qxp-lpcg.o

-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
+clk-imx-$(CONFIG_SOC_IMX1)   += clk-imx1.o
+clk-imx-$(CONFIG_SOC_IMX21)  += clk-imx21.o
+clk-imx-$(CONFIG_SOC_IMX25)  += clk-imx25.o
+clk-imx-$(CONFIG_SOC_IMX27)  += clk-imx27.o
+clk-imx-$(CONFIG_SOC_IMX31)  += clk-imx31.o
+clk-imx-$(CONFIG_SOC_IMX35)  += clk-imx35.o
+clk-imx-$(CONFIG_SOC_IMX5)   += clk-imx5.o
+clk-imx-$(CONFIG_SOC_IMX6Q)  += clk-imx6q.o
+clk-imx-$(CONFIG_SOC_IMX6SL) += clk-imx6sl.o
+clk-imx-$(CONFIG_SOC_IMX6SLL) += clk-imx6sll.o
+clk-imx-$(CONFIG_SOC_IMX6SX) += clk-imx6sx.o
+clk-imx-$(CONFIG_SOC_IMX6UL) += clk-imx6ul.o
+clk-imx-$(CONFIG_SOC_IMX7D)  += clk-imx7d.o
+clk-imx-$(CONFIG_SOC_IMX7ULP) += clk-imx7ulp.o
+clk-imx-$(CONFIG_SOC_VF610)  += clk-vf610.o

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Aisheng Dong]

> From: Arnd Bergmann <arnd@arndb.de>
> Sent: Wednesday, June 24, 2020 3:47 PM
> 
> On Wed, Jun 24, 2020 at 4:19 AM Aisheng Dong <aisheng.dong@nxp.com>
> wrote:
> > > 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.
> 
> The link error can be easily avoided by building all the clk support into a single
> loadable module like below.
> 
> Hower this only works if all drivers that have a runtime dependency on the clk
> driver support deferred probing or are built as loadable modules as well and get
> loaded after the clk driver.
> 

Thanks for the info.
Currently all i.MX6&7/VF clocks driver are not loadable modules (they're using CLK_OF_DECLARE),
so can't be built as m.

If we really want to build i.MX common clock libraries into module, I guess the easiest way
Is as what Patch [2/9] does, select it by ARCH_MXC.
Then users have no chance to build it as module, so no build issues.

Regards
Aisheng

>      Arnd
> 
> 8<---
> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index
> 928f874c73d2..638bc00f5731 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -1,6 +1,8 @@
>  # SPDX-License-Identifier: GPL-2.0
> 
> -obj-$(CONFIG_MXC_CLK) += \
> +obj-$(CONFIG_MXC_CLK) := clk-imx.ko
> +
> +clk-imx-y += \
>         clk.o \
>         clk-busy.o \
>         clk-composite-8m.o \
> @@ -25,24 +27,24 @@ obj-$(CONFIG_MXC_CLK_SCU) += \
>         clk-scu.o \
>         clk-lpcg-scu.o
> 
> -obj-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o
> -obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o
> -obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o
> -obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o
> -obj-$(CONFIG_CLK_IMX8QXP) += clk-imx8qxp.o clk-imx8qxp-lpcg.o
> +clk-imx-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o
> +clk-imx-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o
> +clk-imx-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o
> +clk-imx-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o
> +clk-imx-$(CONFIG_CLK_IMX8QXP) += clk-imx8qxp.o clk-imx8qxp-lpcg.o
> 
> -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
> +clk-imx-$(CONFIG_SOC_IMX1)   += clk-imx1.o
> +clk-imx-$(CONFIG_SOC_IMX21)  += clk-imx21.o
> +clk-imx-$(CONFIG_SOC_IMX25)  += clk-imx25.o
> +clk-imx-$(CONFIG_SOC_IMX27)  += clk-imx27.o
> +clk-imx-$(CONFIG_SOC_IMX31)  += clk-imx31.o
> +clk-imx-$(CONFIG_SOC_IMX35)  += clk-imx35.o
> +clk-imx-$(CONFIG_SOC_IMX5)   += clk-imx5.o
> +clk-imx-$(CONFIG_SOC_IMX6Q)  += clk-imx6q.o
> +clk-imx-$(CONFIG_SOC_IMX6SL) += clk-imx6sl.o
> +clk-imx-$(CONFIG_SOC_IMX6SLL) += clk-imx6sll.o
> +clk-imx-$(CONFIG_SOC_IMX6SX) += clk-imx6sx.o
> +clk-imx-$(CONFIG_SOC_IMX6UL) += clk-imx6ul.o
> +clk-imx-$(CONFIG_SOC_IMX7D)  += clk-imx7d.o
> +clk-imx-$(CONFIG_SOC_IMX7ULP) += clk-imx7ulp.o
> +clk-imx-$(CONFIG_SOC_VF610)  += clk-vf610.o

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Stephen Boyd]

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.

Re: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Dong Aisheng]

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

Re: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Arnd Bergmann]

On Mon, Jun 29, 2020 at 9:18 AM Dong Aisheng <dongas86@gmail.com> wrote:
> On Thu, Jun 25, 2020 at 6:43 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > Quoting Aisheng Dong (2020-06-23 19:59:09)
> > 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.

I would make that

config CLK_IMX6Q
         bool "Clock driver for NXP i.MX6Q"
         depends on SOC_IMX6Q || COMPILE_TEST
         default SOC_IMX6Q
         select MXC_CLK

> 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,

Changing them can be a one or two patches, that's totally
worth it IMHO.

I really don't like the 'select' in arch/arm/mach-imx/Kconfig: if
you've done the work to make the imx8 clk driver modular,
I would expect to see the same at least tried for the other
ones.

For the clk drivers that cannot yet be 'tristate' because of the
CLK_OF_DECLARE(), can you include a list of drivers
that depend on the clocks being available during early
boot?

       Arnd

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Aisheng Dong]

> From: Arnd Bergmann <arnd@arndb.de>
> Sent: Monday, June 29, 2020 4:20 PM
> 
> On Mon, Jun 29, 2020 at 9:18 AM Dong Aisheng <dongas86@gmail.com>
> wrote:
> > On Thu, Jun 25, 2020 at 6:43 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > > Quoting Aisheng Dong (2020-06-23 19:59:09) 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.
> 
> I would make that
> 
> config CLK_IMX6Q
>          bool "Clock driver for NXP i.MX6Q"
>          depends on SOC_IMX6Q || COMPILE_TEST
>          default SOC_IMX6Q
>          select MXC_CLK
> 

Yes, this seems better.
Anson, pls follow this way.

> > 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,
> 
> Changing them can be a one or two patches, that's totally worth it IMHO.
> 
> I really don't like the 'select' in arch/arm/mach-imx/Kconfig: if you've done the
> work to make the imx8 clk driver modular, I would expect to see the same at
> least tried for the other ones.
> 

Got it.

> For the clk drivers that cannot yet be 'tristate' because of the
> CLK_OF_DECLARE(), can you include a list of drivers that depend on the clocks
> being available during early boot?
> 

I guess we can find out them one by one for those 15 platforms when converting them
to modules as well. Currently we're converting ARM64 clock drivers first.

Regards
Aisheng

>        Arnd

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Anson Huang]

> Subject: RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as
> module
> 
> > From: Arnd Bergmann <arnd@arndb.de>
> > Sent: Monday, June 29, 2020 4:20 PM
> >
> > On Mon, Jun 29, 2020 at 9:18 AM Dong Aisheng <dongas86@gmail.com>
> > wrote:
> > > On Thu, Jun 25, 2020 at 6:43 AM Stephen Boyd <sboyd@kernel.org>
> wrote:
> > > > Quoting Aisheng Dong (2020-06-23 19:59:09) 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.
> >
> > I would make that
> >
> > config CLK_IMX6Q
> >          bool "Clock driver for NXP i.MX6Q"
> >          depends on SOC_IMX6Q || COMPILE_TEST
> >          default SOC_IMX6Q
> >          select MXC_CLK
> >
> 
> Yes, this seems better.
> Anson, pls follow this way.

In V4 patch series, I will add a patch to introduce CLK_IMX6Q and other i.MX6/7
clock config following this way.

Anson

RE: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as module

[Anson Huang]

Hi, Arnd/Stephen

> Subject: Re: [PATCH V2 3/9] clk: imx: Support building SCU clock driver as
> module
> 
> Quoting Arnd Bergmann (2020-06-29 01:19:44)
> > On Mon, Jun 29, 2020 at 9:18 AM Dong Aisheng <dongas86@gmail.com>
> wrote:
> > > On Thu, Jun 25, 2020 at 6:43 AM Stephen Boyd <sboyd@kernel.org>
> wrote:
> > > > Quoting Aisheng Dong (2020-06-23 19:59:09) 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.
> >
> > I would make that
> >
> > config CLK_IMX6Q
> >          bool "Clock driver for NXP i.MX6Q"
> >          depends on SOC_IMX6Q || COMPILE_TEST
> >          default SOC_IMX6Q
> >          select MXC_CLK
> 
> Yes, do this.

The COMPILE_TEST existing on the config will introduce some build error
on other ARCH's allyesconfig build on i.MX2/3 platforms, I received some build error report.
So in next patch series, I will drop the COMPILE_TEST for these new added clock configurations,
as I think it is NOT worth to support the COMPILE_TEST for these old legacy platforms which are
NOT supported before.

Thanks,
Anson