Karthik Ramasubramanian 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 15:
(7 comments)
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/in... File src/soc/intel/tigerlake/include/soc/meminit_jsl.h:
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/in... PS14, Line 43: READ_SMBUS, Not used anywhere. Can be added later if the use-case arises. Same for NOT_EXISTING.
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/in... PS14, Line 73: meminit_lpddr4() I think it is meminit_dq_dqs_map that is clearing the unused fields. Also I see that it is set to 0 at the mainboard_config
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/in... PS14, Line 117: uint8_t vref_ca_config; vref_ca config is marked as not applicable for LPDDR4x. Do we still need to configure it? Atleast configured in the mainboard config.
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/in... PS14, Line 132: ) We still need to allow half population.
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/me... File src/soc/intel/tigerlake/meminit_jsl.c:
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/me... PS14, Line 61: / Nit: Fix the indentation.
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/me... PS14, Line 82: /* Early Jasper Lake requires rcomp targets to be 0 */ Based on the comment, did you mean memset RcompTarget to 0. Else the RcompTarget is copied as passed by the mainboard and is not 0.
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/me... PS14, Line 93: mem_cfg->MemorySpdPtr01 = 0; No need to set it to spd_data_ptr for LPDDR4x with 4 slots?