Change in coreboot[master]: soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE...
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35887 )
Change subject: soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE_LENGTH ......................................................................
soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE_LENGTH
Change-Id: Ib9facecc4caa869e65f938a55508741dd48b09b0 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/common/block/systemagent/systemagent_early.c 2 files changed, 4 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/35887/1
diff --git a/src/soc/intel/common/block/systemagent/Kconfig b/src/soc/intel/common/block/systemagent/Kconfig index d7619a0..81a7f9d 100644 --- a/src/soc/intel/common/block/systemagent/Kconfig +++ b/src/soc/intel/common/block/systemagent/Kconfig @@ -8,7 +8,6 @@ default 0x10000000 if (PCIEX_LENGTH_256MB) default 0x8000000 if (PCIEX_LENGTH_128MB) default 0x4000000 if (PCIEX_LENGTH_64MB) - default 0x10000000 help This option allows you to select length of PCIEX region.
diff --git a/src/soc/intel/common/block/systemagent/systemagent_early.c b/src/soc/intel/common/block/systemagent/systemagent_early.c index 8c89c07..5411238 100644 --- a/src/soc/intel/common/block/systemagent/systemagent_early.c +++ b/src/soc/intel/common/block/systemagent/systemagent_early.c @@ -26,6 +26,10 @@
#include "systemagent_def.h"
+#if CONFIG_SA_PCIEX_LENGTH == 0 +#error "SA_PCIEX_LENGTH undefined!" +#endif + void bootblock_systemagent_early_init(void) { uint32_t reg;
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35887 )
Change subject: soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE_LENGTH ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35887/1/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/35887/1/src/soc/intel/common/block/... PS1, Line 31: #endif A Static_assert() might look nicer. but that's a matter of taste, I guess?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35887 )
Change subject: soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE_LENGTH ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35887/2/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/35887/2/src/soc/intel/common/block/... PS2, Line 56: default: NB. This will result in a wrong reservation. And I know at least some Linux versions use the E820 entry to figure the size (yes, ACPI missed to define a way to pass the size to the OS). I guess it's at least worth a warning. Or if we trust our Kconfig, just die().
Or, the long version without trusting Kconfig, trigger the compile-time error if it's not set to one of the three.
Hmmm, after writing this out. What happens with a dead_code() here and without the `#if`? Theoretically, the whole `switch` can be decided at compile-time.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35887 )
Change subject: soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE_LENGTH ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35887/2/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/35887/2/src/soc/intel/common/block/... PS2, Line 56: default:
NB. This will result in a wrong reservation. And I know at least some Linux […]
Hah, dead_code() actually works. Just replace the next line with `dead_code();` and we won't need the `#if`.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35887 )
Change subject: soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE_LENGTH ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35887/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35887/2//COMMIT_MSG@7 PS2, Line 7: soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE_LENGTH Maybe:
Fail build on undefined SA_PCIE_LENGTH
Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35887 )
Change subject: soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE_LENGTH ......................................................................
Abandoned
Fixed in master.
-
Arthur Heymans (Code Review) -
Nico Huber (Code Review)
-
Paul Menzel (Code Review)