2020-07-02 02:18:29

by Anson Huang

[permalink] [raw]
Subject: [PATCH V4 0/5] Support building i.MX8 SoCs clock driver as module

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, i.MX8QXP has linked the common scu/lpcg clock driver into
its clock drivers.

Since i.MX common clk driver will support module build and no longer selected
by default, so for i.MX ARMv7 platforms, add clock configuration for each SoC
and they are selected by default if the corresponding SoC config is selected.

Changes since V3:
- merge all i.MX8M SoCs patches together;
- add module author/description;
- drop __setup_param() change, it is ONLY needed for built-in version,
so just add #if check for module build;
- link common scu and lpcg clock into i.MX8QXP clock driver, then export
is no needed for common scu/lpcg clock driver.

Anson Huang (5):
clk: composite: Export clk_hw_register_composite()
clk: imx: Add clock configuration for ARMv7 platforms
clk: imx: Support building i.MX common clock driver as module
clk: imx8m: Support module build
clk: imx8qxp: Support building i.MX8QXP clock driver as module

drivers/clk/clk-composite.c | 1 +
drivers/clk/imx/Kconfig | 112 ++++++++++++++++++++++++++++++++++---
drivers/clk/imx/Makefile | 79 +++++++++++++-------------
drivers/clk/imx/clk-composite-8m.c | 2 +
drivers/clk/imx/clk-cpu.c | 2 +
drivers/clk/imx/clk-frac-pll.c | 2 +
drivers/clk/imx/clk-gate2.c | 2 +
drivers/clk/imx/clk-imx8mm.c | 4 ++
drivers/clk/imx/clk-imx8mn.c | 4 ++
drivers/clk/imx/clk-imx8mp.c | 4 ++
drivers/clk/imx/clk-imx8mq.c | 4 ++
drivers/clk/imx/clk-imx8qxp-lpcg.c | 10 +++-
drivers/clk/imx/clk-imx8qxp.c | 11 +++-
drivers/clk/imx/clk-pll14xx.c | 5 ++
drivers/clk/imx/clk-sscg-pll.c | 2 +
drivers/clk/imx/clk.c | 22 ++++++--
16 files changed, 209 insertions(+), 57 deletions(-)

--
2.7.4


2020-07-02 02:18:36

by Anson Huang

[permalink] [raw]
Subject: [PATCH V4 2/5] clk: imx: Add clock configuration for ARMv7 platforms

Add CONFIG_CLK_xxx for i.MX ARMv7 platforms, and use it as build option
instead of CONFIG_SOC_xxx, the CONFIG_CLK_xxx will be selected by default
according to CONFIG_SOC_xxx.

Signed-off-by: Anson Huang <[email protected]>
---
Changes since V3:
- Add clock configuration for each ARMv7.
---
drivers/clk/imx/Kconfig | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/imx/Makefile | 30 ++++++++--------
2 files changed, 105 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index db0253f..09fc8ad 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -8,6 +8,96 @@ config MXC_CLK_SCU
bool
depends on IMX_SCU

+config CLK_IMX1
+ bool "IMX1 CCM Clock Driver"
+ depends on SOC_IMX1 || COMPILE_TEST
+ default SOC_IMX1
+ select MXC_CLK
+
+config CLK_IMX21
+ bool "IMX21 CCM Clock Driver"
+ depends on SOC_IMX21 || COMPILE_TEST
+ default SOC_IMX21
+ select MXC_CLK
+
+config CLK_IMX25
+ bool "IMX25 CCM Clock Driver"
+ depends on SOC_IMX25 || COMPILE_TEST
+ default SOC_IMX25
+ select MXC_CLK
+
+config CLK_IMX27
+ bool "IMX27 CCM Clock Driver"
+ depends on SOC_IMX27 || COMPILE_TEST
+ default SOC_IMX27
+ select MXC_CLK
+
+config CLK_IMX31
+ bool "IMX31 CCM Clock Driver"
+ depends on SOC_IMX31 || COMPILE_TEST
+ default SOC_IMX31
+ select MXC_CLK
+
+config CLK_IMX35
+ bool "IMX35 CCM Clock Driver"
+ depends on SOC_IMX35 || COMPILE_TEST
+ default SOC_IMX35
+ select MXC_CLK
+
+config CLK_IMX5
+ bool "IMX5 CCM Clock Driver"
+ depends on SOC_IMX5 || COMPILE_TEST
+ default SOC_IMX5
+ select MXC_CLK
+
+config CLK_IMX6Q
+ bool "IMX6Q CCM Clock Driver"
+ depends on SOC_IMX6Q || COMPILE_TEST
+ default SOC_IMX6Q
+ select MXC_CLK
+
+config CLK_IMX6SL
+ bool "IMX6SL CCM Clock Driver"
+ depends on SOC_IMX6SL || COMPILE_TEST
+ default SOC_IMX6SL
+ select MXC_CLK
+
+config CLK_IMX6SLL
+ bool "IMX6SLL CCM Clock Driver"
+ depends on SOC_IMX6SLL || COMPILE_TEST
+ default SOC_IMX6SLL
+ select MXC_CLK
+
+config CLK_IMX6SX
+ bool "IMX6SX CCM Clock Driver"
+ depends on SOC_IMX6SX || COMPILE_TEST
+ default SOC_IMX6SX
+ select MXC_CLK
+
+config CLK_IMX6UL
+ bool "IMX6UL CCM Clock Driver"
+ depends on SOC_IMX6UL || COMPILE_TEST
+ default SOC_IMX6UL
+ select MXC_CLK
+
+config CLK_IMX7D
+ bool "IMX7D CCM Clock Driver"
+ depends on SOC_IMX7D || COMPILE_TEST
+ default SOC_IMX7D
+ select MXC_CLK
+
+config CLK_IMX7ULP
+ bool "IMX7ULP Clock Driver"
+ depends on SOC_IMX7ULP || COMPILE_TEST
+ default SOC_IMX7ULP
+ select MXC_CLK
+
+config CLK_VF610
+ bool "VF610 Clock Driver"
+ depends on SOC_VF610 || COMPILE_TEST
+ default SOC_VF610
+ select MXC_CLK
+
config CLK_IMX8MM
bool "IMX8MM CCM Clock Driver"
depends on ARCH_MXC
diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 928f874..394ade7 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -31,18 +31,18 @@ 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

-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
+obj-$(CONFIG_CLK_IMX1) += clk-imx1.o
+obj-$(CONFIG_CLK_IMX21) += clk-imx21.o
+obj-$(CONFIG_CLK_IMX25) += clk-imx25.o
+obj-$(CONFIG_CLK_IMX27) += clk-imx27.o
+obj-$(CONFIG_CLK_IMX31) += clk-imx31.o
+obj-$(CONFIG_CLK_IMX35) += clk-imx35.o
+obj-$(CONFIG_CLK_IMX5) += clk-imx5.o
+obj-$(CONFIG_CLK_IMX6Q) += clk-imx6q.o
+obj-$(CONFIG_CLK_IMX6SL) += clk-imx6sl.o
+obj-$(CONFIG_CLK_IMX6SLL) += clk-imx6sll.o
+obj-$(CONFIG_CLK_IMX6SX) += clk-imx6sx.o
+obj-$(CONFIG_CLK_IMX6UL) += clk-imx6ul.o
+obj-$(CONFIG_CLK_IMX7D) += clk-imx7d.o
+obj-$(CONFIG_CLK_IMX7ULP) += clk-imx7ulp.o
+obj-$(CONFIG_CLK_VF610) += clk-vf610.o
--
2.7.4

