Attention is currently required from: Anil Kumar K, Subrata Banik, Paul Menzel, Andrey Petrov, Patrick Rudolph. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59324 )
Change subject: [WIP] drivers/intel/fsp2_0: Add FSP 2.3 support ......................................................................
Patch Set 20: Code-Review+1
(3 comments)
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/59324/comment/56ce7fdf_9715c6ca PS14, Line 38: Features added into FSP 2.3 specification that impact coreboot are: : 1. FSP_INFO_HEADER changes : Updated SpecVersion from 0x22 to 0x23 : Updated HeaderRevision from 5 to 6 : Added ExtendedImageRevision : FSP_INFO_HEADER length changed to 0x50 : 2. Added FSP_NON_VOLATILE_STORAGE_HOB2
I cut short the Kconfig description and added the details in commit message
Ack
File src/drivers/intel/fsp2_0/header_display.c:
https://review.coreboot.org/c/coreboot/+/59324/comment/06dc230b_f8339338 PS3, Line 10: ext_revision.val = 0;
Ack
If we do not have some special quirks around local non-static variables are per standard not initialized and can have any value (including 0) since they are created on the stack (see http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1570.pdf Section 6.7.9 clause 10). So if you have to rely on the variable having a certain value you have to initialize it.
File src/drivers/intel/fsp2_0/header_display.c:
https://review.coreboot.org/c/coreboot/+/59324/comment/c776661d_2b9bc72b PS14, Line 22: ((ext_revision.rev.revision << 8) | revision.rev.revision), : ((ext_revision.rev.bld_num << 8) | revision.rev.bld_num));
Never mind. […]
The value of local variables is undefined per standard. Fine with me now.
-
Werner Zeh (Code Review)