From 2785c118a6e1c7597004160b5688cd8bf3c8e517 Mon Sep 17 00:00:00 2001 From: Arthur Heymans Date: Sun, 10 Sep 2017 20:57:37 +0200 Subject: [PATCH] device/dram/ddr2.c: Improve error returning and debug output This patch outputs decoding errors with BIOS_WARNING instead of depending on CONFIG_DEBUG_RAM_SETUP. Returns SPD_STATUS_INVALID on invalid settings for tRR, bcd and tCK and doesn't try to create a valid setting if an invalid setting is detected. Change-Id: Iee434d1fa1a9d911cc3683b88b260881ed6434ea Signed-off-by: Arthur Heymans Reviewed-on: https://review.coreboot.org/21480 Tested-by: build bot (Jenkins) Reviewed-by: Nico Huber --- src/device/dram/ddr2.c | 138 +++++++++++++++++++++++++++++------------ 1 file changed, 99 insertions(+), 39 deletions(-) diff --git a/src/device/dram/ddr2.c b/src/device/dram/ddr2.c index 76524ceada..6f43563b6e 100644 --- a/src/device/dram/ddr2.c +++ b/src/device/dram/ddr2.c @@ -113,7 +113,7 @@ u8 spd_get_msbs(u8 c) * Decodes a raw SPD data from a DDR2 DIMM. * Returns cycle time in 1/256th ns. */ -static u32 spd_decode_tck_time(u8 c) +static int spd_decode_tck_time(u32 *tck, u8 c) { u8 high, low; @@ -132,11 +132,17 @@ static u32 spd_decode_tck_time(u8 c) case 0xd: low = 75; break; + case 0xe: + case 0xf: + printk(BIOS_WARNING, "Invalid tck setting. " + "lower nibble is 0x%x\n", c & 0xf); + return CB_ERR; default: low = (c & 0xf) * 10; } - return ((high * 100 + low) << 8) / 100; + *tck = ((high * 100 + low) << 8) / 100; + return CB_SUCCESS; } /** @@ -145,14 +151,17 @@ static u32 spd_decode_tck_time(u8 c) * Decodes a raw SPD data from a DDR2 DIMM. * Returns cycle time in 1/256th ns. */ -static u32 spd_decode_bcd_time(u8 c) +static int spd_decode_bcd_time(u32 *bcd, u8 c) { u8 high, low; high = c >> 4; low = c & 0xf; + if (high >= 10 || low >= 10) + return CB_ERR; - return ((high * 10 + low) << 8) / 100; + *bcd = ((high * 10 + low) << 8) / 100; + return CB_SUCCESS; } /** @@ -177,26 +186,26 @@ static u32 spd_decode_quarter_time(u8 c) * Decodes a raw SPD data from a DDR2 DIMM. * Returns cycle time in 1/256th us. */ -static u32 spd_decode_tRR_time(u8 c) +static int spd_decode_tRR_time(u32 *tRR, u8 c) { switch (c) { default: - printk(BIOS_WARNING, - "Unknown tRR value, using default of 15.6us."); - /* Fallthrough */ + printk(BIOS_WARNING, "Invalid tRR value 0x%x\n", c); + return CB_ERR; case 0x80: - return 15625 << 8; + *tRR = 15625 << 8; case 0x81: - return 15625 << 6; + *tRR = 15625 << 6; case 0x82: - return 15625 << 7; + *tRR = 15625 << 7; case 0x83: - return 15625 << 9; + *tRR = 15625 << 9; case 0x84: - return 15625 << 10; + *tRR = 15625 << 10; case 0x85: - return 15625 << 11; + *tRR = 15625 << 11; } + return CB_SUCCESS; } /** @@ -297,21 +306,22 @@ int spd_decode_ddr2(struct dimm_attr_st *dimm, u8 spd[SPD_SIZE_MAX_DDR2]) printram("SPD contains 0x%02x bytes\n", spd_size); if (spd_size < 64 || eeprom_size < 64) { - printram("ERROR: SPD to small\n"); + printk(BIOS_WARNING, "ERROR: SPD to small\n"); dimm->dram_type = SPD_MEMORY_TYPE_UNDEFINED; return SPD_STATUS_INVALID; } if (spd_ddr2_calc_checksum(spd, spd_size) != spd[63]) { - printram("ERROR: SPD checksum error\n"); + printk(BIOS_WARNING, "ERROR: SPD checksum error\n"); dimm->dram_type = SPD_MEMORY_TYPE_UNDEFINED; return SPD_STATUS_CRC_ERROR; } reg8 = spd[62]; if ((reg8 & 0xf0) != 0x10) { - printram("ERROR: Unsupported SPD revision %01x.%01x\n", - reg8 >> 4, reg8 & 0xf); + printk(BIOS_WARNING, + "ERROR: Unsupported SPD revision %01x.%01x\n", + reg8 >> 4, reg8 & 0xf); dimm->dram_type = SPD_MEMORY_TYPE_UNDEFINED; return SPD_STATUS_INVALID; } @@ -321,7 +331,7 @@ int spd_decode_ddr2(struct dimm_attr_st *dimm, u8 spd[SPD_SIZE_MAX_DDR2]) reg8 = spd[2]; printram(" Type : 0x%02x\n", reg8); if (reg8 != 0x08) { - printram("ERROR: Unsupported SPD type %x\n", reg8); + printk(BIOS_WARNING, "ERROR: Unsupported SPD type %x\n", reg8); dimm->dram_type = SPD_MEMORY_TYPE_UNDEFINED; return SPD_STATUS_INVALID; } @@ -331,14 +341,16 @@ int spd_decode_ddr2(struct dimm_attr_st *dimm, u8 spd[SPD_SIZE_MAX_DDR2]) printram(" Rows : %u\n", dimm->row_bits); if ((dimm->row_bits > 31) || ((dimm->row_bits > 15) && (dimm->rev < 0x13))) { - printram(" Invalid number of memory rows\n"); + printk(BIOS_WARNING, + "SPD decode: invalid number of memory rows\n"); ret = SPD_STATUS_INVALID_FIELD; } dimm->col_bits = spd[4]; printram(" Columns : %u\n", dimm->col_bits); if (dimm->col_bits > 15) { - printram(" Invalid number of memory columns\n"); + printk(BIOS_WARNING, + "SPD decode: invalid number of memory columns\n"); ret = SPD_STATUS_INVALID_FIELD; } @@ -348,21 +360,22 @@ int spd_decode_ddr2(struct dimm_attr_st *dimm, u8 spd[SPD_SIZE_MAX_DDR2]) dimm->mod_width = spd[6]; printram(" Module data width : x%u\n", dimm->mod_width); if (!dimm->mod_width) { - printram(" Invalid module data width\n"); + printk(BIOS_WARNING, "SPD decode: invalid module data width\n"); ret = SPD_STATUS_INVALID_FIELD; } dimm->width = spd[13]; printram(" SDRAM width : x%u\n", dimm->width); if (!dimm->width) { - printram(" Invalid SDRAM width\n"); + printk(BIOS_WARNING, "SPD decode: invalid SDRAM width\n"); ret = SPD_STATUS_INVALID_FIELD; } dimm->banks = spd[17]; printram(" Banks : %u\n", dimm->banks); if (!dimm->banks) { - printram(" Invalid module banks count\n"); + printk(BIOS_WARNING, + "SPD decode: invalid module banks count\n"); ret = SPD_STATUS_INVALID_FIELD; } @@ -392,23 +405,26 @@ int spd_decode_ddr2(struct dimm_attr_st *dimm, u8 spd[SPD_SIZE_MAX_DDR2]) printram(" Voltage : 1.8V\n"); break; default: - printram(" Unknown voltage level.\n"); + printk(BIOS_WARNING, "SPD decode: unknown voltage level.\n"); ret = SPD_STATUS_INVALID_FIELD; } dimm->cas_supported = spd[18]; if ((dimm->cas_supported & 0x3) || !dimm->cas_supported) { - printram(" Invalid CAS support advertised.\n"); + printk(BIOS_WARNING, + "SPD decode: invalid CAS support advertised.\n"); ret = SPD_STATUS_INVALID_FIELD; } printram(" Supported CAS mask : 0x%x\n", dimm->cas_supported); if ((dimm->rev < 0x13) && (dimm->cas_supported & 0x80)) { - printram(" Invalid CAS support advertised.\n"); + printk(BIOS_WARNING, + "SPD decode: invalid CAS support advertised.\n"); ret = SPD_STATUS_INVALID_FIELD; } if ((dimm->rev < 0x12) && (dimm->cas_supported & 0x40)) { - printram(" Invalid CAS support advertised.\n"); + printk(BIOS_WARNING, + "SPD decode: invalid CAS support advertised.\n"); ret = SPD_STATUS_INVALID_FIELD; } @@ -416,26 +432,60 @@ int spd_decode_ddr2(struct dimm_attr_st *dimm, u8 spd[SPD_SIZE_MAX_DDR2]) cl = spd_get_msbs(dimm->cas_supported); /* SDRAM Cycle time at Maximum Supported CAS Latency (CL), CL=X */ - dimm->cycle_time[cl] = spd_decode_tck_time(spd[9]); + if (spd_decode_tck_time(&dimm->cycle_time[cl], spd[9]) != CB_SUCCESS) { + printk(BIOS_WARNING, + "SPD decode: invalid min tCL for CAS%d\n", cl); + ret = SPD_STATUS_INVALID_FIELD; + } /* SDRAM Access from Clock */ - dimm->access_time[cl] = spd_decode_bcd_time(spd[10]); + if (spd_decode_bcd_time(&dimm->access_time[cl], spd[10]) + != CB_SUCCESS) { + printk(BIOS_WARNING, + "SPD decode: invalid min tAC for CAS%d\n", cl); + ret = SPD_STATUS_INVALID_FIELD; + } if (dimm->cas_supported & (1 << (cl - 1))) { /* Minimum Clock Cycle at CLX-1 */ - dimm->cycle_time[cl - 1] = spd_decode_tck_time(spd[23]); + if (spd_decode_tck_time(&dimm->cycle_time[cl - 1], spd[23]) + != CB_SUCCESS) { + printk(BIOS_WARNING, + "SPD decode: invalid min tCL for CAS%d\n", + cl - 1); + ret = SPD_STATUS_INVALID_FIELD; + } /* Maximum Data Access Time (tAC) from Clock at CLX-1 */ - dimm->access_time[cl - 1] = spd_decode_bcd_time(spd[24]); + if (spd_decode_bcd_time(&dimm->access_time[cl - 1], spd[24]) + != CB_SUCCESS) { + printk(BIOS_WARNING, + "SPD decode: invalid min tAC for CAS%d\n", + cl - 1); + ret = SPD_STATUS_INVALID_FIELD; + } } if (dimm->cas_supported & (1 << (cl - 2))) { /* Minimum Clock Cycle at CLX-2 */ - dimm->cycle_time[cl - 2] = spd_decode_tck_time(spd[25]); + if (spd_decode_tck_time(&dimm->cycle_time[cl - 2], spd[25]) + != CB_SUCCESS) { + printk(BIOS_WARNING, + "SPD decode: invalid min tCL for CAS%d\n", + cl - 2); + ret = SPD_STATUS_INVALID_FIELD; + } /* Maximum Data Access Time (tAC) from Clock at CLX-2 */ - dimm->access_time[cl - 2] = spd_decode_bcd_time(spd[26]); + if (spd_decode_bcd_time(&dimm->access_time[cl - 2], spd[26]) + != CB_SUCCESS) { + printk(BIOS_WARNING, + "SPD decode: invalid min tAC for CAS%d\n", + cl - 2); + ret = SPD_STATUS_INVALID_FIELD; + } } reg8 = (spd[31] >> 5) | (spd[31] << 3); if (!reg8) { - printram(" Invalid rank density.\n"); + printk(BIOS_WARNING, + "SPD decode: invalid rank density.\n"); ret = SPD_STATUS_INVALID_FIELD; } @@ -449,7 +499,10 @@ int spd_decode_ddr2(struct dimm_attr_st *dimm, u8 spd[SPD_SIZE_MAX_DDR2]) printram(" Capacity : %u GB\n", dimm->size_mb >> 10); /* SDRAM Maximum Cycle Time (tCKmax) */ - dimm->tCK = spd_decode_tck_time(spd[43]); + if (spd_decode_bcd_time(&dimm->tCK, spd[43]) != CB_SUCCESS) { + printk(BIOS_WARNING, "SPD decode: invalid Max tCK\n"); + ret = SPD_STATUS_INVALID_FIELD; + } /* Minimum Write Recovery Time (tWRmin) */ dimm->tWR = spd_decode_quarter_time(spd[36]); /* Minimum RAS# to CAS# Delay Time (tRCDmin) */ @@ -468,9 +521,15 @@ int spd_decode_ddr2(struct dimm_attr_st *dimm, u8 spd[SPD_SIZE_MAX_DDR2]) /* Minimum Internal Read to Precharge Command Delay Time (tRTPmin) */ dimm->tRTP = spd_decode_quarter_time(spd[38]); /* Data Input Setup Time Before Strobe */ - dimm->tDS = spd_decode_bcd_time(spd[34]); + if (spd_decode_bcd_time(&dimm->tDS, spd[34]) != CB_SUCCESS) { + printk(BIOS_WARNING, "SPD decode: invalid tDS\n"); + ret = SPD_STATUS_INVALID_FIELD; + } /* Data Input Hold Time After Strobe */ - dimm->tDH = spd_decode_bcd_time(spd[35]); + if (spd_decode_bcd_time(&dimm->tDH, spd[35]) != CB_SUCCESS) { + printk(BIOS_WARNING, "SPD decode: invalid tDH\n"); + ret = SPD_STATUS_INVALID_FIELD; + } /* SDRAM Device DQS-DQ Skew for DQS and associated DQ signals */ dimm->tDQSQ = (spd[44] << 8) / 100; /* SDRAM Device Maximum Read Data Hold Skew Factor */ @@ -478,7 +537,8 @@ int spd_decode_ddr2(struct dimm_attr_st *dimm, u8 spd[SPD_SIZE_MAX_DDR2]) /* PLL Relock Time in us */ dimm->tPLL = spd[46] << 8; /* Refresh rate in us */ - dimm->tRR = spd_decode_tRR_time(spd[12]); + if (spd_decode_tRR_time(&dimm->tRR, spd[12]) != CB_SUCCESS) + ret = SPD_STATUS_INVALID_FIELD; /* Number of PLLs on DIMM */ if (dimm->rev >= 0x11)