[coreboot-gerrit] Change in coreboot[master]: soc/intel/tigerlake: Support to initialize Memory

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39847 )

Change subject: soc/intel/tigerlake: Support to initialize Memory
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/mem... 
File src/soc/intel/tigerlake/meminit_tgl.c:

https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/mem... 
PS5, Line 179: meminit_ddr4x_sodimm
> I am working on this, I saw you defined DIMMS_PER_CHANNEL to 2, shouldn't it be 1 for deltan?

Code added to SoC will be independent of one particular configuration. I agree that DIMMS_PER_CHANNEL are 1 for Deltan, however, TGL memory controller supports 2 DIMMs per channel. In the future, there can be a board that needs to use it. Hence, SoC code will have to account for that.

Here, board provided information will be useful in determining how to initialize different UPDs. i.e. for Deltan:
topology = SODIMM,
smbus_addr[0] --> Addr for ch0
smbus_addr[1] --> nil since deltan does not support dimm 1 on ch0
smbus_addr[2] --> Addr for ch1
smbus_addr[3] --> nil since deltan does not support dimm 1 on ch1.

SoC code will then have to use this information to fill the SPD data pointers accordingly. If it makes it easier to understand, you can also change smbus_addr to be something like:

struct {
    uint8_t addr_dimm0;
    uint8_t addr_dimm1;
} smbus_info[DDR4_CHANNELS];