linux-kernel - RE: [PATCH V4 3/5] clk: imx: Support building i.MX common clock driver as module

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DB3PR0402MB391633332EFC1A9B9B62FA76F56D0@DB3PR0402MB3916.eurprd04.prod.outlook.com>
Date:   Thu, 2 Jul 2020 06:42:28 +0000
From:   Anson Huang <anson.huang@....com>
To:     Dong Aisheng <dongas86@...il.com>
CC:     Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Sascha Hauer <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        Peng Fan <peng.fan@....com>, Arnd Bergmann <arnd@...db.de>,
        Abel Vesa <abel.vesa@....com>,
        Aisheng Dong <aisheng.dong@....com>,
        Andy Duan <fugang.duan@....com>,
        Daniel Baluta <daniel.baluta@....com>,
        YueHaibing <yuehaibing@...wei.com>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        linux-clk <linux-clk@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH V4 3/5] clk: imx: Support building i.MX common clock
 driver as module



> Subject: Re: [PATCH V4 3/5] clk: imx: Support building i.MX common clock
> driver as module
> 
> On Thu, Jul 2, 2020 at 2:11 PM Anson Huang <anson.huang@....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@....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?

Adjust the function sequence looks like NOT complicated, just need to exchange the
imx_register_uart_clocks() and imx_clk_disable_uart(), then I can use single '#ifndef MODULE',
will go with this way in V5.

Anson