| Message ID | 20201124121740.ytag7rm53umi2qvm@linutronix.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [RFC] clk: imx8qxp: Unbreak auto module building for MXC_CLK_SCU | expand |
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Sent: Tuesday, November 24, 2020 8:18 PM > > Automatic moudule building is broken by adding module support to i.MX8QXP > clock driver. It can be tested by ARM defconfig + CONFIG_IMX_MBOX=m and > CONFIG_MXC_CLK_SCU=m. > > The compile breaks because the modules and source files are mixed. > After fixing that, the build breaks because the SCU driver has no license or > symbols, which are required by the CLK_IMX8QXP driver, are not properly > exported. > > The following patch is against -rc5: > > Compile module clk-imx-scu.o which contains of clk-scu.o clk-lpcg-scu.o if > CONFIG_MXC_CLK_SCU is enabled. > Compile modules clk-imx8qxp.o and clk-imx8qxp-lpcg.o if > CONFIG_CLK_IMX8QXP is enabled. > Add EXPORT_SYMBOL_GPL() to functions which fail to resolve once > CONFIG_CLK_IMX8QXP is enabled as module. > Add License GPL to clk-scu.c. > I think the simplest solution is make MXC_CLK_SCU to be invisible to users and can only be selected by CLK_IMX8QXP option because currently clk-scu.o and clk-imx8qxp.o are built together, it's meaningless and buggy to separate them. Longtermly they will be combined into one driver file. E.g diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index 3b393cb07295..dbacdd70af2e 100644 --- a/drivers/clk/imx/Kconfig +++ b/drivers/clk/imx/Kconfig @@ -5,7 +5,7 @@ config MXC_CLK depends on ARCH_MXC || COMPILE_TEST config MXC_CLK_SCU - tristate "IMX SCU clock" + tristate depends on ARCH_MXC || COMPILE_TEST depends on IMX_SCU && HAVE_ARM_SMCCC > In -next it breaks additionally because device_is_bound() is not made available > to modules. > Any suggestions? Should the original commit be revoked? See fixes here: https://patchwork.kernel.org/project/linux-clk/cover/20201124100802.22775-1-aisheng.dong@nxp.com/ > > Fixes: e0d0d4d86c766 ("clk: imx8qxp: Support building i.MX8QXP clock driver > as module") > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > > drivers/clk/imx/Makefile | 6 +++--- > drivers/clk/imx/clk-lpcg-scu.c | 1 + > drivers/clk/imx/clk-scu.c | 4 ++++ > 3 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index > dd6a737d060b4..b8bf9460c91d7 100644 > --- a/drivers/clk/imx/Makefile > +++ b/drivers/clk/imx/Makefile > @@ -26,9 +26,9 @@ obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o > obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o > obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.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_MXC_CLK_SCU) += clk-imx-scu.o > +clk-imx-scu-y := clk-scu.o clk-lpcg-scu.o We can't do this because clk-scu and clk-lpcg-scu are separate drivers. Regards Aisheng > +obj-$(CONFIG_CLK_IMX8QXP) += clk-imx8qxp.o clk-imx8qxp-lpcg.o > > obj-$(CONFIG_CLK_IMX1) += clk-imx1.o > obj-$(CONFIG_CLK_IMX25) += clk-imx25.o diff --git > a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c index > 1f0e44f921aee..336dce43da82d 100644 > --- a/drivers/clk/imx/clk-lpcg-scu.c > +++ b/drivers/clk/imx/clk-lpcg-scu.c > @@ -115,3 +115,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 > b8b2072742a56..026a33606ae53 100644 > --- a/drivers/clk/imx/clk-scu.c > +++ b/drivers/clk/imx/clk-scu.c > @@ -9,6 +9,7 @@ > #include <linux/clk-provider.h> > #include <linux/err.h> > #include <linux/slab.h> > +#include <linux/module.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,5 @@ > struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents, > > return hw; > } > +EXPORT_SYMBOL_GPL(__imx_clk_scu); > +MODULE_LICENSE("GPL"); > -- > 2.29.2
On 2020-11-25 07:15:24 [+0000], Aisheng Dong wrote: > I think the simplest solution is make MXC_CLK_SCU to be invisible to users and can only > be selected by CLK_IMX8QXP option because currently clk-scu.o and clk-imx8qxp.o are built > together, it's meaningless and buggy to separate them. Longtermly they will be combined into > one driver file. > > E.g > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig > index 3b393cb07295..dbacdd70af2e 100644 > --- a/drivers/clk/imx/Kconfig > +++ b/drivers/clk/imx/Kconfig > @@ -5,7 +5,7 @@ config MXC_CLK > depends on ARCH_MXC || COMPILE_TEST > > config MXC_CLK_SCU > - tristate "IMX SCU clock" > + tristate > depends on ARCH_MXC || COMPILE_TEST > depends on IMX_SCU && HAVE_ARM_SMCCC So it still becomes a module if it gets selected by one, like CLK_IMX8QXP. > See fixes here: > https://patchwork.kernel.org/project/linux-clk/cover/20201124100802.22775-1-aisheng.dong@nxp.com/ Whatever you do, please make sure the issue in v5.10 gets also addressed. Sebastian
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Sent: Wednesday, November 25, 2020 4:40 PM > > On 2020-11-25 07:15:24 [+0000], Aisheng Dong wrote: > > I think the simplest solution is make MXC_CLK_SCU to be invisible to > > users and can only be selected by CLK_IMX8QXP option because currently > > clk-scu.o and clk-imx8qxp.o are built together, it's meaningless and > > buggy to separate them. Longtermly they will be combined into one driver file. > > > > E.g > > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index > > 3b393cb07295..dbacdd70af2e 100644 > > --- a/drivers/clk/imx/Kconfig > > +++ b/drivers/clk/imx/Kconfig > > @@ -5,7 +5,7 @@ config MXC_CLK > > depends on ARCH_MXC || COMPILE_TEST > > > > config MXC_CLK_SCU > > - tristate "IMX SCU clock" > > + tristate > > depends on ARCH_MXC || COMPILE_TEST > > depends on IMX_SCU && HAVE_ARM_SMCCC > > So it still becomes a module if it gets selected by one, like CLK_IMX8QXP. > Yes, as MXC_CLK_SCU and CLK_IMX8QXP will be enabled at the same time and clk-scu.o and clk-imx8qxp.o are compiled together into one module, there will be no such issues reported in this email. > > See fixes here: ... > > Whatever you do, please make sure the issue in v5.10 gets also addressed. Will you send out above patch if you're ok? Or I will do it now if you don't mind. Regards Aisheng > > Sebastian
Hi Sebastian > From: Aisheng Dong <aisheng.dong@nxp.com> > Sent: Wednesday, November 25, 2020 5:53 PM > > > From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > Sent: Wednesday, November 25, 2020 4:40 PM > > > > On 2020-11-25 07:15:24 [+0000], Aisheng Dong wrote: > > > I think the simplest solution is make MXC_CLK_SCU to be invisible to > > > users and can only be selected by CLK_IMX8QXP option because > > > currently clk-scu.o and clk-imx8qxp.o are built together, it's > > > meaningless and buggy to separate them. Longtermly they will be combined > into one driver file. > > > > > > E.g > > > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index > > > 3b393cb07295..dbacdd70af2e 100644 > > > --- a/drivers/clk/imx/Kconfig > > > +++ b/drivers/clk/imx/Kconfig > > > @@ -5,7 +5,7 @@ config MXC_CLK > > > depends on ARCH_MXC || COMPILE_TEST > > > > > > config MXC_CLK_SCU > > > - tristate "IMX SCU clock" > > > + tristate > > > depends on ARCH_MXC || COMPILE_TEST > > > depends on IMX_SCU && HAVE_ARM_SMCCC > > > > So it still becomes a module if it gets selected by one, like CLK_IMX8QXP. > > > > Yes, as MXC_CLK_SCU and CLK_IMX8QXP will be enabled at the same time and > clk-scu.o and clk-imx8qxp.o are compiled together into one module, there will > be no such issues reported in this email. > > > > > See fixes here: > ... > > > > Whatever you do, please make sure the issue in v5.10 gets also addressed. > > Will you send out above patch if you're ok? > Or I will do it now if you don't mind. I just sent out the fix with you CCed. Regards Aisheng > > Regards > Aisheng > > > > > Sebastian
diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index dd6a737d060b4..b8bf9460c91d7 100644 --- a/drivers/clk/imx/Makefile +++ b/drivers/clk/imx/Makefile @@ -26,9 +26,9 @@ obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.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_MXC_CLK_SCU) += clk-imx-scu.o +clk-imx-scu-y := clk-scu.o clk-lpcg-scu.o +obj-$(CONFIG_CLK_IMX8QXP) += clk-imx8qxp.o clk-imx8qxp-lpcg.o obj-$(CONFIG_CLK_IMX1) += clk-imx1.o obj-$(CONFIG_CLK_IMX25) += clk-imx25.o diff --git a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c index 1f0e44f921aee..336dce43da82d 100644 --- a/drivers/clk/imx/clk-lpcg-scu.c +++ b/drivers/clk/imx/clk-lpcg-scu.c @@ -115,3 +115,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 b8b2072742a56..026a33606ae53 100644 --- a/drivers/clk/imx/clk-scu.c +++ b/drivers/clk/imx/clk-scu.c @@ -9,6 +9,7 @@ #include <linux/clk-provider.h> #include <linux/err.h> #include <linux/slab.h> +#include <linux/module.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,5 @@ struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents, return hw; } +EXPORT_SYMBOL_GPL(__imx_clk_scu); +MODULE_LICENSE("GPL");
Automatic moudule building is broken by adding module support to i.MX8QXP clock driver. It can be tested by ARM defconfig + CONFIG_IMX_MBOX=m and CONFIG_MXC_CLK_SCU=m. The compile breaks because the modules and source files are mixed. After fixing that, the build breaks because the SCU driver has no license or symbols, which are required by the CLK_IMX8QXP driver, are not properly exported. The following patch is against -rc5: Compile module clk-imx-scu.o which contains of clk-scu.o clk-lpcg-scu.o if CONFIG_MXC_CLK_SCU is enabled. Compile modules clk-imx8qxp.o and clk-imx8qxp-lpcg.o if CONFIG_CLK_IMX8QXP is enabled. Add EXPORT_SYMBOL_GPL() to functions which fail to resolve once CONFIG_CLK_IMX8QXP is enabled as module. Add License GPL to clk-scu.c. In -next it breaks additionally because device_is_bound() is not made available to modules. Any suggestions? Should the original commit be revoked? Fixes: e0d0d4d86c766 ("clk: imx8qxp: Support building i.MX8QXP clock driver as module") Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- drivers/clk/imx/Makefile | 6 +++--- drivers/clk/imx/clk-lpcg-scu.c | 1 + drivers/clk/imx/clk-scu.c | 4 ++++ 3 files changed, 8 insertions(+), 3 deletions(-)