RE: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module
From: Anson Huang
Date: Thu Jul 02 2020 - 02:11:54 EST
> 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 <anson.huang@xxxxxxx>
> 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