Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40561 )
Change subject: soc/intel/common: Add downgrade support for CSE Firmware ......................................................................
Patch Set 13:
(7 comments)
https://review.coreboot.org/c/coreboot/+/40561/12/src/soc/intel/common/block... File src/soc/intel/common/block/cse/custom_bp.c:
https://review.coreboot.org/c/coreboot/+/40561/12/src/soc/intel/common/block... PS12, Line 93: detected
are detected
Done
https://review.coreboot.org/c/coreboot/+/40561/12/src/soc/intel/common/block... PS12, Line 543: /* Check if CSE reports data mismatch error */
That’s the description for the next function?
No, the comment is for cse_is_rw_dp_valid().
https://review.coreboot.org/c/coreboot/+/40561/12/src/soc/intel/common/block... PS12, Line 556: triggers
trigger (to be in line with *issue*)
Done
https://review.coreboot.org/c/coreboot/+/40561/12/src/soc/intel/common/block... PS12, Line 569: return false;
Add an error message with `die()`?
if this function fails, recovery mode is triggered in the error path.
https://review.coreboot.org/c/coreboot/+/40561/12/src/soc/intel/common/block... PS12, Line 667: return false;
An error message should be returned here, saying that the downgrade is aborted.
Done
https://review.coreboot.org/c/coreboot/+/40561/12/src/soc/intel/common/block... PS12, Line 729: /* If RW blob is present in CBFS, then trigger CSE firmware update */
Remove one space after tab?
Done
https://review.coreboot.org/c/coreboot/+/40561/12/src/soc/intel/common/block... PS12, Line 731: uint8_t rv;
Unnecessary reordering in change.
Done