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 80:
(12 comments)
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/cannonlake/m... File src/soc/intel/cannonlake/me.c:
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/cannonlake/m... PS79, Line 163: #if CONFIG(SOC_INTEL_CSE_CUSTOM_SKU)
I think this can be moved to custom_bp. […]
Done
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/cannonlake/m... PS79, Line 164: BS_PRE_DEVICE
This seems to have changed since last time?
Yes, it has beem changed.
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/cannonlake/m... PS79, Line 164: BS_DEV_INIT
This is not correct. It is supposed to be either BS_ON_EXIT or BS_ON_ENTRY.
Fixed.
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/common/block... File src/soc/intel/common/block/cse/Kconfig:
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/common/block... PS79, Line 21: SKU Custom
nit: Custom SKU
Missed it.I will correct it later.
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/common/block... File src/soc/intel/common/block/cse/custom_bp.c:
PS79:
Name this file custom_sku. […]
Will modify the file to lite_sku.c on a separate patch later.
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/common/block... PS79, Line 89: enum
This will have to fixed width since it is being received from the CSE.
Done
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/common/block... PS79, Line 127: struct
const struct
Done
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/common/block... PS79, Line 128: struct
const struct
Done
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/common/block... PS79, Line 133: struct
const
Done
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/common/block... PS79, Line 135: struct
const
Done
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/common/block... PS79, Line 273: /* Control never reaches here */
Add a die("") here?
Done
https://review.coreboot.org/c/coreboot/+/35402/79/src/soc/intel/common/block... PS79, Line 307: if (vboot_recovery_mode_enabled()) { : printk(BIOS_DEBUG, "cse_bp: Skip switching to RW in the recovery path\n"); : return; : }
I think this can be checked even before custom sku is checked above?
done
-
Sridhar Siricilla (Code Review)