Maulik V Vaghela 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 7:
(5 comments)
https://review.coreboot.org/#/c/31284/7/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/31284/7/src/soc/intel/cannonlake/chip.h@304 PS7, Line 304: uint8_t SerialIoUartMode[SOC_INTEL_CML_UART_DEV_MAX];
I think this file is a perfect place to abstract over […]
Yes, It an make device tree struct constant but it'll be add more complexity in fsp_param.c. Also I feel that it would make the code difficult to understand. This change will provide clear picture of what changes are coming into new fsp.
https://review.coreboot.org/#/c/31284/7/src/soc/intel/cannonlake/fsp_params.... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/#/c/31284/7/src/soc/intel/cannonlake/fsp_params.... PS7, Line 37: #if IS_ENABLED(CONFIG_SOC_INTEL_COMETLAKE)
Please use a C `if` statement instead.
If we use C if statement, compilation would fail since fsp paramaters wouldn't be included in headers.
https://review.coreboot.org/#/c/31284/7/src/soc/intel/cannonlake/fsp_params.... PS7, Line 38: memcpy(params->SerialIoI2cMode, config->SerialIoI2cMode, CONFIG_SOC_INTEL_I2C_DEV_MAX);
The original code below was explicitly written to allow the […]
Ack
https://review.coreboot.org/#/c/31284/7/src/soc/intel/cannonlake/fsp_params.... PS7, Line 184: #if IS_ENABLED(CONFIG_SOC_INTEL_COMETLAKE)
C `if` please.
Compilation would fail since FSP header file will have only one of the parameter.
https://review.coreboot.org/#/c/31284/7/src/soc/intel/cannonlake/include/soc... File src/soc/intel/cannonlake/include/soc/serialio.h:
https://review.coreboot.org/#/c/31284/7/src/soc/intel/cannonlake/include/soc... PS7, Line 26: PchSerialIoSkipInit
AIUI, the last two are only available for UARTs (at least that's […]
It'll be difficult to add FSP version details since these changes are coming up with new CML FSP changes. I can surely pass your feedback regarding enum changes to our FSP team.