2020-07-02 02:18:37

by Anson Huang

[permalink] [raw]
Subject: [PATCH V4 4/5] clk: imx8m: Support module build

Change configuration to "tristate", add module author, description
and license to support building i.MX8M SoCs clock driver as module.

Signed-off-by: Anson Huang <[email protected]>
---
Changes since V3:
- add module author/description, and merge all i.MX8M SoCs patch into
one patch.
---
drivers/clk/imx/Kconfig | 8 ++++----
drivers/clk/imx/clk-imx8mm.c | 4 ++++
drivers/clk/imx/clk-imx8mn.c | 4 ++++
drivers/clk/imx/clk-imx8mp.c | 4 ++++
drivers/clk/imx/clk-imx8mq.c | 4 ++++
5 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index f6ddf76..1867111 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -99,28 +99,28 @@ config CLK_VF610
select MXC_CLK

config CLK_IMX8MM
- bool "IMX8MM CCM Clock Driver"
+ tristate "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"
+ tristate "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"
+ tristate "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"
+ tristate "IMX8MQ 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..0de0be0 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -657,3 +657,7 @@ static struct platform_driver imx8mm_clk_driver = {
},
};
module_platform_driver(imx8mm_clk_driver);
+
+MODULE_AUTHOR("Bai Ping <[email protected]>");
+MODULE_DESCRIPTION("NXP i.MX8MM clock driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index 213cc37..e984de5 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -608,3 +608,7 @@ static struct platform_driver imx8mn_clk_driver = {
},
};
module_platform_driver(imx8mn_clk_driver);
+
+MODULE_AUTHOR("Anson Huang <[email protected]>");
+MODULE_DESCRIPTION("NXP i.MX8MN clock driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index ca74771..f3cedf2 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -773,3 +773,7 @@ static struct platform_driver imx8mp_clk_driver = {
},
};
module_platform_driver(imx8mp_clk_driver);
+
+MODULE_AUTHOR("Anson Huang <[email protected]>");
+MODULE_DESCRIPTION("NXP i.MX8MP clock driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index a64aace..a06cc21 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -643,3 +643,7 @@ static struct platform_driver imx8mq_clk_driver = {
},
};
module_platform_driver(imx8mq_clk_driver);
+
+MODULE_AUTHOR("Abel Vesa <[email protected]>");
+MODULE_DESCRIPTION("NXP i.MX8MQ clock driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2020-07-02 02:19:03

by Anson Huang

[permalink] [raw]
Subject: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module

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 <[email protected]>
---
Changes since V3:
- ONLY include __setup_param() build for built-in, module build no need
to have it.
---
drivers/clk/imx/Kconfig | 8 ++++++--
drivers/clk/imx/Makefile | 40 +++++++++++++++++++-------------------
drivers/clk/imx/clk-composite-8m.c | 2 ++
drivers/clk/imx/clk-cpu.c | 2 ++
drivers/clk/imx/clk-frac-pll.c | 2 ++
drivers/clk/imx/clk-gate2.c | 2 ++
drivers/clk/imx/clk-pll14xx.c | 5 +++++
drivers/clk/imx/clk-sscg-pll.c | 2 ++
drivers/clk/imx/clk.c | 22 +++++++++++++++------
9 files changed, 57 insertions(+), 28 deletions(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index 09fc8ad..f6ddf76 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
bool
@@ -101,24 +101,28 @@ config CLK_VF610
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 394ade7..17f5d12 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -1,25 +1,25 @@
# 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
+mxc-clk-objs += clk-busy.o
+mxc-clk-objs += clk-composite-7ulp.o
+mxc-clk-objs += clk-composite-8m.o
+mxc-clk-objs += clk-cpu.o
+mxc-clk-objs += clk-divider-gate.o
+mxc-clk-objs += clk-fixup-div.o
+mxc-clk-objs += clk-fixup-mux.o
+mxc-clk-objs += clk-frac-pll.o
+mxc-clk-objs += clk-gate2.o
+mxc-clk-objs += clk-gate-exclusive.o
+mxc-clk-objs += clk-pfd.o
+mxc-clk-objs += clk-pfdv2.o
+mxc-clk-objs += clk-pllv1.o
+mxc-clk-objs += clk-pllv2.o
+mxc-clk-objs += clk-pllv3.o
+mxc-clk-objs += clk-pllv4.o
+mxc-clk-objs += clk-pll14xx.o
+mxc-clk-objs += clk-sscg-pll.o
+obj-$(CONFIG_MXC_CLK) += mxc-clk.o

obj-$(CONFIG_MXC_CLK_SCU) += \
clk-scu.o \
diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
index d2b5af8..78fb7e5 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -5,6 +5,7 @@

#include <linux/clk-provider.h>
#include <linux/errno.h>
+#include <linux/export.h>
#include <linux/io.h>
#include <linux/slab.h>

@@ -243,3 +244,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..cb6ca4c 100644
--- a/drivers/clk/imx/clk-cpu.c
+++ b/drivers/clk/imx/clk-cpu.c
@@ -5,6 +5,7 @@

#include <linux/clk.h>
#include <linux/clk-provider.h>
+#include <linux/export.h>
#include <linux/slab.h>
#include "clk.h"

@@ -104,3 +105,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..c703056 100644
--- a/drivers/clk/imx/clk-frac-pll.c
+++ b/drivers/clk/imx/clk-frac-pll.c
@@ -10,6 +10,7 @@

#include <linux/clk-provider.h>
#include <linux/err.h>
+#include <linux/export.h>
#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/slab.h>
@@ -233,3 +234,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..512f675 100644
--- a/drivers/clk/imx/clk-gate2.c
+++ b/drivers/clk/imx/clk-gate2.c
@@ -7,6 +7,7 @@
*/

#include <linux/clk-provider.h>
+#include <linux/export.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/io.h>
@@ -177,3 +178,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..f5c3e7e 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -6,6 +6,7 @@
#include <linux/bitops.h>
#include <linux/clk-provider.h>
#include <linux/err.h>
+#include <linux/export.h>
#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/slab.h>
@@ -68,6 +69,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 +77,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 +440,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..9d6cdff 100644
--- a/drivers/clk/imx/clk-sscg-pll.c
+++ b/drivers/clk/imx/clk-sscg-pll.c
@@ -10,6 +10,7 @@

#include <linux/clk-provider.h>
#include <linux/err.h>
+#include <linux/export.h>
#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/slab.h>
@@ -537,3 +538,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..6f2a94d 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,18 @@ 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;
}
+#ifndef MODULE
__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 +171,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 +185,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

2020-07-02 02:19:35

by Anson Huang

[permalink] [raw]
Subject: [PATCH V4 1/5] clk: composite: Export clk_hw_register_composite()

Export clk_hw_register_composite() to support user built as module.

ERROR: modpost: "clk_hw_register_composite" [drivers/clk/imx/mxc-clk.ko] undefined!

Signed-off-by: Anson Huang <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
---
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

2020-07-02 02:20:36

by Anson Huang

[permalink] [raw]
Subject: [PATCH V4 5/5] clk: imx8qxp: Support building i.MX8QXP clock driver as module

Change configuration to "tristate", use device_initcall() instead
of builtin_platform_driver(), add module author, description and
license to support building i.MX8QXP clock drivers as module.

Signed-off-by: Anson Huang <[email protected]>
---
Changes since V3:
- use device_initcall() instead of builtin_platform_driver();
- add module author/description;
- link common scu/lpcg clock driver to i.MX8QXP clock driver, then
no need to have exports.
---
drivers/clk/imx/Kconfig | 6 +++---
drivers/clk/imx/Makefile | 9 ++++-----
drivers/clk/imx/clk-imx8qxp-lpcg.c | 10 +++++++++-
drivers/clk/imx/clk-imx8qxp.c | 11 ++++++++++-
4 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index 1867111..8340dfe 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -5,8 +5,8 @@ config MXC_CLK
depends on ARCH_MXC

config MXC_CLK_SCU
- bool
- depends on IMX_SCU
+ tristate "IMX SCU clock"
+ depends on ARCH_MXC && IMX_SCU

config CLK_IMX1
bool "IMX1 CCM Clock Driver"
@@ -127,7 +127,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/Makefile b/drivers/clk/imx/Makefile
index 17f5d12..79e53f2 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -21,15 +21,14 @@ mxc-clk-objs += clk-pll14xx.o
mxc-clk-objs += clk-sscg-pll.o
obj-$(CONFIG_MXC_CLK) += mxc-clk.o

-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
+
+obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o
+clk-imx-scu-$(CONFIG_CLK_IMX8QXP) += clk-scu.o clk-imx8qxp.o
+clk-imx-lpcg-scu-$(CONFIG_CLK_IMX8QXP) += clk-lpcg-scu.o clk-imx8qxp-lpcg.o

obj-$(CONFIG_CLK_IMX1) += clk-imx1.o
obj-$(CONFIG_CLK_IMX21) += clk-imx21.o
diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
index 04c8ee3..5b6648e 100644
--- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
+++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
@@ -231,4 +231,12 @@ static struct platform_driver imx8qxp_lpcg_clk_driver = {
.probe = imx8qxp_lpcg_clk_probe,
};

-builtin_platform_driver(imx8qxp_lpcg_clk_driver);
+static int __init imx8qxp_lpcg_clk_init(void)
+{
+ return platform_driver_register(&imx8qxp_lpcg_clk_driver);
+}
+device_initcall(imx8qxp_lpcg_clk_init);
+
+MODULE_AUTHOR("Aisheng Dong <[email protected]>");
+MODULE_DESCRIPTION("NXP i.MX8QXP LPCG clock driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
index 5e2903e..9bcf0d1 100644
--- a/drivers/clk/imx/clk-imx8qxp.c
+++ b/drivers/clk/imx/clk-imx8qxp.c
@@ -151,4 +151,13 @@ static struct platform_driver imx8qxp_clk_driver = {
},
.probe = imx8qxp_clk_probe,
};
-builtin_platform_driver(imx8qxp_clk_driver);
+
+static int __init imx8qxp_clk_init(void)
+{
+ return platform_driver_register(&imx8qxp_clk_driver);
+}
+device_initcall(imx8qxp_clk_init);
+
+MODULE_AUTHOR("Aisheng Dong <[email protected]>");
+MODULE_DESCRIPTION("NXP i.MX8QXP clock driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2020-07-02 02:40:01

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH V4 2/5] clk: imx: Add clock configuration for ARMv7 platforms

> From: Anson Huang <[email protected]>
> Sent: Thursday, July 2, 2020 10:15 AM
>
> Add CONFIG_CLK_xxx for i.MX ARMv7 platforms, and use it as build option
> instead of CONFIG_SOC_xxx, the CONFIG_CLK_xxx will be selected by default
> according to CONFIG_SOC_xxx.
>
> Signed-off-by: Anson Huang <[email protected]>

Reviewed-by: Dong Aisheng <[email protected]>

Regards
Aisheng

> ---
> Changes since V3:
> - Add clock configuration for each ARMv7.
> ---
> drivers/clk/imx/Kconfig | 90
> ++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/clk/imx/Makefile | 30 ++++++++--------
> 2 files changed, 105 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> db0253f..09fc8ad 100644
> --- a/drivers/clk/imx/Kconfig
> +++ b/drivers/clk/imx/Kconfig
> @@ -8,6 +8,96 @@ config MXC_CLK_SCU
> bool
> depends on IMX_SCU
>
> +config CLK_IMX1
> + bool "IMX1 CCM Clock Driver"
> + depends on SOC_IMX1 || COMPILE_TEST
> + default SOC_IMX1
> + select MXC_CLK
> +
> +config CLK_IMX21
> + bool "IMX21 CCM Clock Driver"
> + depends on SOC_IMX21 || COMPILE_TEST
> + default SOC_IMX21
> + select MXC_CLK
> +
> +config CLK_IMX25
> + bool "IMX25 CCM Clock Driver"
> + depends on SOC_IMX25 || COMPILE_TEST
> + default SOC_IMX25
> + select MXC_CLK
> +
> +config CLK_IMX27
> + bool "IMX27 CCM Clock Driver"
> + depends on SOC_IMX27 || COMPILE_TEST
> + default SOC_IMX27
> + select MXC_CLK
> +
> +config CLK_IMX31
> + bool "IMX31 CCM Clock Driver"
> + depends on SOC_IMX31 || COMPILE_TEST
> + default SOC_IMX31
> + select MXC_CLK
> +
> +config CLK_IMX35
> + bool "IMX35 CCM Clock Driver"
> + depends on SOC_IMX35 || COMPILE_TEST
> + default SOC_IMX35
> + select MXC_CLK
> +
> +config CLK_IMX5
> + bool "IMX5 CCM Clock Driver"
> + depends on SOC_IMX5 || COMPILE_TEST
> + default SOC_IMX5
> + select MXC_CLK
> +
> +config CLK_IMX6Q
> + bool "IMX6Q CCM Clock Driver"
> + depends on SOC_IMX6Q || COMPILE_TEST
> + default SOC_IMX6Q
> + select MXC_CLK
> +
> +config CLK_IMX6SL
> + bool "IMX6SL CCM Clock Driver"
> + depends on SOC_IMX6SL || COMPILE_TEST
> + default SOC_IMX6SL
> + select MXC_CLK
> +
> +config CLK_IMX6SLL
> + bool "IMX6SLL CCM Clock Driver"
> + depends on SOC_IMX6SLL || COMPILE_TEST
> + default SOC_IMX6SLL
> + select MXC_CLK
> +
> +config CLK_IMX6SX
> + bool "IMX6SX CCM Clock Driver"
> + depends on SOC_IMX6SX || COMPILE_TEST
> + default SOC_IMX6SX
> + select MXC_CLK
> +
> +config CLK_IMX6UL
> + bool "IMX6UL CCM Clock Driver"
> + depends on SOC_IMX6UL || COMPILE_TEST
> + default SOC_IMX6UL
> + select MXC_CLK
> +
> +config CLK_IMX7D
> + bool "IMX7D CCM Clock Driver"
> + depends on SOC_IMX7D || COMPILE_TEST
> + default SOC_IMX7D
> + select MXC_CLK
> +
> +config CLK_IMX7ULP
> + bool "IMX7ULP Clock Driver"
> + depends on SOC_IMX7ULP || COMPILE_TEST
> + default SOC_IMX7ULP
> + select MXC_CLK
> +
> +config CLK_VF610
> + bool "VF610 Clock Driver"
> + depends on SOC_VF610 || COMPILE_TEST
> + default SOC_VF610
> + select MXC_CLK
> +
> config CLK_IMX8MM
> bool "IMX8MM CCM Clock Driver"
> depends on ARCH_MXC
> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index
> 928f874..394ade7 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -31,18 +31,18 @@ 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
>
> -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
> +obj-$(CONFIG_CLK_IMX1) += clk-imx1.o
> +obj-$(CONFIG_CLK_IMX21) += clk-imx21.o
> +obj-$(CONFIG_CLK_IMX25) += clk-imx25.o
> +obj-$(CONFIG_CLK_IMX27) += clk-imx27.o
> +obj-$(CONFIG_CLK_IMX31) += clk-imx31.o
> +obj-$(CONFIG_CLK_IMX35) += clk-imx35.o
> +obj-$(CONFIG_CLK_IMX5) += clk-imx5.o
> +obj-$(CONFIG_CLK_IMX6Q) += clk-imx6q.o
> +obj-$(CONFIG_CLK_IMX6SL) += clk-imx6sl.o
> +obj-$(CONFIG_CLK_IMX6SLL) += clk-imx6sll.o
> +obj-$(CONFIG_CLK_IMX6SX) += clk-imx6sx.o
> +obj-$(CONFIG_CLK_IMX6UL) += clk-imx6ul.o
> +obj-$(CONFIG_CLK_IMX7D) += clk-imx7d.o
> +obj-$(CONFIG_CLK_IMX7ULP) += clk-imx7ulp.o
> +obj-$(CONFIG_CLK_VF610) += clk-vf610.o
> --
> 2.7.4

2020-07-02 03:05:28

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module

On Thu, Jul 2, 2020 at 10:19 AM Anson Huang <[email protected]> wrote:
>
> 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 <[email protected]>
> ---
> Changes since V3:
> - ONLY include __setup_param() build for built-in, module build no need
> to have it.
> ---
> drivers/clk/imx/Kconfig | 8 ++++++--
> drivers/clk/imx/Makefile | 40 +++++++++++++++++++-------------------
> drivers/clk/imx/clk-composite-8m.c | 2 ++
> drivers/clk/imx/clk-cpu.c | 2 ++
> drivers/clk/imx/clk-frac-pll.c | 2 ++
> drivers/clk/imx/clk-gate2.c | 2 ++
> drivers/clk/imx/clk-pll14xx.c | 5 +++++
> drivers/clk/imx/clk-sscg-pll.c | 2 ++
> drivers/clk/imx/clk.c | 22 +++++++++++++++------
> 9 files changed, 57 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
> index 09fc8ad..f6ddf76 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
> bool
> @@ -101,24 +101,28 @@ config CLK_VF610
> 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 394ade7..17f5d12 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -1,25 +1,25 @@
> # 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
> +mxc-clk-objs += clk-busy.o
> +mxc-clk-objs += clk-composite-7ulp.o
> +mxc-clk-objs += clk-composite-8m.o
> +mxc-clk-objs += clk-cpu.o
> +mxc-clk-objs += clk-divider-gate.o
> +mxc-clk-objs += clk-fixup-div.o
> +mxc-clk-objs += clk-fixup-mux.o
> +mxc-clk-objs += clk-frac-pll.o
> +mxc-clk-objs += clk-gate2.o
> +mxc-clk-objs += clk-gate-exclusive.o
> +mxc-clk-objs += clk-pfd.o
> +mxc-clk-objs += clk-pfdv2.o
> +mxc-clk-objs += clk-pllv1.o
> +mxc-clk-objs += clk-pllv2.o
> +mxc-clk-objs += clk-pllv3.o
> +mxc-clk-objs += clk-pllv4.o
> +mxc-clk-objs += clk-pll14xx.o
> +mxc-clk-objs += clk-sscg-pll.o
> +obj-$(CONFIG_MXC_CLK) += mxc-clk.o
>
> obj-$(CONFIG_MXC_CLK_SCU) += \
> clk-scu.o \
> diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
> index d2b5af8..78fb7e5 100644
> --- a/drivers/clk/imx/clk-composite-8m.c
> +++ b/drivers/clk/imx/clk-composite-8m.c
> @@ -5,6 +5,7 @@
>
> #include <linux/clk-provider.h>
> #include <linux/errno.h>
> +#include <linux/export.h>
> #include <linux/io.h>
> #include <linux/slab.h>
>
> @@ -243,3 +244,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..cb6ca4c 100644
> --- a/drivers/clk/imx/clk-cpu.c
> +++ b/drivers/clk/imx/clk-cpu.c
> @@ -5,6 +5,7 @@
>
> #include <linux/clk.h>
> #include <linux/clk-provider.h>
> +#include <linux/export.h>
> #include <linux/slab.h>
> #include "clk.h"
>
> @@ -104,3 +105,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..c703056 100644
> --- a/drivers/clk/imx/clk-frac-pll.c
> +++ b/drivers/clk/imx/clk-frac-pll.c
> @@ -10,6 +10,7 @@
>
> #include <linux/clk-provider.h>
> #include <linux/err.h>
> +#include <linux/export.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> #include <linux/slab.h>
> @@ -233,3 +234,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..512f675 100644
> --- a/drivers/clk/imx/clk-gate2.c
> +++ b/drivers/clk/imx/clk-gate2.c
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/clk-provider.h>
> +#include <linux/export.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> #include <linux/io.h>
> @@ -177,3 +178,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..f5c3e7e 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -6,6 +6,7 @@
> #include <linux/bitops.h>
> #include <linux/clk-provider.h>
> #include <linux/err.h>
> +#include <linux/export.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> #include <linux/slab.h>
> @@ -68,6 +69,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 +77,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 +440,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..9d6cdff 100644
> --- a/drivers/clk/imx/clk-sscg-pll.c
> +++ b/drivers/clk/imx/clk-sscg-pll.c
> @@ -10,6 +10,7 @@
>
> #include <linux/clk-provider.h>
> #include <linux/err.h>
> +#include <linux/export.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> #include <linux/slab.h>
> @@ -537,3 +538,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..6f2a94d 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,18 @@ 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;
> }
> +#ifndef MODULE
> __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);

