Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31284 )
Change subject: soc/intel/cannonlake: Add required FSP UPD changes for CML ......................................................................
Patch Set 16:
(4 comments)
https://review.coreboot.org/#/c/31284/15/src/soc/intel/cannonlake/fsp_params... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/#/c/31284/15/src/soc/intel/cannonlake/fsp_params... PS15, Line 95: params->SerialIoDevMode[i] = config->SerialIoDevMode[i];
True. Existing logic assigns default value as SerialIoPci in case no value assigned in dt. […]
In this version a `0` / PchSerialIoDisabled / unset was ignored but in the new version it is copied.
https://review.coreboot.org/#/c/31284/15/src/soc/intel/cannonlake/fsp_params... PS15, Line 110: #if IS_ENABLED(CONFIG_SOC_INTEL_COMETLAKE)
Done
Nice, thank you :)
https://review.coreboot.org/#/c/31284/16/src/soc/intel/cannonlake/fsp_params... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/#/c/31284/16/src/soc/intel/cannonlake/fsp_params... PS16, Line 58: if (config->SerialIoDevMode[dev_offset] <= PchSerialIoSkipInit) A value of 0 (i.e. unset) is also <= PchSerialIoSkipInit and would override the Pci default from above.
https://review.coreboot.org/#/c/31284/15/src/soc/intel/cannonlake/include/so... File src/soc/intel/cannonlake/include/soc/serialio.h:
https://review.coreboot.org/#/c/31284/15/src/soc/intel/cannonlake/include/so... PS15, Line 35: #endif
PchSerialIoHidden has 2 different value but does the same job. […]
What about PchSerialIoLegacyUart then? does it do the same job, too? If so, why are there two different values?