powerpc/8xxx: Fix bug in memctrl interleaving & bank interleaving on cs0~cs4

Verified on MPC8641HPCN with four DDR2 dimms. Each dimm has dual
rank with 512MB each rank.

Also check dimm size and rank size for memory controller interleaving

Signed-off-by: York Sun <yorksun@freescale.com>

arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c

@@ -1201,20 +1201,28 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts,
 	/* Chip Select Memory Bounds (CSn_BNDS) */
 	for (i = 0; i < CONFIG_CHIP_SELECTS_PER_CTRL; i++) {
 		unsigned long long ea = 0, sa = 0;
+		unsigned int cs_per_dimm
+			= CONFIG_CHIP_SELECTS_PER_CTRL / CONFIG_DIMM_SLOTS_PER_CTLR;
+		unsigned int dimm_number
+			= i / cs_per_dimm;
+		unsigned long long rank_density
+			= dimm_params[dimm_number].rank_density;
 
-		if (popts->ba_intlv_ctl && (i > 0) &&
+		if (((i == 1) && (popts->ba_intlv_ctl & FSL_DDR_CS0_CS1)) ||
-			((popts->ba_intlv_ctl & 0x60) != FSL_DDR_CS2_CS3 )) {
+			((i == 2) && (popts->ba_intlv_ctl & 0x04)) ||
-			/* Don't set up boundaries for other CS
+			((i == 3) && (popts->ba_intlv_ctl & FSL_DDR_CS2_CS3))) {
-			 * other than CS0, if bank interleaving
+			/*
-			 * is enabled and not CS2+CS3 interleaved.
+			 * Don't set up boundaries for unused CS
+			 * cs1 for cs0_cs1, cs0_cs1_and_cs2_cs3, cs0_cs1_cs2_cs3
+			 * cs2 for cs0_cs1_cs2_cs3
+			 * cs3 for cs2_cs3, cs0_cs1_and_cs2_cs3, cs0_cs1_cs2_cs3
 			 * But we need to set the ODT_RD_CFG and
 			 * ODT_WR_CFG for CS1_CONFIG here.
 			 */
 			set_csn_config(i, ddr, popts, dimm_params);
-			break;
+			continue;
 		}
-
+		if (dimm_params[dimm_number].n_ranks == 0) {
-		if (dimm_params[i/2].n_ranks == 0) {
 			debug("Skipping setup of CS%u "
 				"because n_ranks on DIMM %u is 0\n", i, i/2);
 			continue;
@@ -1222,16 +1230,34 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts,
 		if (popts->memctl_interleaving && popts->ba_intlv_ctl) {
 			/*
 			 * This works superbank 2CS
-			 * There are 2 memory controllers configured
+			 * There are 2 or more memory controllers configured
 			 * identically, memory is interleaved between them,
 			 * and each controller uses rank interleaving within
 			 * itself. Therefore the starting and ending address
 			 * on each controller is twice the amount present on
 			 * each controller.
 			 */
-			unsigned long long rank_density
+			unsigned long long ctlr_density = 0;
-					= dimm_params[0].capacity;
+			switch (popts->ba_intlv_ctl & FSL_DDR_CS0_CS1_CS2_CS3) {
-			ea = (2 * (rank_density >> dbw_cap_adj)) - 1;
+			case FSL_DDR_CS0_CS1:
+			case FSL_DDR_CS0_CS1_AND_CS2_CS3:
+				ctlr_density = dimm_params[0].rank_density * 2;
+				break;
+			case FSL_DDR_CS2_CS3:
+				ctlr_density = dimm_params[0].rank_density;
+				break;
+			case FSL_DDR_CS0_CS1_CS2_CS3:
+				/*
+				 * The four CS interleaving should have been verified by
+				 * populate_memctl_options()
+				 */
+				ctlr_density = dimm_params[0].rank_density * 4;
+				break;
+			default:
+				break;
+			}
+			ea = (CONFIG_NUM_DDR_CONTROLLERS *
+				(ctlr_density >> dbw_cap_adj)) - 1;
 		}
 		else if (!popts->memctl_interleaving && popts->ba_intlv_ctl) {
 			/*
@@ -1243,8 +1269,6 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts,
 			 * controller needs to be programmed into its
 			 * respective CS0_BNDS.
 			 */
-			unsigned long long rank_density
-						= dimm_params[i/2].rank_density;
 			switch (popts->ba_intlv_ctl & FSL_DDR_CS0_CS1_CS2_CS3) {
 			case FSL_DDR_CS0_CS1_CS2_CS3:
 				/* CS0+CS1+CS2+CS3 interleaving, only CS0_CNDS
@@ -1257,9 +1281,13 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts,
 				/* CS0+CS1 and CS2+CS3 interleaving, CS0_CNDS
 				 * and CS2_CNDS need to be set.
 				 */
-				if (!(i&1)) {
+				if ((i == 2) && (dimm_number == 0)) {
-					sa = dimm_params[i/2].base_address;
+					sa = dimm_params[dimm_number].base_address +
-					ea = sa + (i * (rank_density >>
+					      2 * (rank_density >> dbw_cap_adj);
+					ea = sa + 2 * (rank_density >> dbw_cap_adj) - 1;
+				} else {
+					sa = dimm_params[dimm_number].base_address;
+					ea = sa + (2 * (rank_density >>
 						dbw_cap_adj)) - 1;
 				}
 				break;
@@ -1267,16 +1295,31 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts,
 				/* CS0+CS1 interleaving, CS0_CNDS needs
 				 * to be set
 				 */
-				sa = common_dimm->base_address;
+				if (dimm_params[dimm_number].n_ranks > (i % cs_per_dimm)) {
-				ea = sa + (2 * (rank_density >> dbw_cap_adj))-1;
+					sa = dimm_params[dimm_number].base_address;
+					ea = sa + (rank_density >> dbw_cap_adj) - 1;
+					sa += (i % cs_per_dimm) * (rank_density >> dbw_cap_adj);
+					ea += (i % cs_per_dimm) * (rank_density >> dbw_cap_adj);
+				} else {
+					sa = 0;
+					ea = 0;
+				}
+				if (i == 0)
+					ea += (rank_density >> dbw_cap_adj);
 				break;
 			case FSL_DDR_CS2_CS3:
 				/* CS2+CS3 interleaving*/
-				if (i == 2) {
+				if (dimm_params[dimm_number].n_ranks > (i % cs_per_dimm)) {
-					sa = dimm_params[i/2].base_address;
+					sa = dimm_params[dimm_number].base_address;
-					ea = sa + (2 * (rank_density >>
+					ea = sa + (rank_density >> dbw_cap_adj) - 1;
-						dbw_cap_adj)) - 1;
+					sa += (i % cs_per_dimm) * (rank_density >> dbw_cap_adj);
+					ea += (i % cs_per_dimm) * (rank_density >> dbw_cap_adj);
+				} else {
+					sa = 0;
+					ea = 0;
 				}
+				if (i == 2)
+					ea += (rank_density >> dbw_cap_adj);
 				break;
 			default:  /* No bank(chip-select) interleaving */
 				break;
@@ -1292,8 +1335,6 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts,
 			 * memory in the two CS0 ranks.
 			 */
 			if (i == 0) {
-				unsigned long long rank_density
-						= dimm_params[0].rank_density;
 				ea = (2 * (rank_density >> dbw_cap_adj)) - 1;
 			}
 
@@ -1303,20 +1344,14 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts,
 			 * No rank interleaving and no memory controller
 			 * interleaving.
 			 */
-			unsigned long long rank_density
+			sa = dimm_params[dimm_number].base_address;
-						= dimm_params[i/2].rank_density;
-			sa = dimm_params[i/2].base_address;
 			ea = sa + (rank_density >> dbw_cap_adj) - 1;
-			if (i&1) {
+			if (dimm_params[dimm_number].n_ranks > (i % cs_per_dimm)) {
-				if ((dimm_params[i/2].n_ranks == 1)) {
+				sa += (i % cs_per_dimm) * (rank_density >> dbw_cap_adj);
-					/* Odd chip select, single-rank dimm */
+				ea += (i % cs_per_dimm) * (rank_density >> dbw_cap_adj);
-					sa = 0;
+			} else {
-					ea = 0;
+				sa = 0;
-				} else {
+				ea = 0;
-					/* Odd chip select, dual-rank DIMM */
-					sa += rank_density >> dbw_cap_adj;
-					ea += rank_density >> dbw_cap_adj;
-				}
 			}
 		}
 

arch/powerpc/cpu/mpc8xxx/ddr/ddr.h

@@ -73,6 +73,7 @@ extern unsigned int populate_memctl_options(int all_DIMMs_registered,
 				memctl_options_t *popts,
 				dimm_params_t *pdimm,
 				unsigned int ctrl_num);
+extern void check_interleaving_options(fsl_ddr_info_t *pinfo);
 
 extern unsigned int mclk_to_picos(unsigned int mclk);
 extern unsigned int get_memory_clk_period_ps(void);

arch/powerpc/cpu/mpc8xxx/ddr/main.c

@@ -100,8 +100,8 @@ const char * step_to_string(unsigned int step) {
 
 int step_assign_addresses(fsl_ddr_info_t *pinfo,
 			  unsigned int dbw_cap_adj[],
-			  unsigned int *memctl_interleaving,
+			  unsigned int *all_memctl_interleaving,
-			  unsigned int *rank_interleaving)
+			  unsigned int *all_ctlr_rank_interleaving)
 {
 	int i, j;
 
@@ -152,30 +152,30 @@ int step_assign_addresses(fsl_ddr_info_t *pinfo,
 		}
 	}
 
-	/*
-	 * Check if all controllers are configured for memory
-	 * controller interleaving.
-	 */
 	j = 0;
-	for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++) {
+	for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++)
-		if (pinfo->memctl_opts[i].memctl_interleaving) {
+		if (pinfo->memctl_opts[i].memctl_interleaving)
 			j++;
-		}
+	/*
-	}
+	 * Not support less than all memory controllers interleaving
-	if (j == 2)
+	 * if more than two controllers
-		*memctl_interleaving = 1;
+	 */
+	if (j == CONFIG_NUM_DDR_CONTROLLERS)
+		*all_memctl_interleaving = 1;
 
 	/* Check that all controllers are rank interleaving. */
 	j = 0;
-	for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++) {
+	for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++)
-		if (pinfo->memctl_opts[i].ba_intlv_ctl) {
+		if (pinfo->memctl_opts[i].ba_intlv_ctl)
 			j++;
-		}
+	/*
-	}
+	 * All memory controllers must be populated to qualify for
-	if (j == 2)
+	 * all controller rank interleaving
-		*rank_interleaving = 1;
+	 */
+	 if (j == CONFIG_NUM_DDR_CONTROLLERS)
+		*all_ctlr_rank_interleaving = 1;
 
-	if (*memctl_interleaving) {
+	if (*all_memctl_interleaving) {
 		unsigned long long addr, total_mem_per_ctlr = 0;
 		/*
 		 * If interleaving between memory controllers,
@@ -316,7 +316,7 @@ fsl_ddr_compute(fsl_ddr_info_t *pinfo, unsigned int start_step)
 					&pinfo->memctl_opts[i],
 					pinfo->dimm_params[i], i);
 		}
-
+		check_interleaving_options(pinfo);
 	case STEP_ASSIGN_ADDRESSES:
 		/* STEP 5:  Assign addresses to chip selects */
 		step_assign_addresses(pinfo,

arch/powerpc/cpu/mpc8xxx/ddr/options.c

@@ -212,10 +212,9 @@ unsigned int populate_memctl_options(int all_DIMMs_registered,
 	 * Please refer to doc/README.fsl-ddr for the detail.
 	 *
 	 * If memory controller interleaving is enabled, then the data
-	 * bus widths must be programmed identically for the 2 memory
+	 * bus widths must be programmed identically for all memory controllers.
-	 * controllers.
 	 *
-	 * XXX: Attempt to set both controllers to the same chip select
+	 * XXX: Attempt to set all controllers to the same chip select
 	 * interleaving mode. It will do a best effort to get the
 	 * requested ranks interleaved together such that the result
 	 * should be a subset of the requested configuration.
@@ -223,15 +222,17 @@ unsigned int populate_memctl_options(int all_DIMMs_registered,
 #if (CONFIG_NUM_DDR_CONTROLLERS > 1)
 	if (hwconfig_sub("fsl_ddr", "ctlr_intlv")) {
 		if (pdimm[0].n_ranks == 0) {
-			printf("There is no rank on CS0. Because only rank on "
+			printf("There is no rank on CS0 for controller %d. Because only"
-				"CS0 and ranks chip-select interleaved with CS0"
+				" rank on CS0 and ranks chip-select interleaved with CS0"
 				" are controller interleaved, force non memory "
-				"controller interleaving\n");
+				"controller interleaving\n", ctrl_num);
 			popts->memctl_interleaving = 0;
 		} else {
 			popts->memctl_interleaving = 1;
-			/* test null first. if CONFIG_HWCONFIG is not defined
+			/*
-			 * hwconfig_arg_cmp returns non-zero */
+			 * test null first. if CONFIG_HWCONFIG is not defined
+			 * hwconfig_arg_cmp returns non-zero
+			 */
 			if (hwconfig_subarg_cmp("fsl_ddr", "ctlr_intlv", "null")) {
 				popts->memctl_interleaving = 0;
 				debug("memory controller interleaving disabled.\n");
@@ -254,13 +255,12 @@ unsigned int populate_memctl_options(int all_DIMMs_registered,
 		}
 	}
 #endif
-
 	if ((hwconfig_sub("fsl_ddr", "bank_intlv")) &&
 		(CONFIG_CHIP_SELECTS_PER_CTRL > 1)) {
 		/* test null first. if CONFIG_HWCONFIG is not defined,
 		 * hwconfig_arg_cmp returns non-zero */
 		if (hwconfig_subarg_cmp("fsl_ddr", "bank_intlv", "null"))
-			printf("bank interleaving disabled.\n");
+			debug("bank interleaving disabled.\n");
 		else if (hwconfig_subarg_cmp("fsl_ddr", "bank_intlv", "cs0_cs1"))
 			popts->ba_intlv_ctl = FSL_DDR_CS0_CS1;
 		else if (hwconfig_subarg_cmp("fsl_ddr", "bank_intlv", "cs2_cs3"))
@@ -270,30 +270,70 @@ unsigned int populate_memctl_options(int all_DIMMs_registered,
 		else if (hwconfig_subarg_cmp("fsl_ddr", "bank_intlv", "cs0_cs1_cs2_cs3"))
 			popts->ba_intlv_ctl = FSL_DDR_CS0_CS1_CS2_CS3;
 		else
-			printf("hwconfig has unrecognized parameter for ba_intlv_ctl.\n");
+			printf("hwconfig has unrecognized parameter for bank_intlv.\n");
-
 		switch (popts->ba_intlv_ctl & FSL_DDR_CS0_CS1_CS2_CS3) {
 		case FSL_DDR_CS0_CS1_CS2_CS3:
+#if (CONFIG_DIMM_SLOTS_PER_CTLR == 1)
+			if (pdimm[0].n_ranks != 4) {
+				popts->ba_intlv_ctl = 0;
+				printf("Not enough bank(chip-select) for "
+					"CS0+CS1+CS2+CS3 on controller %d, "
+					"force non-interleaving!\n", ctrl_num);
+			}
+#elif (CONFIG_DIMM_SLOTS_PER_CTLR == 2)
+			if ((pdimm[0].n_ranks != 2) && (pdimm[1].n_ranks != 2)) {
+				popts->ba_intlv_ctl = 0;
+				printf("Not enough bank(chip-select) for "
+					"CS0+CS1+CS2+CS3 on controller %d, "
+					"force non-interleaving!\n", ctrl_num);
+			}
+			if (pdimm[0].capacity != pdimm[1].capacity) {
+				popts->ba_intlv_ctl = 0;
+				printf("Not identical DIMM size for "
+					"CS0+CS1+CS2+CS3 on controller %d, "
+					"force non-interleaving!\n", ctrl_num);
+			}
+#endif
+			break;
 		case FSL_DDR_CS0_CS1:
 			if (pdimm[0].n_ranks != 2) {
 				popts->ba_intlv_ctl = 0;
 				printf("Not enough bank(chip-select) for "
-					"CS0+CS1, force non-interleaving!\n");
+					"CS0+CS1 on controller %d, "
+					"force non-interleaving!\n", ctrl_num);
 			}
 			break;
 		case FSL_DDR_CS2_CS3:
-			if (pdimm[1].n_ranks !=2){
+#if (CONFIG_DIMM_SLOTS_PER_CTLR == 1)
+			if (pdimm[0].n_ranks != 4) {
 				popts->ba_intlv_ctl = 0;
-				printf("Not enough bank(CS) for CS2+CS3, "
+				printf("Not enough bank(chip-select) for CS2+CS3 "
-					"force non-interleaving!\n");
+					"on controller %d, force non-interleaving!\n", ctrl_num);
 			}
+#elif (CONFIG_DIMM_SLOTS_PER_CTLR == 2)
+			if (pdimm[1].n_ranks != 2) {
+				popts->ba_intlv_ctl = 0;
+				printf("Not enough bank(chip-select) for CS2+CS3 "
+					"on controller %d, force non-interleaving!\n", ctrl_num);
+			}
+#endif
 			break;
 		case FSL_DDR_CS0_CS1_AND_CS2_CS3:
+#if (CONFIG_DIMM_SLOTS_PER_CTLR == 1)
+			if (pdimm[0].n_ranks != 4) {
+				popts->ba_intlv_ctl = 0;
+				printf("Not enough bank(CS) for CS0+CS1 and "
+					"CS2+CS3 on controller %d, "
+					"force non-interleaving!\n", ctrl_num);
+			}
+#elif (CONFIG_DIMM_SLOTS_PER_CTLR == 2)
 			if ((pdimm[0].n_ranks != 2)||(pdimm[1].n_ranks != 2)) {
 				popts->ba_intlv_ctl = 0;
-				printf("Not enough bank(CS) for CS0+CS1 or "
+				printf("Not enough bank(CS) for CS0+CS1 and "
-					"CS2+CS3, force non-interleaving!\n");
+					"CS2+CS3 on controller %d, "
+					"force non-interleaving!\n", ctrl_num);
 			}
+#endif
 			break;
 		default:
 			popts->ba_intlv_ctl = 0;
@@ -305,3 +345,34 @@ unsigned int populate_memctl_options(int all_DIMMs_registered,
 
 	return 0;
 }
+
+void check_interleaving_options(fsl_ddr_info_t *pinfo)
+{
+	int i, j, check_n_ranks, intlv_fixed = 0;
+	unsigned long long check_rank_density;
+	/*
+	 * Check if all controllers are configured for memory
+	 * controller interleaving. Identical dimms are recommended. At least
+	 * the size should be checked.
+	 */
+	j = 0;
+	check_n_ranks = pinfo->dimm_params[0][0].n_ranks;
+	check_rank_density = pinfo->dimm_params[0][0].rank_density;
+	for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++) {
+		if ((pinfo->memctl_opts[i].memctl_interleaving) && \
+		    (check_rank_density == pinfo->dimm_params[i][0].rank_density) && \
+		    (check_n_ranks == pinfo->dimm_params[i][0].n_ranks)) {
+			j++;
+		}
+	}
+	if (j != CONFIG_NUM_DDR_CONTROLLERS) {
+		for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++)
+			if (pinfo->memctl_opts[i].memctl_interleaving) {
+				pinfo->memctl_opts[i].memctl_interleaving = 0;
+				intlv_fixed = 1;
+			}
+		if (intlv_fixed)
+			printf("Not all DIMMs are identical in size. "
+				"Memory controller interleaving disabled.\n");
+	}
+}

board/freescale/mpc8641hpcn/mpc8641hpcn.c

@@ -60,6 +60,8 @@ int checkboard(void)
 	return 0;
 }
 
+const char *board_hwconfig = "foo:bar=baz";
+const char *cpu_hwconfig = "foo:bar=baz";
 
 phys_size_t
 initdram(int board_type)

doc/README.fsl-ddr

@@ -27,6 +27,9 @@ Table of interleaving modes supported in cpu/8xxx/ddr/
  from each controller. {CS2+CS3} on each controller are only rank
  interleaved on that controller.
 
+ For memory controller interleaving, identical DIMMs are suggested. Software
+ doesn't check the size or organization of interleaved DIMMs.
+
 The ways to configure the ddr interleaving mode
 ==============================================
 1. In board header file(e.g.MPC8572DS.h), add default interleaving setting

include/configs/MPC8641HPCN.h

@@ -122,6 +122,8 @@ extern unsigned long get_board_sys_clk(unsigned long dummy);
 #define CONFIG_SYS_CCSRBAR_PHYS		CONFIG_SYS_CCSRBAR_PHYS_LOW
 #endif
 
+#define CONFIG_HWCONFIG	/* use hwconfig to control memory interleaving */
+
 /*
  * DDR Setup
  */