Attention is currently required from: Kapil Porwal, Shelley Chen, Subrata Banik.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80738?usp=email )
Change subject: vc/google/chromeos: Enable dynamic ChromeOS boot logo selection ......................................................................
Patch Set 5:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80738/comment/a68f5291_b53f6c55 : PS5, Line 7: Enable “Enable” for me is ambiguous. For me it could mean, that the feature is already present, and and only a switch needs to be flipped to *enable* it. Maybe “Implement”?
https://review.coreboot.org/c/coreboot/+/80738/comment/55e2611d_72071b2b : PS5, Line 8: Can you please summarize the problem that is addressed with the new feature?
File src/vendorcode/google/chromeos/tpm_factory_config.c:
https://review.coreboot.org/c/coreboot/+/80738/comment/f9842487_4356de9a : PS5, Line 51: needs need
https://review.coreboot.org/c/coreboot/+/80738/comment/ca73b447_3869622f : PS5, Line 60: return ((factory_config & CHROMEBOOK_PLUS_DEVICE) == CHROMEBOOK_PLUS_DEVICE) ? true : false; Isn’t this equal to
return (factory_config & CHROMEBOOK_PLUS_DEVICE) == CHROMEBOOK_PLUS_DEVICE;
https://review.coreboot.org/c/coreboot/+/80738/comment/f971e9a3_0244d64d : PS5, Line 70: that that is?
https://review.coreboot.org/c/coreboot/+/80738/comment/7317da27_c2d71422 : PS5, Line 89: Chromebook-Plus This is spelled inconsistently in the file – without and with hyphen.
https://review.coreboot.org/c/coreboot/+/80738/comment/ec3d8baa_bf5d1b7a : PS5, Line 92: needs need
https://review.coreboot.org/c/coreboot/+/80738/comment/1aaf9d0e_b162da54 : PS5, Line 105: if (strncmp((char *)device_info, "CAI", 3) == 0) : return true; : else : return false; Should be equivalent to
return strncmp((char *)device_info, "CAI", 3) == 0;