Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 12:
(13 comments)
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 5: AMD products
How about AMD x86 products?
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 6: of "C
of a "Compute...
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 7: (CU) where some parts of the processor are shared between two cores and : some parts are unique for each core.
How about "...(CU); a core pair grouping with certain shared resources.
I don't know. This document is already so small... I would prefer to waste words, though I agree that your way says the same thing.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 10: (see references below).
The placement of this after the "Enterprise, Client, and Embedded designs" kind of bugs me. […]
Added a phrase explicitly stating that a model can support more than 1 market.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 14: Initially with CU, 2 x86 cores have separate integer processors but share cache,
The first CU designs were 2 x86 cores with...
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 15: A die can have up to : 8 CU
...can contain up to CUs. […]
Searched the WEB for technical reviews of AMD models included on family 15h. Did not found anything with more than 8 CU.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 22: Family 15h setups use up to three separate chips
How about "Family 15h products range from SOCs to 3-chip solutions. […]
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 23: (i.e. even when they're headless)
(including headless)
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 26: Some products
Later SOCs
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 26: which
deleting this will make it read better.
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 27: security related measures
I have mixed feelings discussing this here, as no coreboot implementation supports it. […]
Even if there are no plans of making them available to coreboot, that does not mean that some company could not try to do it. I say, leave the information, as it's the PSP reality. They can always as AMD support if needed.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 28: SPI
SPI flash?
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 37: implements
You use a form of this word 3 times in 2 sentences. […]
Done