Philip Chen has uploaded this change for review.
soc/intel/cannonlake: Clean up cannonlake_memcfg_init
The major changes include:
(1) Add enum 'mem_info_read_type' to spd_info.
(2) Add spd_into into cnl_mb_cfg.
(3) Squash meminit_memcfg_spd().
BUG=b:124990009
TEST=build
Change-Id: I686a85996858204c20fd05ef24787a0487817c34
---
M src/mainboard/google/hatch/romstage.c
M src/mainboard/google/hatch/variants/baseboard/memory.c
M src/mainboard/google/sarien/romstage.c
M src/mainboard/intel/coffeelake_rvp/memory.c
M src/mainboard/intel/coffeelake_rvp/romstage.c
M src/soc/intel/cannonlake/cnl_memcfg_init.c
M src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h
7 files changed, 90 insertions(+), 84 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/32513/1
diff --git a/src/mainboard/google/hatch/romstage.c b/src/mainboard/google/hatch/romstage.c
index bdf951b..95db576 100644
--- a/src/mainboard/google/hatch/romstage.c
+++ b/src/mainboard/google/hatch/romstage.c
@@ -25,14 +25,8 @@
{
struct cnl_mb_cfg memcfg;
- const struct spd_info spd = {
- .spd_by_index = true,
- .spd_spec.spd_index = variant_memory_sku(),
- };
-
variant_memory_params(&memcfg);
- cannonlake_memcfg_init(&memupd->FspmConfig,
- &memcfg, &spd);
+ cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg);
}
void mainboard_get_dram_part_num(const char **part_num, size_t *len)
diff --git a/src/mainboard/google/hatch/variants/baseboard/memory.c b/src/mainboard/google/hatch/variants/baseboard/memory.c
index b6a6615..8706332 100644
--- a/src/mainboard/google/hatch/variants/baseboard/memory.c
+++ b/src/mainboard/google/hatch/variants/baseboard/memory.c
@@ -20,6 +20,9 @@
#include <string.h>
static const struct cnl_mb_cfg baseboard_memcfg = {
+ .spd = {
+ .read_type = READ_SPD_CBFS},
+
/*
* The dqs_map arrays map the ddr4 pins to the SoC pins
* for both channels.
@@ -27,14 +30,14 @@
* the index = pin number on ddr4 part
* the value = pin number on SoC
*/
- .dqs_map[DDR_CH0] = { 0, 1, 4, 5, 2, 3, 6, 7 },
- .dqs_map[DDR_CH1] = { 0, 1, 4, 5, 2, 3, 6, 7 },
+ .dqs_map[DDR_CH0] = {0, 1, 4, 5, 2, 3, 6, 7},
+ .dqs_map[DDR_CH1] = {0, 1, 4, 5, 2, 3, 6, 7},
/* Baseboard uses 121, 81 and 100 rcomp resistors */
- .rcomp_resistor = { 121, 81, 100 },
+ .rcomp_resistor = {121, 81, 100},
/* Baseboard Rcomp target values */
- .rcomp_targets = { 100, 40, 20, 20, 26 },
+ .rcomp_targets = {100, 40, 20, 20, 26},
/* Set CaVref config to 2 */
.vref_ca_config = 2,
@@ -46,6 +49,7 @@
void __weak variant_memory_params(struct cnl_mb_cfg *bcfg)
{
memcpy(bcfg, &baseboard_memcfg, sizeof(baseboard_memcfg));
+ bcfg->spd.spd_spec.spd_index = variant_memory_sku();
/*
* GPP_F2 is the MEM_CH_SEL gpio, which is set to 1 for single
* channel skus and 0 for dual channel skus.
diff --git a/src/mainboard/google/sarien/romstage.c b/src/mainboard/google/sarien/romstage.c
index e83cd4a..b66fd70 100644
--- a/src/mainboard/google/sarien/romstage.c
+++ b/src/mainboard/google/sarien/romstage.c
@@ -18,6 +18,14 @@
#include <soc/romstage.h>
static const struct cnl_mb_cfg memcfg = {
+ .spd = {
+ .read_type = READ_SMBUS,
+ .spd_spec = {
+ .spd_smbus_address[0] = 0xa0,
+ .spd_smbus_address[2] = 0xa4,
+ },
+ },
+
/*
* The dqs_map arrays map the ddr4 pins to the SoC pins
* for both channels.
@@ -25,16 +33,16 @@
* the index = pin number on ddr4 part
* the value = pin number on SoC
*/
- .dqs_map[DDR_CH0] = { 0, 1, 4, 5, 2, 3, 6, 7 },
- .dqs_map[DDR_CH1] = { 0, 1, 4, 5, 2, 3, 6, 7 },
+ .dqs_map[DDR_CH0] = {0, 1, 4, 5, 2, 3, 6, 7},
+ .dqs_map[DDR_CH1] = {0, 1, 4, 5, 2, 3, 6, 7},
/* Baseboard uses 121, 81 and 100 rcomp resistors */
- .rcomp_resistor = { 121, 81, 100 },
+ .rcomp_resistor = {121, 81, 100},
/*
* Baseboard Rcomp target values.
*/
- .rcomp_targets = { 100, 40, 20, 20, 26 },
+ .rcomp_targets = {100, 40, 20, 20, 26},
/* Disable Early Command Training */
.ect = 0,
@@ -45,12 +53,7 @@
void mainboard_memory_init_params(FSPM_UPD *memupd)
{
- const struct spd_info spd = {
- .spd_smbus_address[0] = 0xa0,
- .spd_smbus_address[2] = 0xa4
- };
-
wilco_ec_romstage_init();
- cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg, &spd);
+ cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg);
}
diff --git a/src/mainboard/intel/coffeelake_rvp/memory.c b/src/mainboard/intel/coffeelake_rvp/memory.c
index a13000c..a9695e8 100644
--- a/src/mainboard/intel/coffeelake_rvp/memory.c
+++ b/src/mainboard/intel/coffeelake_rvp/memory.c
@@ -20,23 +20,32 @@
#include <soc/cnl_memcfg_init.h>
static const struct cnl_mb_cfg baseboard_memcfg_cfg = {
- /*
+ .spd = {
+ .read_type = READ_SMBUS,
+ .spd_spec = {
+ .spd_smbus_address[0] = 0xA0,
+ .spd_smbus_address[1] = 0xA2,
+ .spd_smbus_address[2] = 0xA4,
+ .spd_smbus_address[3] = 0xA6,
+ },
+ };
+/*
* The dqs_map arrays map the ddr4 pins to the SoC pins
* for both channels.
*
* the index = pin number on ddr4 part
* the value = pin number on SoC
*/
- .dqs_map[DDR_CH0] = { 0, 1, 3, 2, 4, 5, 6, 7 },
- .dqs_map[DDR_CH1] = { 1, 0, 4, 5, 2, 3, 6, 7 },
+.dqs_map[DDR_CH0] = {0, 1, 3, 2, 4, 5, 6, 7},
+ .dqs_map[DDR_CH1] = {1, 0, 4, 5, 2, 3, 6, 7},
/* Baseboard uses 121, 81 and 100 rcomp resistors */
- .rcomp_resistor = { 121, 81, 100 },
+ .rcomp_resistor = {121, 81, 100},
/*
* Baseboard Rcomp target values.
*/
- .rcomp_targets = { 100, 40, 20, 20, 26 },
+ .rcomp_targets = {100, 40, 20, 20, 26},
/* Baseboard is an interleaved design */
.dq_pins_interleaved = 1,
@@ -46,7 +55,8 @@
/* Disable Early Command Training */
.ect = 0,
-};
+}
+;
const struct cnl_mb_cfg *__weak variant_memcfg_config(void)
{
diff --git a/src/mainboard/intel/coffeelake_rvp/romstage.c b/src/mainboard/intel/coffeelake_rvp/romstage.c
index 1ab2d78..09ef148 100644
--- a/src/mainboard/intel/coffeelake_rvp/romstage.c
+++ b/src/mainboard/intel/coffeelake_rvp/romstage.c
@@ -20,13 +20,5 @@
void mainboard_memory_init_params(FSPM_UPD *memupd)
{
- const struct spd_info spd = {
- .spd_smbus_address[0] = 0xA0,
- .spd_smbus_address[1] = 0xA2,
- .spd_smbus_address[2] = 0xA4,
- .spd_smbus_address[3] = 0xA6,
- };
-
- cannonlake_memcfg_init(&memupd->FspmConfig,
- variant_memcfg_config(), &spd);
+ cannonlake_memcfg_init(&memupd->FspmConfig, variant_memcfg_config());
}
diff --git a/src/soc/intel/cannonlake/cnl_memcfg_init.c b/src/soc/intel/cannonlake/cnl_memcfg_init.c
index db001b8..ad7a71c 100644
--- a/src/soc/intel/cannonlake/cnl_memcfg_init.c
+++ b/src/soc/intel/cannonlake/cnl_memcfg_init.c
@@ -20,7 +20,7 @@
#include <string.h>
static void meminit_memcfg(FSP_M_CONFIG *mem_cfg,
- const struct cnl_mb_cfg *board_cfg)
+ const struct cnl_mb_cfg *board_cfg)
{
/*
* DqByteMapChx expects 12 bytes of data, but the last 6 bytes
@@ -29,29 +29,34 @@
*/
memset(&mem_cfg->DqByteMapCh0, 0, sizeof(mem_cfg->DqByteMapCh0));
memcpy(&mem_cfg->DqByteMapCh0, &board_cfg->dq_map[DDR_CH0],
- sizeof(board_cfg->dq_map[DDR_CH0]));
+ sizeof(board_cfg->dq_map[DDR_CH0]));
memset(&mem_cfg->DqByteMapCh1, 0, sizeof(mem_cfg->DqByteMapCh1));
memcpy(&mem_cfg->DqByteMapCh1, &board_cfg->dq_map[DDR_CH1],
- sizeof(board_cfg->dq_map[DDR_CH1]));
+ sizeof(board_cfg->dq_map[DDR_CH1]));
memcpy(&mem_cfg->DqsMapCpu2DramCh0, &board_cfg->dqs_map[DDR_CH0],
- sizeof(board_cfg->dqs_map[DDR_CH0]));
+ sizeof(board_cfg->dqs_map[DDR_CH0]));
memcpy(&mem_cfg->DqsMapCpu2DramCh1, &board_cfg->dqs_map[DDR_CH1],
- sizeof(board_cfg->dqs_map[DDR_CH1]));
+ sizeof(board_cfg->dqs_map[DDR_CH1]));
memcpy(&mem_cfg->RcompResistor, &board_cfg->rcomp_resistor,
- sizeof(mem_cfg->RcompResistor));
+ sizeof(mem_cfg->RcompResistor));
/* Early cannonlake requires rcomp targets to be 0 */
memcpy(&mem_cfg->RcompTarget, &board_cfg->rcomp_targets,
- sizeof(mem_cfg->RcompTarget));
+ sizeof(mem_cfg->RcompTarget));
}
-static void meminit_memcfg_spd(FSP_M_CONFIG *mem_cfg,
- const struct cnl_mb_cfg *cnl_cfg,
- size_t spd_data_len, uintptr_t spd_data_ptr)
+/*
+ * Initialize default memory settings using spd data contained in a buffer.
+ */
+static void meminit_spd_data(FSP_M_CONFIG *mem_cfg,
+ const struct cnl_mb_cfg *cnl_cfg,
+ size_t spd_data_len, uintptr_t spd_data_ptr)
{
+ assert(spd_data_ptr && spd_data_len);
+
mem_cfg->MemorySpdDataLen = spd_data_len;
if (cnl_cfg->channel_empty[0] == 0)
@@ -62,31 +67,20 @@
}
/*
- * Initialize default memory settings using spd data contained in a buffer.
- */
-static void meminit_spd_data(FSP_M_CONFIG *mem_cfg,
- const struct cnl_mb_cfg *cnl_cfg,
- size_t spd_data_len, uintptr_t spd_data_ptr)
-{
- assert(spd_data_ptr && spd_data_len);
- meminit_memcfg_spd(mem_cfg, cnl_cfg, spd_data_len, spd_data_ptr);
-}
-
-/*
* Initialize default memory settings using the spd file specified by
* spd_index. The spd_index is an index into the SPD_SOURCES array defined
* in spd/Makefile.inc.
*/
static void meminit_cbfs_spd_index(FSP_M_CONFIG *mem_cfg,
- const struct cnl_mb_cfg *cnl_cfg,
- int spd_index)
+ const struct cnl_mb_cfg *cnl_cfg)
{
size_t spd_data_len;
uintptr_t spd_data_ptr;
struct region_device spd_rdev;
- printk(BIOS_DEBUG, "SPD INDEX = %d\n", spd_index);
- if (get_spd_cbfs_rdev(&spd_rdev, spd_index) < 0)
+ printk(BIOS_DEBUG, "SPD INDEX = %d\n",
+ cnl_cfg->spd.spd_spec.spd_index);
+ if (get_spd_cbfs_rdev(&spd_rdev, cnl_cfg->spd.spd_spec.spd_index) < 0)
die("spd.bin not found or incorrect index\n");
spd_data_len = region_device_sz(&spd_rdev);
/* Memory leak is ok since we have memory mapped boot media */
@@ -97,10 +91,8 @@
/* Initialize onboard memory configurations for CannonLake */
void cannonlake_memcfg_init(FSP_M_CONFIG *mem_cfg,
- const struct cnl_mb_cfg *cnl_cfg,
- const struct spd_info *spd)
+ const struct cnl_mb_cfg *cnl_cfg)
{
- bool OnModuleSpd = false;
/* Early Command Training Enabled */
mem_cfg->ECT = cnl_cfg->ect;
mem_cfg->DqPinsInterleaved = cnl_cfg->dq_pins_interleaved;
@@ -108,24 +100,26 @@
/* Spd pointer will only be used if all smbus slave address of memory
* sockets on the platform is empty */
- for (int i = 0; i < ARRAY_SIZE(mem_cfg->SpdAddressTable); i++) {
- if (spd->spd_smbus_address[i] != 0) {
- mem_cfg->SpdAddressTable[i] = spd->spd_smbus_address[i];
- OnModuleSpd = true;
+ switch (cnl_cfg->spd.read_type) {
+ case READ_SMBUS:
+ for (int i = 0; i < ARRAY_SIZE(mem_cfg->SpdAddressTable); i++) {
+ if (cnl_cfg->spd.spd_spec.spd_smbus_address[i]) {
+ mem_cfg->SpdAddressTable[i] = cnl_cfg->spd.
+ spd_spec.spd_smbus_address[i];
+ }
}
- }
-
- if (!OnModuleSpd) {
- if (spd->spd_by_index) {
- meminit_cbfs_spd_index(mem_cfg, cnl_cfg,
- spd->spd_spec.spd_index);
- } else {
- meminit_spd_data(mem_cfg, cnl_cfg,
- spd->spd_spec.spd_data_ptr_info.spd_data_len,
- spd->spd_spec.spd_data_ptr_info.spd_data_ptr);
- }
+ break;
+ case READ_SPD_CBFS:
+ meminit_cbfs_spd_index(mem_cfg, cnl_cfg);
+ break;
+ case READ_SPD_MEMPTR:
+ meminit_spd_data(mem_cfg, cnl_cfg,
+ cnl_cfg->spd.spd_spec.spd_data_ptr_info.spd_data_len,
+ cnl_cfg->spd.spd_spec.spd_data_ptr_info.spd_data_ptr);
+ break;
+ default:
+ printk(BIOS_DEBUG, "No valid way to read mem info.");
}
meminit_memcfg(mem_cfg, cnl_cfg);
-
}
diff --git a/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h b/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h
index c602180..42d67a5 100644
--- a/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h
+++ b/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h
@@ -40,17 +40,27 @@
uintptr_t spd_data_ptr;
};
+enum mem_info_read_type {
+ NOT_EXISTING,
+ READ_SMBUS,
+ READ_SPD_CBFS,
+ READ_SPD_MEMPTR
+};
+
struct spd_info {
- bool spd_by_index;
+ enum mem_info_read_type read_type;
union spd_data_by {
int spd_index;
struct spd_by_pointer spd_data_ptr_info;
+ uint8_t spd_smbus_address[4];
} spd_spec;
- uint8_t spd_smbus_address[4];
};
/* Board-specific memory dq mapping information */
struct cnl_mb_cfg {
+ /* Parameters required to access DRAM info. */
+ struct spd_info spd;
+
/*
* For each channel, there are 3 sets of DQ byte mappings,
* where each set has a package 0 and a package 1 value (package 0
@@ -122,7 +132,6 @@
* Initialize default memory configurations for CannonLake.
*/
void cannonlake_memcfg_init(FSP_M_CONFIG *mem_cfg,
- const struct cnl_mb_cfg *cnl_cfg,
- const struct spd_info *spd);
+ const struct cnl_mb_cfg *cnl_cfg);
#endif /* _SOC_CANNONLAKE_MEMCFG_INIT_H_ */
To view, visit change 32513. To unsubscribe, or for help writing mail filters, visit settings.