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; -- To view, visit https://review.coreboot.org/c/coreboot/+/35887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9facecc4caa869e65f938a55508741dd48b09b0 Gerrit-Change-Number: 35887 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-MessageType: newchange
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? -- To view, visit https://review.coreboot.org/c/coreboot/+/35887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9facecc4caa869e65f938a55508741dd48b09b0 Gerrit-Change-Number: 35887 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Tue, 08 Oct 2019 12:58:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
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. -- To view, visit https://review.coreboot.org/c/coreboot/+/35887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9facecc4caa869e65f938a55508741dd48b09b0 Gerrit-Change-Number: 35887 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Mon, 04 Nov 2019 17:34:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
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`.
-- To view, visit https://review.coreboot.org/c/coreboot/+/35887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9facecc4caa869e65f938a55508741dd48b09b0 Gerrit-Change-Number: 35887 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Mon, 04 Nov 2019 17:43:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment
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
-- To view, visit https://review.coreboot.org/c/coreboot/+/35887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9facecc4caa869e65f938a55508741dd48b09b0 Gerrit-Change-Number: 35887 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 05 Nov 2019 16:44:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
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. -- To view, visit https://review.coreboot.org/c/coreboot/+/35887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9facecc4caa869e65f938a55508741dd48b09b0 Gerrit-Change-Number: 35887 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-MessageType: abandon
participants (3)
-
Arthur Heymans (Code Review) -
Nico Huber (Code Review)
-
Paul Menzel (Code Review)