Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36449 )
Change subject: mainboard/google: Allow Hatch variants to read SPD data over SMBus ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36449/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/romstage_spd_smbus.c:
https://review.coreboot.org/c/coreboot/+/36449/2/src/mainboard/google/hatch/... PS2, Line 48: memcfg.spd[0].read_type = READ_SMBUS; Why are we marking this is as READ_SMBUS when get_spd_smbus() is doing the reading and filling in the data contents?
https://review.coreboot.org/c/coreboot/+/36449/2/src/mainboard/google/hatch/... PS2, Line 49: memcfg.spd[0].spd_spec.spd_smbus_address = (uintptr_t)blk.spd_array[0]; spd_smbus_address is expecting an smbus address. 0xa0 (0x50 shifted) or 0xa4 (0x54 shifted). It's not expecting the contents themselves.
https://review.coreboot.org/c/coreboot/+/36449/2/src/mainboard/google/hatch/... PS2, Line 52: 1 I'm not sure this should be 1. There are some bizarre encodings in that I think you need to make this 2 because I believe these entries map to chip selects and for dual rank parts the 2 chip selects will be routed to a single dimm.