Attention is currently required from: Jason Glenesk, Raul Rangel, Martin Roth, Marshall Dawson, Zheng Bao, Felix Held. Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56773 )
Change subject: amdfwtool: Add support for AMD's BIOS A/B recovery feature ......................................................................
Patch Set 28:
(6 comments)
File util/amdfwtool/amdfwtool.h:
https://review.coreboot.org/c/coreboot/+/56773/comment/ea4e9464_ad4034a1 PS26, Line 240: uint8_t recovery_ab;
i'd use a boolean here; probably also the case for some of the struct elements above
Done. https://review.coreboot.org/c/coreboot/+/58942
File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/56773/comment/6fcdc2c9_81fb51a2 PS26, Line 669: int
this is a different type compared to cb_config->recovery_ab
Done
https://review.coreboot.org/c/coreboot/+/56773/comment/bb3c9b79_2d2d9819 PS26, Line 1351: psp_directory_table *pspdir, *pspdir2 = NULL, *pspdir2_b = NULL;
maybe put the 3 definitions on 3 separate lines
Done
https://review.coreboot.org/c/coreboot/+/56773/comment/c6e83534_54d70d73 PS26, Line 1700: pspdir2_b = NULL; /* More explicitly */
since the corresponding if block has curly braces, please also use curly braces around the one state […]
Done
https://review.coreboot.org/c/coreboot/+/56773/comment/1485fa7c_edead8e1 PS26, Line 1707: 0
NULL
Done.
https://review.coreboot.org/c/coreboot/+/58941
https://review.coreboot.org/c/coreboot/+/56773/comment/f5b72fdf_09dee817 PS26, Line 1733: , *biosdir2_b = NULL
i'd put this second definition on a new line to make it more obvious what's done here
Done