Attention is currently required from: Paul Menzel, Paz Zcharya, Subrata Banik.
Gwendal Grignou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80738?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection ......................................................................
Patch Set 7:
(3 comments)
File src/vendorcode/google/chromeos/tpm_factory_config.c:
https://review.coreboot.org/c/coreboot/+/80738/comment/5f8a6855_968b9cfa : PS7, Line 49: * - Bits 3-0 (0x1): Must be 0x1 to signify compliance with Chromebook-Plus
Not true: this field contains the "feature_level" that indicate the chromebook plus generation. […]
To be future-proof, the test on line 61 should be
`return factory_config & CHROMEBOOK_PLUS_DEVICE;`
https://review.coreboot.org/c/coreboot/+/80738/comment/8b4cb17a_a1e66168 : PS7, Line 74: * requirement.
Not true. […]
I assume the design doc is at `go/cros-tiering-dd`. If this CL only applies to new chromebook that have identification in their GSC where `chassis_x_branded` and `hw_x_compliance_version` are properly set, then we don't need to read the VPD at all. Per design [see section `OS Runtime flow`], all the information we need in the GSC.
https://review.coreboot.org/c/coreboot/+/80738/comment/1274f52e_2f4b0617 : PS7, Line 104: return strncmp(vpd_get_feature_device_info(), "CAI", 3) == 0;
This is not acceptable for production code. "CAI" is not defined anywhere in the code base. […]
The previous comment states "all older devices will have legacy config", so we don't need the non-GSC bit parsing logic, do we?