[coreboot-gerrit] Change in coreboot[master]: Chrome EC SOCs: Integrate Chrome EC check

[Furquan Shaikh (Code Review)]

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

Change subject: Chrome EC SOCs: Integrate Chrome EC check
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42258/2/src/soc/amd/picasso/acpi.c 
File src/soc/amd/picasso/acpi.c:

https://review.coreboot.org/c/coreboot/+/42258/2/src/soc/amd/picasso/acpi.c@... 
PS2, Line 259: if (CONFIG(EC_GOOGLE_CHROMEEC)) {
             : 			gnvs->chromeos.vbt2 = google_ec_running_ro() ?
             : 				ACTIVE_ECFW_RO : ACTIVE_ECFW_RW;
             : 		} else {
             : 			gnvs->chromeos.vbt2 = ACTIVE_ECFW_RO;
             : 		}
I think it would be better to move this to src/vendorcode/google/chromeos/gnvs.c to live within the function `chromeos_init_chromeos_acpi()`. It doesn't really need to be repeated for each caller.

Also, the else {} block looks incorrect. I think there should not be any else{} block since there is no CHROMEEC being used and so we don't really set vbt2 to ACTIVE_ECFW_RO or ACTIVE_ECFW_RW. I understand  ACTIVE_ECFW_RO is defined as 0, but setting it explicitly to ACTIVE_ECFW_RO seems confusing.