Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41059 )
Change subject: payloads/libpayload: Fix BIT macro redefinition error ......................................................................
payloads/libpayload: Fix BIT macro redefinition error
/firmware/libpayload/bin/../include/libpayload.h:88: error: "BIT" redefined [-Werror] #define BIT(x) (1ul << (x)) from src/board/tglrvp/board.c:28: usr/include/chromeos/ec/ec_commands.h:49: note: this is the location of the previous definition #define BIT(nr) (1UL << (nr))
Change-Id: I0a5fd069acbd7e59980f6602b63362fcb3220009 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M payloads/libpayload/include/libpayload.h 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/41059/1
diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index e3f8fd3..65aec2e 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -85,7 +85,9 @@ #define MAX(a, b) __CMP(a, b, >)
#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) +#ifndef BIT #define BIT(x) (1ul << (x)) +#endif
#define DIV_ROUND_UP(x, y) ({ \ typeof(x) _div_local_x = (x); \
Hello build bot (Jenkins), Furquan Shaikh, Caveh Jalali, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41059
to look at the new patch set (#2).
Change subject: payloads/libpayload: Fix BIT macro redefinition error ......................................................................
payloads/libpayload: Fix BIT macro redefinition error
Compilation Error: /firmware/libpayload/bin/../include/libpayload.h:88: error: "BIT" redefined [-Werror] #define BIT(x) (1ul << (x)) from src/board/tglrvp/board.c:28: usr/include/chromeos/ec/ec_commands.h:49: note: this is the location of the previous definition #define BIT(nr) (1UL << (nr))
TEST=Able to emerge tglrvp depthcharge and libpayload without any compilation error.
Change-Id: I0a5fd069acbd7e59980f6602b63362fcb3220009 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M payloads/libpayload/include/libpayload.h 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/41059/2
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41059 )
Change subject: payloads/libpayload: Fix BIT macro redefinition error ......................................................................
Patch Set 2:
where are you getting this error - is this from depthcharge/src/board/tglrvp/board.c?
i'd suggest moving <libpayload.h> and <sysinfo.h> up in the up in the include order. also, which file is including ec_commands.h directly? it may be better to include ec.h instead. it's essentially a wrapper and will take care of the libpayload.h dependency and ordering.
ec_commands.h has a guard for a previous BIT, so as long as libpayload.h comes first, it'll be fine.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41059 )
Change subject: payloads/libpayload: Fix BIT macro redefinition error ......................................................................
Patch Set 2:
Patch Set 2:
where are you getting this error - is this from depthcharge/src/board/tglrvp/board.c?
i was doing emerge-tglrvp depthcharge libpayload and could see this issue.
i'd suggest moving <libpayload.h> and <sysinfo.h> up in the up in the include order. also, which file is including ec_commands.h directly? it may be better to include ec.h instead. it's essentially a wrapper and will take care of the libpayload.h dependency and ordering.
ec_commands.h has a guard for a previous BIT, so as long as libpayload.h comes first, it'll be fine.
+#include <libpayload.h> +#include <sysinfo.h> #include "base/init_funcs.h" #include "base/list.h" #include "drivers/ec/cros/lpc.h" @@ -33,25 +35,33 @@ #include "drivers/power/pch.h" #include "drivers/storage/blockdev.h" #include "drivers/storage/nvme.h" -#include <libpayload.h> -#include <sysinfo.h>
this is fixing as well. But do you like to have this hard dependency of include ordering or having this check here make sense. I believe this issue has started coming along with EC_HEADERS CL.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41059 )
Change subject: payloads/libpayload: Fix BIT macro redefinition error ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
where are you getting this error - is this from depthcharge/src/board/tglrvp/board.c?
i was doing emerge-tglrvp depthcharge libpayload and could see this issue.
i'd suggest moving <libpayload.h> and <sysinfo.h> up in the up in the include order. also, which file is including ec_commands.h directly? it may be better to include ec.h instead. it's essentially a wrapper and will take care of the libpayload.h dependency and ordering.
ec_commands.h has a guard for a previous BIT, so as long as libpayload.h comes first, it'll be fine.
+#include <libpayload.h> +#include <sysinfo.h> #include "base/init_funcs.h" #include "base/list.h" #include "drivers/ec/cros/lpc.h" @@ -33,25 +35,33 @@ #include "drivers/power/pch.h" #include "drivers/storage/blockdev.h" #include "drivers/storage/nvme.h" -#include <libpayload.h> -#include <sysinfo.h>
this is fixing as well. But do you like to have this hard dependency of include ordering or having this check here make sense. I believe this issue has started coming along with EC_HEADERS CL.
ya, the issue started with the EC_HEADERS CL. the issue is that care must be taken when sharing headers with another source tree and cpu arch - whether it's a checked-in copy or an auto-exported copy. the wrapper shouldn't have any include ordering issues. it's better to use the wrapper header file than directly using the "low level" header file. over time, there may be other quirks that we'd want to handle in the wrapper as well. we'll make sure the wrapper header file gets us the right thing.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41059 )
Change subject: payloads/libpayload: Fix BIT macro redefinition error ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
where are you getting this error - is this from depthcharge/src/board/tglrvp/board.c?
i was doing emerge-tglrvp depthcharge libpayload and could see this issue.
i'd suggest moving <libpayload.h> and <sysinfo.h> up in the up in the include order. also, which file is including ec_commands.h directly? it may be better to include ec.h instead. it's essentially a wrapper and will take care of the libpayload.h dependency and ordering.
ec_commands.h has a guard for a previous BIT, so as long as libpayload.h comes first, it'll be fine.
+#include <libpayload.h> +#include <sysinfo.h> #include "base/init_funcs.h" #include "base/list.h" #include "drivers/ec/cros/lpc.h" @@ -33,25 +35,33 @@ #include "drivers/power/pch.h" #include "drivers/storage/blockdev.h" #include "drivers/storage/nvme.h" -#include <libpayload.h> -#include <sysinfo.h>
this is fixing as well. But do you like to have this hard dependency of include ordering or having this check here make sense. I believe this issue has started coming along with EC_HEADERS CL.
ya, the issue started with the EC_HEADERS CL. the issue is that care must be taken when sharing headers with another source tree and cpu arch - whether it's a checked-in copy or an auto-exported copy. the wrapper shouldn't have any include ordering issues. it's better to use the wrapper header file than directly using the "low level" header file. over time, there may be other quirks that we'd want to handle in the wrapper as well. we'll make sure the wrapper header file gets us the right thing.
So u r suggesting to modify all possible DC code to add a wrapper file to abstract inclusion of below headers? to bypass this ordering issue? #include <libpayload.h> #include <sysinfo.h>
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41059 )
Change subject: payloads/libpayload: Fix BIT macro redefinition error ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
where are you getting this error - is this from depthcharge/src/board/tglrvp/board.c?
i was doing emerge-tglrvp depthcharge libpayload and could see this issue.
i'd suggest moving <libpayload.h> and <sysinfo.h> up in the up in the include order. also, which file is including ec_commands.h directly? it may be better to include ec.h instead. it's essentially a wrapper and will take care of the libpayload.h dependency and ordering.
ec_commands.h has a guard for a previous BIT, so as long as libpayload.h comes first, it'll be fine.
+#include <libpayload.h> +#include <sysinfo.h> #include "base/init_funcs.h" #include "base/list.h" #include "drivers/ec/cros/lpc.h" @@ -33,25 +35,33 @@ #include "drivers/power/pch.h" #include "drivers/storage/blockdev.h" #include "drivers/storage/nvme.h" -#include <libpayload.h> -#include <sysinfo.h>
this is fixing as well. But do you like to have this hard dependency of include ordering or having this check here make sense. I believe this issue has started coming along with EC_HEADERS CL.
ya, the issue started with the EC_HEADERS CL. the issue is that care must be taken when sharing headers with another source tree and cpu arch - whether it's a checked-in copy or an auto-exported copy. the wrapper shouldn't have any include ordering issues. it's better to use the wrapper header file than directly using the "low level" header file. over time, there may be other quirks that we'd want to handle in the wrapper as well. we'll make sure the wrapper header file gets us the right thing.
So u r suggesting to modify all possible DC code to add a wrapper file to abstract inclusion of below headers? to bypass this ordering issue? #include <libpayload.h> #include <sysinfo.h>
no, we already have ec.h as a wrapper - no action needed there. we may want to remove cros/message.h from cros/lpc.h as that seems to be where things are going wrong.
i do have a bug open to fix message.h - it's basically wrong anyway (b/153581197).
Subrata Banik has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/41059 )
Change subject: payloads/libpayload: Fix BIT macro redefinition error ......................................................................
Abandoned
-
Caveh Jalali (Code Review) -
Subrata Banik (Code Review)