Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42029 )
Change subject: drivers/vpd: Add a config option that would override coreboot version with VPD variable ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42029/2/src/drivers/vpd/Kconfig File src/drivers/vpd/Kconfig:
https://review.coreboot.org/c/coreboot/+/42029/2/src/drivers/vpd/Kconfig@15 PS2, Line 15: VPD_RO and override SMBIOS type 0 version.
Selecting this option will read …
https://review.coreboot.org/c/coreboot/+/42029/2/src/drivers/vpd/Kconfig@23 PS2, Line 23: is "version". The value of the variable is limited to 20 characters for now. Re-flow so both lines length are closer to each other.
https://review.coreboot.org/c/coreboot/+/42029/2/src/drivers/vpd/smbios.c File src/drivers/vpd/smbios.c:
https://review.coreboot.org/c/coreboot/+/42029/2/src/drivers/vpd/smbios.c@5 PS2, Line 5: #include <smbios.h> Sort the entries?
https://review.coreboot.org/c/coreboot/+/42029/2/src/drivers/vpd/smbios.c@16 PS2, Line 16: Get version from VPD: %s\n Just using present tense *Get* with the version from VPD printed is a little confusing to me. Maybe:
Firmware version from VPD: %s\n
or
Retrieved version from VPD: %s\n
https://review.coreboot.org/c/coreboot/+/42029/2/src/drivers/vpd/smbios.c@22 PS2, Line 22: else The else is not needed, but it’s subjective.