[U-Boot] [PATCH v3 0/9] mx23: Make DDR initialization stable

[U-Boot] [PATCH v3 0/9] mx23: Make DDR initialization stable

[Fabio Estevam]

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.

[U-Boot] [PATCH v3 1/7] mx23: Fix pad voltage selection bit

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

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 v2:
- Only place PAD_3V3 inside the if/else block
Changes since v1:
- Newly introduced

 arch/arm/include/asm/arch-mxs/iomux.h |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/include/asm/arch-mxs/iomux.h b/arch/arm/include/asm/arch-mxs/iomux.h
index 4288715..f46895e 100644
--- a/arch/arm/include/asm/arch-mxs/iomux.h
+++ b/arch/arm/include/asm/arch-mxs/iomux.h
@@ -71,7 +71,11 @@ typedef u32 iomux_cfg_t;
 #define PAD_16MA		3
 
 #define PAD_1V8			0
+#if defined CONFIG_MX28
 #define PAD_3V3			1
+#else
+#define PAD_3V3			0
+#endif
 
 #define PAD_NOPULL		0
 #define PAD_PULLUP		1
-- 
1.7.9.5

[U-Boot] [PATCH v3 2/7] mx23evk: Fix DDR pin iomux settings

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

Change MUX_CONFIG_EMI to use the same drive strength as the bootlets code from
Freescale, which results in much better stability.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v2:
- Only change the drive strength
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_PULLUP)
 
 const iomux_cfg_t iomux_setup[] = {
 	/* DUART */
-- 
1.7.9.5

[U-Boot] [PATCH v3 3/7] mx23_olinuxino: Fix DDR pin iomux settings

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

Change MUX_CONFIG_EMI to use the same drive strength as the bootlets code from
Freescale, which results in much better stability.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v2:
- Only change the drive strength
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_PULLUP)
 #define	MUX_CONFIG_SSP	(MXS_PAD_3V3 | MXS_PAD_8MA | MXS_PAD_PULLUP)
 
 const iomux_cfg_t iomux_setup[] = {
-- 
1.7.9.5

[U-Boot] [PATCH v3 4/7] mxs: spl_mem_init: Fix comment about start bit

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

Start bit is part of HW_DRAM_CTL8 register, so  fix the comment.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v2:
- None
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);
-- 
1.7.9.5

[U-Boot] [PATCH v3 5/7] mxs: spl_mem_init: Remove erroneous DDR setting

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

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 v2:
- None
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
 
-- 
1.7.9.5

[U-Boot] [PATCH v3 6/7] mxs: spl_mem_init: Skip the initialization of some DRAM_CTL registers

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

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 v2:
- None
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)
 {
-- 
1.7.9.5

[U-Boot] [PATCH v3 7/7] mxs: spl_mem_init: Change EMI port priority

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

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 v2:
- None
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);
-- 
1.7.9.5

[U-Boot] [PATCH v3 0/9] mx23: Make DDR initialization stable

[Marek Vasut]

Dear Fabio Estevam,

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

PATCH v3 0/9 ? ;-)

Best regards,
Marek Vasut

[U-Boot] [PATCH v3 1/7] mx23: Fix pad voltage selection bit

[Marek Vasut]

Dear Fabio Estevam,

> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> 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>

Acked-by: Marek Vasut <marex@denx.de>

but then, should we not just undefine this PAD_3V3 for MX23 altogether so noone 
is tempted to use it?

> ---
> Changes since v2:
> - Only place PAD_3V3 inside the if/else block
> Changes since v1:
> - Newly introduced
> 
>  arch/arm/include/asm/arch-mxs/iomux.h |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-mxs/iomux.h
> b/arch/arm/include/asm/arch-mxs/iomux.h index 4288715..f46895e 100644
> --- a/arch/arm/include/asm/arch-mxs/iomux.h
> +++ b/arch/arm/include/asm/arch-mxs/iomux.h
> @@ -71,7 +71,11 @@ typedef u32 iomux_cfg_t;
>  #define PAD_16MA		3
> 
>  #define PAD_1V8			0
> +#if defined CONFIG_MX28
>  #define PAD_3V3			1
> +#else
> +#define PAD_3V3			0
> +#endif
> 
>  #define PAD_NOPULL		0
>  #define PAD_PULLUP		1

Best regards,
Marek Vasut

[U-Boot] [PATCH v3 2/7] mx23evk: Fix DDR pin iomux settings

[Marek Vasut]

Dear Fabio Estevam,

> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Change MUX_CONFIG_EMI to use the same drive strength as the bootlets code
> from Freescale, which results in much better stability.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Changes since v2:
> - Only change the drive strength
> 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_PULLUP)

btw. if 3V3 setting doesn't exist, why do we have it here? We should just can it 
then.

Best regards,
Marek Vasut

[U-Boot] [PATCH v3 5/7] mxs: spl_mem_init: Remove erroneous DDR setting

[Marek Vasut]

Dear Fabio Estevam,

> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> 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 v2:
> - None
> 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)))
> -		;

Is there no such similar bit indicating the init completed?

>  }
>  #endif

Best regards,
Marek Vasut

[U-Boot] [PATCH v3 6/7] mxs: spl_mem_init: Skip the initialization of some DRAM_CTL registers

[Marek Vasut]

Dear Fabio Estevam,

> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> 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 v2:
> - None
> 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))

if (foo || bar || baz)
 continue;
writel();

This will be much more readable.

> +			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)
>  {

Best regards,
Marek Vasut

[U-Boot] [PATCH v3 1/7] mx23: Fix pad voltage selection bit

[Otavio Salvador]

On Thu, May 2, 2013 at 11:24 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Fabio Estevam,
>
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> 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>
>
> Acked-by: Marek Vasut <marex@denx.de>
>
> but then, should we not just undefine this PAD_3V3 for MX23 altogether so noone
> is tempted to use it?

I think for code readability it'd be good to have it in place. It
makes it more obvious for people looking at code.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

[U-Boot] [PATCH v3 6/7] mxs: spl_mem_init: Skip the initialization of some DRAM_CTL registers

[Otavio Salvador]

On Thu, May 2, 2013 at 11:26 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Fabio Estevam,
>
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> 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 v2:
>> - None
>> 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))
>
> if (foo || bar || baz)
>  continue;
> writel();
>
> This will be much more readable.

I agree here :)

>> +                     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)
>>  {
>
> Best regards,
> Marek Vasut



--
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

[U-Boot] [PATCH v3 1/7] mx23: Fix pad voltage selection bit

[Marek Vasut]

Dear Otavio Salvador,

> On Thu, May 2, 2013 at 11:24 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Fabio Estevam,
> > 
> >> From: Fabio Estevam <fabio.estevam@freescale.com>
> >> 
> >> 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>
> > 
> > Acked-by: Marek Vasut <marex@denx.de>
> > 
> > but then, should we not just undefine this PAD_3V3 for MX23 altogether so
> > noone is tempted to use it?
> 
> I think for code readability it'd be good to have it in place. It
> makes it more obvious for people looking at code.

I can already see the people wondering why that piece has no effect ...

Best regards,
Marek Vasut

[U-Boot] [PATCH v3 1/7] mx23: Fix pad voltage selection bit

[Fabio Estevam]

On Thu, May 2, 2013 at 11:24 PM, Marek Vasut <marex@denx.de> wrote:

> Acked-by: Marek Vasut <marex@denx.de>
>
> but then, should we not just undefine this PAD_3V3 for MX23 altogether so noone
> is tempted to use it?

No, the EMI pins need to pass MXS_PAD_3V3 to tell the iomux driver
that the voltage select value is 0.

If we don't do that then it will remain on its reset value, which is
1, and marked as invalid in the reference manual.

[U-Boot] [PATCH v3 2/7] mx23evk: Fix DDR pin iomux settings

[Fabio Estevam]

On Thu, May 2, 2013 at 11:24 PM, Marek Vasut <marex@denx.de> wrote:

>>  #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_PULLUP)
>
> btw. if 3V3 setting doesn't exist, why do we have it here? We should just can it
> then.

It exists and it is 0.

We need to have MXS_PAD_3V3 here, so that the pin voltage bit can be cleared.

[U-Boot] [PATCH v3 2/7] mx23evk: Fix DDR pin iomux settings

[Marek Vasut]

Dear Fabio Estevam,

> On Thu, May 2, 2013 at 11:24 PM, Marek Vasut <marex@denx.de> wrote:
> >>  #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_PULLUP)
> > 
> > btw. if 3V3 setting doesn't exist, why do we have it here? We should just
> > can it then.
> 
> It exists and it is 0.
> 
> We need to have MXS_PAD_3V3 here, so that the pin voltage bit can be
> cleared.

Uh, why is it even set ? Does the bootrom screw up with it? But then it's cool, 
just fix those few remaining ramblings of mine and stick my Acks on it.

Best regards,
Marek Vasut

[U-Boot] [PATCH v3 5/7] mxs: spl_mem_init: Remove erroneous DDR setting

[Fabio Estevam]

On Thu, May 2, 2013 at 11:25 PM, Marek Vasut <marex@denx.de> wrote:

> Is there no such similar bit indicating the init completed?

Not that I am aware of.

[U-Boot] [PATCH v3 5/7] mxs: spl_mem_init: Remove erroneous DDR setting

[Marek Vasut]

Dear Fabio Estevam,

> On Thu, May 2, 2013 at 11:25 PM, Marek Vasut <marex@denx.de> wrote:
> > Is there no such similar bit indicating the init completed?
> 
> Not that I am aware of.

That's a tad stupid, but I'll trust you on that.

Best regards,
Marek Vasut

[U-Boot] [PATCH v3 2/7] mx23evk: Fix DDR pin iomux settings

[Fabio Estevam]

On Thu, May 2, 2013 at 11:37 PM, Marek Vasut <marex@denx.de> wrote:

> Uh, why is it even set ? Does the bootrom screw up with it? But then it's cool,
> just fix those few remaining ramblings of mine and stick my Acks on it.

Ok, let's go step by step.

mx23 reference manual says that EMI pins voltage bits can be:
0 - for normal operation
1  - invalid.

After reset this bit is 1 and the iomux driver needs to clear this bit.

Let's look at the defines:

#define MXS_PAD_1V8	((PAD_1V8 << MXS_PAD_VOL_SHIFT) | \
					MXS_PAD_VOL_VALID_MASK)
