[U-Boot] [PATCH v3 0/8] *** iMX6 SabreSD SPL Support ***

[U-Boot] [PATCH v3 0/8] *** iMX6 SabreSD SPL Support ***

[John Tobias]

This patch is for SPL support for iMX6 SabreSD. The said
patches has been tested to work on SD2 and SD3 port of the
said board.

After applying the following patches, it will produces
SPL and u-boot.img binary images. You should run the
two commands below to store it in your SD or eMMC.

sudo dd if=SPL of=/dev/xxx bs=1K seek=1; sync
sudo dd if=u-boot.img of=/dev/xxx bs=1K seek=69

Changes (v2):
Merged the SPL support into the main board file
Remove the compilation warmings

Changes (v3):
Removed sp and gd
Use imx_ddr_size to set the ram_size

John Tobias (8):
  mmc: add spl_board_mmc_init
  mmc: imx6: call spl_board_mmc_init
  imx6: add spl on include header file
  imx6: add some flexibility for defining macros
  imx6: add spl on board configuration
  imx6: add data configuration file for SPL
  imx6: SPL support for iMX6 SabreSD
  kconfig: imx6: add SUPPORT_SPL

 arch/arm/Kconfig                              |   1 +
 board/freescale/mx6sabresd/mx6sabresd.c       | 211 +++++++++++++++++++++++++-
 board/freescale/mx6sabresd/mx6sabresd_spl.cfg |  54 +++++++
 configs/mx6qsabresd_defconfig                 |   7 +-
 drivers/mmc/mmc.c                             |   9 +-
 include/configs/imx6_spl.h                    |   4 +
 include/configs/mx6sabresd.h                  |   7 +
 include/mmc.h                                 |   3 +
 8 files changed, 288 insertions(+), 8 deletions(-)
 create mode 100644 board/freescale/mx6sabresd/mx6sabresd_spl.cfg

-- 
1.9.1

[U-Boot] [PATCH v3 1/8] mmc: add spl_board_mmc_init

[John Tobias]

To avoid compilation warning it need to define it in the header file.
---
 include/mmc.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/mmc.h b/include/mmc.h
index d74a190..37119b8 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -388,6 +388,9 @@ int mmc_legacy_init(int verbose);
 int board_mmc_init(bd_t *bis);
 int cpu_mmc_init(bd_t *bis);
 int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr);
+#ifdef CONFIG_SPL
+int spl_board_mmc_init(bd_t *bis);
+#endif
 
 /* Set block count limit because of 16 bit register limit on some hardware*/
 #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
-- 
1.9.1

[U-Boot] [PATCH v3 2/8] mmc: imx6: call spl_board_mmc_init

[John Tobias]

Add to call spl_board_mmc_init when CONFIG_SPL_MMC_SUPPORT is defined,
by default the mmc_initialize function will call board_mmc_init. But,
the said function is not link to the spl image.
---
 drivers/mmc/mmc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 44a4feb..88c1b03 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1441,11 +1441,14 @@ int mmc_initialize(bd_t *bis)
 	INIT_LIST_HEAD (&mmc_devices);
 	cur_dev_num = 0;
 
-	if (board_mmc_init(bis) < 0)
+#ifdef CONFIG_SPL_BUILD
+	if (spl_board_mmc_init(bis) < 0)
 		cpu_mmc_init(bis);
+#else
+        if (board_mmc_init(bis) < 0)
+                cpu_mmc_init(bis);
 
-#ifndef CONFIG_SPL_BUILD
-	print_mmc_devices(',');
+        print_mmc_devices(',');
 #endif
 
 	do_preinit();
-- 
1.9.1

[U-Boot] [PATCH v3 3/8] imx6: add spl on include header file

[John Tobias]

add the spl on include header file of iMX6 SabreSD
---
 include/configs/mx6sabresd.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/configs/mx6sabresd.h b/include/configs/mx6sabresd.h
index 938030d..4d2e54a 100644
--- a/include/configs/mx6sabresd.h
+++ b/include/configs/mx6sabresd.h
@@ -12,6 +12,13 @@
 #include <asm/arch/imx-regs.h>
 #include <asm/imx-common/gpio.h>
 
+#ifdef CONFIG_SPL
+#define CONFIG_SPL_LIBCOMMON_SUPPORT
+#define CONFIG_SPL_MMC_SUPPORT
+#define CONFIG_SPL_STACK 0x0093FFB8
+#include "imx6_spl.h"
+#endif
+
 #define CONFIG_MACH_TYPE	3980
 #define CONFIG_MXC_UART_BASE	UART1_BASE
 #define CONFIG_CONSOLE_DEV		"ttymxc0"
-- 
1.9.1

[U-Boot] [PATCH v3 4/8] imx6: add some flexibility for defining macros

[John Tobias]

iMX6 SabreSD has different stack address compare
to the default stack address defined on the file.

The CONFIG_SYS_TEXT_BASE is defined in mx6sabre_common.h which is
same address defined on file. At the same time to avoid compilation
warnings.
---
 include/configs/imx6_spl.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
index 5a5f940..4ff37b3 100644
--- a/include/configs/imx6_spl.h
+++ b/include/configs/imx6_spl.h
@@ -29,7 +29,9 @@
 #define CONFIG_SPL_TEXT_BASE		0x00908000
 #define CONFIG_SPL_MAX_SIZE		0x10000
 #define CONFIG_SPL_START_S_PATH		"arch/arm/cpu/armv7"
+#ifndef CONFIG_SPL_STACK
 #define CONFIG_SPL_STACK		0x0091FFB8
+#endif
 #define CONFIG_SPL_LIBCOMMON_SUPPORT
 #define CONFIG_SPL_LIBGENERIC_SUPPORT
 #define CONFIG_SPL_SERIAL_SUPPORT
@@ -66,7 +68,9 @@
 #define CONFIG_SPL_BSS_MAX_SIZE		0x100000	/* 1 MB */
 #define CONFIG_SYS_SPL_MALLOC_START	0x18300000
 #define CONFIG_SYS_SPL_MALLOC_SIZE	0x3200000	/* 50 MB */
+#ifndef CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_TEXT_BASE		0x17800000
 #endif
+#endif
 
 #endif
-- 
1.9.1

[U-Boot] [PATCH v3 4/8] imx6: add some flexibility for defining macros

[Stefano Babic]

On 08/11/2014 22:27, John Tobias wrote:
> iMX6 SabreSD has different stack address compare
> to the default stack address defined on the file.
> 
> The CONFIG_SYS_TEXT_BASE is defined in mx6sabre_common.h which is
> same address defined on file. At the same time to avoid compilation
> warnings.
> ---
>  include/configs/imx6_spl.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
> index 5a5f940..4ff37b3 100644
> --- a/include/configs/imx6_spl.h
> +++ b/include/configs/imx6_spl.h
> @@ -29,7 +29,9 @@
>  #define CONFIG_SPL_TEXT_BASE		0x00908000
>  #define CONFIG_SPL_MAX_SIZE		0x10000
>  #define CONFIG_SPL_START_S_PATH		"arch/arm/cpu/armv7"
> +#ifndef CONFIG_SPL_STACK
>  #define CONFIG_SPL_STACK		0x0091FFB8
> +#endif

Why is this required ?

>  #define CONFIG_SPL_LIBCOMMON_SUPPORT
>  #define CONFIG_SPL_LIBGENERIC_SUPPORT
>  #define CONFIG_SPL_SERIAL_SUPPORT
> @@ -66,7 +68,9 @@
>  #define CONFIG_SPL_BSS_MAX_SIZE		0x100000	/* 1 MB */
>  #define CONFIG_SYS_SPL_MALLOC_START	0x18300000
>  #define CONFIG_SYS_SPL_MALLOC_SIZE	0x3200000	/* 50 MB */
> +#ifndef CONFIG_SYS_TEXT_BASE
>  #define CONFIG_SYS_TEXT_BASE		0x17800000
>  #endif
> +#endif
>  

Why is this required ?

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 v3 4/8] imx6: add some flexibility for defining macros

[John Tobias]

Hi Stefano,

On Sun, Nov 9, 2014 at 1:24 PM, Stefano Babic <sbabic@denx.de> wrote:
>
>
> On 08/11/2014 22:27, John Tobias wrote:
>> iMX6 SabreSD has different stack address compare
>> to the default stack address defined on the file.
>>
>> The CONFIG_SYS_TEXT_BASE is defined in mx6sabre_common.h which is
>> same address defined on file. At the same time to avoid compilation
>> warnings.
>> ---
>>  include/configs/imx6_spl.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
>> index 5a5f940..4ff37b3 100644
>> --- a/include/configs/imx6_spl.h
>> +++ b/include/configs/imx6_spl.h
>> @@ -29,7 +29,9 @@
>>  #define CONFIG_SPL_TEXT_BASE         0x00908000
>>  #define CONFIG_SPL_MAX_SIZE          0x10000
>>  #define CONFIG_SPL_START_S_PATH              "arch/arm/cpu/armv7"
>> +#ifndef CONFIG_SPL_STACK
>>  #define CONFIG_SPL_STACK             0x0091FFB8
>> +#endif
>
> Why is this required ?

Other iMX6 chip has different STACK address and the current defined
address does not
compatibile for the iMX6 SabreSD. The intention is to use as much as
possible the said header
and it should have the ability to change the value of it. e.g defined
in mx6sabresd.h

#ifdef CONFIG_SPL
#define CONFIG_SPL_LIBCOMMON_SUPPORT
#define CONFIG_SPL_MMC_SUPPORT
#define CONFIG_SPL_STACK 0x0093FFB8
#include "imx6_spl.h"
#endif

>
>>  #define CONFIG_SPL_LIBCOMMON_SUPPORT
>>  #define CONFIG_SPL_LIBGENERIC_SUPPORT
>>  #define CONFIG_SPL_SERIAL_SUPPORT
>> @@ -66,7 +68,9 @@
>>  #define CONFIG_SPL_BSS_MAX_SIZE              0x100000        /* 1 MB */
>>  #define CONFIG_SYS_SPL_MALLOC_START  0x18300000
>>  #define CONFIG_SYS_SPL_MALLOC_SIZE   0x3200000       /* 50 MB */
>> +#ifndef CONFIG_SYS_TEXT_BASE
>>  #define CONFIG_SYS_TEXT_BASE         0x17800000
>>  #endif
>> +#endif
>>
>
> Why is this required ?

mentioned above.

Addition to that, on my recent submission for iMX6SL. It requires to
change the value of the following:

CONFIG_SPL_BSS_START_ADDR
CONFIG_SYS_SPL_MALLOC_START


Regards,

john



>
> 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 v3 4/8] imx6: add some flexibility for defining macros

[Stefano Babic]

Hi John,

On 10/11/2014 00:53, John Tobias wrote:

