Hello HAOUAS Elyes, Julius Werner, Angel Pons, Patrick Georgi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32230
to review the following change.
Change subject: kconfig: Drop IS_ENABLED() macro ......................................................................
kconfig: Drop IS_ENABLED() macro
Change-Id: I8fc0d0136ba2316ef393c5c17f2b3ac3a9c6328d Signed-off-by: Nico Huber nico.h@gmx.de --- M payloads/libpayload/include/kconfig.h M src/include/kconfig.h 2 files changed, 0 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/32230/1
diff --git a/payloads/libpayload/include/kconfig.h b/payloads/libpayload/include/kconfig.h index 9cce6ea..203494f 100644 --- a/payloads/libpayload/include/kconfig.h +++ b/payloads/libpayload/include/kconfig.h @@ -17,7 +17,6 @@ #define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0, 0) #define ___config_enabled(__ignored, val, ...) val
-#define IS_ENABLED(option) config_enabled(option) /* deprecated */ #define CONFIG(option) config_enabled(CONFIG_##option)
#endif diff --git a/src/include/kconfig.h b/src/include/kconfig.h index 0478548..50ef302 100644 --- a/src/include/kconfig.h +++ b/src/include/kconfig.h @@ -17,7 +17,6 @@ #define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0, 0) #define ___config_enabled(__ignored, val, ...) val
-#define IS_ENABLED(option) config_enabled(option) /* deprecated */ #define CONFIG(option) config_enabled(CONFIG_##option)
#endif
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32230 )
Change subject: kconfig: Drop IS_ENABLED() macro ......................................................................
Patch Set 1: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32230 )
Change subject: kconfig: Drop IS_ENABLED() macro ......................................................................
Patch Set 1: Code-Review+1
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32230 )
Change subject: kconfig: Drop IS_ENABLED() macro ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32230/1/payloads/libpayload/include/kconfig.... File payloads/libpayload/include/kconfig.h:
https://review.coreboot.org/#/c/32230/1/payloads/libpayload/include/kconfig.... PS1, Line 20: #define CONFIG(option) config_enabled(CONFIG_##option) Are we worried about forcing payloads to change? Depthcharge for example has lots of IS_ENABLED() in it. I'm happy to change those, but other payloads may not be that closely maintained...
Hello HAOUAS Elyes, Julius Werner, Angel Pons, Paul Menzel, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32230
to look at the new patch set (#2).
Change subject: kconfig: Drop IS_ENABLED() macro ......................................................................
kconfig: Drop IS_ENABLED() macro
Change-Id: I8fc0d0136ba2316ef393c5c17f2b3ac3a9c6328d Signed-off-by: Nico Huber nico.h@gmx.de --- M Documentation/getting_started/kconfig.md M payloads/libpayload/include/kconfig.h M src/include/kconfig.h 3 files changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/32230/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32230 )
Change subject: kconfig: Drop IS_ENABLED() macro ......................................................................
Patch Set 3:
(1 comment)
This change is ready for review.
https://review.coreboot.org/#/c/32230/1/payloads/libpayload/include/kconfig.... File payloads/libpayload/include/kconfig.h:
https://review.coreboot.org/#/c/32230/1/payloads/libpayload/include/kconfig.... PS1, Line 20: #define CONFIG(option) config_enabled(CONFIG_##option)
Are we worried about forcing payloads to change? Depthcharge for example has lots of IS_ENABLED() in […]
I reverted this hunk for now. Better check the coreboot tree now and worry about payloads later.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32230 )
Change subject: kconfig: Drop IS_ENABLED() macro ......................................................................
Patch Set 3: Code-Review+2
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32230 )
Change subject: kconfig: Drop IS_ENABLED() macro ......................................................................
kconfig: Drop IS_ENABLED() macro
We keep its definition in libpayload, though, to maintain compatibility with existing payload code. For now.
Change-Id: I8fc0d0136ba2316ef393c5c17f2b3ac3a9c6328d Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/32230 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M Documentation/getting_started/kconfig.md M src/include/kconfig.h 2 files changed, 0 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/Documentation/getting_started/kconfig.md b/Documentation/getting_started/kconfig.md index 249fd46..852ca08 100644 --- a/Documentation/getting_started/kconfig.md +++ b/Documentation/getting_started/kconfig.md @@ -1165,8 +1165,6 @@ - coreboot has added the glob operator '*' for the 'source' keyword. - coreboot’s Kconfig always defines variables except for strings. In other Kconfig implementations, bools set to false/0/no are not defined. -- IS_ENABLED() is ‘false’ for undefined variables and ‘0’ variables. In Linux - (where the macro comes from) it’s ‘true’ as soon as the variable is defined. - coreboot’s version of Kconfig adds the KCONFIG_STRICT environment variable to error out if there are any issues in the Kconfig files. In the Linux kernel, Kconfig will generate a warning, but will still output an updated .config or diff --git a/src/include/kconfig.h b/src/include/kconfig.h index 0478548..50ef302 100644 --- a/src/include/kconfig.h +++ b/src/include/kconfig.h @@ -17,7 +17,6 @@ #define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0, 0) #define ___config_enabled(__ignored, val, ...) val
-#define IS_ENABLED(option) config_enabled(option) /* deprecated */ #define CONFIG(option) config_enabled(CONFIG_##option)
#endif
-
HAOUAS Elyes (Code Review) -
Julius Werner (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)