Attention is currently required from: Jason Glenesk, Raul Rangel, Matt DeVillier, Grzegorz Bernacki, Fred Reitberger, Karthik Ramasubramanian, Felix Held.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74266 )
Change subject: amdfwtool: Add --fsp-version option ......................................................................
Patch Set 1:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/74266/comment/e4703090_3ed8b23f PS1, Line 9: This version tells amdfwtool to create a text file : which contains versions of following blobs: : PSP bootloader (type 0x01), : SMU firmware (type 0x08), : AGESA bootloader 0 (type 0x30).
Passing this option tells amdfwtool to create a text file, containing the versions of the blobs below:
- PSP bootloader (type 0x01),
- SMU firmware (type 0x08),
- AGESA bootloader 0 (type 0x30).
https://review.coreboot.org/c/coreboot/+/74266/comment/fe141243_8212c256 PS1, Line 14: What is the new switch/option going to be used for? Maybe start with the motivation.
https://review.coreboot.org/c/coreboot/+/74266/comment/a1f8a860_ee4a6b0e PS1, Line 16: TEST=Tested on Skyrim device If possible, please paste the commands and the output:
$ amdfwtool --fsp-version bla.txt $ more bla.txt […]
That would also show the format.
Patchset:
PS1: As I do not know, what the option is going to be used for, I think the KISS way would be to just output the versions to standard out. If somebody wants to store the output in a file, they can simply redirect the output with `>` or `| tee`.
No idea, how often this is going to be used, but why not add a switch for each blob: `--print-psp-bootloader-version`, `--print-smu-version`, and `--print-agesa-bootloader-version` (probably too long, but just a suggestion).
-
Paul Menzel (Code Review)