>>> @@ -29,7 +29,9 @@
>>>  #define CONFIG_SPL_TEXT_BASE         0x00908000
>>>  #define CONFIG_SPL_MAX_SIZE          0x10000
>>>  #define CONFIG_SPL_START_S_PATH              "arch/arm/cpu/armv7"
>>> +#ifndef CONFIG_SPL_STACK
>>>  #define CONFIG_SPL_STACK             0x0091FFB8
>>> +#endif
>>
>> Why is this required ?
> 
> Other iMX6 chip has different STACK address and the current defined
> address does not
> compatibile for the iMX6 SabreSD.

This is exactly what I have not understood. Why is this board so special
to require a different value ? SPL will run into the IRAM, whose size
and layout is the same for i.MX6Q. Is there a reserved area in the IRAM
only for sabreSD and if yes, for which reason ?

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 v3 5/8] imx6: add spl on board configuration

[John Tobias]

add the spl on build configuration of iMX6 SabreSD
---
 configs/mx6qsabresd_defconfig | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/configs/mx6qsabresd_defconfig b/configs/mx6qsabresd_defconfig
index 67c1b77..14c80cc 100644
--- a/configs/mx6qsabresd_defconfig
+++ b/configs/mx6qsabresd_defconfig
@@ -1,3 +1,4 @@
-CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/mx6sabresd/mx6q_4x_mt41j128.cfg,MX6Q"
-CONFIG_ARM=y
-CONFIG_TARGET_MX6SABRESD=y
+CONFIG_SPL=y
+CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/mx6sabresd/mx6sabresd_spl.cfg,SPL,MX6Q"
++S:CONFIG_ARM=y
++S:CONFIG_TARGET_MX6SABRESD=y
-- 
1.9.1

[U-Boot] [PATCH v3 6/8] imx6: add data configuration file for SPL

[John Tobias]

This file is the default DCD configuration file for SPL
---
 board/freescale/mx6sabresd/mx6sabresd_spl.cfg | 54 +++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 board/freescale/mx6sabresd/mx6sabresd_spl.cfg

diff --git a/board/freescale/mx6sabresd/mx6sabresd_spl.cfg b/board/freescale/mx6sabresd/mx6sabresd_spl.cfg
new file mode 100644
index 0000000..2e2f850
--- /dev/null
+++ b/board/freescale/mx6sabresd/mx6sabresd_spl.cfg
@@ -0,0 +1,54 @@
+/*
+ * Maintainer : Richard Hu <linuxfae@technexion.com>
+ *
+ * Derived from Nitrogen6x code by Boundary Devices
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ *
+ * Refer doc/README.imximage 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
+
+#define __ASSEMBLY__
+#include <config.h>
+#include "asm/arch/iomux.h"
+#include "asm/arch/crm_regs.h"
+
+/* set the default clock gate to save power */
+DATA 4 0x020C4068 0x00C03F3F /* CCM_CCGR0 */
+DATA 4 0x020C406c 0x0030FC03 /* CCM_CCGR1 */
+DATA 4 0x020C4070 0x0FFFC000 /* CCM_CCGR2 */
+DATA 4 0x020C4074 0x3FF00000 /* CCM_CCGR3 */
+DATA 4 0x020C4078 0xFFF300   /* CCM_CCGR4 */
+DATA 4 0x020C407c 0x0F0000F3 /* CCM_CCGR5 - enable SATA clocks */
+DATA 4 0x020C4080 0x000003FF /* CCM_CCGR6 - enable ushdc and usb clocks */
+
+/* enable AXI cache for VDOA/VPU/IPU */
+DATA 4 0x020e0010 0xF00000CF /* IOMUXC_GPR4 */
+/* set IPU AXI-id0 Qos=0xf(bypass) AXI-id1 Qos=0x7 */
+DATA 4 0x020e0018 0x007F007F /* IOMUXC_GPR6 */
+DATA 4 0x020e001c 0x007F007F /* IOMUXC_GPR7 */
+
+/*
+ * Setup CCM_CCOSR register as follows:
+ *
+ * cko1_en  = 1	   --> CKO1 enabled
+ * cko1_div = 111  --> divide by 8
+ * cko1_sel = 1011 --> ahb_clk_root
+ *
+ * This sets CKO1 at ahb_clk_root/8 = 132/8 = 16.5 MHz
+ */
+DATA 4 0x020c4060 0x000000fb /* CCM_CCOSR */
+
+
-- 
1.9.1

[U-Boot] [PATCH v3 6/8] imx6: add data configuration file for SPL

[Stefano Babic]

Hi John,

On 08/11/2014 22:27, John Tobias wrote:
> This file is the default DCD configuration file for SPL
> ---
>  board/freescale/mx6sabresd/mx6sabresd_spl.cfg | 54 +++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 board/freescale/mx6sabresd/mx6sabresd_spl.cfg
> 
> diff --git a/board/freescale/mx6sabresd/mx6sabresd_spl.cfg b/board/freescale/mx6sabresd/mx6sabresd_spl.cfg
> new file mode 100644
> index 0000000..2e2f850
> --- /dev/null
> +++ b/board/freescale/mx6sabresd/mx6sabresd_spl.cfg
> @@ -0,0 +1,54 @@
> +/*
> + * Maintainer : Richard Hu <linuxfae@technexion.com>
> + *
> + * Derived from Nitrogen6x code by Boundary Devices
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + *
> + * Refer doc/README.imximage for more details about how-to configure
> + * and create imximage boot image
> + *
> + * The syntax is taken as close as possible with the kwbimage
> + */
> +

I am expecting that this file is derived from sabresd .cfg file, not
from something else.

Not only: this file seems to me identical to novena .cfg (setup.cfg),
copyrighted by Marek Vasut. Please carefully check copyright and
licensing of the file you are taking and maintain the original license.

> +/* image version */
> +IMAGE_VERSION 2
> +
> +/*
> + * Boot Device : one of
> + * spi, sd (the board has no nand neither onenand)
> + */
> +BOOT_FROM      sd
> +
> +#define __ASSEMBLY__
> +#include <config.h>
> +#include "asm/arch/iomux.h"
> +#include "asm/arch/crm_regs.h"
> +
> +/* set the default clock gate to save power */
> +DATA 4 0x020C4068 0x00C03F3F /* CCM_CCGR0 */
> +DATA 4 0x020C406c 0x0030FC03 /* CCM_CCGR1 */
> +DATA 4 0x020C4070 0x0FFFC000 /* CCM_CCGR2 */
> +DATA 4 0x020C4074 0x3FF00000 /* CCM_CCGR3 */
> +DATA 4 0x020C4078 0xFFF300   /* CCM_CCGR4 */
> +DATA 4 0x020C407c 0x0F0000F3 /* CCM_CCGR5 - enable SATA clocks */
> +DATA 4 0x020C4080 0x000003FF /* CCM_CCGR6 - enable ushdc and usb clocks */
> +
> +/* enable AXI cache for VDOA/VPU/IPU */
> +DATA 4 0x020e0010 0xF00000CF /* IOMUXC_GPR4 */
> +/* set IPU AXI-id0 Qos=0xf(bypass) AXI-id1 Qos=0x7 */
> +DATA 4 0x020e0018 0x007F007F /* IOMUXC_GPR6 */
> +DATA 4 0x020e001c 0x007F007F /* IOMUXC_GPR7 */
> +
> +/*
> + * Setup CCM_CCOSR register as follows:
> + *
> + * cko1_en  = 1	   --> CKO1 enabled
> + * cko1_div = 111  --> divide by 8
> + * cko1_sel = 1011 --> ahb_clk_root
> + *
> + * This sets CKO1 at ahb_clk_root/8 = 132/8 = 16.5 MHz
> + */
> +DATA 4 0x020c4060 0x000000fb /* CCM_CCOSR */
> +
> +
> 

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 v3 6/8] imx6: add data configuration file for SPL

[John Tobias]

Thanks for the info. I'll double check it again.

Regards,

john

On Sun, Nov 9, 2014 at 1:22 PM, Stefano Babic <sbabic@denx.de> wrote:
> Hi John,
>
> On 08/11/2014 22:27, John Tobias wrote:
>> This file is the default DCD configuration file for SPL
>> ---
>>  board/freescale/mx6sabresd/mx6sabresd_spl.cfg | 54 +++++++++++++++++++++++++++
>>  1 file changed, 54 insertions(+)
>>  create mode 100644 board/freescale/mx6sabresd/mx6sabresd_spl.cfg
>>
>> diff --git a/board/freescale/mx6sabresd/mx6sabresd_spl.cfg b/board/freescale/mx6sabresd/mx6sabresd_spl.cfg
>> new file mode 100644
>> index 0000000..2e2f850
>> --- /dev/null
>> +++ b/board/freescale/mx6sabresd/mx6sabresd_spl.cfg
>> @@ -0,0 +1,54 @@
>> +/*
>> + * Maintainer : Richard Hu <linuxfae@technexion.com>
>> + *
>> + * Derived from Nitrogen6x code by Boundary Devices
>> + *
>> + * SPDX-License-Identifier:  GPL-2.0+
>> + *
>> + * Refer doc/README.imximage for more details about how-to configure
>> + * and create imximage boot image
>> + *
>> + * The syntax is taken as close as possible with the kwbimage
>> + */
>> +
>
> I am expecting that this file is derived from sabresd .cfg file, not
> from something else.
>
> Not only: this file seems to me identical to novena .cfg (setup.cfg),
> copyrighted by Marek Vasut. Please carefully check copyright and
> licensing of the file you are taking and maintain the original license.
>
>> +/* image version */
>> +IMAGE_VERSION 2
>> +
>> +/*
>> + * Boot Device : one of
>> + * spi, sd (the board has no nand neither onenand)
>> + */
>> +BOOT_FROM      sd
>> +
>> +#define __ASSEMBLY__
>> +#include <config.h>
>> +#include "asm/arch/iomux.h"
>> +#include "asm/arch/crm_regs.h"
>> +
>> +/* set the default clock gate to save power */
>> +DATA 4 0x020C4068 0x00C03F3F /* CCM_CCGR0 */
>> +DATA 4 0x020C406c 0x0030FC03 /* CCM_CCGR1 */
>> +DATA 4 0x020C4070 0x0FFFC000 /* CCM_CCGR2 */
>> +DATA 4 0x020C4074 0x3FF00000 /* CCM_CCGR3 */
>> +DATA 4 0x020C4078 0xFFF300   /* CCM_CCGR4 */
>> +DATA 4 0x020C407c 0x0F0000F3 /* CCM_CCGR5 - enable SATA clocks */
>> +DATA 4 0x020C4080 0x000003FF /* CCM_CCGR6 - enable ushdc and usb clocks */
>> +
>> +/* enable AXI cache for VDOA/VPU/IPU */
>> +DATA 4 0x020e0010 0xF00000CF /* IOMUXC_GPR4 */
>> +/* set IPU AXI-id0 Qos=0xf(bypass) AXI-id1 Qos=0x7 */
>> +DATA 4 0x020e0018 0x007F007F /* IOMUXC_GPR6 */
>> +DATA 4 0x020e001c 0x007F007F /* IOMUXC_GPR7 */
>> +
>> +/*
>> + * Setup CCM_CCOSR register as follows:
>> + *
>> + * cko1_en  = 1         --> CKO1 enabled
>> + * cko1_div = 111  --> divide by 8
>> + * cko1_sel = 1011 --> ahb_clk_root
>> + *
>> + * This sets CKO1 at ahb_clk_root/8 = 132/8 = 16.5 MHz
>> + */
>> +DATA 4 0x020c4060 0x000000fb /* CCM_CCOSR */
>> +
>> +
>>
>
> 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 v3 7/8] imx6: SPL support for iMX6 SabreSD

