Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34662 )
Change subject: Documentation/binaries: Add AMD FSP documentation ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34662/6/Documentation/binaries/amd/... File Documentation/binaries/amd/AMD_FSP_family_17h.md:
https://review.coreboot.org/c/coreboot/+/34662/6/Documentation/binaries/amd/... PS6, Line 1: # FSP implementation differences between Intel and AMD Maybe this document should just be about the AMD FSP, with the differences being a subsection?
https://review.coreboot.org/c/coreboot/+/34662/6/Documentation/binaries/amd/... PS6, Line 17: available.
It was going beyond 76 characters... […]
72 characters:
https://doc.coreboot.org/getting_started/writing_documentation.html
https://review.coreboot.org/c/coreboot/+/34662/6/Documentation/binaries/amd/... PS6, Line 25: > obtain the memory map from the PSP. FSP-M is really being used as an initial call into the FSP and no longer really has anything to do with memory. Right now, it could really be considered "FSP-ROMSTAGE" with FSP-S being "FSP-RAMSTAGE", but even that might change at some point as we're still looking at getting rid of romstage and only running ramstage.
We're also still discussing what parts of AGESA need to go into FSP-M and what needs to be in FSP-S.
https://review.coreboot.org/c/coreboot/+/34662/6/Documentation/binaries/cavi... File Documentation/binaries/cavium/index.md:
PS6:
yes
You might want to give an explanation as to what the plan is with these placeholders. Are you planning on filling them out? If not, it might just be better not to have them at all, rather than creating the stub.
https://review.coreboot.org/c/coreboot/+/34662/6/Documentation/binaries/inde... File Documentation/binaries/index.md:
https://review.coreboot.org/c/coreboot/+/34662/6/Documentation/binaries/inde... PS6, Line 2: Are you updating the main documentation page to point to this in a separate commit? Why not just add this as a section to the main page?