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

Hi, Arnd

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:

quoted

quoted

Subject: Re: [PATCH V3 02/10] init.h: Fix the __setup_param() macro
for module build On Mon, Jun 29, 2020 at 1:40 PM Anson Huang
[off-list ref]
wrote:

quoted

It makes sense to drop the __setup() and __serup_param() in the
#else block, just use one definition for all cases, if no one
objects, I will remove

them in next patch series.

Ok, sounds good. Note that there may be users of the plain __setup()
that just get turned into nops right now. Usually those are already
enclosed in "#ifndef MODULE", but if they are not, then removing the
definition would cause a build error.

Have a look if you can find such instances, and either change the
patch to add the missing "#ifndef MODULE" checks, or just drop the
__setup_param() and leave the __setup() if it gets too complicated.

Looks like the __setup_param() defined in "#ifndef MODULE" can NOT be
used for MODULE build at all, so sharing same implementation is NOT
available, so if it is NOT that critical, I plan to keep the #else
block in this patch, let me know if you have further concern or any
other suggestion, below is the build error reported for module build
using
__setup_param() implementation for built in.

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 */

quoted

In file included from ./arch/arm64/include/asm/alternative.h:12,
                 from ./arch/arm64/include/asm/lse.h:15,
                 from ./arch/arm64/include/asm/cmpxchg.h:14,
                 from ./arch/arm64/include/asm/atomic.h:16,
                 from ./include/linux/atomic.h:7,
                 from ./include/asm-generic/bitops/atomic.h:5,
                 from ./arch/arm64/include/asm/bitops.h:26,
                 from ./include/linux/bitops.h:29,
                 from ./include/linux/kernel.h:12,
                 from ./include/linux/clk.h:13,
                 from drivers/clk/imx/clk.c:2:
./include/linux/init.h:177:16: error: variable

‘__setup_imx_keep_uart_earlycon’ has initializer but incomplete type

quoted

  177 |  static struct obs_kernel_param __setup_##unique_id  \
      |                ^~~~~~~~~~~~~~~~
drivers/clk/imx/clk.c:157:1: note: in expansion of macro ‘__setup_param’
  157 | __setup_param("earlycon", imx_keep_uart_earlycon,
      | ^~~~~~~~~~~~~
./include/linux/init.h:180:7: warning: excess elements in struct initializer
  180 |   = { __setup_str_##unique_id, fn, early }
      |       ^~~~~~~~~~~~
drivers/clk/imx/clk.c:157:1: note: in expansion of macro ‘__setup_param’
  157 | __setup_param("earlycon", imx_keep_uart_earlycon,
      | ^~~~~~~~~~~~~
./include/linux/init.h:180:7: note: (near initialization for
‘__setup_imx_keep_uart_earlycon’)

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. 

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