RE: [PATCH V3 04/10] clk: imx: Support building SCU clock driver as module

[Anson Huang]

Hi, Arnd


> Subject: Re: [PATCH V3 04/10] clk: imx: Support building SCU clock driver as
> module
> 
> On Tue, Jun 30, 2020 at 5:16 AM Anson Huang <anson.huang@xxxxxxx>
> wrote:
> >
> > Hi, Arnd
> >
> >
> > > Subject: Re: [PATCH V3 04/10] clk: imx: Support building SCU clock
> > > driver as module
> > >
> > > On Mon, Jun 29, 2020 at 4:52 PM Anson Huang <anson.huang@xxxxxxx>
> > > wrote:
> > > > > Subject: Re: [PATCH V3 04/10] clk: imx: Support building SCU
> > > > > clock driver as module On Mon, Jun 29, 2020 at 2:53 PM Anson
> > > > > Huang
> > > <anson.huang@xxxxxxx> wrote:
> > > > >
> > > > > Sorry, I misread the patch in multiple ways. First of all, you
> > > > > already put clk-scu.o and clk-lpcg-scu.o files into a combined
> > > > > loadable module, and I had only looked at clk-scu.c.
> > > > >
> > > > > What I actually meant here was to link clk-scu.o together with
> > > > > clk-imx8qxp.o (and possibly future chip-specific files) into a
> > > > > loadable module and drop the export.
> > > >
> > > > Sorry, could you please advise more details about how to do it in
> Makefile?
> > > > I tried below but it looks like NOT working. multiple definition
> > > > of
> > > module_init() error reported.
> > > >
> > > > obj-$(CONFIG_MXC_CLK_SCU) := clk-imx.o clk-imx-y += clk-scu.o
> > > > clk-lpcg-scu.o
> > > > clk-imx-$(CONFIG_CLK_IMX8QXP) += clk-imx8qxp.o clk-imx8qxp-lpcg.o
> > >
> > > Right, you can't have multiple module_init() in a module, so what I
> > > suggested earlier won't work any more as soon as you add a second
> > > chip that uses the clk-scu driver, and then you have to use separate
> > > modules, or some hack that calls the init functions one at a time, which is
> probably worse.
> > >
> > > If it's only imx8qxp, you can do it like this:
> > >
> > > obj-$(CONFIG_MXC_CLK_SCU) := clk-imx-scu.o clk-imx-lpcg.o
> > > clk-imx-scu-y         += clk-scu.o clk-imx8qxp.o
> > > clk-imx-lpcq-scu-y += clk-lpcg-scu.o clk-imx8qxp-lpcg.o
> > >
> > > If you already know that the scu driver is going to be used in
> > > future chips, then just stay with what you have now, using a
> > > separate module per file, exporting the symbols as needed.
> > >
> >
> > Thanks, and yes, I know that scu clk driver will be used for future
> > i.MX8X chips with SCU inside, the current i.MX8QXP clock driver can
> > NOT cover all i.MX8X chips, so I will stay with the exporting symbols.
> >
> 
> SCU clock driver is a common driver for all SCU based platforms.
> Current i.MX8QXP SCU clock driver will be extended to support all future SCU
> based platforms.
> So theoretically clk-scu.o and clk-imx8qxp.o can be combined. LPCG  is
> similar.
> Maybe you can give a try as Arnd suggested.
> 

Do we really need to link clk-scu and i.MX8QXP clock driver together just to avoid some export?
I met some build issues if using this method, the i.MX8QXP module build is OK, but other platforms
like i.MX8MM/i.MX8MN/i.MX8MQ/i.MX8MP clock drivers are just skipped for build, even these
i.MX8M clock config are existing in .config, anyone know why? Looks like the change in Makefile for
i.MX8QXP clock driver introduce this issue.

I think doing such big change in order to save some exports looks like NOT that worth, is it acceptable
to just keep the original one in Makefile, just to have mxc-clk.ko, clk-scu.ko and clk-imx8qxp.ko etc..

thanks,
Anson

.config:
CONFIG_MXC_CLK=m
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

Makefile:
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_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_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