I feel not only the __setup_param, the whole logic of keep_uart_clocks
are not needed for Module case. Is it true?

Regards
Aisheng

> +#endif
>
> void imx_register_uart_clocks(struct clk ** const clks[])
> {
> @@ -164,8 +171,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 +185,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
>

2020-07-02 03:06:17

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH V4 4/5] clk: imx8m: Support module build

On Thu, Jul 2, 2020 at 10:18 AM Anson Huang <[email protected]> wrote:
>
> Change configuration to "tristate", add module author, description
> and license to support building i.MX8M SoCs clock driver as module.
>
> Signed-off-by: Anson Huang <[email protected]>

Reviewed-by: Dong Aisheng <[email protected]>

Regards
Aisheng

> ---
> Changes since V3:
> - add module author/description, and merge all i.MX8M SoCs patch into
> one patch.
> ---
> drivers/clk/imx/Kconfig | 8 ++++----
> drivers/clk/imx/clk-imx8mm.c | 4 ++++
> drivers/clk/imx/clk-imx8mn.c | 4 ++++
> drivers/clk/imx/clk-imx8mp.c | 4 ++++
> drivers/clk/imx/clk-imx8mq.c | 4 ++++
> 5 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
> index f6ddf76..1867111 100644
> --- a/drivers/clk/imx/Kconfig
> +++ b/drivers/clk/imx/Kconfig
> @@ -99,28 +99,28 @@ config CLK_VF610
> select MXC_CLK
>
> config CLK_IMX8MM
> - bool "IMX8MM CCM Clock Driver"
> + tristate "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"
> + tristate "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"
> + tristate "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"
> + tristate "IMX8MQ 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..0de0be0 100644
> --- a/drivers/clk/imx/clk-imx8mm.c
> +++ b/drivers/clk/imx/clk-imx8mm.c
> @@ -657,3 +657,7 @@ static struct platform_driver imx8mm_clk_driver = {
> },
> };
> module_platform_driver(imx8mm_clk_driver);
> +
> +MODULE_AUTHOR("Bai Ping <[email protected]>");
> +MODULE_DESCRIPTION("NXP i.MX8MM clock driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
> index 213cc37..e984de5 100644
> --- a/drivers/clk/imx/clk-imx8mn.c
> +++ b/drivers/clk/imx/clk-imx8mn.c
> @@ -608,3 +608,7 @@ static struct platform_driver imx8mn_clk_driver = {
> },
> };
> module_platform_driver(imx8mn_clk_driver);
> +
> +MODULE_AUTHOR("Anson Huang <[email protected]>");
> +MODULE_DESCRIPTION("NXP i.MX8MN clock driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
> index ca74771..f3cedf2 100644
> --- a/drivers/clk/imx/clk-imx8mp.c
> +++ b/drivers/clk/imx/clk-imx8mp.c
> @@ -773,3 +773,7 @@ static struct platform_driver imx8mp_clk_driver = {
> },
> };
> module_platform_driver(imx8mp_clk_driver);
> +
> +MODULE_AUTHOR("Anson Huang <[email protected]>");
> +MODULE_DESCRIPTION("NXP i.MX8MP clock driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> index a64aace..a06cc21 100644
> --- a/drivers/clk/imx/clk-imx8mq.c
> +++ b/drivers/clk/imx/clk-imx8mq.c
> @@ -643,3 +643,7 @@ static struct platform_driver imx8mq_clk_driver = {
> },
> };
> module_platform_driver(imx8mq_clk_driver);
> +
> +MODULE_AUTHOR("Abel Vesa <[email protected]>");
> +MODULE_DESCRIPTION("NXP i.MX8MQ clock driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>

2020-07-02 03:27:03

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module


> Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock
> driver as module
>
> On Thu, Jul 2, 2020 at 10:19 AM Anson Huang <[email protected]>
> wrote:
> >
> > 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 <[email protected]>
> > ---
> > Changes since V3:
> > - ONLY include __setup_param() build for built-in, module build no
> need
> > to have it.
> > ---
> > drivers/clk/imx/Kconfig | 8 ++++++--
> > drivers/clk/imx/Makefile | 40
> +++++++++++++++++++-------------------
> > drivers/clk/imx/clk-composite-8m.c | 2 ++
> > drivers/clk/imx/clk-cpu.c | 2 ++
> > drivers/clk/imx/clk-frac-pll.c | 2 ++
> > drivers/clk/imx/clk-gate2.c | 2 ++
> > drivers/clk/imx/clk-pll14xx.c | 5 +++++
> > drivers/clk/imx/clk-sscg-pll.c | 2 ++
> > drivers/clk/imx/clk.c | 22 +++++++++++++++------
> > 9 files changed, 57 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> > 09fc8ad..f6ddf76 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
> > bool
> > @@ -101,24 +101,28 @@ config CLK_VF610 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
> > 394ade7..17f5d12 100644
> > --- a/drivers/clk/imx/Makefile
> > +++ b/drivers/clk/imx/Makefile
> > @@ -1,25 +1,25 @@
> > # 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
> > +mxc-clk-objs += clk-busy.o
> > +mxc-clk-objs += clk-composite-7ulp.o
> > +mxc-clk-objs += clk-composite-8m.o
> > +mxc-clk-objs += clk-cpu.o
> > +mxc-clk-objs += clk-divider-gate.o
> > +mxc-clk-objs += clk-fixup-div.o
> > +mxc-clk-objs += clk-fixup-mux.o
> > +mxc-clk-objs += clk-frac-pll.o
> > +mxc-clk-objs += clk-gate2.o
> > +mxc-clk-objs += clk-gate-exclusive.o
> > +mxc-clk-objs += clk-pfd.o
> > +mxc-clk-objs += clk-pfdv2.o
> > +mxc-clk-objs += clk-pllv1.o
> > +mxc-clk-objs += clk-pllv2.o
> > +mxc-clk-objs += clk-pllv3.o
> > +mxc-clk-objs += clk-pllv4.o
> > +mxc-clk-objs += clk-pll14xx.o
> > +mxc-clk-objs += clk-sscg-pll.o
> > +obj-$(CONFIG_MXC_CLK) += mxc-clk.o
> >
> > obj-$(CONFIG_MXC_CLK_SCU) += \
> > clk-scu.o \
> > diff --git a/drivers/clk/imx/clk-composite-8m.c
> > b/drivers/clk/imx/clk-composite-8m.c
> > index d2b5af8..78fb7e5 100644
> > --- a/drivers/clk/imx/clk-composite-8m.c
> > +++ b/drivers/clk/imx/clk-composite-8m.c
> > @@ -5,6 +5,7 @@
> >
> > #include <linux/clk-provider.h>
> > #include <linux/errno.h>
> > +#include <linux/export.h>
> > #include <linux/io.h>
> > #include <linux/slab.h>
> >
> > @@ -243,3 +244,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..cb6ca4c 100644
> > --- a/drivers/clk/imx/clk-cpu.c
> > +++ b/drivers/clk/imx/clk-cpu.c
> > @@ -5,6 +5,7 @@
> >
> > #include <linux/clk.h>
> > #include <linux/clk-provider.h>
> > +#include <linux/export.h>
> > #include <linux/slab.h>
> > #include "clk.h"
> >
> > @@ -104,3 +105,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..c703056 100644
> > --- a/drivers/clk/imx/clk-frac-pll.c
> > +++ b/drivers/clk/imx/clk-frac-pll.c
> > @@ -10,6 +10,7 @@
> >
> > #include <linux/clk-provider.h>
> > #include <linux/err.h>
> > +#include <linux/export.h>
> > #include <linux/io.h>
> > #include <linux/iopoll.h>
> > #include <linux/slab.h>
> > @@ -233,3 +234,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..512f675 100644
> > --- a/drivers/clk/imx/clk-gate2.c
> > +++ b/drivers/clk/imx/clk-gate2.c
> > @@ -7,6 +7,7 @@
> > */
> >
> > #include <linux/clk-provider.h>
> > +#include <linux/export.h>
> > #include <linux/module.h>
> > #include <linux/slab.h>
> > #include <linux/io.h>
> > @@ -177,3 +178,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..f5c3e7e 100644
> > --- a/drivers/clk/imx/clk-pll14xx.c
> > +++ b/drivers/clk/imx/clk-pll14xx.c
> > @@ -6,6 +6,7 @@
> > #include <linux/bitops.h>
> > #include <linux/clk-provider.h>
> > #include <linux/err.h>
> > +#include <linux/export.h>
> > #include <linux/io.h>
> > #include <linux/iopoll.h>
> > #include <linux/slab.h>
> > @@ -68,6 +69,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 +77,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 +440,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..9d6cdff 100644
> > --- a/drivers/clk/imx/clk-sscg-pll.c
> > +++ b/drivers/clk/imx/clk-sscg-pll.c
> > @@ -10,6 +10,7 @@
> >
> > #include <linux/clk-provider.h>
> > #include <linux/err.h>
> > +#include <linux/export.h>
> > #include <linux/io.h>
> > #include <linux/iopoll.h>
> > #include <linux/slab.h>
> > @@ -537,3 +538,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..6f2a94d 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,18 @@ 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;
> > }
> > +#ifndef MODULE
> > __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);
>
> I feel not only the __setup_param, the whole logic of keep_uart_clocks
> are not needed for Module case. Is it true?

