Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27369 )
Change subject: soc/intel/basecode: Add support for updating ucode loaded via FIT ......................................................................
Patch Set 47:
(5 comments)
https://review.coreboot.org/c/coreboot/+/27369/46//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/27369/46//COMMIT_MSG@36 PS46, Line 36: current slot MCU and RW staging MCU are same. If so, update the staging area
why do you need to update the staging area if it's the same microcode?
Done
https://review.coreboot.org/c/coreboot/+/27369/46//COMMIT_MSG@39 PS46, Line 39: Also, make sure that the top is enabled in normal/developer mode and disabled
top swap?
Done
https://review.coreboot.org/c/coreboot/+/27369/46/src/soc/intel/common/basec... File src/soc/intel/common/basecode/fw_update/ucode_update.c:
https://review.coreboot.org/c/coreboot/+/27369/46/src/soc/intel/common/basec... PS46, Line 33: __weak void ucode_update_reboot(void)
I don't see the need for weak here
provide an option for the caller to define their own reset function, if they want to do any housekeeping. If not defined this will do a board reset.
https://review.coreboot.org/c/coreboot/+/27369/46/src/soc/intel/common/basec... PS46, Line 143: /* Get slot MCU */
why is it called slot? […]
CBFS makes sense.
https://review.coreboot.org/c/coreboot/+/27369/46/src/soc/intel/common/basec... PS46, Line 160: /* Compare */
that's clear by memcmp. […]
Done