[coreboot-gerrit] [L] Change in coreboot[main]: soc/intel/common/block/imc: add Integrated Memory Controller driver

[Shuo Liu (Code Review)]

Attention is currently required from: [e-mail].

Shuo Liu has posted comments on this change by [e-mail]. ( https://review.coreboot.org/c/coreboot/+/83320?usp=email )

Change subject: soc/intel/common/block/imc: add Integrated Memory Controller driver
......................................................................

Patch Set 3:

(4 comments)

File src/soc/intel/common/block/imc/Kconfig:

https://review.coreboot.org/c/coreboot/+/83320/comment/97eda60a_99d6dc77?usp... :
PS3, Line 4: 	bool
I guess you are using an IMC with an SMBUS in it? Would it be possible to be more explicit on naming?

File src/soc/intel/common/block/imc/imc.c:

https://review.coreboot.org/c/coreboot/+/83320/comment/51982270_8abfef5d?usp... :
PS3, Line 57: void imc_smbus_spd_init(pci_devfn_t dev)
imc_spd_smbus_init?

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

https://review.coreboot.org/c/coreboot/+/83320/comment/d3de02c6_6ee8b429?usp... :
PS3, Line 10: {
Can we not using weak since it would be confusing?
IMO, there are 2 implementation, one is the default, one is the IMC SMBUS based.

https://review.coreboot.org/c/coreboot/+/83320/comment/7844c74e_f0c9e8e6?usp... :
PS3, Line 96: 		if (!CONFIG(SOC_INTEL_COMMON_BLOCK_IMC) && blk->addr_map[i] == 0) {
I guess SOC_INTEL_COMMON_BLOCK_IMC will provide SPD data not depending on blk->addr_map[i]?