[coreboot-gerrit] [XS] Change in coreboot[master]: soc/intel/common: Restore to page 0 before reading SPD

[Kyösti Mälkki (Code Review)]

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.