Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32365 )
Change subject: coreboot: Run mainboard specific code before Cr50 reset ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/#/c/32365/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32365/1//COMMIT_MSG@18 PS1, Line 18: Unexpected Cr50 TPM mode 3 Isn't TPM mode 3 actually an error: https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master...
https://review.coreboot.org/#/c/32365/1/src/vendorcode/google/chromeos/cr50_... File src/vendorcode/google/chromeos/cr50_enable_update.c:
https://review.coreboot.org/#/c/32365/1/src/vendorcode/google/chromeos/cr50_... PS1, Line 36: cr50_reset_if_needed Can we split this function into two?
cr50_is_reset_needed() --> returns true like it does right now, but does not reset
cr50_reset_now() --> calls tlcl_cr50_immediate_reset(...)
https://review.coreboot.org/#/c/32365/1/src/vendorcode/google/chromeos/cr50_... PS1, Line 147: cr50_reset_if_needed(C50_RESET_DELAY_MS) This can be then changed to: cr50_reset_reqd = cr50_is_reset_needed();
if (!cr50_reset_reqd) return;
// print message // elog add event
That way the logging would happen irrespective of cr50 firmware version.
https://review.coreboot.org/#/c/32365/1/src/vendorcode/google/chromeos/cr50_... PS1, Line 163: and you can add the reset here: if (cr50_reset_reqd) cr50_reset_now(C50_RESET_DELAY_MS);