Yes, but the 'keep_uart_clocks' is false by default and the function imx_keep_uart_clocks_param()
already has '__maybe_unused', it does NOT impact anything if it is for module build, so I did NOT
add the #ifndef check for them, just to keep code easy and clean.

Thanks,
Anson

2020-07-02 03:31:46

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH V4 5/5] clk: imx8qxp: Support building i.MX8QXP clock driver as module

On Thu, Jul 2, 2020 at 10:20 AM Anson Huang <[email protected]> wrote:
>
> Change configuration to "tristate", use device_initcall() instead
> of builtin_platform_driver(), add module author, description and
> license to support building i.MX8QXP clock drivers as module.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> Changes since V3:
> - use device_initcall() instead of builtin_platform_driver();
> - add module author/description;
> - link common scu/lpcg clock driver to i.MX8QXP clock driver, then
> no need to have exports.
> ---
> drivers/clk/imx/Kconfig | 6 +++---
> drivers/clk/imx/Makefile | 9 ++++-----
> drivers/clk/imx/clk-imx8qxp-lpcg.c | 10 +++++++++-
> drivers/clk/imx/clk-imx8qxp.c | 11 ++++++++++-
> 4 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
> index 1867111..8340dfe 100644
> --- a/drivers/clk/imx/Kconfig
> +++ b/drivers/clk/imx/Kconfig
> @@ -5,8 +5,8 @@ config MXC_CLK
> depends on ARCH_MXC
>
> config MXC_CLK_SCU
> - bool
> - depends on IMX_SCU
> + tristate "IMX SCU clock"
> + depends on ARCH_MXC && IMX_SCU
>
> config CLK_IMX1
> bool "IMX1 CCM Clock Driver"
> @@ -127,7 +127,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/Makefile b/drivers/clk/imx/Makefile
> index 17f5d12..79e53f2 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -21,15 +21,14 @@ mxc-clk-objs += clk-pll14xx.o
> mxc-clk-objs += clk-sscg-pll.o
> obj-$(CONFIG_MXC_CLK) += mxc-clk.o
>
> -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
> +
> +obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o
> +clk-imx-scu-$(CONFIG_CLK_IMX8QXP) += clk-scu.o clk-imx8qxp.o
> +clk-imx-lpcg-scu-$(CONFIG_CLK_IMX8QXP) += clk-lpcg-scu.o clk-imx8qxp-lpcg.o
>
> obj-$(CONFIG_CLK_IMX1) += clk-imx1.o
> obj-$(CONFIG_CLK_IMX21) += clk-imx21.o
> diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> index 04c8ee3..5b6648e 100644
> --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
> +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> @@ -231,4 +231,12 @@ static struct platform_driver imx8qxp_lpcg_clk_driver = {
> .probe = imx8qxp_lpcg_clk_probe,
> };
>
> -builtin_platform_driver(imx8qxp_lpcg_clk_driver);
> +static int __init imx8qxp_lpcg_clk_init(void)

