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 [off-list ref] wrote:

quoted

quoted

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 [off-list ref]
wrote:
[...]

quoted

quoted

quoted

@@ -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.

quoted

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.

quoted

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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel