Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35402 )
Change subject: soc/intel/common/block/cse: Add boot partition related APIs ......................................................................
Patch Set 19:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/block... File src/soc/intel/common/block/cse/Kconfig:
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/block... PS12, Line 10: 3
Then, maybe its better to just have this as a macro if you are not expecting it to change?
For chrome systems, CSE region will have 3 paritions only if CSE supports FW update. Otherwise , CSE will have only 2 partitions. The valid boot partition are BP1, and BP2 in chrome systems.
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/block... PS12, Line 781: partition_id != BP1 && partition_id != BP2
Those details should be captured in comments. […]
Sure, I will capture the details in the comments.I will check on spec, and get back to you on it.
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/block... PS12, Line 153: enum
Can those instances use something like BP(1) and BP(2)?
For chrome system,CSE region will have max 3 partitions. So, no change to existing enum definations. Thanks
https://review.coreboot.org/c/coreboot/+/35402/6/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35402/6/src/soc/intel/common/block/... PS6, Line 147: MAX_ME_PARTITIONS
Yes, that would be ideal.
I will move definition of CSE_MAX_BOOT_PARTITIONS to header file.
-
Sridhar Siricilla (Code Review)