[John Tobias]

This patch will enable the support for SPL on iMX6 SabreSD.
It tested on SD2 and SD3 mmc port.
---
 board/freescale/mx6sabresd/mx6sabresd.c | 211 +++++++++++++++++++++++++++++++-
 1 file changed, 209 insertions(+), 2 deletions(-)

diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c
index 3d81fff..f443337 100644
--- a/board/freescale/mx6sabresd/mx6sabresd.c
+++ b/board/freescale/mx6sabresd/mx6sabresd.c
@@ -55,8 +55,7 @@ DECLARE_GLOBAL_DATA_PTR;
 
 int dram_init(void)
 {
-	gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
-
+	gd->ram_size = imx_ddr_size();
 	return 0;
 }
 
@@ -607,3 +606,211 @@ int checkboard(void)
 	puts("Board: MX6-SabreSD\n");
 	return 0;
 }
+
+#ifdef CONFIG_SPL_BUILD
+#include <spl.h>
+#include <libfdt.h>
+
+#define BOOT_CFG	0x20D8004
+#define __REG(x)        (*((volatile u32 *)(x)))
+
+struct fsl_esdhc_cfg spl_usdhc_cfg;
+/*
+ * Got it from mx6q_4x_mt41j128.cfg file
+ */
+void set_mt41j128_ddr(void)
+{
+	__REG(0x020e05a8) = 0x00000028;
+	__REG(0x020e05b0) = 0x00000028;
+	__REG(0x020e0524) = 0x00000028;
+	__REG(0x020e051c) = 0x00000028;
+
+	__REG(0x020e0518) = 0x00000028;
+	__REG(0x020e050c) = 0x00000028;
+	__REG(0x020e05b8) = 0x00000028;
+	__REG(0x020e05c0) = 0x00000028;
+
+	__REG(0x020e05ac) = 0x00000028;
+	__REG(0x020e05b4) = 0x00000028;
+	__REG(0x020e0528) = 0x00000028;
+	__REG(0x020e0520) = 0x00000028;
+
+	__REG(0x020e0514) = 0x00000028;
+	__REG(0x020e0510) = 0x00000028;
+	__REG(0x020e05bc) = 0x00000028;
+	__REG(0x020e05c4) = 0x00000028;
+
+	__REG(0x020e056c) = 0x00000030;
+	__REG(0x020e0578) = 0x00000030;
+	__REG(0x020e0588) = 0x00000030;
+	__REG(0x020e0594) = 0x00000030;
+
+	__REG(0x020e057c) = 0x00000030;
+	__REG(0x020e0590) = 0x00000030;
+	__REG(0x020e0598) = 0x00000030;
+	__REG(0x020e058c) = 0x00000000;
+
+	__REG(0x020e059c) = 0x00003030;
+	__REG(0x020e05a0) = 0x00003030;
+	__REG(0x020e0784) = 0x00000028;
+	__REG(0x020e0788) = 0x00000028;
+
+	__REG(0x020e0794) = 0x00000028;
+	__REG(0x020e079c) = 0x00000028;
+	__REG(0x020e07a0) = 0x00000028;
+	__REG(0x020e07a4) = 0x00000028;
+
+	__REG(0x020e07a8) = 0x00000028;
+	__REG(0x020e0748) = 0x00000028;
+	__REG(0x020e074c) = 0x00000030;
+	__REG(0x020e0750) = 0x00020000;
+
+	__REG(0x020e0758) = 0x00000000;
+	__REG(0x020e0774) = 0x00020000;
+	__REG(0x020e078c) = 0x00000030;
+	__REG(0x020e0798) = 0x000C0000;
+
+	__REG(0x021b081c) = 0x33333333;
+	__REG(0x021b0820) = 0x33333333;
+	__REG(0x021b0824) = 0x33333333;
+	__REG(0x021b0828) = 0x33333333;
+
+	__REG(0x021b481c) = 0x33333333;
+	__REG(0x021b4820) = 0x33333333;
+	__REG(0x021b4824) = 0x33333333;
+	__REG(0x021b4828) = 0x33333333;
+
+	__REG(0x021b0018) = 0x00001740;
+
+	__REG(0x021b001c) = 0x00008000;
+	__REG(0x021b000c) = 0x8A8F7975;
+	__REG(0x021b0010) = 0xFF538E64;
+	__REG(0x021b0014) = 0x01FF00DB;
+	__REG(0x021b002c) = 0x000026D2;
+
+	__REG(0x021b0030) = 0x008F0E21;
+	__REG(0x021b0008) = 0x09444040;
+	__REG(0x021b0004) = 0x00020036;
+	__REG(0x021b0040) = 0x00000047;
+	__REG(0x021b0000) = 0x841A0000;
+
+	__REG(0x021b001c) = 0x04088032;
+	__REG(0x021b001c) = 0x00008033;
+	__REG(0x021b001c) = 0x00428031;
+	__REG(0x021b001c) = 0x09408030;
+
+	__REG(0x021b001c) = 0x04008040;
+	__REG(0x021b0800) = 0xA1380003;
+	__REG(0x021b0020) = 0x00005800;
+	__REG(0x021b0818) = 0x00000007;
+	__REG(0x021b4818) = 0x00000007;
+
+	/* Calibration values based on ARD and 528MHz */
+	__REG(0x021b083c) = 0x434B0358;
+	__REG(0x021b0840) = 0x033D033C;
+	__REG(0x021b483c) = 0x03520362;
+	__REG(0x021b4840) = 0x03480318;
+	__REG(0x021b0848) = 0x41383A3C;
+	__REG(0x021b4848) = 0x3F3C374A;
+	__REG(0x021b0850) = 0x42434444;
+	__REG(0x021b4850) = 0x4932473A;
+
+	__REG(0x021b080c) = 0x001F001F;
+	__REG(0x021b0810) = 0x001F001F;
+
+	__REG(0x021b480c) = 0x001F001F;
+	__REG(0x021b4810) = 0x001F001F;
+
+	__REG(0x021b08b8) = 0x00000800;
+	__REG(0x021b48b8) = 0x00000800;
+
+	__REG(0x021b0404) = 0x00011006;
+	__REG(0x021b0004) = 0x00025576;
+
+	__REG(0x021b001c) = 0x00000000;
+
+	__REG(0x020c4068) = 0x00C03F3F;
+	__REG(0x020c406c) = 0x0030FC00;
+	__REG(0x020c4070) = 0x0FFFC000;
+	__REG(0x020c4074) = 0x3FF00000;
+	__REG(0x020c4078) = 0x00FFF300;
+	__REG(0x020c407c) = 0x0F0000C3;
+	__REG(0x020c4080) = 0x000003FF;
+}
+
+/*
+ * This section require the differentiation
+ * between iMX6 Sabre Families.
+ * But for now, it will configure only for
+ * SabreSD.
+ */
+static void spl_dram_init(void)
+{
+	set_mt41j128_ddr();
+}
+
+int spl_board_mmc_init(bd_t *bis)
+{
+	unsigned reg = readl(BOOT_CFG);
+	/*
+	 * Upon reading BOOT_CFG register the following map is done:
+	 * (U-boot device node)    (Physical Port)
+	 * 0x840                   SD2
+	 * 0x1040                  SD3
+	 * 0x1840                  eMMC
+	 */
+	switch (reg) {
+	case 0x840:
+		imx_iomux_v3_setup_multiple_pads(
+			usdhc2_pads, ARRAY_SIZE(usdhc2_pads));
+		spl_usdhc_cfg.esdhc_base = USDHC2_BASE_ADDR;
+		spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK);
+		gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
+		break;
+	case 0x1040:
+		imx_iomux_v3_setup_multiple_pads(
+			usdhc3_pads, ARRAY_SIZE(usdhc3_pads));
+		spl_usdhc_cfg.esdhc_base = USDHC3_BASE_ADDR;
+		spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
+		gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
+		break;
+	case 0x1840:
+		imx_iomux_v3_setup_multiple_pads(
+			usdhc4_pads, ARRAY_SIZE(usdhc4_pads));
+		spl_usdhc_cfg.esdhc_base = USDHC4_BASE_ADDR;
+		spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK);
+		gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
+		break;
+	}
+
+	return fsl_esdhc_initialize(bis, &spl_usdhc_cfg);
+}
+
+void board_init_f(ulong dummy)
+{
+	/* setup AIPS and disable watchdog */
+	arch_cpu_init();
+
+	/* iomux and setup of i2c */
+	board_early_init_f();
+
+	/* setup GP timer */
+	timer_init();
+
+	/* UART clocks enabled and gd valid - init serial console */
+	preloader_console_init();
+
+	/* DDR initialization */
+	spl_dram_init();
+
+	/* Clear the BSS. */
+	memset(__bss_start, 0, __bss_end - __bss_start);
+
+	/* load/boot image from boot device */
+	board_init_r(NULL, 0);
+}
+
+void reset_cpu(ulong addr)
+{
+}
+#endif
-- 
1.9.1

[U-Boot] [PATCH v3 7/8] imx6: SPL support for iMX6 SabreSD

[Stefano Babic]

Hi John,

