Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1593656074-10092-1-git-send-email-Anson.Huang@nxp.com>
 <1593656074-10092-4-git-send-email-Anson.Huang@nxp.com> <CAA+hA=S0G7Na0ieEpPq3aN6GN1BEHtYp9vbF9x2tjmknDSVjZw@mail.gmail.com>
 <DB3PR0402MB391610595D40C11CF26CD990F56D0@DB3PR0402MB3916.eurprd04.prod.outlook.com>
 <CAA+hA=Rtkm_FpkoBvHNnB0TSeTrqXaRVwOzkywsL7QO6ec_K7Q@mail.gmail.com> <AM6PR0402MB3911049CC1B136327345F11CF56D0@AM6PR0402MB3911.eurprd04.prod.outlook.com>
In-Reply-To: <AM6PR0402MB3911049CC1B136327345F11CF56D0@AM6PR0402MB3911.eurprd04.prod.outlook.com>
From:   Dong Aisheng <dongas86@gmail.com>
Date:   Thu, 2 Jul 2020 14:25:58 +0800
Message-ID: <CAA+hA=S9ENXtD7q362=y84qKAtR090pZOd6MF6uN8W223UK-+Q@mail.gmail.com>
Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock
 driver as module
To:     Anson Huang <anson.huang@nxp.com>
Cc:     Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Sascha Hauer <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        Peng Fan <peng.fan@nxp.com>, Arnd Bergmann <arnd@arndb.de>,
        Abel Vesa <abel.vesa@nxp.com>,
        Aisheng Dong <aisheng.dong@nxp.com>,
        Andy Duan <fugang.duan@nxp.com>,
        Daniel Baluta <daniel.baluta@nxp.com>,
        YueHaibing <yuehaibing@huawei.com>,
        Stephen Rothwell <sfr@canb.auug.org.au>,
        linux-clk <linux-clk@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@lists.infradead.org>,
        dl-linux-imx <linux-imx@nxp.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jul 2, 2020 at 2:11 PM Anson Huang <anson.huang@nxp.com> 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 <anson.huang@nxp.com>
> > 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