[U-Boot] [PATCH] Disable unused chip-select for DDR controller interleaving

When DDR controller interleaving is eabled and less than all bank (chip-select) interleaving is seletected, the unused chip-select should be disabled.
Signed-off-by: York Sun yorksun@freescale.com --- arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c b/arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c index e82082e..e981262 100644 --- a/arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c +++ b/arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c @@ -1184,6 +1184,7 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts, unsigned int sr_it; unsigned int zq_en; unsigned int wrlvl_en; + int cs_en = 1;
memset(ddr, 0, sizeof(fsl_ddr_cfg_regs_t));
@@ -1250,16 +1251,20 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts, * 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. + * each controller.When */ unsigned long long ctlr_density = 0; switch (popts->ba_intlv_ctl & FSL_DDR_CS0_CS1_CS2_CS3) { case FSL_DDR_CS0_CS1: case FSL_DDR_CS0_CS1_AND_CS2_CS3: ctlr_density = dimm_params[0].rank_density * 2; + if (i > 1) + cs_en = 0; break; case FSL_DDR_CS2_CS3: ctlr_density = dimm_params[0].rank_density; + if (i > 0) + cs_en = 0; break; case FSL_DDR_CS0_CS1_CS2_CS3: /* @@ -1379,8 +1384,10 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts, );
debug("FSLDDR: cs[%d]_bnds = 0x%08x\n", i, ddr->cs[i].bnds); - set_csn_config(dimm_number, i, ddr, popts, dimm_params); - set_csn_config_2(i, ddr); + if (cs_en) { + set_csn_config(dimm_number, i, ddr, popts, dimm_params); + set_csn_config_2(i, ddr); + } }
set_ddr_eor(ddr, popts);

Dear York Sun,
In message 1286983650-3911-1-git-send-email-yorksun@freescale.com you wrote:
When DDR controller interleaving is eabled and less than all bank (chip-select) interleaving is seletected, the unused chip-select should be disabled.
...
@@ -1250,16 +1251,20 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts, * 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.
* each controller.When */
Seems you wanted to add some comment here, but then forgot it?
@@ -1379,8 +1384,10 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts, );
debug("FSLDDR: cs[%d]_bnds = 0x%08x\n", i, ddr->cs[i].bnds);
set_csn_config(dimm_number, i, ddr, popts, dimm_params);
set_csn_config_2(i, ddr);
if (cs_en) {
set_csn_config(dimm_number, i, ddr, popts, dimm_params);
set_csn_config_2(i, ddr);
}
Would it not be helpful to inform the user that we disabled a memory bank? Eventually this wa snot intentional...
Best regards,
Wolfgang Denk

On Wed, 2010-10-13 at 18:24 +0200, Wolfgang Denk wrote:
Dear York Sun,
In message 1286983650-3911-1-git-send-email-yorksun@freescale.com you wrote:
When DDR controller interleaving is eabled and less than all bank (chip-select) interleaving is seletected, the unused chip-select should be disabled.
...
@@ -1250,16 +1251,20 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts, * 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.
* each controller.When */
Seems you wanted to add some comment here, but then forgot it?
Oop. Nice catch. I didn't know how I missed this.
@@ -1379,8 +1384,10 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts, );
debug("FSLDDR: cs[%d]_bnds = 0x%08x\n", i, ddr->cs[i].bnds);
set_csn_config(dimm_number, i, ddr, popts, dimm_params);
set_csn_config_2(i, ddr);
if (cs_en) {
set_csn_config(dimm_number, i, ddr, popts, dimm_params);
set_csn_config_2(i, ddr);
}
Would it not be helpful to inform the user that we disabled a memory bank? Eventually this wa snot intentional...
I can add a message but maybe not necessary. The only case the CS left disable is the incomplete interleaving. For example the controller interleaving is enabled, but the bank interleaving is disabled or less than all populated CS are interleaving. In these cases, the unused CSs (or maybe CS'es, my bad English) are not accessible, even enabled. This patch doesn't change the hardware behavior, but to disable those unused CS to avoid confusion.
York

Dear York Sun,
In message 1286988197.5737.6.camel@oslab-l1 you wrote:
Would it not be helpful to inform the user that we disabled a memory bank? Eventually this wa snot intentional...
I can add a message but maybe not necessary. The only case the CS left disable is the incomplete interleaving. For example the controller interleaving is enabled, but the bank interleaving is disabled or less than all populated CS are interleaving. In these cases, the unused CSs (or maybe CS'es, my bad English) are not accessible, even enabled. This patch doesn't change the hardware behavior, but to disable those unused CS to avoid confusion.
I understand this. But I think the conditions that cause such behaviour are pretty unusual, and most probably not something that is actually intended by the end user - for example, he might have installed an incompatible DIMM or such. It would be nice if he was made aware that somthing unusual is going on, so he can double-check his configuration.
Best regards,
Wolfgang Denk

On Wed, 2010-10-13 at 19:41 +0200, Wolfgang Denk wrote:
Dear York Sun,
In message 1286988197.5737.6.camel@oslab-l1 you wrote:
Would it not be helpful to inform the user that we disabled a memory bank? Eventually this wa snot intentional...
I can add a message but maybe not necessary. The only case the CS left disable is the incomplete interleaving. For example the controller interleaving is enabled, but the bank interleaving is disabled or less than all populated CS are interleaving. In these cases, the unused CSs (or maybe CS'es, my bad English) are not accessible, even enabled. This patch doesn't change the hardware behavior, but to disable those unused CS to avoid confusion.
I understand this. But I think the conditions that cause such behaviour are pretty unusual, and most probably not something that is actually intended by the end user - for example, he might have installed an incompatible DIMM or such. It would be nice if he was made aware that somthing unusual is going on, so he can double-check his configuration.
Points taken. I will add a message for this case.
Thanks.
York
participants (2)
-
Wolfgang Denk
-
York Sun