On Tue, Dec 25, 2018 at 12:14:57AM +0900, Masahiro Yamada wrote:
> The whole code of fallback_table.c is surrounded by #ifdef of
> CONFIG_FW_LOADER_USER_HELPER.
>
> Move the CONFIG_FW_LOADER_USER_HELPER switch to Makefile so that
> it is not compiled at all when this CONFIG is turned off.
>
> I also removed the confusing comment, "Module or buit-in [sic]".
> CONFIG_FW_LOADER_USER_HELPER is a boolean option.
> (If it were a module, CONFIG_FW_LOADER_USER_HELPER_MODULE would
> be defined instead.)
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
> drivers/base/firmware_loader/Makefile | 2 +-
> drivers/base/firmware_loader/fallback_table.c | 5 -----
> 2 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/base/firmware_loader/Makefile
> b/drivers/base/firmware_loader/Makefile
> index a97eeb0..5c7f422 100644
> --- a/drivers/base/firmware_loader/Makefile
> +++ b/drivers/base/firmware_loader/Makefile
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
> # Makefile for the Linux firmware loader
>
> -obj-y := fallback_table.o
> +obj-$(CONFIG_FW_LOADER_USER_HELPER) += fallback_table.o
Phew ... CONFIG_FW_LOADER_USER_HELPER is bool.
So no new modules. You had me scared for a bit.
The reason this was built-in was that we *need* this table for built-in
code which *can* request firmware *pre* module loading. The table thing
was designed to be built-in if the firmware interface was enabled in
any way.
Fortunately I don't see this regressing as CONFIG_FW_LOADER_USER_HELPER
is a bool. It is hard to enforce other than with careful review.
> obj-$(CONFIG_FW_LOADER) += firmware_class.o
> firmware_class-objs := main.o
> firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += fallback.o
> diff --git a/drivers/base/firmware_loader/fallback_table.c
> b/drivers/base/firmware_loader/fallback_table.c
> index 7428659..776dd69 100644
> --- a/drivers/base/firmware_loader/fallback_table.c
> +++ b/drivers/base/firmware_loader/fallback_table.c
> @@ -16,9 +16,6 @@
> * firmware fallback configuration table
> */
>
> -/* Module or buit-in */
> -#ifdef CONFIG_FW_LOADER_USER_HELPER
> -
> static unsigned int zero;
> static unsigned int one = 1;
>
> @@ -51,5 +48,3 @@ struct ctl_table firmware_config_table[] = {
> { }
> };
> EXPORT_SYMBOL_GPL(firmware_config_table);
> -
> -#endif
Just a random early spring cleaning note for the future:
Although we cannot rename the firmware module to something more
sensible, we could just make the old sysfs firmware loading interface
*its own module firmware_class* and move the non-sysfs code to its own
first citizen file.
Luis