On 08/11/2014 22:27, John Tobias wrote:
> This patch will enable the support for SPL on iMX6 SabreSD.
> It tested on SD2 and SD3 mmc port.
> ---
>  board/freescale/mx6sabresd/mx6sabresd.c | 211 +++++++++++++++++++++++++++++++-
>  1 file changed, 209 insertions(+), 2 deletions(-)
> 
> diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c
> index 3d81fff..f443337 100644
> --- a/board/freescale/mx6sabresd/mx6sabresd.c
> +++ b/board/freescale/mx6sabresd/mx6sabresd.c
> @@ -55,8 +55,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  
>  int dram_init(void)
>  {
> -	gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
> -
> +	gd->ram_size = imx_ddr_size();
>  	return 0;
>  }
>  
> @@ -607,3 +606,211 @@ int checkboard(void)
>  	puts("Board: MX6-SabreSD\n");
>  	return 0;
>  }
> +
> +#ifdef CONFIG_SPL_BUILD
> +#include <spl.h>
> +#include <libfdt.h>
> +
> +#define BOOT_CFG	0x20D8004
> +#define __REG(x)        (*((volatile u32 *)(x)))
> +
> +struct fsl_esdhc_cfg spl_usdhc_cfg;
> +/*
> + * Got it from mx6q_4x_mt41j128.cfg file
> + */
> +void set_mt41j128_ddr(void)
> +{
> +	__REG(0x020e05a8) = 0x00000028;
> +	__REG(0x020e05b0) = 0x00000028;
> +	__REG(0x020e0524) = 0x00000028;
> +	__REG(0x020e051c) = 0x00000028;
> +
> +	__REG(0x020e0518) = 0x00000028;
> +	__REG(0x020e050c) = 0x00000028;
> +	__REG(0x020e05b8) = 0x00000028;
> +	__REG(0x020e05c0) = 0x00000028;
> +
> +	__REG(0x020e05ac) = 0x00000028;
> +	__REG(0x020e05b4) = 0x00000028;
> +	__REG(0x020e0528) = 0x00000028;
> +	__REG(0x020e0520) = 0x00000028;
> +
> +	__REG(0x020e0514) = 0x00000028;
> +	__REG(0x020e0510) = 0x00000028;
> +	__REG(0x020e05bc) = 0x00000028;
> +	__REG(0x020e05c4) = 0x00000028;
> +
> +	__REG(0x020e056c) = 0x00000030;
> +	__REG(0x020e0578) = 0x00000030;
> +	__REG(0x020e0588) = 0x00000030;
> +	__REG(0x020e0594) = 0x00000030;
> +
> +	__REG(0x020e057c) = 0x00000030;
> +	__REG(0x020e0590) = 0x00000030;
> +	__REG(0x020e0598) = 0x00000030;
> +	__REG(0x020e058c) = 0x00000000;
> +
> +	__REG(0x020e059c) = 0x00003030;
> +	__REG(0x020e05a0) = 0x00003030;
> +	__REG(0x020e0784) = 0x00000028;
> +	__REG(0x020e0788) = 0x00000028;
> +
> +	__REG(0x020e0794) = 0x00000028;
> +	__REG(0x020e079c) = 0x00000028;
> +	__REG(0x020e07a0) = 0x00000028;
> +	__REG(0x020e07a4) = 0x00000028;
> +
> +	__REG(0x020e07a8) = 0x00000028;
> +	__REG(0x020e0748) = 0x00000028;
> +	__REG(0x020e074c) = 0x00000030;
> +	__REG(0x020e0750) = 0x00020000;
> +
> +	__REG(0x020e0758) = 0x00000000;
> +	__REG(0x020e0774) = 0x00020000;
> +	__REG(0x020e078c) = 0x00000030;
> +	__REG(0x020e0798) = 0x000C0000;
> +
> +	__REG(0x021b081c) = 0x33333333;
> +	__REG(0x021b0820) = 0x33333333;
> +	__REG(0x021b0824) = 0x33333333;
> +	__REG(0x021b0828) = 0x33333333;
> +
> +	__REG(0x021b481c) = 0x33333333;
> +	__REG(0x021b4820) = 0x33333333;
> +	__REG(0x021b4824) = 0x33333333;
> +	__REG(0x021b4828) = 0x33333333;
> +
> +	__REG(0x021b0018) = 0x00001740;
> +
> +	__REG(0x021b001c) = 0x00008000;
> +	__REG(0x021b000c) = 0x8A8F7975;
> +	__REG(0x021b0010) = 0xFF538E64;
> +	__REG(0x021b0014) = 0x01FF00DB;
> +	__REG(0x021b002c) = 0x000026D2;
> +
> +	__REG(0x021b0030) = 0x008F0E21;
> +	__REG(0x021b0008) = 0x09444040;
> +	__REG(0x021b0004) = 0x00020036;
> +	__REG(0x021b0040) = 0x00000047;
> +	__REG(0x021b0000) = 0x841A0000;
> +
> +	__REG(0x021b001c) = 0x04088032;
> +	__REG(0x021b001c) = 0x00008033;
> +	__REG(0x021b001c) = 0x00428031;
> +	__REG(0x021b001c) = 0x09408030;
> +
> +	__REG(0x021b001c) = 0x04008040;
> +	__REG(0x021b0800) = 0xA1380003;
> +	__REG(0x021b0020) = 0x00005800;
> +	__REG(0x021b0818) = 0x00000007;
> +	__REG(0x021b4818) = 0x00000007;
> +
> +	/* Calibration values based on ARD and 528MHz */
> +	__REG(0x021b083c) = 0x434B0358;
> +	__REG(0x021b0840) = 0x033D033C;
> +	__REG(0x021b483c) = 0x03520362;
> +	__REG(0x021b4840) = 0x03480318;
> +	__REG(0x021b0848) = 0x41383A3C;
> +	__REG(0x021b4848) = 0x3F3C374A;
> +	__REG(0x021b0850) = 0x42434444;
> +	__REG(0x021b4850) = 0x4932473A;
> +
> +	__REG(0x021b080c) = 0x001F001F;
> +	__REG(0x021b0810) = 0x001F001F;
> +
> +	__REG(0x021b480c) = 0x001F001F;
> +	__REG(0x021b4810) = 0x001F001F;
> +
> +	__REG(0x021b08b8) = 0x00000800;
> +	__REG(0x021b48b8) = 0x00000800;
> +
> +	__REG(0x021b0404) = 0x00011006;
> +	__REG(0x021b0004) = 0x00025576;
> +
> +	__REG(0x021b001c) = 0x00000000;
> +
> +	__REG(0x020c4068) = 0x00C03F3F;
> +	__REG(0x020c406c) = 0x0030FC00;
> +	__REG(0x020c4070) = 0x0FFFC000;
> +	__REG(0x020c4074) = 0x3FF00000;
> +	__REG(0x020c4078) = 0x00FFF300;
> +	__REG(0x020c407c) = 0x0F0000C3;
> +	__REG(0x020c4080) = 0x000003FF;
> +}
> +

This cannot be accepted. Firstly, you cannot use fixed offset. The code
you propose here is very difficult to maintain.

Then, please take a look at the i.MX6 boards that are already supporting
SPL (ventana and novena). There is a logical separation between iomux,
calibration, and so on. You have to provide tables, and you should then
call the common functions:

        mx6dq_dram_iocfg();
        mx6_dram_cfg();

Again, please take a look at the boards I mentioned.

> +/*
> + * This section require the differentiation
> + * between iMX6 Sabre Families.
> + * But for now, it will configure only for
> + * SabreSD.
> + */
> +static void spl_dram_init(void)
> +{
> +	set_mt41j128_ddr();
> +}
> +
> +int spl_board_mmc_init(bd_t *bis)
> +{
> +	unsigned reg = readl(BOOT_CFG);
> +	/*
> +	 * Upon reading BOOT_CFG register the following map is done:
> +	 * (U-boot device node)    (Physical Port)
> +	 * 0x840                   SD2
> +	 * 0x1040                  SD3
> +	 * 0x1840                  eMMC
> +	 */

You add a new entry point here (spl_board_mmc_init), but why cannot you
do this in the functions already supplied by SPL ? Why not in board_init_f ?

> +	switch (reg) {
> +	case 0x840:
> +		imx_iomux_v3_setup_multiple_pads(
> +			usdhc2_pads, ARRAY_SIZE(usdhc2_pads));
> +		spl_usdhc_cfg.esdhc_base = USDHC2_BASE_ADDR;
> +		spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK);
> +		gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
> +		break;
> +	case 0x1040:
> +		imx_iomux_v3_setup_multiple_pads(
> +			usdhc3_pads, ARRAY_SIZE(usdhc3_pads));
> +		spl_usdhc_cfg.esdhc_base = USDHC3_BASE_ADDR;
> +		spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
> +		gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
> +		break;
> +	case 0x1840:
> +		imx_iomux_v3_setup_multiple_pads(
> +			usdhc4_pads, ARRAY_SIZE(usdhc4_pads));
> +		spl_usdhc_cfg.esdhc_base = USDHC4_BASE_ADDR;
> +		spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK);
> +		gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
> +		break;
> +	}
> +
> +	return fsl_esdhc_initialize(bis, &spl_usdhc_cfg);
> +}
> +
> +void board_init_f(ulong dummy)
> +{
> +	/* setup AIPS and disable watchdog */
> +	arch_cpu_init();
> +
> +	/* iomux and setup of i2c */
> +	board_early_init_f();

??

A "early" function is generally called by common code at a different
point. It makes no sense to call it inside board_init_f()

> +
> +	/* setup GP timer */
> +	timer_init();
> +

Then I am expecting iomux setuup---

> +	/* UART clocks enabled and gd valid - init serial console */
> +	preloader_console_init();
> +
> +	/* DDR initialization */
> +	spl_dram_init();

and here calling the already supplied functions.

> +
> +	/* Clear the BSS. */
> +	memset(__bss_start, 0, __bss_end - __bss_start);
> +
> +	/* load/boot image from boot device */
> +	board_init_r(NULL, 0);
> +}
> +
> +void reset_cpu(ulong addr)
> +{
> +}
> +#endif
> 

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 v3 7/8] imx6: SPL support for iMX6 SabreSD

[John Tobias]

Hi Stefano,


