[coreboot-gerrit] Change in ...coreboot[master]: soc/intel/cannonlake: Add required FSP UPD changes for CML

[Subrata Banik (Code Review)]

Subrata Banik 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 9:

(2 comments)

https://review.coreboot.org/#/c/31284/9/src/soc/intel/cannonlake/fsp_params.... 
File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/#/c/31284/9/src/soc/intel/cannonlake/fsp_params.... 
PS9, Line 38: #if IS_ENABLED(CONFIG_SOC_INTEL_COMETLAKE)
> Can we split this into separate functions to make it easier to read: […]

good idea Furquan.

Also i have one open.

can we create something like soc/soctree.cb to move all FSP UPD related changes from devicetree.cb to soctree.cb ?

in that sense we might not need to copy those common FSP UPD across different mb/devicetree.cb

https://review.coreboot.org/#/c/31284/9/src/soc/intel/cannonlake/include/soc... 
File src/soc/intel/cannonlake/include/soc/serialio.h:

https://review.coreboot.org/#/c/31284/9/src/soc/intel/cannonlake/include/soc... 
PS9, Line 20: typedef enum {
            : 	PchSerialIoDisabled,
            : 	PchSerialIoPci,
            : 	PchSerialIoAcpi,
            : 	PchSerialIoHidden,
            : #if IS_ENABLED(CONFIG_SOC_INTEL_COMETLAKE)
            : 	PchSerialIoSkipInit
            : #endif
            : } PCH_SERIAL_IO_MODE;
> @Maulik: lets redefine this as below […]

Note above is just a proposal that I'm working internally to see if we can implement the same to reduce duplicate effort