[coreboot-gerrit] Change in coreboot[master]: cr50: verify if the new image needs to be turned on and act ...

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/18946 )

Change subject: cr50: verify if the new image needs to be turned on and act on it
......................................................................


Patch Set 1:

(4 comments)

https://review.coreboot.org/#/c/18946/1/src/vendorcode/google/chromeos/cr50_turn_on.c
File src/vendorcode/google/chromeos/cr50_turn_on.c:

PS1, Line 28: if (ret != VB2_SUCCESS) {
            : 		printk(BIOS_ERR, "%s: tlcl_lib_init() failed: %x\n",
            : 		       __func__, ret);
            : 		return;
            : 	}
Do we want to trigger reboot or recovery mode when there is failure?


PS1, Line 38: printk(BIOS_ERR, "%s Attempt to turn update on failed: %x\n",
            : 		       __func__, ret);
I am guessing we do not want any reboot or recovery mode trigger here since we would be hitting this with all devices running older version of cr50.


PS1, Line 44: while(1)
            : 				;
Can we have a timed loop here? That way, if the reset fails for any reason, we can at least trigger reboot into recovery?


PS1, Line 49: BS_PAYLOAD_LOAD
Why are we doing this so late? Since it might require a reboot, wouldn't it be better to do this as soon as possible after boot-up?


-- 
To view, visit https://review.coreboot.org/18946
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45fd6058c03f32ff8edccd56ca2aa5359d9b21b1
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Vadim Bendebury <vbendeb at chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Vadim Bendebury <vbendeb at chromium.org>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes