[2/6] kconfig: Add support for conditional values

Message ID 20200522020223.230834-2-sjg@chromium.org
State New
Headers show
Series
  • [1/6] bootstage: Fix 'stacked' typo
Related show

Commit Message

Simon Glass May 22, 2020, 2:02 a.m.
At present if an optional Kconfig value needs to be used it must be
bracketed by #ifdef. For example, with this Kconfig setup:

config WIBBLE
	bool "Support wibbles, the world needs more wibbles"

config WIBBLE_ADDR
	hex "Address of the wibble"
	depends on WIBBLE

then the following code must be used:

 #ifdef CONFIG_WIBBLE
 static void handle_wibble(void)
 {
 	int val = CONFIG_WIBBLE_ADDR;

	...
 }
 #endif

 static void init_machine()
 {
 ...
 #ifdef CONFIG_WIBBLE
	handle_wibble();
 #endif
 }

Add a new IF_ENABLED_INT() to help with this. So now it is possible to
write, without #ifdefs:

 static void handle_wibble(void)
 {
        int val = IF_ENABLED_INT(CONFIG_WIBBLE, CONFIG_WIBBLE_ADDR);

	...
 }

 static void init_machine()
 {
 ...
 if (IS_ENABLED(CONFIG_WIBBLE))
	handle_wibble();
 }

The value will be 0 if CONFIG_WIBBLE is not defined, and
CONFIG_WIBBLE_ADDR if it is. This allows us to reduce the use of #ifdef in
the code, ensuring that the compiler still checks the code even if it is
not ultimately used for a particular build.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

 include/linux/kconfig.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Masahiro Yamada May 22, 2020, 2:17 a.m. | #1
On Fri, May 22, 2020 at 11:02 AM Simon Glass <sjg at chromium.org> wrote:
>
> At present if an optional Kconfig value needs to be used it must be
> bracketed by #ifdef. For example, with this Kconfig setup:
>
> config WIBBLE
>         bool "Support wibbles, the world needs more wibbles"
>
> config WIBBLE_ADDR
>         hex "Address of the wibble"
>         depends on WIBBLE




I am not sure if this is a good idea.


If you want to always use CONFIG_WIBBLE_ADDR,
you can get rid of 'depends on WIBBLE'.




