[U-Boot] [PATCH v2 0/9] mx23: Make DDR initialization stable
Prior to this series running 'memtester' utility in Linux on a mx23evk always resulted in many errors during stress testing, if the kernel is loaded via U-boot.
Running the same test and loading the kernel via FSL bootlets resulted on zero errors.
Adjust U-boot so that it can also pass the 'memtester' stress test.
After this series was applied, no more DDR errors were observed on a mx23evk.
On mx23 the pad voltage selection bit needs to be always '0', since '1' is a reserved value.
For example:
Pin 108, EMI_A06 pin voltage selection: 0= 1.8V (mDDR) or 2.5V (DDR1); 1= reserved.
Fix the pad voltage definitions for the mx23 case.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - Newly introduced
arch/arm/include/asm/arch-mxs/iomux.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/arch/arm/include/asm/arch-mxs/iomux.h b/arch/arm/include/asm/arch-mxs/iomux.h index 4288715..66dcd36 100644 --- a/arch/arm/include/asm/arch-mxs/iomux.h +++ b/arch/arm/include/asm/arch-mxs/iomux.h @@ -70,8 +70,13 @@ typedef u32 iomux_cfg_t; #define PAD_12MA 2 #define PAD_16MA 3
+#if defined CONFIG_MX28 #define PAD_1V8 0 #define PAD_3V3 1 +#else +#define PAD_1V8 0 +#define PAD_3V3 0 +#endif
#define PAD_NOPULL 0 #define PAD_PULLUP 1
On mx23 and mx28 the pullup bits are defined as:
- 0: Enable the internal pad keeper - 1: Disable the internal pad keeper
Fix the definitions as they are currently the opposite.
Cc: Lauri Hintsala lauri.hintsala@bluegiga.com Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- This affects mx28 as well, so also adding Lauri.
I also tested on mx28evk and behavior is fine after this change.
Changes since v1: - Newly introduced
arch/arm/include/asm/arch-mxs/iomux.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/arch-mxs/iomux.h b/arch/arm/include/asm/arch-mxs/iomux.h index 66dcd36..70300d5 100644 --- a/arch/arm/include/asm/arch-mxs/iomux.h +++ b/arch/arm/include/asm/arch-mxs/iomux.h @@ -78,8 +78,8 @@ typedef u32 iomux_cfg_t; #define PAD_3V3 0 #endif
-#define PAD_NOPULL 0 -#define PAD_PULLUP 1 +#define PAD_PULLUP 0 +#define PAD_NOPULL 1
#define MXS_PAD_4MA ((PAD_4MA << MXS_PAD_MA_SHIFT) | \ MXS_PAD_MA_VALID_MASK)
Hi Fabio,
On mx23 and mx28 the pullup bits are defined as:
- 0: Enable the internal pad keeper
- 1: Disable the internal pad keeper
Fix the definitions as they are currently the opposite.
Sorry, but I think this is not correct. Please have a look at the reference manual for i.MX28 page 786: It seems that some pins have an internal pullup, but other pins "only" have the internal keeper you mentioned.
Quote from : bank 0 pin 24: "Set this bit to one to _enable_ the internal pullup..." bank 0 pin 25: "Set this bit to one to _disable_ the internal keeper..."
So I think the current implementation is correct.
However, if you are trying to enable a pullup for a pin which does not have this function (e.g. because the hardware guys trimmed the BOM and did not spend an external pullup) you are actually disabling the keeper which makes the situation even worse in some situations.
I'm still wondering what an ideal solution could be...
Best regards, Michael
Hi Michael,
On Thu, May 2, 2013 at 3:44 PM, Michael Heimpold mhei@heimpold.de wrote:
Sorry, but I think this is not correct. Please have a look at the reference manual for i.MX28 page 786: It seems that some pins have an internal pullup, but other pins "only" have the internal keeper you mentioned.
Quote from : bank 0 pin 24: "Set this bit to one to _enable_ the internal pullup..." bank 0 pin 25: "Set this bit to one to _disable_ the internal keeper..."
So I think the current implementation is correct.
Ok, I see.
mx23 does not have the "Set this bit to one to _enable_ the internal pullup" option.
However, if you are trying to enable a pullup for a pin which does not have this function (e.g. because the hardware guys trimmed the BOM and did not spend an external pullup) you are actually disabling the keeper which makes the situation even worse in some situations.
I'm still wondering what an ideal solution could be...
So I plan to keep mx28 bits untouched and do the reverse definition only for mx23:
#if defined CONFIG_MX28 #define PAD_PULLUP 1 #define PAD_NOPULL 0 #else #define PAD_PULLUP 0 #define PAD_NOPULL 1 #endif
Dear Fabio Estevam,
Hi Michael,
On Thu, May 2, 2013 at 3:44 PM, Michael Heimpold mhei@heimpold.de wrote:
Sorry, but I think this is not correct. Please have a look at the reference manual for i.MX28 page 786: It seems that some pins have an internal pullup, but other pins "only" have the internal keeper you mentioned.
Quote from : bank 0 pin 24: "Set this bit to one to _enable_ the internal pullup..." bank 0 pin 25: "Set this bit to one to _disable_ the internal keeper..."
So I think the current implementation is correct.
Ok, I see.
mx23 does not have the "Set this bit to one to _enable_ the internal pullup" option.
However, if you are trying to enable a pullup for a pin which does not have this function (e.g. because the hardware guys trimmed the BOM and did not spend an external pullup) you are actually disabling the keeper which makes the situation even worse in some situations.
I'm still wondering what an ideal solution could be...
So I plan to keep mx28 bits untouched and do the reverse definition only for mx23:
#if defined CONFIG_MX28 #define PAD_PULLUP 1 #define PAD_NOPULL 0 #else #define PAD_PULLUP 0 #define PAD_NOPULL 1 #endif
Wait, this code is pulled from Linux kernel. How does Linux solve that? Is it also broken in Linux?
Best regards, Marek Vasut
Hi Fabio,
mx23 does not have the "Set this bit to one to _enable_ the internal pullup" option.
Hm, in the RM for i.MX233, page 1480 and above (http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf) I can only find definitions like "Set this bit to one to _disable_ the internal pad keeper and _enable_ the internal pull up resistor..." or for EMI pads at bank 3 "Set this bit to one to _disable_ the internal keeper" (without a note about pull ups).
Could you please link me to your reference where the reverse definition comes from?
Best regards, Michael
Hi Michael,
On Thu, May 2, 2013 at 6:45 PM, Michael Heimpold mhei@heimpold.de wrote:
Could you please link me to your reference where the reverse definition comes from?
Sorry for the confusion, current code is correct for both mx23 and mx28.
I discarded this patch and will send v3 shortly.
Thanks,
Fabio Estevam
Change MUX_CONFIG_EMI to use the same voltage selection/drive strength and pullup settings as the bootlets code from Freescale, which results in much better stability.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - Only adjust MUX_CONFIG_EMI
board/freescale/mx23evk/spl_boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mx23evk/spl_boot.c b/board/freescale/mx23evk/spl_boot.c index b6f4e7e..fd6b3d9 100644 --- a/board/freescale/mx23evk/spl_boot.c +++ b/board/freescale/mx23evk/spl_boot.c @@ -26,7 +26,7 @@ #include <asm/arch/sys_proto.h>
#define MUX_CONFIG_SSP1 (MXS_PAD_3V3 | MXS_PAD_8MA | MXS_PAD_PULLUP) -#define MUX_CONFIG_EMI (MXS_PAD_3V3 | MXS_PAD_16MA | MXS_PAD_PULLUP) +#define MUX_CONFIG_EMI (MXS_PAD_3V3 | MXS_PAD_12MA | MXS_PAD_NOPULL)
const iomux_cfg_t iomux_setup[] = { /* DUART */
Change MUX_CONFIG_EMI to use the same voltage selection/drive strength and pullup settings as the bootlets code from Freescale, which results in much better stability.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - Only adjust MUX_CONFIG_EMI
board/olimex/mx23_olinuxino/spl_boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/olimex/mx23_olinuxino/spl_boot.c b/board/olimex/mx23_olinuxino/spl_boot.c index a96c293..e55947f 100644 --- a/board/olimex/mx23_olinuxino/spl_boot.c +++ b/board/olimex/mx23_olinuxino/spl_boot.c @@ -29,7 +29,7 @@ #include <asm/arch/imx-regs.h> #include <asm/arch/sys_proto.h>
-#define MUX_CONFIG_EMI (MXS_PAD_3V3 | MXS_PAD_16MA | MXS_PAD_PULLUP) +#define MUX_CONFIG_EMI (MXS_PAD_3V3 | MXS_PAD_12MA | MXS_PAD_NOPULL) #define MUX_CONFIG_SSP (MXS_PAD_3V3 | MXS_PAD_8MA | MXS_PAD_PULLUP)
const iomux_cfg_t iomux_setup[] = {
Start bit is part of HW_DRAM_CTL8 register, so fix the comment.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - Newly introduced as the previous patch is now splitted.
arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 4950490..300da0a 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -267,7 +267,7 @@ static void mx23_mem_init(void)
initialize_dram_values();
- /* Set START bit in DRAM_CTL16 */ + /* Set START bit in DRAM_CTL8 */ setbits_le32(MXS_DRAM_BASE + 0x20, 1 << 16);
clrbits_le32(MXS_DRAM_BASE + 0x40, 1 << 17);
On mx23 there is no 'DRAM init complete' in register HW_DRAM_CTL18.
Remove this erroneous setting.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - Newly introduced as the previous patch is now splitted.
arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 300da0a..df25535 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -279,10 +279,6 @@ static void mx23_mem_init(void)
setbits_le32(MXS_DRAM_BASE + 0x40, 1 << 19); setbits_le32(MXS_DRAM_BASE + 0x40, 1 << 11); - - /* Wait for bit 10 (DRAM init complete) in DRAM_CTL18 */ - while (!(readl(MXS_DRAM_BASE + 0x48) & (1 << 10))) - ; } #endif
HW_DRAM_CTL27, HW_DRAM_CTL28 and HW_DRAM_CTL35 are not initialized as per FSL bootlets code.
mx23 Reference Manual mark HW_DRAM_CTL27 and HW_DRAM_CTL28 as "reserved".
HW_DRAM_CTL8 is setup as the last element.
So skip the initialization of these DRAM_CTL registers. Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - To avoid polluting the mx28 case, separate the function definition in mx23 and for mx28.
arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index df25535..bf58058 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -110,6 +110,7 @@ __weak void mxs_adjust_memory_params(uint32_t *dram_vals) { }
+#ifdef CONFIG_MX28 static void initialize_dram_values(void) { int i; @@ -118,15 +119,26 @@ static void initialize_dram_values(void)
for (i = 0; i < ARRAY_SIZE(dram_vals); i++) writel(dram_vals[i], MXS_DRAM_BASE + (4 * i)); +} +#else +static void initialize_dram_values(void) +{ + int i; + + mxs_adjust_memory_params(dram_vals); + + for (i = 0; i < ARRAY_SIZE(dram_vals); i++) { + if (!(i == 8 || i == 27 || i == 28 || i == 35)) + writel(dram_vals[i], MXS_DRAM_BASE + (4 * i)); + }
-#ifdef CONFIG_MX23 /* * Enable tRAS lockout in HW_DRAM_CTL08 ; it must be the last * element to be set */ writel((1 << 24), MXS_DRAM_BASE + (4 * 8)); -#endif } +#endif
static void mxs_mem_init_clock(void) {
FSL bootlets code set the PORT_PRIORITY_ORDER field of register HW_EMI_CTRL as 0x2, which means:
PORT0231 = 0x02 Priority Order: AXI0, AHB2, AHB3, AHB1
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - None
arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index bf58058..5d881da 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c @@ -286,7 +286,7 @@ static void mx23_mem_init(void) early_delay(20000);
/* Adjust EMI port priority. */ - clrsetbits_le32(0x80020000, 0x1f << 16, 0x8); + clrsetbits_le32(0x80020000, 0x1f << 16, 0x2); early_delay(20000);
setbits_le32(MXS_DRAM_BASE + 0x40, 1 << 19);
participants (4)
-
Fabio Estevam -
Fabio Estevam -
Marek Vasut -
Michael Heimpold