Does __init work for module?

> +{
> + return platform_driver_register(&imx8qxp_lpcg_clk_driver);
> +}
> +device_initcall(imx8qxp_lpcg_clk_init);

Any reason to change to device_initcall which looks a bit strange?
Is it because the following line?
+obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o
But it looks to me they're still two modules. Aren't they?

Regards
Aisheng

> +
> +MODULE_AUTHOR("Aisheng Dong <[email protected]>");
> +MODULE_DESCRIPTION("NXP i.MX8QXP LPCG clock driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
> index 5e2903e..9bcf0d1 100644
> --- a/drivers/clk/imx/clk-imx8qxp.c
> +++ b/drivers/clk/imx/clk-imx8qxp.c
> @@ -151,4 +151,13 @@ static struct platform_driver imx8qxp_clk_driver = {
> },
> .probe = imx8qxp_clk_probe,
> };
> -builtin_platform_driver(imx8qxp_clk_driver);
> +
> +static int __init imx8qxp_clk_init(void)
> +{
> + return platform_driver_register(&imx8qxp_clk_driver);
> +}
> +device_initcall(imx8qxp_clk_init);
> +
> +MODULE_AUTHOR("Aisheng Dong <[email protected]>");
> +MODULE_DESCRIPTION("NXP i.MX8QXP clock driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>

2020-07-02 03:56:26

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH V4 5/5] clk: imx8qxp: Support building i.MX8QXP clock driver as module



