Hi, Arnd

Subject: Re: [PATCH V3 02/10] init.h: Fix the __setup_param() macro for
module build

On Wed, Jul 1, 2020 at 11:27 AM Anson Huang [off-list ref]
wrote:

quoted

quoted

Subject: Re: [PATCH V3 02/10] init.h: Fix the __setup_param() macro
for module build

On Wed, Jul 1, 2020 at 7:14 AM Anson Huang [off-list ref]
wrote:

I don't understand what your plan is here. Do you mean you will
leave that part of the clk driver as built-in?

I meant I will leave the #else block of __setup_param() defined as
nothing as below to make module build passed.

#define __setup_param(str, unique_id, fn, early)        /* nothing */

No, I think that is  mistake. It will mean that other drivers with the same bug
as the imx-clk driver will appear to build fine, but not work correctly.

A build error is better than silently dropping the command line parsing in my
opinion.

quoted

quoted

This error just means you can't have a __setup_param() call in a
loadable module, which we already knew. If you need to do something
with the clocks early on, that has to be in built-in code and cannot
be in a module. If you don't need that code, then you should just
remove it from both the modular version and the built-in version.

What is the purpose of that __setup_param() argument parsing in the
clock driver?

We need the code for proper uart clock management of earlycon, from
the code, it is trying to keep console uart clock enabled during kernel boot

up.

Why not move this all to a separate file then and only build it when
CONFIG_CLK_IMX=y?
It seems that you don't need the imx_keep_uart_clocks_param() if the clk
driver is loaded as a module, but then you also don't need the
imx_clk_disable_uart() and imx_register_uart_clocks() functions or the
associated variables.

If so, how about just adding "#ifndef MODULE" check for this part of code? I think
it should be easier/better than adding a file and build it conditionally?

Thanks,
Anson
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel