Meera Ravindranath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39135 )
Change subject: src/soc/tigerlake: Add memory configuration support for Jasper Lake ......................................................................
Patch Set 7:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/jsl_memcfg_init.h:
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/inc... PS7, Line 70: struct spd_info spd[NUM_DIMM_SLOT];
Does JSL support different types of memories for each slot? If not, mainboard can just pass in SPD i […]
No it does not support different memories for each slot. Yes, the implementation like TGL is under progress. Will post a new CL soon.
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/inc... PS7, Line 103: uint16_t rcomp_resistor[3];
On TGL, we were told that rcomp_resistor and rcomp_target need to be set to auto configurable by MRC […]
In JSL, we can't auto configure it as the board values deviates and need to be passed in mainboard.
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/inc... PS7, Line 136: void jasperlake_memcfg_init(FSP_M_CONFIG *mem_cfg, : const struct jsl_mb_cfg *jsl_cfg);
Quick comment: Please take a look at TGL memcfg_init. […]
Ack
https://review.coreboot.org/c/coreboot/+/39135/2/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/jsl_memcfg_init.h:
https://review.coreboot.org/c/coreboot/+/39135/2/src/soc/intel/tigerlake/inc... PS2, Line 48: READ_SMBUS,
Since this is SoC code, prefer to have all the methods to provide SPD info. […]
Ack
https://review.coreboot.org/c/coreboot/+/39135/2/src/soc/intel/tigerlake/inc... PS2, Line 136: jasperlake
Even though both SoCs configure memory differently, only one of them will be enabled/used at a time […]
Ack