> Subject: Re: [PATCH V4 5/5] clk: imx8qxp: Support building i.MX8QXP clock
> driver as module
>
> On Thu, Jul 2, 2020 at 10:20 AM Anson Huang <[email protected]>
> wrote:
> >
> > Change configuration to "tristate", use device_initcall() instead of
> > builtin_platform_driver(), add module author, description and license
> > to support building i.MX8QXP clock drivers as module.
> >
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > Changes since V3:
> > - use device_initcall() instead of builtin_platform_driver();
> > - add module author/description;
> > - link common scu/lpcg clock driver to i.MX8QXP clock driver, then
> > no need to have exports.
> > ---
> > drivers/clk/imx/Kconfig | 6 +++---
> > drivers/clk/imx/Makefile | 9 ++++-----
> > drivers/clk/imx/clk-imx8qxp-lpcg.c | 10 +++++++++-
> > drivers/clk/imx/clk-imx8qxp.c | 11 ++++++++++-
> > 4 files changed, 26 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index
> > 1867111..8340dfe 100644
> > --- a/drivers/clk/imx/Kconfig
> > +++ b/drivers/clk/imx/Kconfig
> > @@ -5,8 +5,8 @@ config MXC_CLK
> > depends on ARCH_MXC
> >
> > config MXC_CLK_SCU
> > - bool
> > - depends on IMX_SCU
> > + tristate "IMX SCU clock"
> > + depends on ARCH_MXC && IMX_SCU
> >
> > config CLK_IMX1
> > bool "IMX1 CCM Clock Driver"
> > @@ -127,7 +127,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/Makefile b/drivers/clk/imx/Makefile index
> > 17f5d12..79e53f2 100644
> > --- a/drivers/clk/imx/Makefile
> > +++ b/drivers/clk/imx/Makefile
> > @@ -21,15 +21,14 @@ mxc-clk-objs += clk-pll14xx.o mxc-clk-objs +=
> > clk-sscg-pll.o
> > obj-$(CONFIG_MXC_CLK) += mxc-clk.o
> >
> > -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
> > +
> > +obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o
> > +clk-imx-scu-$(CONFIG_CLK_IMX8QXP) += clk-scu.o clk-imx8qxp.o
> > +clk-imx-lpcg-scu-$(CONFIG_CLK_IMX8QXP) += clk-lpcg-scu.o
> > +clk-imx8qxp-lpcg.o
> >
> > obj-$(CONFIG_CLK_IMX1) += clk-imx1.o
> > obj-$(CONFIG_CLK_IMX21) += clk-imx21.o diff --git
> > a/drivers/clk/imx/clk-imx8qxp-lpcg.c
> > b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> > index 04c8ee3..5b6648e 100644
> > --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
> > +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> > @@ -231,4 +231,12 @@ static struct platform_driver
> imx8qxp_lpcg_clk_driver = {
> > .probe = imx8qxp_lpcg_clk_probe, };
> >
> > -builtin_platform_driver(imx8qxp_lpcg_clk_driver);
> > +static int __init imx8qxp_lpcg_clk_init(void)
>
> Does __init work for module?

__init is NOT working/necessary for module, but if the driver is built-in, it is better to
have it, checked other drivers which support module build, they all have it, so I think
it is better to keep it especially when the driver supports both built-in and loadable module.

drivers/clk/qcom/gcc-sdm845.c
drivers/clk/qcom/gcc-msm8939.c