On Sun, Nov 9, 2014 at 1:16 PM, Stefano Babic <sbabic@denx.de> wrote:
> Hi John,
>
> On 08/11/2014 22:27, John Tobias wrote:
>> This patch will enable the support for SPL on iMX6 SabreSD.
>> It tested on SD2 and SD3 mmc port.
>> ---
>>  board/freescale/mx6sabresd/mx6sabresd.c | 211 +++++++++++++++++++++++++++++++-
>>  1 file changed, 209 insertions(+), 2 deletions(-)
>>
>> diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c
>> index 3d81fff..f443337 100644
>> --- a/board/freescale/mx6sabresd/mx6sabresd.c
>> +++ b/board/freescale/mx6sabresd/mx6sabresd.c
>> @@ -55,8 +55,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>
>>  int dram_init(void)
>>  {
>> -     gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
>> -
>> +     gd->ram_size = imx_ddr_size();
>>       return 0;
>>  }
>>
>> @@ -607,3 +606,211 @@ int checkboard(void)
>>       puts("Board: MX6-SabreSD\n");
>>       return 0;
>>  }
>> +
>> +#ifdef CONFIG_SPL_BUILD
>> +#include <spl.h>
>> +#include <libfdt.h>
>> +
>> +#define BOOT_CFG     0x20D8004
>> +#define __REG(x)        (*((volatile u32 *)(x)))
>> +
>> +struct fsl_esdhc_cfg spl_usdhc_cfg;
>> +/*
>> + * Got it from mx6q_4x_mt41j128.cfg file
>> + */
>> +void set_mt41j128_ddr(void)
>> +{
>> +     __REG(0x020e05a8) = 0x00000028;
>> +     __REG(0x020e05b0) = 0x00000028;
>> +     __REG(0x020e0524) = 0x00000028;
>> +     __REG(0x020e051c) = 0x00000028;
>> +
>> +     __REG(0x020e0518) = 0x00000028;
>> +     __REG(0x020e050c) = 0x00000028;
>> +     __REG(0x020e05b8) = 0x00000028;
>> +     __REG(0x020e05c0) = 0x00000028;
>> +
>> +     __REG(0x020e05ac) = 0x00000028;
>> +     __REG(0x020e05b4) = 0x00000028;
>> +     __REG(0x020e0528) = 0x00000028;
>> +     __REG(0x020e0520) = 0x00000028;
>> +
>> +     __REG(0x020e0514) = 0x00000028;
>> +     __REG(0x020e0510) = 0x00000028;
>> +     __REG(0x020e05bc) = 0x00000028;
>> +     __REG(0x020e05c4) = 0x00000028;
>> +
>> +     __REG(0x020e056c) = 0x00000030;
>> +     __REG(0x020e0578) = 0x00000030;
>> +     __REG(0x020e0588) = 0x00000030;
>> +     __REG(0x020e0594) = 0x00000030;
>> +
>> +     __REG(0x020e057c) = 0x00000030;
>> +     __REG(0x020e0590) = 0x00000030;
>> +     __REG(0x020e0598) = 0x00000030;
>> +     __REG(0x020e058c) = 0x00000000;
>> +
>> +     __REG(0x020e059c) = 0x00003030;
>> +     __REG(0x020e05a0) = 0x00003030;
>> +     __REG(0x020e0784) = 0x00000028;
>> +     __REG(0x020e0788) = 0x00000028;
>> +
>> +     __REG(0x020e0794) = 0x00000028;
>> +     __REG(0x020e079c) = 0x00000028;
>> +     __REG(0x020e07a0) = 0x00000028;
>> +     __REG(0x020e07a4) = 0x00000028;
>> +
>> +     __REG(0x020e07a8) = 0x00000028;
>> +     __REG(0x020e0748) = 0x00000028;
>> +     __REG(0x020e074c) = 0x00000030;
>> +     __REG(0x020e0750) = 0x00020000;
>> +
>> +     __REG(0x020e0758) = 0x00000000;
>> +     __REG(0x020e0774) = 0x00020000;
>> +     __REG(0x020e078c) = 0x00000030;
>> +     __REG(0x020e0798) = 0x000C0000;
>> +
>> +     __REG(0x021b081c) = 0x33333333;
>> +     __REG(0x021b0820) = 0x33333333;
>> +     __REG(0x021b0824) = 0x33333333;
>> +     __REG(0x021b0828) = 0x33333333;
>> +
>> +     __REG(0x021b481c) = 0x33333333;
>> +     __REG(0x021b4820) = 0x33333333;
>> +     __REG(0x021b4824) = 0x33333333;
>> +     __REG(0x021b4828) = 0x33333333;
>> +
>> +     __REG(0x021b0018) = 0x00001740;
>> +
>> +     __REG(0x021b001c) = 0x00008000;
>> +     __REG(0x021b000c) = 0x8A8F7975;
>> +     __REG(0x021b0010) = 0xFF538E64;
>> +     __REG(0x021b0014) = 0x01FF00DB;
>> +     __REG(0x021b002c) = 0x000026D2;
>> +
>> +     __REG(0x021b0030) = 0x008F0E21;
>> +     __REG(0x021b0008) = 0x09444040;
>> +     __REG(0x021b0004) = 0x00020036;
>> +     __REG(0x021b0040) = 0x00000047;
>> +     __REG(0x021b0000) = 0x841A0000;
>> +
>> +     __REG(0x021b001c) = 0x04088032;
>> +     __REG(0x021b001c) = 0x00008033;
>> +     __REG(0x021b001c) = 0x00428031;
>> +     __REG(0x021b001c) = 0x09408030;
>> +
>> +     __REG(0x021b001c) = 0x04008040;
>> +     __REG(0x021b0800) = 0xA1380003;
>> +     __REG(0x021b0020) = 0x00005800;
>> +     __REG(0x021b0818) = 0x00000007;
>> +     __REG(0x021b4818) = 0x00000007;
>> +
>> +     /* Calibration values based on ARD and 528MHz */
>> +     __REG(0x021b083c) = 0x434B0358;
>> +     __REG(0x021b0840) = 0x033D033C;
>> +     __REG(0x021b483c) = 0x03520362;
>> +     __REG(0x021b4840) = 0x03480318;
>> +     __REG(0x021b0848) = 0x41383A3C;
>> +     __REG(0x021b4848) = 0x3F3C374A;
>> +     __REG(0x021b0850) = 0x42434444;
>> +     __REG(0x021b4850) = 0x4932473A;
>> +
>> +     __REG(0x021b080c) = 0x001F001F;
>> +     __REG(0x021b0810) = 0x001F001F;
>> +
>> +     __REG(0x021b480c) = 0x001F001F;
>> +     __REG(0x021b4810) = 0x001F001F;
>> +
>> +     __REG(0x021b08b8) = 0x00000800;
>> +     __REG(0x021b48b8) = 0x00000800;
>> +
>> +     __REG(0x021b0404) = 0x00011006;
>> +     __REG(0x021b0004) = 0x00025576;
>> +
>> +     __REG(0x021b001c) = 0x00000000;
>> +
>> +     __REG(0x020c4068) = 0x00C03F3F;
>> +     __REG(0x020c406c) = 0x0030FC00;
>> +     __REG(0x020c4070) = 0x0FFFC000;
>> +     __REG(0x020c4074) = 0x3FF00000;
>> +     __REG(0x020c4078) = 0x00FFF300;
>> +     __REG(0x020c407c) = 0x0F0000C3;
>> +     __REG(0x020c4080) = 0x000003FF;
>> +}
>> +
>
> This cannot be accepted. Firstly, you cannot use fixed offset. The code
> you propose here is very difficult to maintain.
>
> Then, please take a look at the i.MX6 boards that are already supporting
> SPL (ventana and novena). There is a logical separation between iomux,
> calibration, and so on. You have to provide tables, and you should then
> call the common functions:
>
>         mx6dq_dram_iocfg();
>         mx6_dram_cfg();

The reason why I did not use the two functions you've mentioned is that the
contents of mx6q_4x_mt41j128.cfg are very difficult to convert according to the
data structures being required by the two functions above.

It's barely plain text and it requires a lot of time to look for the
register address
name it the datasheet.

>
> Again, please take a look at the boards I mentioned.
>
>> +/*
>> + * This section require the differentiation
>> + * between iMX6 Sabre Families.
>> + * But for now, it will configure only for
>> + * SabreSD.
>> + */
>> +static void spl_dram_init(void)
>> +{
>> +     set_mt41j128_ddr();
>> +}
>> +
>> +int spl_board_mmc_init(bd_t *bis)
>> +{
>> +     unsigned reg = readl(BOOT_CFG);
>> +     /*
>> +      * Upon reading BOOT_CFG register the following map is done:
>> +      * (U-boot device node)    (Physical Port)
>> +      * 0x840                   SD2
>> +      * 0x1040                  SD3
>> +      * 0x1840                  eMMC
>> +      */
>
> You add a new entry point here (spl_board_mmc_init), but why cannot you
> do this in the functions already supplied by SPL ? Why not in board_init_f ?

When the spl_mmc_load_image function being called, it call
mmc_initialize. By default, the mmc_initialize
call board_mmc_init. By looking the said function, it initialize all mmc ports.

While, in spl_board_mmc_init, only initialize the current mmc port.

>
>> +     switch (reg) {
>> +     case 0x840:
>> +             imx_iomux_v3_setup_multiple_pads(
>> +                     usdhc2_pads, ARRAY_SIZE(usdhc2_pads));
>> +             spl_usdhc_cfg.esdhc_base = USDHC2_BASE_ADDR;
>> +             spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK);
>> +             gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
>> +             break;
>> +     case 0x1040:
>> +             imx_iomux_v3_setup_multiple_pads(
>> +                     usdhc3_pads, ARRAY_SIZE(usdhc3_pads));
>> +             spl_usdhc_cfg.esdhc_base = USDHC3_BASE_ADDR;
>> +             spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
>> +             gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
>> +             break;
>> +     case 0x1840:
>> +             imx_iomux_v3_setup_multiple_pads(
>> +                     usdhc4_pads, ARRAY_SIZE(usdhc4_pads));
>> +             spl_usdhc_cfg.esdhc_base = USDHC4_BASE_ADDR;
>> +             spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK);
>> +             gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
>> +             break;
>> +     }
>> +
>> +     return fsl_esdhc_initialize(bis, &spl_usdhc_cfg);
>> +}
>> +
>> +void board_init_f(ulong dummy)
>> +{
>> +     /* setup AIPS and disable watchdog */
>> +     arch_cpu_init();
>> +
>> +     /* iomux and setup of i2c */
>> +     board_early_init_f();
>
> ??
>
> A "early" function is generally called by common code at a different
> point. It makes no sense to call it inside board_init_f()
>
>> +
>> +     /* setup GP timer */
>> +     timer_init();
>> +
>
> Then I am expecting iomux setuup---

The iomux setup being called in board_early_init_f...

>
>> +     /* UART clocks enabled and gd valid - init serial console */
>> +     preloader_console_init();
>> +
>> +     /* DDR initialization */
>> +     spl_dram_init();
>
> and here calling the already supplied functions.
>
>> +
>> +     /* Clear the BSS. */
>> +     memset(__bss_start, 0, __bss_end - __bss_start);
>> +
>> +     /* load/boot image from boot device */
>> +     board_init_r(NULL, 0);
>> +}
>> +
>> +void reset_cpu(ulong addr)
>> +{
>> +}
>> +#endif
>>
>
> 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
> =====================================================================


Regards,

john

[U-Boot] [PATCH v3 7/8] imx6: SPL support for iMX6 SabreSD

[Stefano Babic]

Hi John,

