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 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45278/5/src/mainboard/google/zork/s... File src/mainboard/google/zork/spd/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45278/5/src/mainboard/google/zork/s... PS5, Line 17: --hex \ The switch to binary has stalled. So --hex is still needed for now.
https://review.coreboot.org/c/coreboot/+/45278/5/src/mainboard/google/zork/s... PS5, Line 20: $(if $(APCB_POPULATE_2ND_CHANNEL), --chan_mask 5, --chan_mask 1) \ Where does chan_mask 5 come from?
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
Yes, unfortunately it is needed. […]
Does add_opt_prefix return "" when SPD_BLOB is undefined? This seems fragile. Maybe this ifdef should be around OPT_SPD_BLOB?
https://review.coreboot.org/c/coreboot/+/45278/5/util/apcb/apcb_edit.py File util/apcb/apcb_edit.py:
https://review.coreboot.org/c/coreboot/+/45278/5/util/apcb/apcb_edit.py@128 PS5, Line 128: # specifying which channels are actually populated (no need for dedicated GPIO84) This does not replace GPIO84. GPIO84 is needed on boards that support both 1 and 2 channel dram. On these boards both must be populated. Boards that never support 2 channels need to only populate 1 channel here because GPIO84 isn't set on these boards.