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/+/32226
to review the following change.
Change subject: commonlib/cbfs: Check for presence of CONFIG() macro ......................................................................
commonlib/cbfs: Check for presence of CONFIG() macro
Check for CONFIG not IS_ENABLED, as we use the former now.
Change-Id: I7e1b67bc0894ca6f0149039054449656b58bcdd3 Signed-off-by: Nico Huber nico.h@gmx.de --- M src/commonlib/cbfs.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/32226/1
diff --git a/src/commonlib/cbfs.c b/src/commonlib/cbfs.c index 250a72a..4379099 100644 --- a/src/commonlib/cbfs.c +++ b/src/commonlib/cbfs.c @@ -25,7 +25,7 @@ #if !defined(LOG) #define LOG(x...) printk(BIOS_INFO, "CBFS: " x) #endif -#if defined(IS_ENABLED) +#if defined(CONFIG)
#if CONFIG(DEBUG_CBFS) #define DEBUG(x...) printk(BIOS_SPEW, "CBFS: " x)
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32226 )
Change subject: commonlib/cbfs: Check for presence of CONFIG() macro ......................................................................
Patch Set 1: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32226 )
Change subject: commonlib/cbfs: Check for presence of CONFIG() macro ......................................................................
Patch Set 1: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32226 )
Change subject: commonlib/cbfs: Check for presence of CONFIG() macro ......................................................................
Patch Set 1: Code-Review+1
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32226 )
Change subject: commonlib/cbfs: Check for presence of CONFIG() macro ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/32226/1/src/commonlib/cbfs.c File src/commonlib/cbfs.c:
https://review.coreboot.org/#/c/32226/1/src/commonlib/cbfs.c@28 PS1, Line 28: #if defined(CONFIG) This is very weird code. Why doesn't it do #ifdef printk instead? Looks like that's really what it wants to do. (Also the #elif/#endif below could really use a comment.)
...but not your fault.
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32226 )
Change subject: commonlib/cbfs: Check for presence of CONFIG() macro ......................................................................
commonlib/cbfs: Check for presence of CONFIG() macro
Check for CONFIG not IS_ENABLED, as we use the former now.
Change-Id: I7e1b67bc0894ca6f0149039054449656b58bcdd3 Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/32226 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Julius Werner jwerner@chromium.org --- M src/commonlib/cbfs.c 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve HAOUAS Elyes: Looks good to me, approved Julius Werner: Looks good to me, approved
diff --git a/src/commonlib/cbfs.c b/src/commonlib/cbfs.c index 250a72a..4379099 100644 --- a/src/commonlib/cbfs.c +++ b/src/commonlib/cbfs.c @@ -25,7 +25,7 @@ #if !defined(LOG) #define LOG(x...) printk(BIOS_INFO, "CBFS: " x) #endif -#if defined(IS_ENABLED) +#if defined(CONFIG)
#if CONFIG(DEBUG_CBFS) #define DEBUG(x...) printk(BIOS_SPEW, "CBFS: " x)
-
HAOUAS Elyes (Code Review) -
Julius Werner (Code Review)
-
Kyösti Mälkki (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)