>
> > +{
> > + return platform_driver_register(&imx8qxp_lpcg_clk_driver);
> > +}
> > +device_initcall(imx8qxp_lpcg_clk_init);
>
> Any reason to change to device_initcall which looks a bit strange?
> Is it because the following line?
> +obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o
> But it looks to me they're still two modules. Aren't they?

It is suggested by Arnd to NOT use builtin_platform_driver(), in order to support module
unload, although the clock driver normally does NOT support remove, but it is better to
follow the right way.

The change in Makefile is just to link scu/lpcg library to i.MX8QXP clk driver, so that we can
get rid of exports and below 2 .ko are needed for all i.MX SoCs with SCU inside as per your
saying of i.MX8QXP clock driver can be extended for future SoCs with SCU.

clk-imx-lpcg-scu.ko
clk-imx-scu.ko

Thanks,
Anson

2020-07-02 05:43:48

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH V4 5/5] clk: imx8qxp: Support building i.MX8QXP clock driver as module

On Thu, Jul 2, 2020 at 11:55 AM Anson Huang <[email protected]> wrote:
[...]
> > > +{
> > > + return platform_driver_register(&imx8qxp_lpcg_clk_driver);
> > > +}
> > > +device_initcall(imx8qxp_lpcg_clk_init);
> >
> > Any reason to change to device_initcall which looks a bit strange?
> > Is it because the following line?
> > +obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o
> > But it looks to me they're still two modules. Aren't they?
>
> It is suggested by Arnd to NOT use builtin_platform_driver(), in order to support module
> unload, although the clock driver normally does NOT support remove, but it is better to
> follow the right way.
>

By expanding builtin_platform_driver() marcro, you will find your
patch is exactly doing the same thing
as buildin_platform_driver() which obivously is unneccesary.

#define builtin_platform_driver(__platform_driver) \
builtin_driver(__platform_driver, platform_driver_register)
#define builtin_driver(__driver, __register, ...) \

static int __init __driver##_init(void) \
{ \
return __register(&(__driver) , ##__VA_ARGS__); \
} \
device_initcall(__driver##_init);

If we want to support unload, we need a .remove() callback as current
clocks are not allocated by devm_().
If don't support, we probably can use builtin_platform_driver() first
and switch to module_platform_driver()
in the future once the driver supports release resource properly.

Regards
Aisheng

> The change in Makefile is just to link scu/lpcg library to i.MX8QXP clk driver, so that we can
> get rid of exports and below 2 .ko are needed for all i.MX SoCs with SCU inside as per your
> saying of i.MX8QXP clock driver can be extended for future SoCs with SCU.
>
> clk-imx-lpcg-scu.ko
> clk-imx-scu.ko
>
> Thanks,
> Anson

2020-07-02 05:53:19

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH V4 5/5] clk: imx8qxp: Support building i.MX8QXP clock driver as module



> Subject: Re: [PATCH V4 5/5] clk: imx8qxp: Support building i.MX8QXP clock
> driver as module
>
> On Thu, Jul 2, 2020 at 11:55 AM Anson Huang <[email protected]>
> wrote:
> [...]
> > > > +{
> > > > + return platform_driver_register(&imx8qxp_lpcg_clk_driver);
> > > > +}
> > > > +device_initcall(imx8qxp_lpcg_clk_init);
> > >
> > > Any reason to change to device_initcall which looks a bit strange?
> > > Is it because the following line?
> > > +obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o
> > > But it looks to me they're still two modules. Aren't they?
> >
> > It is suggested by Arnd to NOT use builtin_platform_driver(), in order
> > to support module unload, although the clock driver normally does NOT
> > support remove, but it is better to follow the right way.
> >
>
> By expanding builtin_platform_driver() marcro, you will find your patch is
> exactly doing the same thing as buildin_platform_driver() which obivously is
> unneccesary.
>
> #define builtin_platform_driver(__platform_driver) \
> builtin_driver(__platform_driver, platform_driver_register) #define
> builtin_driver(__driver, __register, ...) \
>
> static int __init __driver##_init(void) \ { \
> return __register(&(__driver) , ##__VA_ARGS__); \ } \
> device_initcall(__driver##_init);
>
> If we want to support unload, we need a .remove() callback as current clocks
> are not allocated by devm_().
> If don't support, we probably can use builtin_platform_driver() first and
> switch to module_platform_driver() in the future once the driver supports
> release resource properly.
>

Yes, that is why I use the device_initcall() to make it exactly same as builtin_driver(),
and also yes that i.MX clock driver does NOT support module unload, so .remove()
is NOT implemented, I am fine with either way, just try to address Arnd's comment.

Hi, Arnd
What do you think? Do you agree to keep using the builtin_driver()?

Thanks,
Anson

2020-07-02 05:55:16

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module

On Thu, Jul 2, 2020 at 11:26 AM Anson Huang <[email protected]> wrote:
[...]
> > > @@ -143,16 +148,18 @@ 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;
> > > }
> > > +#ifndef MODULE
> > > __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);
> >
> > I feel not only the __setup_param, the whole logic of keep_uart_clocks
> > are not needed for Module case. Is it true?
>
> Yes, but the 'keep_uart_clocks' is false by default and the function imx_keep_uart_clocks_param()
> already has '__maybe_unused', it does NOT impact anything if it is for module build, so I did NOT
> add the #ifndef check for them, just to keep code easy and clean.
>

IMHO do not compile them is a more easy and clean way. Then users
don't have to look into the code logic
which is meaingless for Module case.

BTW, it really does not make any sense to only condionally compile
__setup_parm() but left
the param functions definition to be handled by __maybe_unnused.
They're together part of code, aren't they?

Regards
Aisheng

Regards
Aisheng

> Thanks,
> Anson
>

2020-07-02 06:11:54

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module



> Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock
> driver as module
>
> On Thu, Jul 2, 2020 at 11:26 AM Anson Huang <[email protected]>
> wrote:
> [...]
> > > > @@ -143,16 +148,18 @@ 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;
> > > > }
> > > > +#ifndef MODULE
> > > > __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);
> > >
> > > I feel not only the __setup_param, the whole logic of
> > > keep_uart_clocks are not needed for Module case. Is it true?
> >
> > Yes, but the 'keep_uart_clocks' is false by default and the function
> > imx_keep_uart_clocks_param() already has '__maybe_unused', it does NOT
> > impact anything if it is for module build, so I did NOT add the #ifndef check
> for them, just to keep code easy and clean.
> >
>
> IMHO do not compile them is a more easy and clean way. Then users don't
> have to look into the code logic which is meaingless for Module case.
>
> BTW, it really does not make any sense to only condionally compile
> __setup_parm() but left
> the param functions definition to be handled by __maybe_unnused.
> They're together part of code, aren't they?

I am fine of adding the '#ifndef MODULE' to imx_clk_disable_uart() and imx_keep_uart_clocks_param()
as well in next patch series. Others like ' imx_keep_uart_clocks ' and imx_register_uart_clocks() need to
be kept always built, since they are used by each clock driver no matter built-in or module build.

So that means I have to add another 'ifndef MODULE' or I need to adjust some code sequence to make
those code can be built-out in same block and just use single 'ifndef MODULE', I think adjust the code
sequence should be better, will go with this way.

Anson

2020-07-02 06:42:27

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module

