Re: [PATCH 2/6] kconfig: Add support for conditional values
Hi, On Fri, 22 May 2020 at 00:18, Heinrich Schuchardt wrote: > > On 5/22/20 4:17 AM, Masahiro Yamada wrote: > > On Fri, May 22, 2020 at 11:02 AM Simon Glass 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
Re: [PATCH 2/6] kconfig: Add support for conditional values
On 5/22/20 4:17 AM, Masahiro Yamada wrote:
> On Fri, May 22, 2020 at 11:02 AM Simon Glass 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
>> ---
>>
>> 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
>>
>
>
Re: [PATCH 2/6] kconfig: Add support for conditional values
On Fri, May 22, 2020 at 11:02 AM Simon Glass 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
> ---
>
> 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
>
--
Best Regards
Masahiro Yamada
