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 3:
(4 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
Instead of taking family/model-min/model-max as inputs, would it make sense to have family name as i […]
Sounds good, will see if I can make it work well
https://review.coreboot.org/c/coreboot/+/42566/3/util/amdfwtool/amdfwtool.c@... PS3, Line 239: <HEX_VAL>
I think this will have to be a boolean flag or a tri-state flag like […]
This sounds reasonable. The only downside is if other part vendors are added to this exception field in later architectures. I am not sure that is going to ever happen though.
https://review.coreboot.org/c/coreboot/+/42566/3/util/amdfwtool/amdfwtool.c@... PS3, Line 1088: LONGOPT_FAMILY = 256, : LONGOPT_MODEL = 257,
These are not really used.
Oops, forgot to remove these from an earlier revision.
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; : }
Use `else of` to make sure, these branches cannot be entered twice (by making a mistake in the condi […]
I cannot think of any combinations of these 3 numbers where any of these conditionals would be executed more than once. Let me know if I am missing any