[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Benoît Thébaudeau]

FSL 2.6.35 kernel assumes that the bootloader passes the CONFIG_REVISION_TAG
information.

If this data is not present, the kernel misconfigures the TZIC, which results in
the timer interrupt handler never being called, so the kernel deadlocks while
calibrating its delay.

Suggested-by: Greg Topmiller <Greg.Topmiller@jdsu.com>
Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
---
 .../board/freescale/mx51evk/mx51evk.c              |    5 +++++
 .../include/configs/mx51evk.h                      |    1 +
 2 files changed, 6 insertions(+)

diff --git u-boot-imx-1d9b033.orig/board/freescale/mx51evk/mx51evk.c u-boot-imx-1d9b033/board/freescale/mx51evk/mx51evk.c
index 7a0682a..2e877c1 100644
--- u-boot-imx-1d9b033.orig/board/freescale/mx51evk/mx51evk.c
+++ u-boot-imx-1d9b033/board/freescale/mx51evk/mx51evk.c
@@ -60,6 +60,11 @@ int dram_init(void)
 	return 0;
 }
 
+u32 get_board_rev(void)
+{
+	return get_cpu_rev();
+}
+
 static void setup_iomux_uart(void)
 {
 	unsigned int pad = PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE |
diff --git u-boot-imx-1d9b033.orig/include/configs/mx51evk.h u-boot-imx-1d9b033/include/configs/mx51evk.h
index ba4a4a6..7b027b4 100644
--- u-boot-imx-1d9b033.orig/include/configs/mx51evk.h
+++ u-boot-imx-1d9b033/include/configs/mx51evk.h
@@ -44,6 +44,7 @@
 #define CONFIG_CMDLINE_TAG			/* enable passing of ATAGs */
 #define CONFIG_SETUP_MEMORY_TAGS
 #define CONFIG_INITRD_TAG
+#define CONFIG_REVISION_TAG
 
 #define CONFIG_OF_LIBFDT
 

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Fabio Estevam]

Hi Beno?t ,

On Mon, Sep 17, 2012 at 4:04 PM, Beno?t Th?baudeau
<benoit.thebaudeau@advansee.com> wrote:

> +u32 get_board_rev(void)
> +{
> +       return get_cpu_rev();
> +}

Is this enough?  Looking at FSL U-boot there is also a:

static inline void set_board_rev(void)
{
	if ((__REG(GPIO1_BASE_ADDR + 0x0) & (0x1 << 22)) == 0)
		system_rev |= BOARD_REV_2_0 << BOARD_VER_OFFSET;

}

On mx53loco I had very weird problems when kernel was running and it
took me sometime to find out that it was due to wrong detection of the
board revision, so I suggest to double check this to avoid this kind
of problem on mx51evk too.

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Fabio Estevam]

Hi Greg,

On Tue, Sep 18, 2012 at 10:00 AM, Greg Topmiller
<Greg.Topmiller@jdsu.com> wrote:
> Fabio/Benoit,
>
> I tried the get_board_rev alone and it worked on my mx51evk.
>
> In the FSL u-boot version I am using the set_board_rev is:
> static inline void set_board_rev(int rev)
> {
>         system_rev |= (rev & 0xF) << 8;
> }

So you should call it as "set_board_rev(BOARD_REV2)" , where:
#define BOARD_REV2 0x1

I don't think we have boards older than rev2 anymore.

Looking at the FSL kernel, there are are some tests for "if
(board_is_rev(BOARD_REV_2))"
and they should return true from what I can see from the schematics.

 841                 if (board_is_rev(BOARD_REV_2))
 842                         /* BB2.5 */
 843                         ret = gpio_get_value(BABBAGE_SD2_CD_2_5);
 844                 else
 845                         /* BB2.0 */
 846                         ret = gpio_get_value(BABBAGE_SD2_CD_2_0);
 847                 return ret;

With the original patch from Benoit "board_is_rev(BOARD_REV_2)" would
return falsem which would then assign the wrong SD2 card detect pin,
for example.

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Benoît Thébaudeau]

Hi Greg, Fabio,

