Attention is currently required from: CoolStar, Matt DeVillier, Sean Rhodes, Tim Crawford.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76382?usp=email )
Change subject: soc/intel/common: Restore to page 0 before reading SPD ......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/block/smbus/smbuslib.c:
https://review.coreboot.org/c/coreboot/+/76382/comment/f2e5ed25_4baa646d : PS3, Line 44: smbus_write_byte(SPD_PAGE_0, 0, 0);
since we can't know the DRAM type ahead of time, how about only restoring to page 0 in the event the […]
I had another look at the smbus host code. I think it does not hit the long timeout even when there is no response to SPD_PAGE_0/1 address requests. And some of this SPD code is really slow anyways so the extra read should be hardly measurabls.
Since the switch to SPD_PAGE_1 is guarded by a Konfig (and runtime SPD data), I see little reason why the return to SPD_PAGE_0 should appear in the source without the static conditional. The risk is if some old hardware has something else responding to those same addresses.
-
Kyösti Mälkki (Code Review)