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 42:
(14 comments)
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... File src/soc/intel/common/block/cse/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 4: bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CSE) += cse_bp.c : romstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CSE) += cse_bp.c
We don't need this in bootblock and romstage
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse_bp.c:
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 103: force_cse_init
Then you don't need this complicated implementation. you only need 2 things […]
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 105: cse_bp_init
Agreed. […]
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 166: if (!cse_init_bp_info(true)) { : printk(BIOS_ERR, "Failed to update boot partition info\n"); : return false; : }
Please see my first comment
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 183: cse_init_bp_info(false)
check comment on cse_init_bp_info()
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 192: cse_init_bp_info
Please see my first comment.
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 201: cse_init_bp_info
Please see my first comment.
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 210: cse_init_bp_info
Please see my first comment.
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 221: cse_init_bp_info
Please see my first comment.
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 232: cse_init_bp_info
Please see my first comment.
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 304: cse_init_bp_info
Please see my first comment.
Done
https://review.coreboot.org/c/coreboot/+/35402/40/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse_bp.c:
https://review.coreboot.org/c/coreboot/+/35402/40/src/soc/intel/common/block... PS40, Line 47: uint8_t
The structure definition is align with the response buffer of get_boot_partition_info. […]
Done
https://review.coreboot.org/c/coreboot/+/35402/40/src/soc/intel/common/block... PS40, Line 64: bool
bool type seems to be ok here.
Done
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35402/38/src/soc/intel/common/block... PS38, Line 166: cse_wait_opmode_soft_temp_disable
Is this defined in this patch?
Done