Messages in this thread | | | Subject | Re: [PATCH V2 4/9] clk: imx: Support building i.MX common clock driver as module | From | Stephen Boyd <> | Date | Mon, 22 Jun 2020 00:05:46 -0700 |
| |
Quoting Anson Huang (2020-06-09 00:32:08) > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile > index 1af8cff..1291f9b 100644 > --- a/drivers/clk/imx/Makefile > +++ b/drivers/clk/imx/Makefile > @@ -1,25 +1,10 @@ > # SPDX-License-Identifier: GPL-2.0 > > -obj-$(CONFIG_MXC_CLK) += \ > - clk.o \ > - clk-busy.o \ > - clk-composite-8m.o \ > - clk-cpu.o \ > - clk-composite-7ulp.o \ > - clk-divider-gate.o \ > - clk-fixup-div.o \ > - clk-fixup-mux.o \ > - clk-frac-pll.o \ > - clk-gate-exclusive.o \ > - clk-gate2.o \ > - clk-pfd.o \ > - clk-pfdv2.o \ > - clk-pllv1.o \ > - clk-pllv2.o \ > - clk-pllv3.o \ > - clk-pllv4.o \ > - clk-sscg-pll.o \ > - clk-pll14xx.o > +mxc-clk-objs += clk.o clk-busy.o clk-composite-8m.o clk-cpu.o clk-composite-7ulp.o \ > + clk-divider-gate.o clk-fixup-div.o clk-fixup-mux.o clk-frac-pll.o \ > + clk-gate-exclusive.o clk-gate2.o clk-pfd.o clk-pfdv2.o clk-pllv1.o \ > + clk-pllv2.o clk-pllv3.o clk-pllv4.o clk-sscg-pll.o clk-pll14xx.o > +obj-$(CONFIG_MXC_CLK) += mxc-clk.o
I enjoyed it when the files were all on their own line. Can you keep it like that and just add mxc-clk-objs += to all the lines and remove the trailing slash?
> > mxc-clk-scu-objs += clk-scu.o clk-lpcg-scu.o > obj-$(CONFIG_MXC_CLK_SCU) += mxc-clk-scu.o > diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c > index d2b5af8..73e064b 100644 > --- a/drivers/clk/imx/clk-composite-8m.c > +++ b/drivers/clk/imx/clk-composite-8m.c > @@ -243,3 +243,4 @@ struct clk_hw *imx8m_clk_hw_composite_flags(const char *name, > kfree(mux); > return ERR_CAST(hw); > } > +EXPORT_SYMBOL_GPL(imx8m_clk_hw_composite_flags);
Are all these files including <linux/export.h>? Because they should unless they're using something from <linux/module.h> like MODULE_LICENSE().
> diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c > index 87ab8db..cc894b5 100644 > --- a/drivers/clk/imx/clk.c > +++ b/drivers/clk/imx/clk.c > @@ -143,16 +148,24 @@ void imx_cscmr1_fixup(u32 *val) > static int imx_keep_uart_clocks; > static struct clk ** const *imx_uart_clocks; > > -static int __init imx_keep_uart_clocks_param(char *str) > +static int __maybe_unused imx_keep_uart_clocks_param(char *str) > { > imx_keep_uart_clocks = 1; > > return 0; > } > + > +#ifdef MODULE > +__setup_param("earlycon", imx_keep_uart_earlycon, > + imx_keep_uart_clocks_param); > +__setup_param("earlyprintk", imx_keep_uart_earlyprintk, > + imx_keep_uart_clocks_param);
Why not fix __setup_param() to take the same number of arguments in either case? That macro looks broken.
> +#else > __setup_param("earlycon", imx_keep_uart_earlycon, > imx_keep_uart_clocks_param, 0); > __setup_param("earlyprintk", imx_keep_uart_earlyprintk, > imx_keep_uart_clocks_param, 0); > +#endif > > void imx_register_uart_clocks(struct clk ** const clks[]) > {
| |