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:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42566/3/util/amdfwtool/amdfwtool.c File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/42566/3/util/amdfwtool/amdfwtool.c@... PS3, Line 218: --soc-family
Sounds good, will see if I can make it work well
Done
https://review.coreboot.org/c/coreboot/+/42566/3/util/amdfwtool/amdfwtool.c@... PS3, Line 239: <HEX_VAL>
This sounds reasonable. […]
Ack
https://review.coreboot.org/c/coreboot/+/42566/3/util/amdfwtool/amdfwtool.c@... PS3, Line 243: 0xaa for only Micron parts.
Use the same wording as for F17h M 30h-3fh?
No longer applicable to the new patch
https://review.coreboot.org/c/coreboot/+/42566/3/util/amdfwtool/amdfwtool.c@... PS3, Line 1088: LONGOPT_FAMILY = 256, : LONGOPT_MODEL = 257,
Oops, forgot to remove these from an earlier revision.
Done
https://review.coreboot.org/c/coreboot/+/42566/3/util/amdfwtool/amdfwtool.c@... PS3, Line 1617: if (soc_family == 0x17 && soc_model_min >= 0x0 && soc_model_max <= 0x2f) { : amd_romsig->second_gen_efs = 0; : amd_romsig->spi_readmode_f17_mod_00_2f = efs_spi_readmode; : amd_romsig->spi_fastspeed_f17_mod_00_2f = efs_spi_speed; : amd_romsig->qpr_dummy_cycle_f17_mod_00_2f = efs_spi_micron_flag; : } : if (soc_family == 0x17 && soc_model_min >= 0x30 && soc_model_max <= 0x3f) { : amd_romsig->second_gen_efs = 1; : amd_romsig->spi_readmode_f17_mod_30_3f = efs_spi_readmode; : amd_romsig->spi_fastspeed_f17_mod_30_3f = efs_spi_speed; : amd_romsig->micron_detect_f17_mod_30_3f = efs_spi_micron_flag; : }
I cannot think of any combinations of these 3 numbers where any of these conditionals would be execu […]
Done