On Tuesday, September 18, 2012 3:00:58 PM, Greg Topmiller wrote:
> Fabio/Benoit,
> 
> I tried the get_board_rev alone and it worked on my mx51evk.
> 
> In the FSL u-boot version I am using the set_board_rev is:
> static inline void set_board_rev(int rev)
> {
> 	system_rev |= (rev & 0xF) << 8;
> }
> 
> It is not called for the mx51evk but is used for mx35_3stack.
> 
> The set_board_rev code below doesn't make sense when looking at the
> FSL u-boot code or the main line and comparing to the mx51evk
> schematic.  I don't see where either the FSL u-boot or the main line
> u-boot set up iomux for GPIO0_22.  There are four pin possibilities,
> EIM_EB2, NANDF_CS6, CSPI1_MOSI, and UART3_RXD.  On the schematic the
> UART3_RXD is tied low so that would be the only pin to make sense in
> this case.  To make the "set_board_rev" code below work the mx51evk
> board specific code would have to set up the UART3_RXD to be
> GPIO0_22 by making its function ALT3.  That being said the code
> below would not be a problem since the if logic would most likely be
> positive in any case and the system_rev would be or'd with board rev
> 2.
> 
> Let me know if you want the schematic I'm looking at.  It's a PDF.
> 
> Best Regards,
> 
> Greg
> 
> 
> 
> -----Original Message-----
> From: Fabio Estevam [mailto:festevam at gmail.com]
> Sent: Monday, September 17, 2012 11:25 PM
> To: Beno?t Th?baudeau
> Cc: U-Boot-Users ML; Greg Topmiller; Stefano Babic
> Subject: Re: [PATCH] mx51evk: Add CONFIG_REVISION_TAG
> 
> Hi Beno?t ,
> 
> On Mon, Sep 17, 2012 at 4:04 PM, Beno?t Th?baudeau
> <benoit.thebaudeau@advansee.com> wrote:
> 
> > +u32 get_board_rev(void)
> > +{
> > +       return get_cpu_rev();
> > +}
> 
> Is this enough?  Looking at FSL U-boot there is also a:
> 
> static inline void set_board_rev(void)
> {
> 	if ((__REG(GPIO1_BASE_ADDR + 0x0) & (0x1 << 22)) == 0)
> 		system_rev |= BOARD_REV_2_0 << BOARD_VER_OFFSET;
> 
> }
> 
> On mx53loco I had very weird problems when kernel was running and it
> took me sometime to find out that it was due to wrong detection of
> the board revision, so I suggest to double check this to avoid this
> kind of problem on mx51evk too.
> 
> Regards,
> 
> Fabio Estevam

It looks like there is confusion between the 3stack and Babbage boards here:
Fabio, the code you refer to in FSL U-Boot comes from the Babbage board, while
AFAIK, the mx51evk mainline U-Boot board corresponds to the 3stack board (this
is also FSL's naming for this board on their website).

Best regards,
Beno?t

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Fabio Estevam]

On Tue, Sep 18, 2012 at 10:41 AM, Beno?t Th?baudeau
<benoit.thebaudeau@advansee.com> wrote:

> It looks like there is confusion between the 3stack and Babbage boards here:
> Fabio, the code you refer to in FSL U-Boot comes from the Babbage board, while
> AFAIK, the mx51evk mainline U-Boot board corresponds to the 3stack board (this
> is also FSL's naming for this board on their website).

Not really,

mx51evk = mx51babbage

mx51_3stack is a different board.

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Benoît Thébaudeau]

On Tuesday, September 18, 2012 3:38:02 PM, Fabio Estevam wrote:
> On Tue, Sep 18, 2012 at 10:41 AM, Beno?t Th?baudeau
> <benoit.thebaudeau@advansee.com> wrote:
> 
> > It looks like there is confusion between the 3stack and Babbage
> > boards here:
> > Fabio, the code you refer to in FSL U-Boot comes from the Babbage
> > board, while
> > AFAIK, the mx51evk mainline U-Boot board corresponds to the 3stack
> > board (this
> > is also FSL's naming for this board on their website).
> 
> Not really,
> 
> mx51evk = mx51babbage
> 
> mx51_3stack is a different board.

Now, I'm totally confused. FSL's website shows a board that really looks like a
"3-stack" board for its i.MX51 EVK:
http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=MCIMX51EVKJ&parentCode=i.MX515&fpsp=1

Babbage here, different from i.MX51 EVK:
http://ossguy.com/?p=317

So, do you mean that mainline U-Boot board naming is broken for mx51evk?

Best regards,
Beno?t

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Fabio Estevam]