On 10/11/2014 01:23, John Tobias wrote:
> Hi Stefano,
> 
> 
> On Sun, Nov 9, 2014 at 1:16 PM, Stefano Babic <sbabic@denx.de> wrote:
>> Hi John,
>>
>> On 08/11/2014 22:27, John Tobias wrote:
>>> This patch will enable the support for SPL on iMX6 SabreSD.
>>> It tested on SD2 and SD3 mmc port.
>>> ---
>>>  board/freescale/mx6sabresd/mx6sabresd.c | 211 +++++++++++++++++++++++++++++++-
>>>  1 file changed, 209 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c
>>> index 3d81fff..f443337 100644
>>> --- a/board/freescale/mx6sabresd/mx6sabresd.c
>>> +++ b/board/freescale/mx6sabresd/mx6sabresd.c
>>> @@ -55,8 +55,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>>
>>>  int dram_init(void)
>>>  {
>>> -     gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
>>> -
>>> +     gd->ram_size = imx_ddr_size();
>>>       return 0;
>>>  }
>>>
>>> @@ -607,3 +606,211 @@ int checkboard(void)
>>>       puts("Board: MX6-SabreSD\n");
>>>       return 0;
>>>  }
>>> +
>>> +#ifdef CONFIG_SPL_BUILD
>>> +#include <spl.h>
>>> +#include <libfdt.h>
>>> +
>>> +#define BOOT_CFG     0x20D8004
>>> +#define __REG(x)        (*((volatile u32 *)(x)))
>>> +
>>> +struct fsl_esdhc_cfg spl_usdhc_cfg;
>>> +/*
>>> + * Got it from mx6q_4x_mt41j128.cfg file
>>> + */
>>> +void set_mt41j128_ddr(void)
>>> +{
>>> +     __REG(0x020e05a8) = 0x00000028;
>>> +     __REG(0x020e05b0) = 0x00000028;
>>> +     __REG(0x020e0524) = 0x00000028;
>>> +     __REG(0x020e051c) = 0x00000028;
>>> +
>>> +     __REG(0x020e0518) = 0x00000028;
>>> +     __REG(0x020e050c) = 0x00000028;
>>> +     __REG(0x020e05b8) = 0x00000028;
>>> +     __REG(0x020e05c0) = 0x00000028;
>>> +
>>> +     __REG(0x020e05ac) = 0x00000028;
>>> +     __REG(0x020e05b4) = 0x00000028;
>>> +     __REG(0x020e0528) = 0x00000028;
>>> +     __REG(0x020e0520) = 0x00000028;
>>> +
>>> +     __REG(0x020e0514) = 0x00000028;
>>> +     __REG(0x020e0510) = 0x00000028;
>>> +     __REG(0x020e05bc) = 0x00000028;
>>> +     __REG(0x020e05c4) = 0x00000028;
>>> +
>>> +     __REG(0x020e056c) = 0x00000030;
>>> +     __REG(0x020e0578) = 0x00000030;
>>> +     __REG(0x020e0588) = 0x00000030;
>>> +     __REG(0x020e0594) = 0x00000030;
>>> +
>>> +     __REG(0x020e057c) = 0x00000030;
>>> +     __REG(0x020e0590) = 0x00000030;
>>> +     __REG(0x020e0598) = 0x00000030;
>>> +     __REG(0x020e058c) = 0x00000000;
>>> +
>>> +     __REG(0x020e059c) = 0x00003030;
>>> +     __REG(0x020e05a0) = 0x00003030;
>>> +     __REG(0x020e0784) = 0x00000028;
>>> +     __REG(0x020e0788) = 0x00000028;
>>> +
>>> +     __REG(0x020e0794) = 0x00000028;
>>> +     __REG(0x020e079c) = 0x00000028;
>>> +     __REG(0x020e07a0) = 0x00000028;
>>> +     __REG(0x020e07a4) = 0x00000028;
>>> +
>>> +     __REG(0x020e07a8) = 0x00000028;
>>> +     __REG(0x020e0748) = 0x00000028;
>>> +     __REG(0x020e074c) = 0x00000030;
>>> +     __REG(0x020e0750) = 0x00020000;
>>> +
>>> +     __REG(0x020e0758) = 0x00000000;
>>> +     __REG(0x020e0774) = 0x00020000;
>>> +     __REG(0x020e078c) = 0x00000030;
>>> +     __REG(0x020e0798) = 0x000C0000;
>>> +
>>> +     __REG(0x021b081c) = 0x33333333;
>>> +     __REG(0x021b0820) = 0x33333333;
>>> +     __REG(0x021b0824) = 0x33333333;
>>> +     __REG(0x021b0828) = 0x33333333;
>>> +
>>> +     __REG(0x021b481c) = 0x33333333;
>>> +     __REG(0x021b4820) = 0x33333333;
>>> +     __REG(0x021b4824) = 0x33333333;
>>> +     __REG(0x021b4828) = 0x33333333;
>>> +
>>> +     __REG(0x021b0018) = 0x00001740;
>>> +
>>> +     __REG(0x021b001c) = 0x00008000;
>>> +     __REG(0x021b000c) = 0x8A8F7975;
>>> +     __REG(0x021b0010) = 0xFF538E64;
>>> +     __REG(0x021b0014) = 0x01FF00DB;
>>> +     __REG(0x021b002c) = 0x000026D2;
>>> +
>>> +     __REG(0x021b0030) = 0x008F0E21;
>>> +     __REG(0x021b0008) = 0x09444040;
>>> +     __REG(0x021b0004) = 0x00020036;
>>> +     __REG(0x021b0040) = 0x00000047;
>>> +     __REG(0x021b0000) = 0x841A0000;
>>> +
>>> +     __REG(0x021b001c) = 0x04088032;
>>> +     __REG(0x021b001c) = 0x00008033;
>>> +     __REG(0x021b001c) = 0x00428031;
>>> +     __REG(0x021b001c) = 0x09408030;
>>> +
>>> +     __REG(0x021b001c) = 0x04008040;
>>> +     __REG(0x021b0800) = 0xA1380003;
>>> +     __REG(0x021b0020) = 0x00005800;
>>> +     __REG(0x021b0818) = 0x00000007;
>>> +     __REG(0x021b4818) = 0x00000007;
>>> +
>>> +     /* Calibration values based on ARD and 528MHz */
>>> +     __REG(0x021b083c) = 0x434B0358;
>>> +     __REG(0x021b0840) = 0x033D033C;
>>> +     __REG(0x021b483c) = 0x03520362;
>>> +     __REG(0x021b4840) = 0x03480318;
>>> +     __REG(0x021b0848) = 0x41383A3C;
>>> +     __REG(0x021b4848) = 0x3F3C374A;
>>> +     __REG(0x021b0850) = 0x42434444;
>>> +     __REG(0x021b4850) = 0x4932473A;
>>> +
>>> +     __REG(0x021b080c) = 0x001F001F;
>>> +     __REG(0x021b0810) = 0x001F001F;
>>> +
>>> +     __REG(0x021b480c) = 0x001F001F;
>>> +     __REG(0x021b4810) = 0x001F001F;
>>> +
>>> +     __REG(0x021b08b8) = 0x00000800;
>>> +     __REG(0x021b48b8) = 0x00000800;
>>> +
>>> +     __REG(0x021b0404) = 0x00011006;
>>> +     __REG(0x021b0004) = 0x00025576;
>>> +
>>> +     __REG(0x021b001c) = 0x00000000;
>>> +
>>> +     __REG(0x020c4068) = 0x00C03F3F;
>>> +     __REG(0x020c406c) = 0x0030FC00;
>>> +     __REG(0x020c4070) = 0x0FFFC000;
>>> +     __REG(0x020c4074) = 0x3FF00000;
>>> +     __REG(0x020c4078) = 0x00FFF300;
>>> +     __REG(0x020c407c) = 0x0F0000C3;
>>> +     __REG(0x020c4080) = 0x000003FF;
>>> +}
>>> +
>>
>> This cannot be accepted. Firstly, you cannot use fixed offset. The code
>> you propose here is very difficult to maintain.
>>
>> Then, please take a look at the i.MX6 boards that are already supporting
>> SPL (ventana and novena). There is a logical separation between iomux,
>> calibration, and so on. You have to provide tables, and you should then
>> call the common functions:
>>
>>         mx6dq_dram_iocfg();
>>         mx6_dram_cfg();
> 
> The reason why I did not use the two functions you've mentioned is that the
> contents of mx6q_4x_mt41j128.cfg are very difficult to convert according to the
> data structures being required by the two functions above.

Yes, but these functions are the result of a very long discussion on the
ML about how it is possible to support several flavours of the SOC
(MX6Q, MX6DL), having one single binary for all of them.

> 
> It's barely plain text and it requires a lot of time to look for the
> register address
> name it the datasheet.

This is also a reason to get rid of them. DCD tables are more difficult
to maintain. However, the .cfg file can be parsed by the compiler. The
sabresd was added to mainline before the patch for gcc was added. See
other i.MX6 boards, for example nitrogen6x.

> 
>>
>> Again, please take a look at the boards I mentioned.
>>
>>> +/*
>>> + * This section require the differentiation
>>> + * between iMX6 Sabre Families.
>>> + * But for now, it will configure only for
>>> + * SabreSD.
>>> + */
>>> +static void spl_dram_init(void)
>>> +{
>>> +     set_mt41j128_ddr();
>>> +}
>>> +
>>> +int spl_board_mmc_init(bd_t *bis)
>>> +{
>>> +     unsigned reg = readl(BOOT_CFG);
>>> +     /*
>>> +      * Upon reading BOOT_CFG register the following map is done:
>>> +      * (U-boot device node)    (Physical Port)
>>> +      * 0x840                   SD2
>>> +      * 0x1040                  SD3
>>> +      * 0x1840                  eMMC
>>> +      */
>>
>> You add a new entry point here (spl_board_mmc_init), but why cannot you
>> do this in the functions already supplied by SPL ? Why not in board_init_f ?
> 
> When the spl_mmc_load_image function being called, it call
> mmc_initialize. By default, the mmc_initialize
> call board_mmc_init. By looking the said function, it initialize all mmc ports.
> 
> While, in spl_board_mmc_init, only initialize the current mmc port.

ok - but which is the issue by initializing all ports ? I mean, if
board_mmc_init initialize all ports including what you need in SPL,
which is the reason to exclude the other ones ?

> 
>>
>>> +     switch (reg) {
>>> +     case 0x840:
>>> +             imx_iomux_v3_setup_multiple_pads(
>>> +                     usdhc2_pads, ARRAY_SIZE(usdhc2_pads));
>>> +             spl_usdhc_cfg.esdhc_base = USDHC2_BASE_ADDR;
>>> +             spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK);
>>> +             gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
>>> +             break;
>>> +     case 0x1040:
>>> +             imx_iomux_v3_setup_multiple_pads(
>>> +                     usdhc3_pads, ARRAY_SIZE(usdhc3_pads));
>>> +             spl_usdhc_cfg.esdhc_base = USDHC3_BASE_ADDR;
>>> +             spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
>>> +             gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
>>> +             break;
>>> +     case 0x1840:
>>> +             imx_iomux_v3_setup_multiple_pads(
>>> +                     usdhc4_pads, ARRAY_SIZE(usdhc4_pads));
>>> +             spl_usdhc_cfg.esdhc_base = USDHC4_BASE_ADDR;
>>> +             spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK);
>>> +             gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
>>> +             break;
>>> +     }
>>> +
>>> +     return fsl_esdhc_initialize(bis, &spl_usdhc_cfg);
>>> +}
>>> +
>>> +void board_init_f(ulong dummy)
>>> +{
>>> +     /* setup AIPS and disable watchdog */
>>> +     arch_cpu_init();
>>> +
>>> +     /* iomux and setup of i2c */
>>> +     board_early_init_f();
>>
>> ??
>>
>> A "early" function is generally called by common code at a different
>> point. It makes no sense to call it inside board_init_f()
>>
>>> +
>>> +     /* setup GP timer */
>>> +     timer_init();
>>> +
>>
>> Then I am expecting iomux setuup---
> 
> The iomux setup being called in board_early_init_f...
> 
>>
>>> +     /* UART clocks enabled and gd valid - init serial console */
>>> +     preloader_console_init();
>>> +
>>> +     /* DDR initialization */
>>> +     spl_dram_init();
>>
>> and here calling the already supplied functions.
>>
>>> +
>>> +     /* Clear the BSS. */
>>> +     memset(__bss_start, 0, __bss_end - __bss_start);
>>> +
>>> +     /* load/boot image from boot device */
>>> +     board_init_r(NULL, 0);
>>> +}
>>> +
>>> +void reset_cpu(ulong addr)
>>> +{
>>> +}
>>> +#endif
>>>
>>

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 v3 7/8] imx6: SPL support for iMX6 SabreSD

