All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] drivers/ddr: Fix klockworks warnings
@ 2014-04-01 21:20 York Sun
  0 siblings, 0 replies; only message in thread
From: York Sun @ 2014-04-01 21:20 UTC (permalink / raw)
  To: u-boot

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@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

^ permalink raw reply related	[flat|nested] only message in thread
only message in thread, other threads:[~2014-04-01 21:20 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-01 21:20 [U-Boot] [PATCH] drivers/ddr: Fix klockworks warnings York Sun

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.