Hi Beno?t,

On Tue, Sep 18, 2012 at 10:54 AM, Beno?t Th?baudeau
<benoit.thebaudeau@advansee.com> wrote:

> Now, I'm totally confused. FSL's website shows a board that really looks like a
> "3-stack" board for its i.MX51 EVK:
> http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=MCIMX51EVKJ&parentCode=i.MX515&fpsp=1

Yes, the board names are confusing indeed ;-)

Let me clarify:  This is the official mx51 development board that
Freescale makes available. The website calls it mx51evk, which is the
name that we use in mainline U-boot.

Yes, you can attach a separate LCD and a daughter card to this mx51evk.

When the FSL development team started to work on kernel and bootloader
for this board, they used the internal name for it: mx51 babbage.

The first mx51 board that was designed was called mx51 3stack and it
is more of an internal board to the FSL development teams.

Looking at FSL u-boot source code, you will see mx51_bbg and
mx51_3stack targets.

>
> Babbage here, different from i.MX51 EVK:
> http://ossguy.com/?p=317

This is a mx51evk indeed, but without a WVGA LCD and the daughter card
and put in a plastic shell.

> So, do you mean that mainline U-Boot board naming is broken for mx51evk?

No, the name in U-boot mainline is fine and it matches the name in the website.

Hopefully I managed to clarify the board name confusion.

But back to the original patch: all you need to do is to pass
set_board_rev(BOARD_REV2), so that the kernel can correctly identify
the board revision correctly.

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Fabio Estevam]

On Tue, Sep 18, 2012 at 11:02 AM, Fabio Estevam <festevam@gmail.com> wrote:

> But back to the original patch: all you need to do is to pass
> set_board_rev(BOARD_REV2), so that the kernel can correctly identify
> the board revision correctly.

To make it clearer:

get_board_rev should return 0x51100 , where 0x51 is SoC type and the
8th bit (1 is the board revision).

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Fabio Estevam]

On Tue, Sep 18, 2012 at 11:16 AM, Greg Topmiller
<Greg.Topmiller@jdsu.com> wrote:
> So how about something like this in the board specific code?
>
> u32 get_board_rev(void)
> {
>     u32 myrev = get_cpu_rev();
>     myrev |= 0x100; /* BOARD_REV_2 */
>     return myrev;
> }

Looks good, Greg.

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Stefano Babic]

On 18/09/2012 16:02, Fabio Estevam wrote:

> 
>> So, do you mean that mainline U-Boot board naming is broken for mx51evk?
> 
> No, the name in U-boot mainline is fine and it matches the name in the website.

Right. I dislike if the name is different as the official name. As we
can see, this generates a lot of confusion. IMHO we should also align
some other boards, such as mx53loco that is officially called mx53qsb.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Stefano Babic]