[John Tobias]

On Tue, Nov 11, 2014 at 12:44 AM, Stefano Babic <sbabic@denx.de> wrote:
> Hi John,
>
> On 10/11/2014 01:23, John Tobias wrote:
>> Hi Stefano,
>>
>>
>> On Sun, Nov 9, 2014 at 1:16 PM, Stefano Babic <sbabic@denx.de> wrote:
>>> Hi John,
>>>
>>> On 08/11/2014 22:27, John Tobias wrote:
>>>> This patch will enable the support for SPL on iMX6 SabreSD.
>>>> It tested on SD2 and SD3 mmc port.
>>>> ---
>>>>  board/freescale/mx6sabresd/mx6sabresd.c | 211 +++++++++++++++++++++++++++++++-
>>>>  1 file changed, 209 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c
>>>> index 3d81fff..f443337 100644
>>>> --- a/board/freescale/mx6sabresd/mx6sabresd.c
>>>> +++ b/board/freescale/mx6sabresd/mx6sabresd.c
>>>> @@ -55,8 +55,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>>>
>>>>  int dram_init(void)
>>>>  {
>>>> -     gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
>>>> -
>>>> +     gd->ram_size = imx_ddr_size();
>>>>       return 0;
>>>>  }
>>>>
>>>> @@ -607,3 +606,211 @@ int checkboard(void)
>>>>       puts("Board: MX6-SabreSD\n");
>>>>       return 0;
>>>>  }
>>>> +
>>>> +#ifdef CONFIG_SPL_BUILD
>>>> +#include <spl.h>
>>>> +#include <libfdt.h>
>>>> +
>>>> +#define BOOT_CFG     0x20D8004
>>>> +#define __REG(x)        (*((volatile u32 *)(x)))
>>>> +
>>>> +struct fsl_esdhc_cfg spl_usdhc_cfg;
>>>> +/*
>>>> + * Got it from mx6q_4x_mt41j128.cfg file
>>>> + */
>>>> +void set_mt41j128_ddr(void)
>>>> +{
>>>> +     __REG(0x020e05a8) = 0x00000028;
>>>> +     __REG(0x020e05b0) = 0x00000028;
>>>> +     __REG(0x020e0524) = 0x00000028;
>>>> +     __REG(0x020e051c) = 0x00000028;
>>>> +
>>>> +     __REG(0x020e0518) = 0x00000028;
>>>> +     __REG(0x020e050c) = 0x00000028;
>>>> +     __REG(0x020e05b8) = 0x00000028;
>>>> +     __REG(0x020e05c0) = 0x00000028;
>>>> +
>>>> +     __REG(0x020e05ac) = 0x00000028;
>>>> +     __REG(0x020e05b4) = 0x00000028;
>>>> +     __REG(0x020e0528) = 0x00000028;
>>>> +     __REG(0x020e0520) = 0x00000028;
>>>> +
>>>> +     __REG(0x020e0514) = 0x00000028;
>>>> +     __REG(0x020e0510) = 0x00000028;
>>>> +     __REG(0x020e05bc) = 0x00000028;
>>>> +     __REG(0x020e05c4) = 0x00000028;
>>>> +
>>>> +     __REG(0x020e056c) = 0x00000030;
>>>> +     __REG(0x020e0578) = 0x00000030;
>>>> +     __REG(0x020e0588) = 0x00000030;
>>>> +     __REG(0x020e0594) = 0x00000030;
>>>> +
>>>> +     __REG(0x020e057c) = 0x00000030;
>>>> +     __REG(0x020e0590) = 0x00000030;
>>>> +     __REG(0x020e0598) = 0x00000030;
>>>> +     __REG(0x020e058c) = 0x00000000;
>>>> +
>>>> +     __REG(0x020e059c) = 0x00003030;
>>>> +     __REG(0x020e05a0) = 0x00003030;
>>>> +     __REG(0x020e0784) = 0x00000028;
>>>> +     __REG(0x020e0788) = 0x00000028;
>>>> +
>>>> +     __REG(0x020e0794) = 0x00000028;
>>>> +     __REG(0x020e079c) = 0x00000028;
>>>> +     __REG(0x020e07a0) = 0x00000028;
>>>> +     __REG(0x020e07a4) = 0x00000028;
>>>> +
>>>> +     __REG(0x020e07a8) = 0x00000028;
>>>> +     __REG(0x020e0748) = 0x00000028;
>>>> +     __REG(0x020e074c) = 0x00000030;
>>>> +     __REG(0x020e0750) = 0x00020000;
>>>> +
>>>> +     __REG(0x020e0758) = 0x00000000;
>>>> +     __REG(0x020e0774) = 0x00020000;
>>>> +     __REG(0x020e078c) = 0x00000030;
>>>> +     __REG(0x020e0798) = 0x000C0000;
>>>> +
>>>> +     __REG(0x021b081c) = 0x33333333;
>>>> +     __REG(0x021b0820) = 0x33333333;
>>>> +     __REG(0x021b0824) = 0x33333333;
>>>> +     __REG(0x021b0828) = 0x33333333;
>>>> +
>>>> +     __REG(0x021b481c) = 0x33333333;
>>>> +     __REG(0x021b4820) = 0x33333333;
>>>> +     __REG(0x021b4824) = 0x33333333;
>>>> +     __REG(0x021b4828) = 0x33333333;
>>>> +
>>>> +     __REG(0x021b0018) = 0x00001740;
>>>> +
>>>> +     __REG(0x021b001c) = 0x00008000;
>>>> +     __REG(0x021b000c) = 0x8A8F7975;
>>>> +     __REG(0x021b0010) = 0xFF538E64;
>>>> +     __REG(0x021b0014) = 0x01FF00DB;
>>>> +     __REG(0x021b002c) = 0x000026D2;
>>>> +
>>>> +     __REG(0x021b0030) = 0x008F0E21;
>>>> +     __REG(0x021b0008) = 0x09444040;
>>>> +     __REG(0x021b0004) = 0x00020036;
>>>> +     __REG(0x021b0040) = 0x00000047;
>>>> +     __REG(0x021b0000) = 0x841A0000;
>>>> +
>>>> +     __REG(0x021b001c) = 0x04088032;
>>>> +     __REG(0x021b001c) = 0x00008033;
>>>> +     __REG(0x021b001c) = 0x00428031;
>>>> +     __REG(0x021b001c) = 0x09408030;
>>>> +
>>>> +     __REG(0x021b001c) = 0x04008040;
>>>> +     __REG(0x021b0800) = 0xA1380003;
>>>> +     __REG(0x021b0020) = 0x00005800;
>>>> +     __REG(0x021b0818) = 0x00000007;
>>>> +     __REG(0x021b4818) = 0x00000007;
>>>> +
>>>> +     /* Calibration values based on ARD and 528MHz */
>>>> +     __REG(0x021b083c) = 0x434B0358;
>>>> +     __REG(0x021b0840) = 0x033D033C;
>>>> +     __REG(0x021b483c) = 0x03520362;
>>>> +     __REG(0x021b4840) = 0x03480318;
>>>> +     __REG(0x021b0848) = 0x41383A3C;
>>>> +     __REG(0x021b4848) = 0x3F3C374A;
>>>> +     __REG(0x021b0850) = 0x42434444;
>>>> +     __REG(0x021b4850) = 0x4932473A;
>>>> +
>>>> +     __REG(0x021b080c) = 0x001F001F;
>>>> +     __REG(0x021b0810) = 0x001F001F;
>>>> +
>>>> +     __REG(0x021b480c) = 0x001F001F;
>>>> +     __REG(0x021b4810) = 0x001F001F;
>>>> +
>>>> +     __REG(0x021b08b8) = 0x00000800;
>>>> +     __REG(0x021b48b8) = 0x00000800;
>>>> +
>>>> +     __REG(0x021b0404) = 0x00011006;
>>>> +     __REG(0x021b0004) = 0x00025576;
>>>> +
>>>> +     __REG(0x021b001c) = 0x00000000;
>>>> +
>>>> +     __REG(0x020c4068) = 0x00C03F3F;
>>>> +     __REG(0x020c406c) = 0x0030FC00;
>>>> +     __REG(0x020c4070) = 0x0FFFC000;
>>>> +     __REG(0x020c4074) = 0x3FF00000;
>>>> +     __REG(0x020c4078) = 0x00FFF300;
>>>> +     __REG(0x020c407c) = 0x0F0000C3;
>>>> +     __REG(0x020c4080) = 0x000003FF;
>>>> +}
>>>> +
>>>
>>> This cannot be accepted. Firstly, you cannot use fixed offset. The code
>>> you propose here is very difficult to maintain.
>>>
>>> Then, please take a look at the i.MX6 boards that are already supporting
>>> SPL (ventana and novena). There is a logical separation between iomux,
>>> calibration, and so on. You have to provide tables, and you should then
>>> call the common functions:
>>>
>>>         mx6dq_dram_iocfg();
>>>         mx6_dram_cfg();
>>
>> The reason why I did not use the two functions you've mentioned is that the
>> contents of mx6q_4x_mt41j128.cfg are very difficult to convert according to the
>> data structures being required by the two functions above.
>
> Yes, but these functions are the result of a very long discussion on the
> ML about how it is possible to support several flavours of the SOC
> (MX6Q, MX6DL), having one single binary for all of them.
>
>>
>> It's barely plain text and it requires a lot of time to look for the
>> register address
>> name it the datasheet.
>
> This is also a reason to get rid of them. DCD tables are more difficult
> to maintain. However, the .cfg file can be parsed by the compiler. The
> sabresd was added to mainline before the patch for gcc was added. See
> other i.MX6 boards, for example nitrogen6x.
>>
>>>
>>> Again, please take a look at the boards I mentioned.
>>>
>>>> +/*
>>>> + * This section require the differentiation
>>>> + * between iMX6 Sabre Families.
>>>> + * But for now, it will configure only for
>>>> + * SabreSD.
>>>> + */
>>>> +static void spl_dram_init(void)
>>>> +{
>>>> +     set_mt41j128_ddr();
>>>> +}
>>>> +
>>>> +int spl_board_mmc_init(bd_t *bis)
>>>> +{
>>>> +     unsigned reg = readl(BOOT_CFG);
>>>> +     /*
>>>> +      * Upon reading BOOT_CFG register the following map is done:
>>>> +      * (U-boot device node)    (Physical Port)
>>>> +      * 0x840                   SD2
>>>> +      * 0x1040                  SD3
>>>> +      * 0x1840                  eMMC
>>>> +      */
>>>
>>> You add a new entry point here (spl_board_mmc_init), but why cannot you
>>> do this in the functions already supplied by SPL ? Why not in board_init_f ?
>>
>> When the spl_mmc_load_image function being called, it call
>> mmc_initialize. By default, the mmc_initialize
>> call board_mmc_init. By looking the said function, it initialize all mmc ports.
>>
>> While, in spl_board_mmc_init, only initialize the current mmc port.
>
> ok - but which is the issue by initializing all ports ? I mean, if
> board_mmc_init initialize all ports including what you need in SPL,
> which is the reason to exclude the other ones ?
>

When board_mmc_init initialize all ports, we can issue a command
at uboot console to switch to different port.
e.g:

mmc dev 0 (e.g. SD4)
mmc dev 1  (e.g. SD2)
mmc dev 2 (e.g. SD3)

We can also re-map which is index 0, 1 and 2 by re-arranging the contents of

struct fsl_esdhc_cfg usdhc_cfg[3] = {
    {USDHC4_BASE_ADDR},
    {USDHC2_BASE_ADDR},
    {USDHC3_BASE_ADDR},
};

Assuming the bootstrap is configured to port 3 (USDHC3). The SPL image will
call spl_mmc_load_image to load the uboot image. In order to do that,
it will call
the find_mmc_device(0), based on example above the index 0 is port 4, index 1
is port 2 and index 2 is port 3. The find_mmc_device will return with errors
because it couldn't initialize the said port.

So by default, the user should switch back the bootstrap to port 4 in
order to load the
uboot correctly. Then, if the user really want to use the different
ports, they have
to change the arrangement of the contents of usdhc_cfg and compile the
u-boot again.

In spl_board_mmc_init function, it will read the bootstrap and configure it. The
settings are passed to fsl_esdhc_initialize which is to be stored in
index 0. So,
find_mmc_device(0) could get a correct information.

Then, switching to different bootstrap doesn't require to re-compile the image.

>>
>>>
>>>> +     switch (reg) {
>>>> +     case 0x840:
>>>> +             imx_iomux_v3_setup_multiple_pads(
>>>> +                     usdhc2_pads, ARRAY_SIZE(usdhc2_pads));
>>>> +             spl_usdhc_cfg.esdhc_base = USDHC2_BASE_ADDR;
>>>> +             spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK);
>>>> +             gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
>>>> +             break;
>>>> +     case 0x1040:
>>>> +             imx_iomux_v3_setup_multiple_pads(
>>>> +                     usdhc3_pads, ARRAY_SIZE(usdhc3_pads));
>>>> +             spl_usdhc_cfg.esdhc_base = USDHC3_BASE_ADDR;
>>>> +             spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
>>>> +             gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
>>>> +             break;
>>>> +     case 0x1840:
>>>> +             imx_iomux_v3_setup_multiple_pads(
>>>> +                     usdhc4_pads, ARRAY_SIZE(usdhc4_pads));
>>>> +             spl_usdhc_cfg.esdhc_base = USDHC4_BASE_ADDR;
>>>> +             spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK);
>>>> +             gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk;
>>>> +             break;
>>>> +     }
>>>> +
>>>> +     return fsl_esdhc_initialize(bis, &spl_usdhc_cfg);
>>>> +}
>>>> +
>>>> +void board_init_f(ulong dummy)
>>>> +{
>>>> +     /* setup AIPS and disable watchdog */
>>>> +     arch_cpu_init();
>>>> +
>>>> +     /* iomux and setup of i2c */
>>>> +     board_early_init_f();
>>>
>>> ??
>>>
>>> A "early" function is generally called by common code at a different
>>> point. It makes no sense to call it inside board_init_f()
>>>
>>>> +
>>>> +     /* setup GP timer */
>>>> +     timer_init();
>>>> +
>>>
>>> Then I am expecting iomux setuup---
>>
>> The iomux setup being called in board_early_init_f...
>>
>>>
>>>> +     /* UART clocks enabled and gd valid - init serial console */
>>>> +     preloader_console_init();
>>>> +
>>>> +     /* DDR initialization */
>>>> +     spl_dram_init();
>>>
>>> and here calling the already supplied functions.
>>>
>>>> +
>>>> +     /* Clear the BSS. */
>>>> +     memset(__bss_start, 0, __bss_end - __bss_start);
>>>> +
>>>> +     /* load/boot image from boot device */
>>>> +     board_init_r(NULL, 0);
>>>> +}
>>>> +
>>>> +void reset_cpu(ulong addr)
>>>> +{
>>>> +}
>>>> +#endif
>>>>
>>>
>
> 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 v3 7/8] imx6: SPL support for iMX6 SabreSD

