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 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42566/7/util/amdfwtool/amdfwtool.c File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/42566/7/util/amdfwtool/amdfwtool.c@... PS7, Line 222: speed
nit: fast speed
Done
https://review.coreboot.org/c/coreboot/+/42566/7/util/amdfwtool/amdfwtool.c@... PS7, Line 1245: strcmp
nit: Use strcasecmp here and below?
Done
https://review.coreboot.org/c/coreboot/+/42566/7/util/amdfwtool/amdfwtool.c@... PS7, Line 1254: 0
It would be good to have enums for these values. Just easier to read.
Done
https://review.coreboot.org/c/coreboot/+/42566/7/util/amdfwtool/amdfwtool.c@... PS7, Line 1511: soc_name = optarg;
Just a thought. Probably, we should do strcasecmp right away and set platform_id to some enum […]
Yea this would be good thing to do so future changes do not require a refactoring, should not be too hard.