Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45278 )
Change subject: WIP: Use SPDs from PSP BIOS table type 0x69 based on GPIO straps ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45278/3/src/mainboard/google/zork/s... File src/mainboard/google/zork/spd/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45278/3/src/mainboard/google/zork/s... PS3, Line 7: SPD_BLOB:=$(obj)/SPD_array.bin Does this need to be here? It's constructed in the picasso makefile.
https://review.coreboot.org/c/coreboot/+/45278/3/src/mainboard/google/zork/s... PS3, Line 10: APCB_SOURCES:=$(FIRMWARE_LOCATE)/APCB_magic.bin Why is one even needed? Is this just to pass the validation check? If so, let's remove the check or make in an OR.
https://review.coreboot.org/c/coreboot/+/45278/3/src/soc/amd/picasso/Makefil... File src/soc/amd/picasso/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45278/3/src/soc/amd/picasso/Makefil... PS3, Line 243: #ifdef SPD_SOURCES Is this ifdef necessary? PSP_SPD_FILES will just be empty when its not defined.
https://review.coreboot.org/c/coreboot/+/45278/3/src/soc/amd/picasso/Makefil... PS3, Line 247: SPD_BLOB:=$(obj)/SPD_array.bin Maybe this be SPD_ARRAY_BLOB since it may contain multiple SPDs