Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39195 )
Change subject: mb/intel/jasperlake_rvp: Add memory config for JSLRVP ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39195/7/src/mainboard/intel/jasperl... File src/mainboard/intel/jasperlake_rvp/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39195/7/src/mainboard/intel/jasperl... PS7, Line 30: const struct spd_info lpddr4_spd_info = { : .read_type = READ_SPD_CBFS, : .spd_spec.spd_index = 0x0, : }; : : const struct spd_info ddr4_spd_info = { : .read_type = READ_SMBUS, : .spd_spec = {.spd_smbus_address[1] = 0xA0, : .spd_smbus_address[2] = 0xA2, : .spd_smbus_address[3] = 0xA4, : .spd_smbus_address[1] = 0xA6} : }; can we initialize it within the boardid checks below
https://review.coreboot.org/c/coreboot/+/39195/11/src/mainboard/intel/jasper... File src/mainboard/intel/jasperlake_rvp/variants/jslrvp/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39195/11/src/mainboard/intel/jasper... PS11, Line 17: romstage-$(CONFIG_BOARD_INTEL_JASPERLAKE_RVP) += memory_lp4x.c : romstage-$(CONFIG_BOARD_INTEL_JASPERLAKE_RVP_EXT_EC) += memory_lp4x.c : romstage-$(CONFIG_BOARD_INTEL_JASPERLAKE_RVP_DDR4) += memory_ddr4.c since we are using the board id function to differentiate on the board type, can we eliminate the extra variant, maintain the lpprd4 and ddr4 mem config in memory.c and board id as parameter to variant_mem_config and return the baseboard_mem_cfg(drr4/lpddr4 configs )