On 18/09/2012 15:41, Beno?t Th?baudeau wrote:
> Hi Greg, Fabio,
> 
> On Tuesday, September 18, 2012 3:00:58 PM, Greg Topmiller wrote:
>> Fabio/Benoit,
>>
>> I tried the get_board_rev alone and it worked on my mx51evk.
>>
>> In the FSL u-boot version I am using the set_board_rev is:
>> static inline void set_board_rev(int rev)
>> {
>> 	system_rev |= (rev & 0xF) << 8;
>> }
>>
>> It is not called for the mx51evk but is used for mx35_3stack.
>>
>> The set_board_rev code below doesn't make sense when looking at the
>> FSL u-boot code or the main line and comparing to the mx51evk
>> schematic.  I don't see where either the FSL u-boot or the main line
>> u-boot set up iomux for GPIO0_22.  There are four pin possibilities,
>> EIM_EB2, NANDF_CS6, CSPI1_MOSI, and UART3_RXD.  On the schematic the
>> UART3_RXD is tied low so that would be the only pin to make sense in
>> this case.  To make the "set_board_rev" code below work the mx51evk
>> board specific code would have to set up the UART3_RXD to be
>> GPIO0_22 by making its function ALT3.  That being said the code
>> below would not be a problem since the if logic would most likely be
>> positive in any case and the system_rev would be or'd with board rev
>> 2.
>>
>> Let me know if you want the schematic I'm looking at.  It's a PDF.
>>
>> Best Regards,
>>
>> Greg
>>
>>
>>
>> -----Original Message-----
>> From: Fabio Estevam [mailto:festevam at gmail.com]
>> Sent: Monday, September 17, 2012 11:25 PM
>> To: Beno?t Th?baudeau
>> Cc: U-Boot-Users ML; Greg Topmiller; Stefano Babic
>> Subject: Re: [PATCH] mx51evk: Add CONFIG_REVISION_TAG
>>
>> Hi Beno?t ,
>>
>> On Mon, Sep 17, 2012 at 4:04 PM, Beno?t Th?baudeau
>> <benoit.thebaudeau@advansee.com> wrote:
>>
>>> +u32 get_board_rev(void)
>>> +{
>>> +       return get_cpu_rev();
>>> +}
>>
>> Is this enough?  Looking at FSL U-boot there is also a:
>>
>> static inline void set_board_rev(void)
>> {
>> 	if ((__REG(GPIO1_BASE_ADDR + 0x0) & (0x1 << 22)) == 0)
>> 		system_rev |= BOARD_REV_2_0 << BOARD_VER_OFFSET;
>>
>> }
>>
>> On mx53loco I had very weird problems when kernel was running and it
>> took me sometime to find out that it was due to wrong detection of
>> the board revision, so I suggest to double check this to avoid this
>> kind of problem on mx51evk too.
>>
>> Regards,
>>
>> Fabio Estevam
> 
> It looks like there is confusion between the 3stack and Babbage boards here:
> Fabio, the code you refer to in FSL U-Boot comes from the Babbage board, while
> AFAIK, the mx51evk mainline U-Boot board corresponds to the 3stack board (this
> is also FSL's naming for this board on their website).

There has been always confusion about Freescale's names.

Babbage was the internal name for the first mx51 development board. When
this board becomes available, it was referenced on Freescale's website
as "mx51evk". So babbage = mx51evk.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Benoît Thébaudeau]

Fabio,

On Tuesday, September 18, 2012 5:24:57 AM, Fabio Estevam wrote:
> Hi Beno?t ,
> 
> On Mon, Sep 17, 2012 at 4:04 PM, Beno?t Th?baudeau
> <benoit.thebaudeau@advansee.com> wrote:
> 
> > +u32 get_board_rev(void)
> > +{
> > +       return get_cpu_rev();
> > +}
> 
> Is this enough?  Looking at FSL U-boot there is also a:
> 
> static inline void set_board_rev(void)
> {
> 	if ((__REG(GPIO1_BASE_ADDR + 0x0) & (0x1 << 22)) == 0)
> 		system_rev |= BOARD_REV_2_0 << BOARD_VER_OFFSET;
> 
> }

I have a question regarding this code: Is it normal that it does not enable a
weak pull-up (100 k) on UART3_RXD's pad? According to the schematic history, the
pull-down R6001 (10 k) has been added to UART3_RXD for revision 2. By default,
the pad of this pin has its keeper enabled, not its pull, so everything is fine
with this code for revision 2, but what about revision 1?

To reformulate my question: Was there anything on this board before R6001
inputting a high level on UART3_RXD, or does the pad keeper guarantee some
default init level by design if the pin is externally left floating?

Best regards,
Beno?t

[U-Boot] [PATCH] mx51evk: Add CONFIG_REVISION_TAG

[Fabio Estevam]

Beno?t,

On Tue, Sep 18, 2012 at 1:01 PM, Beno?t Th?baudeau
<benoit.thebaudeau@advansee.com> wrote:

> I have a question regarding this code: Is it normal that it does not enable a
> weak pull-up (100 k) on UART3_RXD's pad? According to the schematic history, the
> pull-down R6001 (10 k) has been added to UART3_RXD for revision 2. By default,
> the pad of this pin has its keeper enabled, not its pull, so everything is fine
> with this code for revision 2, but what about revision 1?

I don't know the history about this change between rev1 versus rev2. I
would need to check with the board designer.

Regards,

Fabio Estevam

[U-Boot] [PATCH v2] mx51evk: Add CONFIG_REVISION_TAG

[Benoît Thébaudeau]

FSL 2.6.35 kernel assumes that the bootloader passes the CONFIG_REVISION_TAG
information.