On Thu, Jul 2, 2020 at 2:11 PM Anson Huang <[email protected]> wrote:
>
> > Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock
> > driver as module
> >
> > On Thu, Jul 2, 2020 at 11:26 AM Anson Huang <[email protected]>
> > wrote:
> > [...]
> > > > > @@ -143,16 +148,18 @@ 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;
> > > > > }
> > > > > +#ifndef MODULE
> > > > > __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);
> > > >
> > > > I feel not only the __setup_param, the whole logic of
> > > > keep_uart_clocks are not needed for Module case. Is it true?
> > >
> > > Yes, but the 'keep_uart_clocks' is false by default and the function
> > > imx_keep_uart_clocks_param() already has '__maybe_unused', it does NOT
> > > impact anything if it is for module build, so I did NOT add the #ifndef check
> > for them, just to keep code easy and clean.
> > >
> >
> > IMHO do not compile them is a more easy and clean way. Then users don't
> > have to look into the code logic which is meaingless for Module case.
> >
> > BTW, it really does not make any sense to only condionally compile
> > __setup_parm() but left
> > the param functions definition to be handled by __maybe_unnused.
> > They're together part of code, aren't they?
>
> I am fine of adding the '#ifndef MODULE' to imx_clk_disable_uart() and imx_keep_uart_clocks_param()
> as well in next patch series. Others like ' imx_keep_uart_clocks ' and imx_register_uart_clocks() need to
> be kept always built, since they are used by each clock driver no matter built-in or module build.
>
> So that means I have to add another 'ifndef MODULE' or I need to adjust some code sequence to make
> those code can be built-out in same block and just use single 'ifndef MODULE', I think adjust the code
> sequence should be better, will go with this way.

What if we condionally compile it in clk.h? Will that be easiser?

Regards
Aisheng

>
> Anson

2020-07-02 06:43:08

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module



> Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock
> driver as module
>
> On Thu, Jul 2, 2020 at 2:11 PM Anson Huang <[email protected]> wrote:
> >
> > > Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common
> > > clock driver as module
> > >
> > > On Thu, Jul 2, 2020 at 11:26 AM Anson Huang <[email protected]>
> > > wrote:
> > > [...]
> > > > > > @@ -143,16 +148,18 @@ 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;
> > > > > > }
> > > > > > +#ifndef MODULE
> > > > > > __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);
> > > > >
> > > > > I feel not only the __setup_param, the whole logic of
> > > > > keep_uart_clocks are not needed for Module case. Is it true?
> > > >
> > > > Yes, but the 'keep_uart_clocks' is false by default and the
> > > > function
> > > > imx_keep_uart_clocks_param() already has '__maybe_unused', it does
> > > > NOT impact anything if it is for module build, so I did NOT add
> > > > the #ifndef check
> > > for them, just to keep code easy and clean.
> > > >
> > >
> > > IMHO do not compile them is a more easy and clean way. Then users
> > > don't have to look into the code logic which is meaingless for Module case.
> > >
> > > BTW, it really does not make any sense to only condionally compile
> > > __setup_parm() but left
> > > the param functions definition to be handled by __maybe_unnused.
> > > They're together part of code, aren't they?
> >
> > I am fine of adding the '#ifndef MODULE' to imx_clk_disable_uart() and
> > imx_keep_uart_clocks_param() as well in next patch series. Others like
> > ' imx_keep_uart_clocks ' and imx_register_uart_clocks() need to be kept
> always built, since they are used by each clock driver no matter built-in or
> module build.
> >
> > So that means I have to add another 'ifndef MODULE' or I need to
> > adjust some code sequence to make those code can be built-out in same
> > block and just use single 'ifndef MODULE', I think adjust the code sequence
> should be better, will go with this way.
>
> What if we condionally compile it in clk.h? Will that be easiser?

Adjust the function sequence looks like NOT complicated, just need to exchange the
imx_register_uart_clocks() and imx_clk_disable_uart(), then I can use single '#ifndef MODULE',
will go with this way in V5.

Anson

2020-07-02 07:04:25

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module

On Thu, Jul 2, 2020 at 8:40 AM Dong Aisheng <[email protected]> wrote:
> On Thu, Jul 2, 2020 at 2:11 PM Anson Huang <[email protected]> wrote:
> > > Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock
> >
> > I am fine of adding the '#ifndef MODULE' to imx_clk_disable_uart() and imx_keep_uart_clocks_param()
> > as well in next patch series. Others like ' imx_keep_uart_clocks ' and imx_register_uart_clocks() need to
> > be kept always built, since they are used by each clock driver no matter built-in or module build.
> >
> > So that means I have to add another 'ifndef MODULE' or I need to adjust some code sequence to make
> > those code can be built-out in same block and just use single 'ifndef MODULE', I think adjust the code
> > sequence should be better, will go with this way.
>
> What if we condionally compile it in clk.h? Will that be easiser?

Yes, that's what I expected to see in v4 after the previous discussion. If
imx_register_uart_clocks() is an empty inline function, then the arrays
passed to it are unused and also get dropped by the compiler.

The question is whether the #ifdef check in the header to test for MODULE
(only calling it if this particular soc has a built-in clk driver,
which is sufficient)
or for IS_MODULE(CONFIG_MXC_CLK) (call it if _any_ clk driver is built-in
and the function exists, which leaves extra code in the driver but is a more
conventional check).

Arnd

2020-07-02 07:16:13

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module

Hi, Arnd


> Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock
> driver as module
>
> On Thu, Jul 2, 2020 at 8:40 AM Dong Aisheng <[email protected]> wrote:
> > On Thu, Jul 2, 2020 at 2:11 PM Anson Huang <[email protected]>
> wrote:
> > > > Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common
> > > > clock
> > >
> > > I am fine of adding the '#ifndef MODULE' to imx_clk_disable_uart()
> > > and imx_keep_uart_clocks_param() as well in next patch series.
> > > Others like ' imx_keep_uart_clocks ' and imx_register_uart_clocks() need to
> be kept always built, since they are used by each clock driver no matter
> built-in or module build.
> > >
> > > So that means I have to add another 'ifndef MODULE' or I need to
> > > adjust some code sequence to make those code can be built-out in
> > > same block and just use single 'ifndef MODULE', I think adjust the code
> sequence should be better, will go with this way.
> >
> > What if we condionally compile it in clk.h? Will that be easiser?
>
> Yes, that's what I expected to see in v4 after the previous discussion. If
> imx_register_uart_clocks() is an empty inline function, then the arrays passed
> to it are unused and also get dropped by the compiler.
>
> The question is whether the #ifdef check in the header to test for MODULE
> (only calling it if this particular soc has a built-in clk driver, which is sufficient)
> or for IS_MODULE(CONFIG_MXC_CLK) (call it if _any_ clk driver is built-in and
> the function exists, which leaves extra code in the driver but is a more
> conventional check).
>

So you prefer to add an empty inline function for imx_register_uart_clocks() with check
of MODULE build? Even with this, we still need to add MODULE build check to whole block
of this earlycon uart clock handler in imx/clk.c.

Thanks,
Anson

2020-07-02 07:43:59

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module

On Thu, Jul 2, 2020 at 9:12 AM Anson Huang <[email protected]> wrote:
> >
> > The question is whether the #ifdef check in the header to test for MODULE
> > (only calling it if this particular soc has a built-in clk driver, which is sufficient)
> > or for IS_MODULE(CONFIG_MXC_CLK) (call it if _any_ clk driver is built-in and
> > the function exists, which leaves extra code in the driver but is a more
> > conventional check).
> >
>
> So you prefer to add an empty inline function for imx_register_uart_clocks() with check
> of MODULE build?

Yes, I want the empty inline stub, but either symbol check would work

> Even with this, we still need to add MODULE build check to whole block
> of this earlycon uart clock handler in imx/clk.c.

Correct.

Arnd