Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42201 )
Change subject: soc/intel/skylake: Check Kconfig symbols in C instead of preprocessor ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42201/3/src/soc/intel/skylake/acpi.... File src/soc/intel/skylake/acpi.c:
https://review.coreboot.org/c/coreboot/+/42201/3/src/soc/intel/skylake/acpi.... PS3, Line 179: else {
please why this new 'else' ? […]
The code as-is looks a bit non-sensical (first setting vbt2 to ACTIVE_ECFW_RO/RW depending on running_ro(), then always setting it to _RO), so if anything this is a bug fix. Cannon-, Ice-, Jasper-, Tiger-Lake also have the semantics of the new version, while Broadwell also has the old one here (and probably should be fixed as well).
The code seems to be originally form Broadwell, and introduced by Duncan so looping in him in case I (and all the editors of the newer intel chipset code) missed something.