[U-Boot] [PATCH] drivers/ddr: Fix klockworks warnings

York Sun yorksun at freescale.com
Tue Apr 1 23:20:49 CEST 2014


This is a possible out of bounds error, caught by Klockworks. Adding
check before using as array index. Klockworks also gives warning when
pre-compiling conditions should be used instead of runtime.

Signed-off-by: York Sun <yorksun at freescale.com>
---
 drivers/ddr/fsl/ctrl_regs.c |    8 ++--
 drivers/ddr/fsl/main.c      |    6 +++
 drivers/ddr/fsl/options.c   |  108 +++++++++++++++++++++----------------------
 3 files changed, 64 insertions(+), 58 deletions(-)

diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
index d01eea0..78e82bb 100644
--- a/drivers/ddr/fsl/ctrl_regs.c
+++ b/drivers/ddr/fsl/ctrl_regs.c
@@ -507,8 +507,8 @@ static void set_timing_cfg_1(fsl_ddr_cfg_regs_t *ddr,
 	wrrec_mclk = picos_to_mclk(common_dimm->twr_ps);
 	acttoact_mclk = max(picos_to_mclk(common_dimm->trrds_ps), 4);
 	wrtord_mclk = max(2, picos_to_mclk(2500));
-	if (wrrec_mclk > 24)
-		printf("Error: WRREC doesn't support more than 24 clocks\n");
+	if ((wrrec_mclk < 1) || (wrrec_mclk > 24))
+		printf("Error: WRREC doesn't support %d clocks\n", wrrec_mclk);
 	else
 		wrrec_mclk = wrrec_table[wrrec_mclk - 1];
 #else
@@ -516,8 +516,8 @@ static void set_timing_cfg_1(fsl_ddr_cfg_regs_t *ddr,
 	wrrec_mclk = picos_to_mclk(common_dimm->twr_ps);
 	acttoact_mclk = picos_to_mclk(common_dimm->trrd_ps);
 	wrtord_mclk = picos_to_mclk(common_dimm->twtr_ps);
-	if (wrrec_mclk > 16)
-		printf("Error: WRREC doesn't support more than 16 clocks\n");
+	if ((wrrec_mclk < 1) || (wrrec_mclk > 16))
+		printf("Error: WRREC doesn't support %d clocks\n", wrrec_mclk);
 	else
 		wrrec_mclk = wrrec_table[wrrec_mclk - 1];
 #endif
diff --git a/drivers/ddr/fsl/main.c b/drivers/ddr/fsl/main.c
index f95704f..5e001fc 100644
--- a/drivers/ddr/fsl/main.c
+++ b/drivers/ddr/fsl/main.c
@@ -220,6 +220,11 @@ const char * step_to_string(unsigned int step) {
 	if ((1 << s) != step)
 		return step_string_tbl[7];
 
+	if (s >= ARRAY_SIZE(step_string_tbl)) {
+		printf("Error for the step in %s\n", __func__);
+		s = 0;
+	}
+
 	return step_string_tbl[s];
 }
 
@@ -520,6 +525,7 @@ fsl_ddr_compute(fsl_ddr_info_t *pinfo, unsigned int start_step,
 		/* STEP 5:  Assign addresses to chip selects */
 		check_interleaving_options(pinfo);
 		total_mem = step_assign_addresses(pinfo, dbw_capacity_adjust);
+		debug("Total mem %llu assigned\n", total_mem);
 
 	case STEP_COMPUTE_REGS:
 		/* STEP 6:  compute controller register values */
diff --git a/drivers/ddr/fsl/options.c b/drivers/ddr/fsl/options.c
index bf0d1da..5986e1a 100644
--- a/drivers/ddr/fsl/options.c
+++ b/drivers/ddr/fsl/options.c
@@ -525,67 +525,66 @@ unsigned int populate_memctl_options(int all_dimms_registered,
 	defined(CONFIG_SYS_FSL_DDR2) || \
 	defined(CONFIG_SYS_FSL_DDR4)
 	/* Chip select options. */
-	if (CONFIG_DIMM_SLOTS_PER_CTLR == 1) {
-		switch (pdimm[0].n_ranks) {
-		case 1:
-			pdodt = single_S;
-			break;
+#if (CONFIG_DIMM_SLOTS_PER_CTLR == 1)
+	switch (pdimm[0].n_ranks) {
+	case 1:
+		pdodt = single_S;
+		break;
+	case 2:
+		pdodt = single_D;
+		break;
+	case 4:
+		pdodt = single_Q;
+		break;
+	}
+#elif (CONFIG_DIMM_SLOTS_PER_CTLR == 2)
+	switch (pdimm[0].n_ranks) {
+#ifdef CONFIG_FSL_DDR_FIRST_SLOT_QUAD_CAPABLE
+	case 4:
+		pdodt = single_Q;
+		if (pdimm[1].n_ranks)
+			printf("Error: Quad- and Dual-rank DIMMs cannot be used together\n");
+		break;
+#endif
+	case 2:
+		switch (pdimm[1].n_ranks) {
 		case 2:
-			pdodt = single_D;
+			pdodt = dual_DD;
 			break;
-		case 4:
-			pdodt = single_Q;
+		case 1:
+			pdodt = dual_DS;
 			break;
-		}
-	} else if (CONFIG_DIMM_SLOTS_PER_CTLR == 2) {
-		switch (pdimm[0].n_ranks) {
-#ifdef CONFIG_FSL_DDR_FIRST_SLOT_QUAD_CAPABLE
-		case 4:
-			pdodt = single_Q;
-			if (pdimm[1].n_ranks)
-				printf("Error: Quad- and Dual-rank DIMMs "
-					"cannot be used together\n");
+		case 0:
+			pdodt = dual_D0;
 			break;
-#endif
+		}
+		break;
+	case 1:
+		switch (pdimm[1].n_ranks) {
 		case 2:
-			switch (pdimm[1].n_ranks) {
-			case 2:
-				pdodt = dual_DD;
-				break;
-			case 1:
-				pdodt = dual_DS;
-				break;
-			case 0:
-				pdodt = dual_D0;
-				break;
-			}
+			pdodt = dual_SD;
 			break;
 		case 1:
-			switch (pdimm[1].n_ranks) {
-			case 2:
-				pdodt = dual_SD;
-				break;
-			case 1:
-				pdodt = dual_SS;
-				break;
-			case 0:
-				pdodt = dual_S0;
-				break;
-			}
+			pdodt = dual_SS;
 			break;
 		case 0:
-			switch (pdimm[1].n_ranks) {
-			case 2:
-				pdodt = dual_0D;
-				break;
-			case 1:
-				pdodt = dual_0S;
-				break;
-			}
+			pdodt = dual_S0;
 			break;
 		}
+		break;
+	case 0:
+		switch (pdimm[1].n_ranks) {
+		case 2:
+			pdodt = dual_0D;
+			break;
+		case 1:
+			pdodt = dual_0S;
+			break;
+		}
+		break;
 	}
-#endif
+#endif	/* CONFIG_DIMM_SLOTS_PER_CTLR */
+#endif	/* CONFIG_SYS_FSL_DDR2, 3, 4 */
 
 	/* Pick chip-select local options. */
 	for (i = 0; i < CONFIG_CHIP_SELECTS_PER_CTRL; i++) {
@@ -847,8 +846,7 @@ unsigned int populate_memctl_options(int all_dimms_registered,
 	popts->memctl_interleaving_mode = FSL_DDR_256B_INTERLEAVING;
 	popts->memctl_interleaving = 1;
 	debug("256 Byte interleaving\n");
-	goto done;
-#endif
+#else
 	/*
 	 * test null first. if CONFIG_HWCONFIG is not defined
 	 * hwconfig_arg_cmp returns non-zero
@@ -930,8 +928,9 @@ unsigned int populate_memctl_options(int all_dimms_registered,
 		popts->memctl_interleaving = 0;
 		printf("hwconfig has unrecognized parameter for ctlr_intlv.\n");
 	}
+#endif	/* CONFIG_SYS_FSL_DDR_INTLV_256B */
 done:
-#endif
+#endif /* CONFIG_NUM_DDR_CONTROLLERS > 1 */
 	if ((hwconfig_sub_f("fsl_ddr", "bank_intlv", buf)) &&
 		(CONFIG_CHIP_SELECTS_PER_CTRL > 1)) {
 		/* test null first. if CONFIG_HWCONFIG is not defined,
@@ -1106,10 +1105,11 @@ void check_interleaving_options(fsl_ddr_info_t *pinfo)
 		case FSL_DDR_PAGE_INTERLEAVING:
 		case FSL_DDR_BANK_INTERLEAVING:
 		case FSL_DDR_SUPERBANK_INTERLEAVING:
-			if (3 == CONFIG_NUM_DDR_CONTROLLERS)
+#if (3 == CONFIG_NUM_DDR_CONTROLLERS)
 				k = 2;
-			else
+#else
 				k = CONFIG_NUM_DDR_CONTROLLERS;
+#endif
 			break;
 		case FSL_DDR_3WAY_1KB_INTERLEAVING:
 		case FSL_DDR_3WAY_4KB_INTERLEAVING:
-- 
1.7.9.5




More information about the U-Boot mailing list