Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38512 )
Change subject: soc/intel/skylake: Only reserve TPM area for !CONFIG_TPM_CR50 device ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38512/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38512/3//COMMIT_MSG@10 PS3, Line 10: and the size of the MMIO area is 20KB (0x5000). This explains nothing about the actual change.
https://review.coreboot.org/c/coreboot/+/38512/3/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/systemagent.asl:
https://review.coreboot.org/c/coreboot/+/38512/3/src/soc/intel/skylake/acpi/... PS3, Line 185: #if !CONFIG(TPM_CR50) This is highly irritating. By excluding one case when it is wrong, you make it look like it's correct otherwise. When is this reservation actually correct?
I'm not an expert, but a hunch tells me this should be `#if CONFIG(LPC_TPM)`.