#define MXS_PAD_3V3	((PAD_3V3 << MXS_PAD_VOL_SHIFT) | \
					MXS_PAD_VOL_VALID_MASK)

and then in the iomux;c driver:

	if (PAD_VOL_VALID(pad)) {
		bp = PAD_PIN(pad) % 8 * 4 + 2;
		mxs_reg = (struct mxs_register_32 *)(iomux_base + ofs);
		if (PAD_VOL(pad))
			writel(1 << bp, &mxs_reg->reg_set);
		else
			writel(1 << bp, &mxs_reg->reg_clr);
	}

So the only way that the iomux driver can clear the voltage bit is if
PAD_VOL_VALID(pad) is true, and the only way that PAD_VOL_VALID(pad)
can be true is if either MXS_PAD_3V3 or MXS_PAD_1V8 are defined, since
they have the MXS_PAD_VOL_VALID_MASK bit in their definitions.

If it is still not clear, just let me know.

[U-Boot] [PATCH v3 2/7] mx23evk: Fix DDR pin iomux settings

[Marek Vasut]

Dear Fabio Estevam,

> On Thu, May 2, 2013 at 11:37 PM, Marek Vasut <marex@denx.de> wrote:
> > Uh, why is it even set ? Does the bootrom screw up with it? But then it's
> > cool, just fix those few remaining ramblings of mine and stick my Acks
> > on it.
> 
> Ok, let's go step by step.
> 
> mx23 reference manual says that EMI pins voltage bits can be:
> 0 - for normal operation
> 1  - invalid.
> 
> After reset this bit is 1 and the iomux driver needs to clear this bit.
> 
> Let's look at the defines:
> 
> #define MXS_PAD_1V8	((PAD_1V8 << MXS_PAD_VOL_SHIFT) | \
> 					MXS_PAD_VOL_VALID_MASK)
> #define MXS_PAD_3V3	((PAD_3V3 << MXS_PAD_VOL_SHIFT) | \
> 					MXS_PAD_VOL_VALID_MASK)
> 
> and then in the iomux;c driver:
> 
> 	if (PAD_VOL_VALID(pad)) {
> 		bp = PAD_PIN(pad) % 8 * 4 + 2;
> 		mxs_reg = (struct mxs_register_32 *)(iomux_base + ofs);
> 		if (PAD_VOL(pad))
> 			writel(1 << bp, &mxs_reg->reg_set);
> 		else
> 			writel(1 << bp, &mxs_reg->reg_clr);
> 	}
> 
> So the only way that the iomux driver can clear the voltage bit is if
> PAD_VOL_VALID(pad) is true, and the only way that PAD_VOL_VALID(pad)
> can be true is if either MXS_PAD_3V3 or MXS_PAD_1V8 are defined, since
> they have the MXS_PAD_VOL_VALID_MASK bit in their definitions.

Yuck. But then, why not configure the pins as 1V8 ? But either way WFM.

Best regards,
Marek Vasut