Matt Papageorge has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42566 )
Change subject: util/amdfwtool: Add support for EFS SPI values for F17h and F15h ......................................................................
Patch Set 6:
(6 comments)
https://review.coreboot.org/c/coreboot/+/42566/4/util/amdfwtool/amdfwtool.c File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/42566/4/util/amdfwtool/amdfwtool.c@... PS4, Line 219: are ST, RV, PCO, RN, or LCN\
nit: It might be helpful to add full names as well so that it is clear what they mean: […]
Done
https://review.coreboot.org/c/coreboot/+/42566/4/util/amdfwtool/amdfwtool.c@... PS4, Line 1593: if (soc_name != NULL) {
nit: This block can be moved into its own function. Something like `set_spi_params()`.
Done
https://review.coreboot.org/c/coreboot/+/42566/4/util/amdfwtool/amdfwtool.c@... PS4, Line 1603: : if
+1. Then you can also drop the `soc_name_set` variable.
Done
https://review.coreboot.org/c/coreboot/+/42566/4/util/amdfwtool/amdfwtool.c@... PS4, Line 1625: amd_romsig->micron_detect_f17_mod_30_3f = efs_spi_micron_flag;
This is not correct. […]
You are right, my bad
https://review.coreboot.org/c/coreboot/+/42566/4/util/amdfwtool/amdfwtool.c@... PS4, Line 1629: qpr_dummy_cycle_f17_mod_00_2f
This is not used by Fam17H Mod30-3fH. […]
Ditto
https://review.coreboot.org/c/coreboot/+/42566/4/util/amdfwtool/amdfwtool.c@... PS4, Line 1642: if (!soc_name_set)
else
Done