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

login
register
mail settings
Submitter Fabio Estevam
Date May 2, 2013, 10:44 p.m.
Message ID <1367534661-13502-3-git-send-email-festevam@gmail.com>
Download mbox | patch
Permalink /patch/241115/
State Superseded
Headers show

Comments

Fabio Estevam - May 2, 2013, 10:44 p.m.
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(-)
Marek Vasut - May 3, 2013, 2:24 a.m.
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
Fabio Estevam - May 3, 2013, 2:35 a.m.
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.
Marek Vasut - May 3, 2013, 2:37 a.m.
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
Fabio Estevam - May 3, 2013, 2:51 a.m.
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.
Marek Vasut - May 3, 2013, 3:22 a.m.
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

Patch

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 */