Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38698 )
Change subject: vc/amd/fsp/picasso: Add PCIe and DDI helpers ......................................................................
Patch Set 1:
(7 comments)
https://review.coreboot.org/c/coreboot/+/38698/1/src/vendorcode/amd/fsp/pica... File src/vendorcode/amd/fsp/picasso/platform_descriptors.h:
https://review.coreboot.org/c/coreboot/+/38698/1/src/vendorcode/amd/fsp/pica... PS1, Line 37: GEN_MAX, // Maximum supported I'd add a = 0 here to make t clearer that the absolute values matter here and not only the symbolic names. the resulting values stay the same
https://review.coreboot.org/c/coreboot/+/38698/1/src/vendorcode/amd/fsp/pica... PS1, Line 46: SATA_CHANNEL_OTHER, // Default Channel Type same here
https://review.coreboot.org/c/coreboot/+/38698/1/src/vendorcode/amd/fsp/pica... PS1, Line 68: ASPM_DISABLED, // Disabled same here
https://review.coreboot.org/c/coreboot/+/38698/1/src/vendorcode/amd/fsp/pica... PS1, Line 77: AUX1, // Aux1 same here
https://review.coreboot.org/c/coreboot/+/38698/1/src/vendorcode/amd/fsp/pica... PS1, Line 88: HDP1, // Hdp1 same here
https://review.coreboot.org/c/coreboot/+/38698/1/src/vendorcode/amd/fsp/pica... PS1, Line 99: DP, // DP same here
https://review.coreboot.org/c/coreboot/+/38698/1/src/vendorcode/amd/fsp/pica... PS1, Line 131: unsigned int i'd strongly prefer uint32_t here, since it makes the sizes more obvious. same below