[M] Change in coreboot[master]: soc/common/smbus: Add support for reading spd data via smbus for DDR5

[Tim Crawford (Code Review)]

Attention is currently required from: Angel Pons, Balaji Manigandan, Kapil Porwal, Karthik Ramasubramanian, Kyösti Mälkki, Maulik Vaghela, Meera Ravindranath, Nick Vaccaro, Patrick Rudolph, Raj Astekar, Reka Norman, Subrata Banik, Tarun Tuli, Tim Wawrzynczak.

Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52731?usp=email )

Change subject: soc/common/smbus: Add support for reading spd data via smbus for DDR5
......................................................................

Patch Set 14:

(3 comments)

File src/include/spd_bin.h:

https://review.coreboot.org/c/coreboot/+/52731/comment/b72d93eb_877099b4 :
PS14, Line 39: #define MAX_SPD_SIZE		(SPD_PAGE_LEN * SPD_SN_LEN)
> SPD_SN_LEN is the number of bytes used for storing DIMM serial number as binary and there is no relation to number of pages to choose from.

Yes, a new define would make more sense. I guess it was used just because it's also 4.
> SPD_PAGE_LEN is defined as 256, while SPD_HUB implementation uses pages of 128 bytes?

Looks correct. Page size is 128 in 1 byte addressing mode (`MR11[3]` = 0).

File src/soc/intel/common/block/smbus/smbuslib.c:

https://review.coreboot.org/c/coreboot/+/52731/comment/cf2a067d_8e46a702 :
PS13, Line 10: {   0,               1 }, /* General Configuration section */
             : {   2,               2 }, /* General Configuration section */
             : {   3,              47 }, /* General Configuration section */
> If this code is correct, then these are not contiguous. […]

I see, it's inclusive. So yes, they are contiguous. Probably just copied from the FSP then. And at least on the version I have, the ranges are different as well (0-1, 2-3, 4-47).

File src/soc/intel/common/block/smbus/smbuslib.c:

https://review.coreboot.org/c/coreboot/+/52731/comment/3715a6a4_da803c1b :
PS14, Line 93: 	spd_hub_byte = smbus_read_byte(spd_addr, SPD5_MEMREG_REG(SPD5_MR0));
> SPD5_MEMREG_REG() with a defined constant register in range [0..0x7f] is a no-op.

Probably to make it explicit that it's a register access and not NVM access? (Same with `reset_page_spd5()`.)