mx6qsabresd: Add basic support

Add basic support for mx6qsabresd board.

Signed-off-by Fabio Estevam <[email protected]>

Could you please add a bit information which sort of board this is, where to find documetnation about it, etc.?

Review

Quoted 3 lines

(quoted diff) +2 -0

+#define CONFIG_SYS_PROMPT_HUSH_PS2 ? ? "> "
+#define CONFIG_SYS_PROMPT ? ? ? ? ? ? ?"MX6QSABRESD U-Boot > "

Do we really need such verbose prompts? ?This appears to be overkill, and is becoming more and more a nuisance. [I wonder when I will see the first board which puts the full version string in the prompt :-( ]

Review

Quoted 3 lines

(quoted diff) +2 -0

+#define CONFIG_SYS_MEMTEST_START ? ? ? 0x10000000
+#define CONFIG_SYS_MEMTEST_END ? ? ? ? 0x10010000

Quoted 3 lines

Review

Quoted 3 lines

(quoted diff) +2 -0

+#define CONFIG_SYS_MEMTEST_START ? ? ? 0x10000000
+#define CONFIG_SYS_MEMTEST_END ? ? ? ? 0x10010000

0x10000000 is the start address for DDR on mx6.

mx6qarm2 and mx6qsabrelite do the same approach.

Doesn't this seem OK?

Does testing just the first 64 KiB of RAM make any sense? Not to me.

BTW: where are the exception vectors located?

Quoted 1 line · Wolfgang Denk

Does testing just the first 64 KiB of RAM make any sense? Not to me.

Ok, I will expand the mtest range.

Quoted 1 line

BTW: where are the exception vectors located?

I don't know actually, but would like to learn about it. Looked at the generated u-boot.map and could not find it out.

Stefano,

Can you please help me to find a safer DDR location for performing mtest on mx6? I think we probably have the same issues on other i.MX processors as well. Do you know how can I locate the address range that the exception vectors occupy?

Regarding the exception vectors:

I would think that the code of the U-Boot exception vectors are located at

#define CONFIG_SYS_TEXT_BASE ? ? ? ? ? 0x17800000

and

#define CONFIG_LOADADDR ? ? ? ? ? ? ? ? ? ? ? ?0x10800000

So, would it be OK to use 0x10000000 - 0x177fffff as the memory range for mtest?

Quoted 1 line · Fabio Estevam

So, would it be OK to use 0x10000000 - 0x177fffff as the memory range for mtest?

Looks like I forgot the relocation. So the range for mtest would be from 0x10000000 until 0x10800000 - 1, right?

Quoted 1 line

So, would it be OK to use 0x10000000 - 0x177fffff as the memory range for mtest?

Some considerations about this issue. hopefully I am not OT. The values put in the configuration file are the default parameters taken by the mtest command if no parameters are issued. I agree they must not be set to wrong values (I mean, outside the adressable RAM), but in any case mtest is not run automatically and the range can be adjusted in the console. I can always send a "mtest 0x0x10000000 - 0x177fffff" even if a restricted range is set in the config file, for example as it is set now for the mx6qsabrelite (64k).

So I will be for a patch that changes the behavior of mtest and computes automatically the last testable address if the second parameter is not given, dropping CONFIG_SYS_MEMTEST_END - I know, this is not part of your patch, I have already said I can be OT ;-)

Something like the patch below? (Build tested only - no hardware handy right now)

Quoted 3 lines

Something like the patch below? (Build tested only - no hardware handy right now)

Sorry, I meant the patch below:

common/cmd_mem.c +2 -0