If this data is not present, the kernel misconfigures the TZIC, which results in
the timer interrupt handler never being called, so the kernel deadlocks while
calibrating its delay.

Suggested-by: Greg Topmiller <Greg.Topmiller@jdsu.com>
Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
---
Changes for v2:
 - Add board revision to SoC revision from GPIO1[22] pin information.

 .../arch/arm/include/asm/arch-mx5/imx-regs.h       |    2 ++
 .../board/freescale/mx51evk/mx51evk.c              |    8 ++++++++
 .../include/configs/mx51evk.h                      |    1 +
 3 files changed, 11 insertions(+)

diff --git u-boot-imx-1d9b033.orig/arch/arm/include/asm/arch-mx5/imx-regs.h u-boot-imx-1d9b033/arch/arm/include/asm/arch-mx5/imx-regs.h
index d1ef15d..46017f4 100644
--- u-boot-imx-1d9b033.orig/arch/arm/include/asm/arch-mx5/imx-regs.h
+++ u-boot-imx-1d9b033/arch/arm/include/asm/arch-mx5/imx-regs.h
@@ -321,6 +321,8 @@
 #define BOARD_REV_1_0           0x0
 #define BOARD_REV_2_0           0x1
 
+#define BOARD_VER_OFFSET	0x8
+
 #define IMX_IIM_BASE            (IIM_BASE_ADDR)
 
 #if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__))
diff --git u-boot-imx-1d9b033.orig/board/freescale/mx51evk/mx51evk.c u-boot-imx-1d9b033/board/freescale/mx51evk/mx51evk.c
index 7a0682a..a94701c 100644
--- u-boot-imx-1d9b033.orig/board/freescale/mx51evk/mx51evk.c
+++ u-boot-imx-1d9b033/board/freescale/mx51evk/mx51evk.c
@@ -60,6 +60,14 @@ int dram_init(void)
 	return 0;
 }
 
+u32 get_board_rev(void)
+{
+	u32 rev = get_cpu_rev();
+	if (!gpio_get_value(IMX_GPIO_NR(1, 22)))
+		rev |= BOARD_REV_2_0 << BOARD_VER_OFFSET;
+	return rev;
+}
+
 static void setup_iomux_uart(void)
 {
 	unsigned int pad = PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE |
diff --git u-boot-imx-1d9b033.orig/include/configs/mx51evk.h u-boot-imx-1d9b033/include/configs/mx51evk.h
index ba4a4a6..7b027b4 100644
--- u-boot-imx-1d9b033.orig/include/configs/mx51evk.h
+++ u-boot-imx-1d9b033/include/configs/mx51evk.h
@@ -44,6 +44,7 @@
 #define CONFIG_CMDLINE_TAG			/* enable passing of ATAGs */
 #define CONFIG_SETUP_MEMORY_TAGS
 #define CONFIG_INITRD_TAG
+#define CONFIG_REVISION_TAG
 
 #define CONFIG_OF_LIBFDT
 

[U-Boot] [PATCH v2] mx51evk: Add CONFIG_REVISION_TAG

[Fabio Estevam]

On Tue, Sep 18, 2012 at 11:48 AM, Beno?t Th?baudeau
<benoit.thebaudeau@advansee.com> wrote:
> FSL 2.6.35 kernel assumes that the bootloader passes the CONFIG_REVISION_TAG
> information.
>
> If this data is not present, the kernel misconfigures the TZIC, which results in
> the timer interrupt handler never being called, so the kernel deadlocks while
> calibrating its delay.
>
> Suggested-by: Greg Topmiller <Greg.Topmiller@jdsu.com>
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>

Acked-by: Fabio Estevam <fabio.estevam@freescale.com>

[U-Boot] [PATCH v2] mx51evk: Add CONFIG_REVISION_TAG

[Stefano Babic]

On 18/09/2012 16:48, Beno?t Th?baudeau wrote:
> FSL 2.6.35 kernel assumes that the bootloader passes the CONFIG_REVISION_TAG
> information.
> 
> If this data is not present, the kernel misconfigures the TZIC, which results in
> the timer interrupt handler never being called, so the kernel deadlocks while
> calibrating its delay.
> 
> Suggested-by: Greg Topmiller <Greg.Topmiller@jdsu.com>
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> ---

Applied to u-boot-imx (fix), thanks.

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================