Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang.
Subrata Banik has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84104?usp=email )
Change subject: soc/intel/common/block/pmc: Add GPE1 functions ......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/84104/comment/76a6f2dd_c3ae42da?usp... : PS3, Line 367: pmc_clear_std_gpe_status
I would have implemented this logic in a more abstract way (w/ an assumption that we have either one Kconfig that tells me if GPE1 exists or use a macro as per https://review.coreboot.org/c/coreboot/+/84103/comment/39abef01_50206d52/
static void pmc_clear_std_gpe_status(void) { print_std_gpe0_sts(reset_std_gpe0_status()); if (GPE1_STS(0)) print_std_gpe1_sts(reset_std_gpe1_status()); }
or
static void pmc_clear_std_gpe_status(void) { print_std_gpe0_sts(reset_std_gpe0_status()); if (SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1) print_std_gpe1_sts(reset_std_gpe1_status()); }
or maybe something like this ? then you don't need to pass `_gpeX_` for each functions
``` static void pmc_clear_std_gpe_status(void) { print_std_gpe0_sts(reset_std_gpe_status(0)); if (GPE1_STS(0)) print_std_gpe1_sts(reset_std_gpe_status(1)); } ```
or
``` static void pmc_clear_std_gpe_status(void) { print_std_gpe0_sts(reset_std_gpe_status(0)); if (SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1) print_std_gpe1_sts(reset_std_gpe_status(1)); } ```