> then the following code must be used:
>
>  #ifdef CONFIG_WIBBLE
>  static void handle_wibble(void)
>  {
>         int val = CONFIG_WIBBLE_ADDR;
>
>         ...
>  }
>  #endif
>
>  static void init_machine()
>  {
>  ...
>  #ifdef CONFIG_WIBBLE
>         handle_wibble();
>  #endif
>  }
>
> Add a new IF_ENABLED_INT() to help with this. So now it is possible to
> write, without #ifdefs:
>
>  static void handle_wibble(void)
>  {
>         int val = IF_ENABLED_INT(CONFIG_WIBBLE, CONFIG_WIBBLE_ADDR);
>
>         ...
>  }
>
>  static void init_machine()
>  {
>  ...
>  if (IS_ENABLED(CONFIG_WIBBLE))
>         handle_wibble();
>  }
>
> The value will be 0 if CONFIG_WIBBLE is not defined, and
> CONFIG_WIBBLE_ADDR if it is. This allows us to reduce the use of #ifdef in
> the code, ensuring that the compiler still checks the code even if it is
> not ultimately used for a particular build.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>  include/linux/kconfig.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> index 3a2da738c4..86cd266540 100644
> --- a/include/linux/kconfig.h
> +++ b/include/linux/kconfig.h
> @@ -79,6 +79,18 @@
>   */
>  #define CONFIG_VAL(option)  config_val(option)
>
> +/* This use a similar mechanism to config_enabled() above */
> +#define config_opt_enabled(cfg, opt_cfg) _config_opt_enabled(cfg, opt_cfg)
> +#define _config_opt_enabled(cfg_val, opt_value) \
> +       __config_opt_enabled(__ARG_PLACEHOLDER_##cfg_val, opt_value)
> +#define __config_opt_enabled(arg1_or_junk, arg2) \
> +       ___config_opt_enabled(arg1_or_junk arg2, 0)
> +#define ___config_opt_enabled(__ignored, val, ...) val
> +
> +/* Evaluates to 0 if option is not defined, int_option if it is defined */
> +#define IF_ENABLED_INT(option, int_option) \
> +       config_opt_enabled(option, int_option)
> +
>  /*
>   * CONFIG_IS_ENABLED(FOO) evaluates to
>   *  1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y' or 'm',
> --
> 2.27.0.rc0.183.gde8f92d652-goog
>
Heinrich Schuchardt May 22, 2020, 6:17 a.m. | #2
On 5/22/20 4:17 AM, Masahiro Yamada wrote:
> On Fri, May 22, 2020 at 11:02 AM Simon Glass <sjg at chromium.org> wrote:
>>
>> At present if an optional Kconfig value needs to be used it must be
>> bracketed by #ifdef. For example, with this Kconfig setup:
>>
>> config WIBBLE
>>         bool "Support wibbles, the world needs more wibbles"
>>
>> config WIBBLE_ADDR
>>         hex "Address of the wibble"
>>         depends on WIBBLE
>
>
>
>
> I am not sure if this is a good idea.
>
>
> If you want to always use CONFIG_WIBBLE_ADDR,
> you can get rid of 'depends on WIBBLE'.
>
>

Hello Simon,

what is the effect on the code size if you eliminate the #ifdefs?

Isn't this move growing the size of the U-Boot binary?

Best regards

Heinrich

>
>
>> then the following code must be used:
>>
>>  #ifdef CONFIG_WIBBLE
>>  static void handle_wibble(void)
>>  {
>>         int val = CONFIG_WIBBLE_ADDR;
>>
>>         ...
>>  }
>>  #endif
>>
>>  static void init_machine()
>>  {
>>  ...
>>  #ifdef CONFIG_WIBBLE
>>         handle_wibble();
>>  #endif
>>  }
>>
>> Add a new IF_ENABLED_INT() to help with this. So now it is possible to
>> write, without #ifdefs:
>>
>>  static void handle_wibble(void)
>>  {
>>         int val = IF_ENABLED_INT(CONFIG_WIBBLE, CONFIG_WIBBLE_ADDR);
>>
>>         ...
>>  }
>>
>>  static void init_machine()
>>  {
>>  ...
>>  if (IS_ENABLED(CONFIG_WIBBLE))
>>         handle_wibble();
>>  }
>>
>> The value will be 0 if CONFIG_WIBBLE is not defined, and
>> CONFIG_WIBBLE_ADDR if it is. This allows us to reduce the use of #ifdef in
>> the code, ensuring that the compiler still checks the code even if it is
>> not ultimately used for a particular build.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>>  include/linux/kconfig.h | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
>> index 3a2da738c4..86cd266540 100644
>> --- a/include/linux/kconfig.h
>> +++ b/include/linux/kconfig.h
>> @@ -79,6 +79,18 @@
>>   */
>>  #define CONFIG_VAL(option)  config_val(option)
>>
>> +/* This use a similar mechanism to config_enabled() above */
>> +#define config_opt_enabled(cfg, opt_cfg) _config_opt_enabled(cfg, opt_cfg)
>> +#define _config_opt_enabled(cfg_val, opt_value) \
>> +       __config_opt_enabled(__ARG_PLACEHOLDER_##cfg_val, opt_value)
>> +#define __config_opt_enabled(arg1_or_junk, arg2) \
>> +       ___config_opt_enabled(arg1_or_junk arg2, 0)
>> +#define ___config_opt_enabled(__ignored, val, ...) val
>> +
>> +/* Evaluates to 0 if option is not defined, int_option if it is defined */
>> +#define IF_ENABLED_INT(option, int_option) \
>> +       config_opt_enabled(option, int_option)
>> +
>>  /*
>>   * CONFIG_IS_ENABLED(FOO) evaluates to
>>   *  1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y' or 'm',
>> --
>> 2.27.0.rc0.183.gde8f92d652-goog
>>
>
>
Simon Glass May 22, 2020, 2:24 p.m. | #3
Hi,

On Fri, 22 May 2020 at 00:18, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 5/22/20 4:17 AM, Masahiro Yamada wrote:
> > On Fri, May 22, 2020 at 11:02 AM Simon Glass <sjg at chromium.org> wrote:
> >>
> >> At present if an optional Kconfig value needs to be used it must be
> >> bracketed by #ifdef. For example, with this Kconfig setup:
> >>
> >> config WIBBLE
> >>         bool "Support wibbles, the world needs more wibbles"
> >>
> >> config WIBBLE_ADDR
> >>         hex "Address of the wibble"
> >>         depends on WIBBLE
> >
> >
> >
> >
> > I am not sure if this is a good idea.
> >
> >
> > If you want to always use CONFIG_WIBBLE_ADDR,
> > you can get rid of 'depends on WIBBLE'.

Yes that's right.

But I worry that we end up clutting the Kconfig with unused things.

Another option would be for Kconfig to emit hex CONFIGs always, even
if optional?

> >
> >
>
> Hello Simon,
>
> what is the effect on the code size if you eliminate the #ifdefs?
>
> Isn't this move growing the size of the U-Boot binary?

No it is handled at compile time so the code doesn't make it into the image.

[..]

Regards,
Simon

Patch

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 3a2da738c4..86cd266540 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -79,6 +79,18 @@ 
  */
 #define CONFIG_VAL(option)  config_val(option)
 
+/* This use a similar mechanism to config_enabled() above */
+#define config_opt_enabled(cfg, opt_cfg) _config_opt_enabled(cfg, opt_cfg)
+#define _config_opt_enabled(cfg_val, opt_value) \
+	__config_opt_enabled(__ARG_PLACEHOLDER_##cfg_val, opt_value)
+#define __config_opt_enabled(arg1_or_junk, arg2) \
+	___config_opt_enabled(arg1_or_junk arg2, 0)
+#define ___config_opt_enabled(__ignored, val, ...) val
+
+/* Evaluates to 0 if option is not defined, int_option if it is defined */
+#define IF_ENABLED_INT(option, int_option) \
+	config_opt_enabled(option, int_option)
+
 /*
  * CONFIG_IS_ENABLED(FOO) evaluates to
  *  1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y' or 'm',