@@ -40,6 +40,8 @@
 #define PRINTF(fmt,args...)
 #endif
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static int mod_mem(cmd_tbl_t *, int, int, int, char * const []);
 
 /* Display values from last command.

Something like this, but not exactly this. After sending my answer I remembered that the stack is *before* u-boot code, and not after as I wrote - using gd->relocaddr is wrong. We should subtract the memory reserve for stack and heap from this value.

Dear Stefano,

Quoted 5 lines · In message <[email protected]> you wrote:

Something like this, but not exactly this. After sending my answer I remembered that the stack is *before* u-boot code, and not after as I wrote - using gd->relocaddr is wrong. We should subtract the memory reserve for stack and heap from this value.

There is no builtin limit for the stack size in U-Boot. It gows downward unchecked.

Something like the patch below? (Build tested only - no hardware handy right now)

The start address CONFIG_SYS_MEMTEST_START is the lowest address of the SDRAM that can be accessed, that is MMDC0_ARB_BASE_ADDR (0x10000000) for i.MX6.

dynamically, also the last RAM address that can be tested should be computed at run time and not at compile time with CONFIG_SYS_MEMTEST_END.

So I will be for a patch that changes the behavior of mtest and computes automatically the last testable address if the second parameter is not given, dropping CONFIG_SYS_MEMTEST_END - I know, this is not part of your patch, I have already said I can be OT ;-)

Dear Stefano,

Quoted 4 lines · In message <[email protected]> you wrote:

The start address CONFIG_SYS_MEMTEST_START is the lowest address of the SDRAM that can be accessed, that is MMDC0_ARB_BASE_ADDR (0x10000000) for i.MX6.

But it _should_ be the lowest address that can _safely_ be used for such tests, i. e. the range MEMTEST_START - MEMTEST_START must neither contain the exception vectors, not U-Boot's code, stack or malloc arena.

Quoted 2 lines

dynamically, also the last RAM address that can be tested should be computed at run time and not at compile time with CONFIG_SYS_MEMTEST_END.

CONFIG_SYS_MEMTEST_END is NOT the end of physical RAM, but the end of the range that can be tested without crashing U-Boot...

Such a patch would be wrong.

Am 14/04/2012 22:13, schrieb Wolfgang Denk:

Quoted 6 lines

Dear Stefano,

Quoted 3 lines · In message <[email protected]> you wrote:

The start address CONFIG_SYS_MEMTEST_START is the lowest address of the SDRAM that can be accessed, that is MMDC0_ARB_BASE_ADDR (0x10000000) for i.MX6.

Quoted 8 lines

But it _should_ be the lowest address that can _safely_ be used for such tests, i. e. the range MEMTEST_START - MEMTEST_START must neither contain the exception vectors, not U-Boot's code, stack or malloc arena.

Quoted 2 lines

dynamically, also the last RAM address that can be tested should be computed at run time and not at compile time with CONFIG_SYS_MEMTEST_END.

CONFIG_SYS_MEMTEST_END is NOT the end of physical RAM, but the end of the range that can be tested without crashing U-Boot...

Well, but why do we have to define at compile time this value, that in most cases can be wrong ? Why should "mtest" use a default value if not all parameters are passed ? IMHO it is better if mtest returns with an error if end address is not provided.

Quoted 1 line

Such a patch would be wrong.

Really I think it is better to get rid completely of CONFIG_SYS_MEMTEST_START and CONFIG_SYS_MEMTEST_END without trying to make some (maybe wrong) assumptions, and let mtest doing its job

Will we get an update of this patch?

Dear Fabio Estevam,

Quoted 3 lines · In message <[email protected]> you wrote:

Add basic support for mx6qsabresd board.

Signed-off-by Fabio Estevam <[email protected]>

Could you please add a bit information which sort of board this is, where to find documetnation about it, etc.?

include/configs/mx6qsabresd.h line 119

Quoted 2 lines

include/configs/mx6qsabresd.h new file +2 -0

+#define CONFIG_SYS_PROMPT_HUSH_PS2     "> "
+#define CONFIG_SYS_PROMPT              "MX6QSABRESD U-Boot > "

Do we really need such verbose prompts? This appears to be overkill, and is becoming more and more a nuisance. [I wonder when I will see the first board which puts the full version string in the prompt :-( ]

include/configs/mx6qsabresd.h line 129

Quoted 3 lines

include/configs/mx6qsabresd.h new file +2 -0

+#define CONFIG_SYS_MEMTEST_START       0x10000000
+#define CONFIG_SYS_MEMTEST_END         0x10010000

Does this make sense?

Best regards,

Wolfgang Denk

Add basic support for mx6qsabresd board.

Signed-off-by Fabio Estevam <[email protected]>

---
 MAINTAINERS                               |    1 +
 board/freescale/mx6qsabresd/Makefile      |   37 ++++++
 board/freescale/mx6qsabresd/imximage.cfg  |  170 +++++++++++++++++++++++++++

 board/freescale/mx6qsabresd/mx6qsabresd.c |  111 ++++++++++++++++++
 boards.cfg                                |    1 +
 include/configs/mx6qsabresd.h             |  178 +++++++++++++++++++++++++++++
 6 files changed, 498 insertions(+), 0 deletions(-)
 create mode 100644 board/freescale/mx6qsabresd/Makefile
 create mode 100644 board/freescale/mx6qsabresd/imximage.cfg
 create mode 100644 board/freescale/mx6qsabresd/mx6qsabresd.c
 create mode 100644 include/configs/mx6qsabresd.h

(quoted diff) +1 -0

+#########################################################################

board/freescale/mx6qsabresd/imximage.cfg new file +160 -0

new file mode 100644
index 0000000..c389427
@@ -0,0 +1,170 @@
+# Copyright (C) 2011 Freescale Semiconductor, Inc.
+# Jason Liu <[email protected]>
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not write to the Free Software
+# Foundation Inc. 51 Franklin Street Fifth Floor Boston,
+# MA 02110-1301 USA
+#
+# Refer docs/README.imxmage for more details about how-to configure
+# and create imximage boot image
+#
+# The syntax is taken as close as possible with the kwbimage
+
+# image version
+
+IMAGE_VERSION 2
+
+# Boot Device : one of
+# spi, sd (the board has no nand neither onenand)
+
+BOOT_FROM      sd
+
+# Device Configuration Data (DCD)
+#
+# Each entry must have the format:
+# Addr-type           Address        Value
+#
+# where:
+#      Addr-type register length (1,2 or 4 bytes)
+#      Address   absolute address of the register
+#      value     value to be stored in the register
+DATA 4 0x020e05a8 0x00000030
+DATA 4 0x020e05b0 0x00000030
+DATA 4 0x020e0524 0x00000030
+DATA 4 0x020e051c 0x00000030
+
+DATA 4 0x020e0518 0x00000030
+DATA 4 0x020e050c 0x00000030
+DATA 4 0x020e05b8 0x00000030
+DATA 4 0x020e05c0 0x00000030
+
+DATA 4 0x020e05ac 0x00020030
+DATA 4 0x020e05b4 0x00020030
+DATA 4 0x020e0528 0x00020030
+DATA 4 0x020e0520 0x00020030
+
+DATA 4 0x020e0514 0x00020030
+DATA 4 0x020e0510 0x00020030
+DATA 4 0x020e05bc 0x00020030
+DATA 4 0x020e05c4 0x00020030
+
+DATA 4 0x020e056c 0x00020030
+DATA 4 0x020e0578 0x00020030
+DATA 4 0x020e0588 0x00020030
+DATA 4 0x020e0594 0x00020030
+
+DATA 4 0x020e057c 0x00020030
+DATA 4 0x020e0590 0x00003000
+DATA 4 0x020e0598 0x00003000
+DATA 4 0x020e058c 0x00000000
+
+DATA 4 0x020e059c 0x00003030
+DATA 4 0x020e05a0 0x00003030
+DATA 4 0x020e0784 0x00000030
+DATA 4 0x020e0788 0x00000030
+
+DATA 4 0x020e0794 0x00000030
+DATA 4 0x020e079c 0x00000030
+DATA 4 0x020e07a0 0x00000030
+DATA 4 0x020e07a4 0x00000030
+
+DATA 4 0x020e07a8 0x00000030
+DATA 4 0x020e0748 0x00000030
+DATA 4 0x020e074c 0x00000030
+DATA 4 0x020e0750 0x00020000
+
+DATA 4 0x020e0758 0x00000000
+DATA 4 0x020e0774 0x00020000
+DATA 4 0x020e078c 0x00000030
+DATA 4 0x020e0798 0x000C0000
+
+DATA 4 0x021b081c 0x33333333
+DATA 4 0x021b0820 0x33333333
+DATA 4 0x021b0824 0x33333333
+DATA 4 0x021b0828 0x33333333
+
+DATA 4 0x021b481c 0x33333333
+DATA 4 0x021b4820 0x33333333
+DATA 4 0x021b4824 0x33333333
+DATA 4 0x021b4828 0x33333333
+
+DATA 4 0x021b0018 0x00081740
+
+DATA 4 0x021b001c 0x00008000
+DATA 4 0x021b000c 0x555A7975
+DATA 4 0x021b0010 0xFF538E64
+DATA 4 0x021b0014 0x01FF00DB
+DATA 4 0x021b002c 0x000026D2
+
+DATA 4 0x021b0030 0x005B0E21
+DATA 4 0x021b0008 0x09444040
+DATA 4 0x021b0004 0x00025576
+DATA 4 0x021b0040 0x00000027
+DATA 4 0x021b0000 0x831A0000
+
+DATA 4 0x021b001c 0x04088032
+DATA 4 0x021b001c 0x0408803A
+DATA 4 0x021b001c 0x00008033
+DATA 4 0x021b001c 0x0000803B
+DATA 4 0x021b001c 0x00428031
+DATA 4 0x021b001c 0x00428039
+DATA 4 0x021b001c 0x09408030
+DATA 4 0x021b001c 0x09408038
+
+DATA 4 0x021b001c 0x04008040
+DATA 4 0x021b001c 0x04008048
+DATA 4 0x021b0800 0xA1380003
+DATA 4 0x021b4800 0xA1380003
+DATA 4 0x021b0020 0x00005800
+DATA 4 0x021b0818 0x00022227
+DATA 4 0x021b4818 0x00022227
+
+DATA 4 0x021b083c 0x434B0350
+DATA 4 0x021b0840 0x034C0359
+DATA 4 0x021b483c 0x434B0350
+DATA 4 0x021b4840 0x03650348
+DATA 4 0x021b0848 0x4436383B
+DATA 4 0x021b4848 0x39393341
+DATA 4 0x021b0850 0x35373933
+DATA 4 0x021b4850 0x48254A36
+
+DATA 4 0x021b080c 0x001F001F
+DATA 4 0x021b0810 0x001F001F
+
+DATA 4 0x021b480c 0x00440044
+DATA 4 0x021b4810 0x00440044
+
+DATA 4 0x021b08b8 0x00000800
+DATA 4 0x021b48b8 0x00000800
+
+DATA 4 0x021b001c 0x00000000
+DATA 4 0x021b0404 0x00011006
+
+# set the default clock gate to save power
+DATA 4 0x020c4068 0x00C03F3F
+DATA 4 0x020c406c 0x0030FC03
+DATA 4 0x020c4070 0x0FFFC000

board/freescale/mx6qsabresd/mx6qsabresd.c new file +28 -0

new file mode 100644
index 0000000..7df2823
@@ -0,0 +1,111 @@
+/*
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ *
+ * Author: Fabio Estevam <[email protected]>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/imx-regs.h>
+#include <asm/arch/mx6x_pins.h>
+#include <asm/arch/iomux-v3.h>
+#include <asm/errno.h>
+#include <asm/gpio.h>
+#include <mmc.h>
+#include <fsl_esdhc.h>

board/freescale/mx6qsabresd/mx6qsabresd.c new file +15 -0

+#define CONFIG_EXTRA_ENV_SETTINGS \
+	"script=boot.scr\0" \
+	"uimage=uImage\0" \
+	"console=ttymxc0\0" \
+	"fdt_high=0xffffffff\0"	  \
+	"initrd_high=0xffffffff\0" \
+	"mmcdev=0\0" \
+	"mmcpart=2\0" \
+	"mmcroot=/dev/mmcblk0p3 rootwait rw\0" \
+	"mmcargs=setenv bootargs console=${console},${baudrate} " \
+		"root=${mmcroot}\0" \
+	"loadbootscript=" \
+		"fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${script};\0" \
+	"bootscript=echo Running bootscript from mmc ...; " \
+		"source\0" \

This file is identical to imximage.cfg for the mx6qsabrelite board. I can imagine this is derived board. Why cannot we implement it as a variant of the original one ? We have several example in u-boot, for example the efika (MX51), or the TAM3517 (ok, I admit I know this very well because I did it...), or imx27-lite /magnesium, or....

Hi Stefano,

Quoted 6 lines

This file is identical to imximage.cfg for the mx6qsabrelite board. I can imagine this is derived board. Why cannot we implement it as a variant of the original one ? We have several example in u-boot, for example the efika (MX51), or the TAM3517 (ok, I admit I know this very well because I did it...), or imx27-lite /magnesium, or....

After a long time, I am returning on adding support to mx6qsabresd.

I have been comparing mx6qsabrelite against mx6qsabresd and I have started to do as you suggested: unify the 2 boards into mx6qsabrelite.c.

What I realize is that the differences are relevant: UART1 pin muxing, SDHC ports, SDHC card detect GPIO, USB Host enable port, I2C devices, Ethernet PHY, etc.

It seems to me that the code is becoming polluted by all the ifdef's I need to place in order to handle both boards, and I am starting to think if it wouldn't be better to follow with the original approach of adding a board/freescale/mx6qsabresd directory.

After I finish mx6qsabresd, I also plan to add one more mx6q board, and this would mean even more ifdefs, which would make the code even harder to read.

Hi Stefano,

Quoted 5 lines · Stefano Babic

This file is identical to imximage.cfg for the mx6qsabrelite board. I can imagine this is derived board. Why cannot we implement it as a variant of the original one ? We have several example in u-boot, for example the efika (MX51), or the TAM3517 (ok, I admit I know this very well because I did it...), or imx27-lite /magnesium, or....

After a long time, I am returning on adding support to mx6qsabresd.

I have been comparing mx6qsabrelite against mx6qsabresd and I have started to do as you suggested: unify the 2 boards into mx6qsabrelite.c.

What I realize is that the differences are relevant: UART1 pin muxing, SDHC ports, SDHC card detect GPIO, USB Host enable port, I2C devices, Ethernet PHY, etc.

It seems to me that the code is becoming polluted by all the ifdef's I need to place in order to handle both boards, and I am starting to think if it wouldn't be better to follow with the original approach of adding a board/freescale/mx6qsabresd directory.

After I finish mx6qsabresd, I also plan to add one more mx6q board, and this would mean even more ifdefs, which would make the code even harder to read.

Please let me know what you think.

Hi Stefano,

Quoted 5 lines · Stefano Babic

This file is identical to imximage.cfg for the mx6qsabrelite board. I can imagine this is derived board. Why cannot we implement it as a variant of the original one ? We have several example in u-boot, for example the efika (MX51), or the TAM3517 (ok, I admit I know this very well because I did it...), or imx27-lite /magnesium, or....

After a long time, I am returning on adding support to mx6qsabresd.

I have been comparing mx6qsabrelite against mx6qsabresd and I have started to do as you suggested: unify the 2 boards into mx6qsabrelite.c.

What I realize is that the differences are relevant: UART1 pin muxing, SDHC ports, SDHC card detect GPIO, USB Host enable port, I2C devices, Ethernet PHY, etc.

It seems to me that the code is becoming polluted by all the ifdef's I need to place in order to handle both boards, and I am starting to think if it wouldn't be better to follow with the original approach of adding a board/freescale/mx6qsabresd directory.

After I finish mx6qsabresd, I also plan to add one more mx6q board, and this would mean even more ifdefs, which would make the code even harder to read.

Please let me know what you think.

On 09/10/2012 08:56 PM, Fabio Estevam wrote:
We also need to add support for a board (our Nitrogen6x) which is a slight variation on Sabre Lite.

We have been patching the board/freescale/mx6qsabrelite, and our needs can in general be handled without ifdefs, but it seems that this approach doesn't scale well.

When other vendors produce boards which are also slight variations, where do we draw the line, and more importantly, how do we track and test the needs of each?

Our thinking is that if it's a different board, it should have a different directory, and common code should be clearly separated.

Quoted 25 lines

Hi Stefano,

Quoted 5 lines · Stefano Babic

This file is identical to imximage.cfg for the mx6qsabrelite board. I can imagine this is derived board. Why cannot we implement it as a variant of the original one ? We have several example in u-boot, for example the efika (MX51), or the TAM3517 (ok, I admit I know this very well because I did it...), or imx27-lite /magnesium, or....

After a long time, I am returning on adding support to mx6qsabresd.

I have been comparing mx6qsabrelite against mx6qsabresd and I have started to do as you suggested: unify the 2 boards into mx6qsabrelite.c.

What I realize is that the differences are relevant: UART1 pin muxing, SDHC ports, SDHC card detect GPIO, USB Host enable port, I2C devices, Ethernet PHY, etc.

It seems to me that the code is becoming polluted by all the ifdef's I need to place in order to handle both boards, and I am starting to think if it wouldn't be better to follow with the original approach of adding a board/freescale/mx6qsabresd directory.

Quoted 6 lines

After I finish mx6qsabresd, I also plan to add one more mx6q board, and this would mean even more ifdefs, which would make the code even harder to read.

Please let me know what you think.

Hi Fabio and Stefano,

We also need to add support for a board (our Nitrogen6x) which is a slight variation on Sabre Lite.

We have been patching the board/freescale/mx6qsabrelite, and our needs can in general be handled without ifdefs, but it seems that this approach doesn't scale well.

When other vendors produce boards which are also slight variations, where do we draw the line, and more importantly, how do we track and test the needs of each?

Our thinking is that if it's a different board, it should have a different directory, and common code should be clearly separated.