Sean Rhodes has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86164?usp=email )
Change subject: soc/intel: Allow zero values for PMC GPE0 DW registers ......................................................................
soc/intel: Allow zero values for PMC GPE0 DW registers
The `pmc_gpe0_different_values` function previously asserted if any two of the GPE0 DW registers (DW0, DW1, DW2) had the same value, as introduced in commit 640a41f3ee938b794b14 ("soc/intel: Assert if `pmc_/gpe0_dwX` values are not unique"). This prevented platforms from configuring GPE routing via PMC as per default register (MISCCFG) value.
This commit modifies the check to allow all DW registers to be zero. This enables platforms that rely on MISCCFG register for PMC-controlled GPE routing to boot without triggering the assertion.
The change was verified by testing the following scenarios:
- All DWs zero: The system boots using the default GPE route. No assertion occurs. - Duplicate DWs (e.g., DW0=1, DW1=2, DW2=2): The existing assertion is triggered as expected. - Unique DWs (e.g., DW0=1, DW1=2, DW2=3): No errors occur.
TEST=Built and booted normally. No assertion failure observed.
Change-Id: Ie66d6dbcf49d5400b3fc3e4da113a569fe52dd51 Signed-off-by: Subrata Banik subratabanik@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/86164 Reviewed-by: Jérémy Compostella jeremy.compostella@intel.com Reviewed-by: Matt DeVillier matt.devillier@gmail.com Reviewed-by: Dinesh Gehlot digehlot@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/alderlake/pmutil.c M src/soc/intel/apollolake/pmutil.c M src/soc/intel/cannonlake/pmutil.c M src/soc/intel/elkhartlake/pmutil.c M src/soc/intel/jasperlake/pmutil.c M src/soc/intel/meteorlake/pmutil.c M src/soc/intel/pantherlake/pmutil.c M src/soc/intel/skylake/pmutil.c M src/soc/intel/tigerlake/pmutil.c 9 files changed, 63 insertions(+), 18 deletions(-)
Approvals: Dinesh Gehlot: Looks good to me, approved Matt DeVillier: Looks good to me, approved build bot (Jenkins): Verified Jérémy Compostella: Looks good to me, approved
diff --git a/src/soc/intel/alderlake/pmutil.c b/src/soc/intel/alderlake/pmutil.c index 60d6e10..c459100 100644 --- a/src/soc/intel/alderlake/pmutil.c +++ b/src/soc/intel/alderlake/pmutil.c @@ -156,11 +156,16 @@
static void pmc_gpe0_different_values(const struct soc_intel_alderlake_config *config) { - bool result = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && + bool all_zero = (config->pmc_gpe0_dw0 == 0) && + (config->pmc_gpe0_dw1 == 0) && + (config->pmc_gpe0_dw2 == 0); + + /* Check if all values are different AND not all zero */ + bool all_different = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw2) && (config->pmc_gpe0_dw1 != config->pmc_gpe0_dw2);
- assert(result); + assert(all_different || all_zero); }
void soc_get_gpi_gpe_configs(uint8_t *dw0, uint8_t *dw1, uint8_t *dw2) diff --git a/src/soc/intel/apollolake/pmutil.c b/src/soc/intel/apollolake/pmutil.c index 5366ca8..ddc5647 100644 --- a/src/soc/intel/apollolake/pmutil.c +++ b/src/soc/intel/apollolake/pmutil.c @@ -140,11 +140,16 @@
static void gpe0_different_values(const struct soc_intel_apollolake_config *config) { - bool result = (config->gpe0_dw1 != config->gpe0_dw2) && + bool all_zero = (config->gpe0_dw1 == 0) && + (config->gpe0_dw2 == 0) && + (config->gpe0_dw3 == 0); + + /* Check if all values are different AND not all zero */ + bool all_different = (config->gpe0_dw1 != config->gpe0_dw2) && (config->gpe0_dw1 != config->gpe0_dw3) && (config->gpe0_dw2 != config->gpe0_dw3);
- assert(result); + assert(all_different || all_zero); }
void soc_get_gpi_gpe_configs(uint8_t *dw0, uint8_t *dw1, uint8_t *dw2) diff --git a/src/soc/intel/cannonlake/pmutil.c b/src/soc/intel/cannonlake/pmutil.c index 63eed16..f7d9345 100644 --- a/src/soc/intel/cannonlake/pmutil.c +++ b/src/soc/intel/cannonlake/pmutil.c @@ -150,11 +150,16 @@
static void gpe0_different_values(const struct soc_intel_cannonlake_config *config) { - bool result = (config->gpe0_dw0 != config->gpe0_dw1) && + bool all_zero = (config->gpe0_dw0 == 0) && + (config->gpe0_dw1 == 0) && + (config->gpe0_dw2 == 0); + + /* Check if all values are different AND not all zero */ + bool all_different = (config->gpe0_dw0 != config->gpe0_dw1) && (config->gpe0_dw0 != config->gpe0_dw2) && (config->gpe0_dw1 != config->gpe0_dw2);
- assert(result); + assert(all_different || all_zero); }
void soc_get_gpi_gpe_configs(uint8_t *dw0, uint8_t *dw1, uint8_t *dw2) diff --git a/src/soc/intel/elkhartlake/pmutil.c b/src/soc/intel/elkhartlake/pmutil.c index a862bdd..72f9bd6 100644 --- a/src/soc/intel/elkhartlake/pmutil.c +++ b/src/soc/intel/elkhartlake/pmutil.c @@ -149,11 +149,16 @@
static void pmc_gpe0_different_values(const struct soc_intel_elkhartlake_config *config) { - bool result = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && + bool all_zero = (config->pmc_gpe0_dw0 == 0) && + (config->pmc_gpe0_dw1 == 0) && + (config->pmc_gpe0_dw2 == 0); + + /* Check if all values are different AND not all zero */ + bool all_different = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw2) && (config->pmc_gpe0_dw1 != config->pmc_gpe0_dw2);
- assert(result); + assert(all_different || all_zero); }
void soc_get_gpi_gpe_configs(uint8_t *dw0, uint8_t *dw1, uint8_t *dw2) diff --git a/src/soc/intel/jasperlake/pmutil.c b/src/soc/intel/jasperlake/pmutil.c index 569eac7..09eb57f 100644 --- a/src/soc/intel/jasperlake/pmutil.c +++ b/src/soc/intel/jasperlake/pmutil.c @@ -149,11 +149,16 @@
static void pmc_gpe0_different_values(const struct soc_intel_jasperlake_config *config) { - bool result = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && + bool all_zero = (config->pmc_gpe0_dw0 == 0) && + (config->pmc_gpe0_dw1 == 0) && + (config->pmc_gpe0_dw2 == 0); + + /* Check if all values are different AND not all zero */ + bool all_different = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw2) && (config->pmc_gpe0_dw1 != config->pmc_gpe0_dw2);
- assert(result); + assert(all_different || all_zero); }
void soc_get_gpi_gpe_configs(uint8_t *dw0, uint8_t *dw1, uint8_t *dw2) diff --git a/src/soc/intel/meteorlake/pmutil.c b/src/soc/intel/meteorlake/pmutil.c index 154c3be..a0bba92 100644 --- a/src/soc/intel/meteorlake/pmutil.c +++ b/src/soc/intel/meteorlake/pmutil.c @@ -152,11 +152,16 @@
static void pmc_gpe0_different_values(const struct soc_intel_meteorlake_config *config) { - bool result = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && + bool all_zero = (config->pmc_gpe0_dw0 == 0) && + (config->pmc_gpe0_dw1 == 0) && + (config->pmc_gpe0_dw2 == 0); + + /* Check if all values are different AND not all zero */ + bool all_different = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw2) && (config->pmc_gpe0_dw1 != config->pmc_gpe0_dw2);
- assert(result); + assert(all_different || all_zero); }
void soc_get_gpi_gpe_configs(uint8_t *dw0, uint8_t *dw1, uint8_t *dw2) diff --git a/src/soc/intel/pantherlake/pmutil.c b/src/soc/intel/pantherlake/pmutil.c index 306d834..7ce1dac 100644 --- a/src/soc/intel/pantherlake/pmutil.c +++ b/src/soc/intel/pantherlake/pmutil.c @@ -147,11 +147,16 @@
static void pmc_gpe0_different_values(const struct soc_intel_pantherlake_config *config) { - bool result = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && + bool all_zero = (config->pmc_gpe0_dw0 == 0) && + (config->pmc_gpe0_dw1 == 0) && + (config->pmc_gpe0_dw2 == 0); + + /* Check if all values are different AND not all zero */ + bool all_different = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw2) && (config->pmc_gpe0_dw1 != config->pmc_gpe0_dw2);
- assert(result); + assert(all_different || all_zero); }
void soc_get_gpi_gpe_configs(uint8_t *dw0, uint8_t *dw1, uint8_t *dw2) diff --git a/src/soc/intel/skylake/pmutil.c b/src/soc/intel/skylake/pmutil.c index ed1ab8b..91f0836 100644 --- a/src/soc/intel/skylake/pmutil.c +++ b/src/soc/intel/skylake/pmutil.c @@ -156,11 +156,16 @@
static void gpe0_different_values(const struct soc_intel_skylake_config *config) { - bool result = (config->gpe0_dw0 != config->gpe0_dw1) && + bool all_zero = (config->gpe0_dw0 == 0) && + (config->gpe0_dw1 == 0) && + (config->gpe0_dw2 == 0); + + /* Check if all values are different AND not all zero */ + bool all_different = (config->gpe0_dw0 != config->gpe0_dw1) && (config->gpe0_dw0 != config->gpe0_dw2) && (config->gpe0_dw1 != config->gpe0_dw2);
- assert(result); + assert(all_different || all_zero); }
void soc_get_gpi_gpe_configs(uint8_t *dw0, uint8_t *dw1, uint8_t *dw2) diff --git a/src/soc/intel/tigerlake/pmutil.c b/src/soc/intel/tigerlake/pmutil.c index 7903264..0c5a1b59 100644 --- a/src/soc/intel/tigerlake/pmutil.c +++ b/src/soc/intel/tigerlake/pmutil.c @@ -155,11 +155,16 @@
static void pmc_gpe0_different_values(const struct soc_intel_tigerlake_config *config) { - bool result = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && + bool all_zero = (config->pmc_gpe0_dw0 == 0) && + (config->pmc_gpe0_dw1 == 0) && + (config->pmc_gpe0_dw2 == 0); + + /* Check if all values are different AND not all zero */ + bool all_different = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw2) && (config->pmc_gpe0_dw1 != config->pmc_gpe0_dw2);
- assert(result); + assert(all_different || all_zero); }
void soc_get_gpi_gpe_configs(uint8_t *dw0, uint8_t *dw1, uint8_t *dw2)
-
Sean Rhodes (Code Review)