Attention is currently required from: Gwendal Grignou, Kapil Porwal, Paul Menzel, Paz Zcharya.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80738?usp=email )
Change subject: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection ......................................................................
Patch Set 7:
(4 comments)
Patchset:
PS7:
which one?
marking resolved based on offline discussion.
File src/vendorcode/google/chromeos/tpm_factory_config.c:
https://review.coreboot.org/c/coreboot/+/80738/comment/5a276793_41d71ae9 : PS7, Line 49: * - Bits 3-0 (0x1): Must be 0x1 to signify compliance with Chromebook-Plus
To be future-proof, the test on line 61 should be
`return factory_config & CHROMEBOOK_PLUS_DEVICE;`
If you read the bug ticket number attached to the commit message, you will see that the issue this CL aims to remedy is due to the fact that we were originally returning `return factory_config & CHROMEBOOK_PLUS_DEVICE;`
When we `return factory_config & CHROMEBOOK_PLUS_DEVICE;`, that means we are considering a device qualified as CBX if `Bit 4 is set` or `Bit 0-3 is set` or `both are set`.
However, in practice, we have seen devices that are CBX HW compliant (bit 0-3) but are not branded as CBX. As a result, we are displaying the CBX logo instead of the correct device segment logo (CB). This CL is intended to fix that issue, as described in the commit message.
- If the GSC bit indicates that both Bit-4 and Bit 0-3 are set to 1, then we know that this is a HW branded CBX.
- If bit 0-3 is set, then it can be either a SB CBX or a regular CB. To detect which one it is properly, we are relying on VPD read.
- If none of the above logic is being met, then this device is definitely a CB.
marking resolved based on offline discussion.
https://review.coreboot.org/c/coreboot/+/80738/comment/91e18a20_06bb8248 : PS7, Line 74: * requirement.
Requested comment access to the doc. If there are corner cases, then line 97 is incorrect.
marking resolved based on offline discussion.
https://review.coreboot.org/c/coreboot/+/80738/comment/3f207026_9fd5f859 : PS7, Line 104: return strncmp(vpd_get_feature_device_info(), "CAI", 3) == 0;
As discussed in the meeting, in chromeos_device_branded_plus_soft() we can completely ignore GSC information and go by reading the VPD.
yes, agreed. done.
Until protobuf decoding is added to coreboot, the device will be a regular chromebook when vpd_get_feature_device_info() is empty or indicates the feature_level is 0.
marking resolved based on offline discussion.