Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29284 )
Change subject: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode ......................................................................
Patch Set 8:
Patch Set 8: Code-Review-1
FSP MR2 does a check for ACPI mode and change the device mode. Supplying incorrect configuration will be corrected by FSP, but to be futher proof supply correct configuration to FSP.
I can't say that I understand this yet. You are saying FSP allows a value of 2 (PCH_ACPI_MODE). But then you say FSP would be correcting incorrect configuration, which translates to me as it reads the hardware state and ignores the diffe- rence between a 1 and a 2.
And about being future proof. That's generally a good idea, but how can we anticipate the future here? How can we tell that if FSP is being corrected, if the code will be made to match the header file or the other way around?
The goal is to supply correct mode settings to FSP. For this reason the modification is made. On the other hand FSP (MR2) will modify settings of the child devices if incorrect. What FSP is doing is not wrong, but for those without access to FSP sources, we should ensure that coreboot is configuring the correct mode.