[Stefano Babic]

Hi John,

On 11/11/2014 18:15, John Tobias wrote:

>>>> You add a new entry point here (spl_board_mmc_init), but why cannot you
>>>> do this in the functions already supplied by SPL ? Why not in board_init_f ?
>>>
>>> When the spl_mmc_load_image function being called, it call
>>> mmc_initialize. By default, the mmc_initialize
>>> call board_mmc_init. By looking the said function, it initialize all mmc ports.
>>>
>>> While, in spl_board_mmc_init, only initialize the current mmc port.
>>
>> ok - but which is the issue by initializing all ports ? I mean, if
>> board_mmc_init initialize all ports including what you need in SPL,
>> which is the reason to exclude the other ones ?
>>
> 
> When board_mmc_init initialize all ports, we can issue a command
> at uboot console to switch to different port.
> e.g:
> 
> mmc dev 0 (e.g. SD4)
> mmc dev 1  (e.g. SD2)
> mmc dev 2 (e.g. SD3)
> 
> We can also re-map which is index 0, 1 and 2 by re-arranging the contents of
> 
> struct fsl_esdhc_cfg usdhc_cfg[3] = {
>     {USDHC4_BASE_ADDR},
>     {USDHC2_BASE_ADDR},
>     {USDHC3_BASE_ADDR},
> };
> 
> Assuming the bootstrap is configured to port 3 (USDHC3). The SPL image will
> call spl_mmc_load_image to load the uboot image. In order to do that,
> it will call
> the find_mmc_device(0), based on example above the index 0 is port 4, index 1
> is port 2 and index 2 is port 3. The find_mmc_device will return with errors
> because it couldn't initialize the said port.
> 
> So by default, the user should switch back the bootstrap to port 4 in
> order to load the
> uboot correctly. Then, if the user really want to use the different
> ports, they have
> to change the arrangement of the contents of usdhc_cfg and compile the
> u-boot again.
> 
> In spl_board_mmc_init function, it will read the bootstrap and configure it. The
> settings are passed to fsl_esdhc_initialize which is to be stored in
> index 0. So,
> find_mmc_device(0) could get a correct information.
> 
> Then, switching to different bootstrap doesn't require to re-compile the image.

This is not enough for adding a new exported weak function, that reamins
undocumented, and does not avoid using #ifdef, only moved into
drivers/mmc/mmc.c.

We have other cases where the behavior is different between SPL and
U-Boot, and this is handled in board file using the same #ifdef
CONFIG_SPL_BUILD.

The question is: why do you need to change the API (as I said, it
remains undocumented) instead of doing :

int board_mmc_init(bd_t *bis) {

#ifdef CONFIG_SPL_BUILD
	<initialization single port based on bootstrap>
#else
	<instantiate all ports as usual>
#endif

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 v3 7/8] imx6: SPL support for iMX6 SabreSD

[John Tobias]

Hi Stefano,

That's fine.. I'll fix it.

Regards,

john

On Wed, Nov 12, 2014 at 2:13 AM, Stefano Babic <sbabic@denx.de> wrote:
> Hi John,
>
> On 11/11/2014 18:15, John Tobias wrote:
>
>>>>> You add a new entry point here (spl_board_mmc_init), but why cannot you
>>>>> do this in the functions already supplied by SPL ? Why not in board_init_f ?
>>>>
>>>> When the spl_mmc_load_image function being called, it call
>>>> mmc_initialize. By default, the mmc_initialize
>>>> call board_mmc_init. By looking the said function, it initialize all mmc ports.
>>>>
>>>> While, in spl_board_mmc_init, only initialize the current mmc port.
>>>
>>> ok - but which is the issue by initializing all ports ? I mean, if
>>> board_mmc_init initialize all ports including what you need in SPL,
>>> which is the reason to exclude the other ones ?
>>>
>>
>> When board_mmc_init initialize all ports, we can issue a command
>> at uboot console to switch to different port.
>> e.g:
>>
>> mmc dev 0 (e.g. SD4)
>> mmc dev 1  (e.g. SD2)
>> mmc dev 2 (e.g. SD3)
>>
>> We can also re-map which is index 0, 1 and 2 by re-arranging the contents of
>>
>> struct fsl_esdhc_cfg usdhc_cfg[3] = {
>>     {USDHC4_BASE_ADDR},
>>     {USDHC2_BASE_ADDR},
>>     {USDHC3_BASE_ADDR},
>> };
>>
>> Assuming the bootstrap is configured to port 3 (USDHC3). The SPL image will
>> call spl_mmc_load_image to load the uboot image. In order to do that,
>> it will call
>> the find_mmc_device(0), based on example above the index 0 is port 4, index 1
>> is port 2 and index 2 is port 3. The find_mmc_device will return with errors
>> because it couldn't initialize the said port.
>>
>> So by default, the user should switch back the bootstrap to port 4 in
>> order to load the
>> uboot correctly. Then, if the user really want to use the different
>> ports, they have
>> to change the arrangement of the contents of usdhc_cfg and compile the
>> u-boot again.
>>
>> In spl_board_mmc_init function, it will read the bootstrap and configure it. The
>> settings are passed to fsl_esdhc_initialize which is to be stored in
>> index 0. So,
>> find_mmc_device(0) could get a correct information.
>>
>> Then, switching to different bootstrap doesn't require to re-compile the image.
>
> This is not enough for adding a new exported weak function, that reamins
> undocumented, and does not avoid using #ifdef, only moved into
> drivers/mmc/mmc.c.
>
> We have other cases where the behavior is different between SPL and
> U-Boot, and this is handled in board file using the same #ifdef
> CONFIG_SPL_BUILD.
>
> The question is: why do you need to change the API (as I said, it
> remains undocumented) instead of doing :
>
> int board_mmc_init(bd_t *bis) {
>
> #ifdef CONFIG_SPL_BUILD
>         <initialization single port based on bootstrap>
> #else
>         <instantiate all ports as usual>
> #endif
>
> 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 v3 8/8] kconfig: imx6: add SUPPORT_SPL

[John Tobias]

add SUPPORT_SPL for iMX6 SabreSD by default
---
 arch/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 22eb2d5..ab0d284 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -609,6 +609,7 @@ config TARGET_MX6QSABREAUTO
 config TARGET_MX6SABRESD
 	bool "Support mx6sabresd"
 	select CPU_V7
+	select SUPPORT_SPL
 
 config TARGET_MX6SLEVK
 	bool "Support mx6slevk"
-- 
1.9.1