Change in coreboot[master]: soc/intel/common: Add support for CSE IOM/NPHY sub-parition update

[Tim Wawrzynczak (Code Review)]

Attention is currently required from: Julius Werner, Sridhar Siricilla, Krishna P Bhat D.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59685 )

Change subject: soc/intel/common: Add support for CSE IOM/NPHY sub-parition update
......................................................................

Patch Set 14:

(13 comments)

File src/soc/intel/alderlake/romstage/romstage.c:

PS14: 
Separate CL to enable this for ADL please

https://review.coreboot.org/c/coreboot/+/59685/comment/56f81a79_647632fe 
PS14, Line 29: 	/* If SOC is not Alderlake A2, skip update */
             : 	if (cpu_get_cpuid() != CPUID_ALDERLAKE_A2) {
             : 		printk(BIOS_INFO, "CSE Sub-partition update not required\n");
             : 		return true;
             : 	}
             : 	return false;
I would rather keep the print in common code, and then this function just becomes:

`return cpu_get_cpuid() != CPUID_ALDERLAKE_A2;`

File src/soc/intel/common/block/cse/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/59685/comment/81c32eea_f87d39b8 
PS11, Line 110:
> These blobs are part of FW_MAIN_A/FW_MAIN_B/COREBOOT, they are verified by vboot.

Ah yes I see now.

File src/soc/intel/common/block/cse/cse_lite.c:

https://review.coreboot.org/c/coreboot/+/59685/comment/34e52af9_5d49fe6f 
PS11, Line 958: 	/* Get sub-partition blob's version */
> These blobs are part of FW_MAIN_A/FW_MAIN_B/COREBOOT, they are verified by vboot.

Ack

File src/soc/intel/common/block/cse/cse_lite.c:

https://review.coreboot.org/c/coreboot/+/59685/comment/023c21b5_fae4ae03 
PS14, Line 743: uint16_t
`enum bpdt_entry_type`

https://review.coreboot.org/c/coreboot/+/59685/comment/b13f1f26_cbb681ee 
PS14, Line 756: uint16_t
`enum bpdt_entry_type `

https://review.coreboot.org/c/coreboot/+/59685/comment/8a26bfee_528b90cd 
PS14, Line 799: uint16_t
`enum bpdt_entry_type`

https://review.coreboot.org/c/coreboot/+/59685/comment/54b0ccfc_8bba5739 
PS14, Line 833: ptr + SUBPART_HEADER_SZ
It is technically illegal C to do pointer arithmetic on a `void *`, but GCC does allow it (and probably clang too, I'm not sure). Would prefer if you use `uint8_t *` or `uintptr_t` and then cast to pointer later.

https://review.coreboot.org/c/coreboot/+/59685/comment/3995e2b7_1a138785 
PS14, Line 855: uint16_t
`enum bpdt_entry_type`

https://review.coreboot.org/c/coreboot/+/59685/comment/bfc09456_12bfcec5 
PS14, Line 913: 
nit: extra blank line

https://review.coreboot.org/c/coreboot/+/59685/comment/dd6c4f63_87213be9 
PS14, Line 997: /*
              : 		 * If system is in recovery mode, don't trigger recovery again */
nit: single-line comment

https://review.coreboot.org/c/coreboot/+/59685/comment/2ab1d9fb_e9583b72 
PS14, Line 1018:  
nit: extra space

https://review.coreboot.org/c/coreboot/+/59685/comment/20943132_c326b31b 
PS14, Line 1020: 		return;
nit: blank line after `}`