Attention is currently required from: Maulik V Vaghela, Ravishankar Sarawadi, Reka Norman, Subrata Banik, Meera Ravindranath, Nick Vaccaro, Balaji Manigandan, Raj Astekar, Patrick Rudolph, Karthik Ramasubramanian. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52731 )
Change subject: soc/common/smbus: Add support for reading spd data via smbus for DDR5 ......................................................................
Patch Set 10:
(5 comments)
File src/soc/intel/common/block/smbus/smbuslib.c:
https://review.coreboot.org/c/coreboot/+/52731/comment/245ef31f_d8aa1e70 PS7, Line 10: { 0, 1 }, : { 2, 2 }, : { 3, 47 }, : { 126, 127 }, : { 192, 213 }, : { 230, 235 }, : { 512, 520 }, : { 521, 550 },
SPD5118, SPD5108 HUB AND SERIAL PRESENCE DETECT DEVICE STANDARD […]
ping here
File src/soc/intel/common/block/smbus/smbuslib.c:
https://review.coreboot.org/c/coreboot/+/52731/comment/66972b75_94157e2e PS10, Line 56: /* By default, SPD5 hub accepts 1 byte addressing pointing : to the first 128 bytes of memory. MR11[2:0] selects the page : pointer to address the entire 1024 bytes of non-volatile memory */ nit: multi-line comment like this should be indented on 2nd & 3rd line, e.g.
``` /* By default, an SPD5 hub accepts 1 byte addressing pointing to the first 128 bytes of memory. MR11[2:0] selects the page pointer to address the entire 1024 bytes of non-volatile memory. */ ```
https://review.coreboot.org/c/coreboot/+/52731/comment/635a1b1a_fb8289b5 PS10, Line 62: : /* Read the SPD data over the SMBus, at the specified SPD address, : starting at the specified starting offset and read the given amount of data */ Function header comments usually look like: ``` /* * Read the SPD data over SMBus, at the specified SPD address, starting at the * specified starting offset and read the given amount of data. */ ```
https://review.coreboot.org/c/coreboot/+/52731/comment/e6e6b3e1_ae5a3be3 PS10, Line 95: /* Reset the SPD page back to page 0 on an SPD5 Hub device at the : input SPD SMbus address*/ ``` /* * Reset the SPD page back to page 0 on an SPD5 Hub device at the * input SPD SMBus address. */ ```
https://review.coreboot.org/c/coreboot/+/52731/comment/4b65b0df_c5234266 PS10, Line 124: table_select Can you drop `table_select` and just use `spd_ddr5_table` here?