[U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM

Stefano Babic sbabic at denx.de
Wed Aug 28 18:08:37 CEST 2013


Hi Tapani,

On 28/08/2013 13:23, Tapani wrote:
> 
>     Add support for TechNexion edm-cf-imx6 SoM
> 
>     The edm1-cf-imx6 SoM comes in three variants, one with imx6 solo cpu,
>     one with an imx6 dual lite cpu and one with an imx6 quad cpu.
> 
>     This patch adds basic support for the module that utilizes SPL boot 
>     mechanism for detecting imx6 CPU runtime and sets the system accordingly.
> 
>     Signed-off-by: Richard Hu <richard.hu at technexion.com>
>     Signed-off-by: Tapani Utriainen <tapani at technexion.com>
>     Signed-off-by: Edward Lin <edward.lin at technexion.com>
> ---
>  MAINTAINERS                                     |   4 +
>  arch/arm/include/asm/arch-mx6/spl.h             |  19 +
>  board/technexion/edm_cf_imx6/Makefile           |  26 +
>  board/technexion/edm_cf_imx6/README             |  30 +
>  board/technexion/edm_cf_imx6/clocks.cfg         |  44 ++
>  board/technexion/edm_cf_imx6/edm_cf_imx6.c      | 801 ++++++++++++++++++++++++
>  board/technexion/edm_cf_imx6/edm_cf_imx6_pins.h |  44 ++
>  board/technexion/edm_cf_imx6/imximage.cfg       |  17 +
>  boards.cfg                                      |   1 +
>  include/configs/edm_cf_imx6.h                   | 140 +++++
>  10 files changed, 1126 insertions(+)
>  create mode 100644 arch/arm/include/asm/arch-mx6/spl.h
>  create mode 100644 board/technexion/edm_cf_imx6/Makefile
>  create mode 100644 board/technexion/edm_cf_imx6/README
>  create mode 100644 board/technexion/edm_cf_imx6/clocks.cfg
>  create mode 100644 board/technexion/edm_cf_imx6/edm_cf_imx6.c
>  create mode 100644 board/technexion/edm_cf_imx6/edm_cf_imx6_pins.h
>  create mode 100644 board/technexion/edm_cf_imx6/imximage.cfg
>  create mode 100644 include/configs/edm_cf_imx6.h
> 

The patch should be split into separate patches, and each of them fixes
a specific issue. From our previous discussion, we agree about:

- we need to clean up conflicts in pad definitions - see Eric's answer.
- we need a way to simply defines pins for the different SOC variations.
Eric and Troy have already proposed a schema adding tables *only* into
the board file, and the generation of the table is quite automatic.
Let's say, if I am a board maintainer and I want to add support for a
board having all iMX6 variations, I would like to define only once which
pins I need, without replicating for each SOC variant.
- the same should be done with DDR and clocks, if necessary.

After these preparation patches, there should be a patch preparing i.MX6
for SPL - changes in i.MX6 common code should go here.

Last, there will be a patch on top of the rest adding support to your board.

> diff --git a/arch/arm/include/asm/arch-mx6/spl.h b/arch/arm/include/asm/arch-mx6/spl.h
> new file mode 100644
> index 0000000..dd04088
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-mx6/spl.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright (C) 2013 TechNexion Ltd.
> + *
> + * 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.
> + */

Please change all license text according to the new rule with
SPDX-License-Identifier.

> +
> +Flashing U-boot into the SD card
> +--------------------------------
> +
> +- After the 'make u-boot.img' command completes, the generated 'SPL' and
> +'u-boot.img' binary must be flashed into the SD card:
> +
> +# dd if=SPL of=/dev/$dev bs=1k seek=1
> +
> +# dd if=u-boot.img of=/dev/$dev bs=64k seek=1; sync

Maybe you should add some comments explaining that this is your
decision, not due to the SOC. Only the first dd is mandatory by iMX
(offset is 0x400 in flash). For u-boot, you have decided to put it into
raw data exactly after SPL and not into a filesystem.

> +
> +Only raw mmc boot has been verified to work.

The phrase is misleading, and let me think the other ways do not work. I
assume that you tested only raw mmc, so please rephrase to explain this.

> diff --git a/board/technexion/edm_cf_imx6/clocks.cfg b/board/technexion/edm_cf_imx6/clocks.cfg
> new file mode 100644
> index 0000000..9a182c8
> --- /dev/null
> +++ b/board/technexion/edm_cf_imx6/clocks.cfg
> @@ -0,0 +1,44 @@
> +/*
> + * Copyright (C) 2013 TechNexion Ltd.
> + *

Where have you taken the file ? If this comes as it seems from nitrogen,
you cannot drop their copyright.

> diff --git a/board/technexion/edm_cf_imx6/edm_cf_imx6.c b/board/technexion/edm_cf_imx6/edm_cf_imx6.c
> new file mode 100644
> index 0000000..1a98168
> --- /dev/null
> +++ b/board/technexion/edm_cf_imx6/edm_cf_imx6.c
> @@ -0,0 +1,801 @@
> +/*
> + * Copyright (C) 2013 TechNexion Ltd.
> + *
> + * Author: Richard Hu (richard.hu at technexion.com)
> + *         Tapani Utriainen (tapani at technexion.com)
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <asm/arch/clock.h>
> +#include <asm/arch/crm_regs.h>
> +#include <asm/arch/iomux.h>
> +#include <asm/arch/imx-regs.h>
> +#ifdef CONFIG_SPL
> +#include <spl.h>
> +#endif
> +#include <asm/arch/sys_proto.h>
> +#include <asm/gpio.h>
> +#include <asm/imx-common/iomux-v3.h>
> +#include <asm/imx-common/boot_mode.h>
> +#include <asm/io.h>
> +#include <asm/sizes.h>
> +#include <common.h>
> +#include <fsl_esdhc.h>
> +#include <mmc.h>
> +
> +#include "edm_cf_imx6_pins.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define UART_PAD_CTRL  (PAD_CTL_PUS_100K_UP |			\
> +	PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm |			\
> +	PAD_CTL_SRE_FAST  | PAD_CTL_HYS)
> +
> +#define USDHC_PAD_CTRL (PAD_CTL_PUS_47K_UP |			\
> +	PAD_CTL_SPEED_LOW | PAD_CTL_DSE_80ohm |			\
> +	PAD_CTL_SRE_FAST  | PAD_CTL_HYS)
> +
> +#define ENET_PAD_CTRL  (PAD_CTL_PUS_100K_UP |			\
> +	PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm | PAD_CTL_HYS)
> +
> +#define USDHC1_CD_GPIO		IMX_GPIO_NR(1, 2)
> +#define USDHC3_CD_GPIO		IMX_GPIO_NR(3, 9)
> +
> +enum boot_device {
> +	SD0_BOOT,
> +	SD1_BOOT,
> +	MMC_BOOT,
> +	NAND_BOOT,
> +	WEIM_NOR_BOOT,
> +	ONE_NAND_BOOT,
> +	PATA_BOOT,
> +	SATA_BOOT,
> +	I2C_BOOT,
> +	SPI_NOR_BOOT,
> +	UNKNOWN_BOOT,
> +	BOOT_DEV_NUM = UNKNOWN_BOOT,
> +};
> +
> +
> +static enum boot_device boot_dev;
> +enum boot_device get_boot_device(void);
> +
> +static inline void setup_boot_device(void)
> +{
> +	uint soc_sbmr = readl(SRC_BASE_ADDR + 0x4);
> +	uint bt_mem_ctl = (soc_sbmr & 0x000000FF) >> 4 ;
> +	uint bt_mem_type = (soc_sbmr & 0x00000008) >> 3;
> +	uint bt_mem_mmc = (soc_sbmr & 0x00001000) >> 12;
> +
> +	switch (bt_mem_ctl) {
> +	case 0x0:
> +		if (bt_mem_type)
> +			boot_dev = ONE_NAND_BOOT;
> +		else
> +			boot_dev = WEIM_NOR_BOOT;
> +		break;
> +	case 0x2:
> +			boot_dev = SATA_BOOT;
> +		break;
> +	case 0x3:
> +		if (bt_mem_type)
> +			boot_dev = I2C_BOOT;
> +		else
> +			boot_dev = SPI_NOR_BOOT;
> +		break;
> +	case 0x4:
> +	case 0x5:
> +		if (bt_mem_mmc)
> +			boot_dev = SD0_BOOT;
> +		else
> +			boot_dev = SD1_BOOT;
> +		break;
> +	case 0x6:
> +	case 0x7:
> +		boot_dev = MMC_BOOT;
> +		break;
> +	case 0x8 ... 0xf:
> +		boot_dev = NAND_BOOT;
> +		break;
> +	default:
> +		boot_dev = UNKNOWN_BOOT;
> +		break;
> +	}
> +}
> +
> +enum boot_device get_boot_device(void) {
> +	return boot_dev;
> +}
> +
> 

Let's say: boot device is not board dependent, and the required SPL
function should be moved into general code (imx_common or
arch/arm/cpu/armv7/mx6).

+
> +int dram_init(void)
> +{
> +	uint cpurev, imxtype;
> +	u32 sdram_size;
> +
> +	cpurev = get_cpu_rev();
> +	imxtype = (cpurev & 0xFF000) >> 12;

I am expecting to have a global function getting the cputype, with
macros of the type is_cpu_mx6q() (I see you use it later) to help us the
usage. Not here in board code.

> +static iomux_v3_cfg_t const edmdl_uart1_pads[] = {
> +	MX6DL_PAD_CSI0_DAT10__UART1_TXD | MUX_PAD_CTRL(UART_PAD_CTRL),
> +	MX6DL_PAD_CSI0_DAT11__UART1_RXD | MUX_PAD_CTRL(UART_PAD_CTRL),
> +};
> +
> +static iomux_v3_cfg_t const edmq_uart1_pads[] = {
> +	MX6Q_PAD_CSI0_DAT10__UART1_TXD | MUX_PAD_CTRL(UART_PAD_CTRL),
> +	MX6Q_PAD_CSI0_DAT11__UART1_RXD | MUX_PAD_CTRL(UART_PAD_CTRL),
> +};
> +
> +static iomux_v3_cfg_t const edmdl_usdhc1_pads[] = {
> +	MX6DL_PAD_SD1_CLK__USDHC1_CLK   | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	MX6DL_PAD_SD1_CMD__USDHC1_CMD   | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	MX6DL_PAD_SD1_DAT0__USDHC1_DAT0 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	MX6DL_PAD_SD1_DAT1__USDHC1_DAT1 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	MX6DL_PAD_SD1_DAT2__USDHC1_DAT2 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	MX6DL_PAD_SD1_DAT3__USDHC1_DAT3 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	/* Card detect */
> +	MX6DL_PAD_GPIO_2__GPIO_1_2      | MUX_PAD_CTRL(NO_PAD_CTRL),
> +};
> +
> +static iomux_v3_cfg_t const edmq_usdhc1_pads[] = {
> +	MX6Q_PAD_SD1_CLK__USDHC1_CLK   | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	MX6Q_PAD_SD1_CMD__USDHC1_CMD   | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	MX6Q_PAD_SD1_DAT0__USDHC1_DAT0 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	MX6Q_PAD_SD1_DAT1__USDHC1_DAT1 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	MX6Q_PAD_SD1_DAT2__USDHC1_DAT2 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	MX6Q_PAD_SD1_DAT3__USDHC1_DAT3 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
> +	/* Card detect */
> +	MX6Q_PAD_GPIO_2__GPIO_1_2      | MUX_PAD_CTRL(NO_PAD_CTRL),
> +};

I do not like this solution: this is a bare duplication of the pads, and
it is prone to errors. The definitions of the table must be in some way
automatic. I want to define a pin: if SD1_CLK is used for MMC, this does
not depend from the SOC variant.

What about Troy's solution ?

> +
> +static bool cpu_is_mx6q(void)
> +{
> +	u32 cpurev, imxtype;
> +
> +	cpurev = get_cpu_rev();
> +	imxtype = (cpurev & 0xFF000) >> 12;
> +
> +        return (imxtype == MXC_CPU_MX6Q);
> +}
> +
> +static void setup_iomux_uart(void)
> +{
> +	const iomux_v3_cfg_t *uart1_pads = NULL;
> +	u32 uart1_pads_cnt;
> +
> +	if (cpu_is_mx6q())
> +	{
> +		uart1_pads = edmq_uart1_pads;
> +		uart1_pads_cnt = ARRAY_SIZE(edmq_uart1_pads);
> +	}
> +	else
> +	{
> +		uart1_pads = edmdl_uart1_pads;
> +		uart1_pads_cnt = ARRAY_SIZE(edmdl_uart1_pads);
> +	}
> +	imx_iomux_v3_setup_multiple_pads(uart1_pads, uart1_pads_cnt);
> +}

Let's say : this does not scale well. For each peripheral we have
exactly the same code. The if..then..else should be hidden in some way,
using macros to select the right table.

> +int board_mmc_init(bd_t *bis)
> +{
> +	s32 status = 0;
> +	u32 index = 0;
> +	const iomux_v3_cfg_t *usdhc3_pads = NULL;
> +	u32 usdhc3_pads_cnt;
> +	const iomux_v3_cfg_t *usdhc1_pads = NULL;
> +	u32 usdhc1_pads_cnt;
> +	/*
> +	 * Following map is done:
> +	 * (U-boot device node)    (Physical Port)
> +	 * mmc0                    SOM MicroSD
> +	 * mmc1                    Carrier board MicroSD
> +	 */
> +	switch (get_boot_device()) {
> +		case SD1_BOOT:
> +			usdhc_cfg[0].esdhc_base = USDHC1_BASE_ADDR;
> +			usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK);
> +			usdhc_cfg[0].max_bus_width = 4;
> +			usdhc_cfg[1].esdhc_base = USDHC3_BASE_ADDR;
> +			usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
> +			usdhc_cfg[1].max_bus_width = 4;
> +		break;
> +		case SD0_BOOT:
> +		default:
> +			usdhc_cfg[0].esdhc_base = USDHC3_BASE_ADDR;
> +			usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
> +			usdhc_cfg[0].max_bus_width = 4;
> +			usdhc_cfg[1].esdhc_base = USDHC1_BASE_ADDR;
> +			usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK);
> +			usdhc_cfg[1].max_bus_width = 4;
> +		break;
> +	}
> +
> +	for (index = 0; index < CONFIG_SYS_FSL_USDHC_NUM; ++index) {
> +		switch (index) {
> +		case 0:
> +
> +			if (cpu_is_mx6q())
> +			{
> +				usdhc3_pads = edmq_usdhc3_pads;
> +				usdhc3_pads_cnt = ARRAY_SIZE(edmq_usdhc3_pads);
> +			}
> +			else
> +			{
> +				usdhc3_pads = edmdl_usdhc3_pads;
> +				usdhc3_pads_cnt = ARRAY_SIZE(edmdl_usdhc3_pads);
> +			}

As you see, we can have a lot of some code.

> +			imx_iomux_v3_setup_multiple_pads(
> +				usdhc3_pads, usdhc3_pads_cnt);
> +
> +			gpio_direction_input(USDHC3_CD_GPIO);
> +			break;
> +		case 1:
> +			if (cpu_is_mx6q())
> +			{
> +				usdhc1_pads = edmq_usdhc1_pads;
> +				usdhc1_pads_cnt = ARRAY_SIZE(edmq_usdhc1_pads);
> +			}
> +			else
> +			{
> +				usdhc1_pads = edmdl_usdhc1_pads;
> +				usdhc1_pads_cnt = ARRAY_SIZE(edmdl_usdhc1_pads);
> +			}

Again here.

> +
> +int board_early_init_f(void)
> +{
> +	setup_iomux_uart();
> +	return 0;
> +}
> +
> +/*
> + * Do not overwrite the console
> + * Use always serial for U-Boot console
> + */
> +int overwrite_console(void)
> +{
> +	return 1;
> +}

I have not seen CONFIG_VIDEO in your configuration file. Is this dead code ?

> +#if defined(CONFIG_SPL_BUILD)
> +void board_init_f(ulong dummy)
> +{
> +	/* Set the stack pointer. */
> +	asm volatile("mov sp, %0\n" : : "r"(CONFIG_SPL_STACK));
> +
> +	/* Clear the BSS. */
> +	memset(__bss_start, 0, __bss_end - __bss_start);
> +
> +	/* Set global data pointer. */
> +	gd = &gdata;
> +
> +	arch_cpu_init();
> +	board_early_init_f();
> +	timer_init();
> +	preloader_console_init();
> +
> +	board_init_r(NULL, 0);
> +}

None of this stuff is board specific - we need to factorize this, making
available for all i.MX6 boards. I will say, for i.MX5, too.

> +
> +static void spl_dram_init_mx6solo_512mb(void)
> +{
> +	/* DDR3 initialization based on the MX6Solo Auto Reference Design (ARD) */
> +	/* DDR IO TYPE */
> +	writel(0x000c0000, IOMUXC_BASE_ADDR + 0x774);
> +	writel(0x00000000, IOMUXC_BASE_ADDR + 0x754);
> +	/* Clock */
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x4ac);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x4b0);
> +	/* Address */
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x464);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x490);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x74c);
> +	/* Control */
> +	writel(0x000c0030, IOMUXC_BASE_ADDR + 0x494);
> +	writel(0x00003000, IOMUXC_BASE_ADDR + 0x4a4);
> +	writel(0x00003000, IOMUXC_BASE_ADDR + 0x4a8);
> +	writel(0x00000000, IOMUXC_BASE_ADDR + 0x4a0);
> +	writel(0x00003030, IOMUXC_BASE_ADDR + 0x4b4);
> +	writel(0x00003030, IOMUXC_BASE_ADDR + 0x4b8);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x76c);
> +	/* Strobe */
> +	writel(0x00020000, IOMUXC_BASE_ADDR + 0x750);
> +	writel(0x00000038, IOMUXC_BASE_ADDR + 0x4bc);
> +	writel(0x00000038, IOMUXC_BASE_ADDR + 0x4c0);
> +	writel(0x00000038, IOMUXC_BASE_ADDR + 0x4c4);
> +	writel(0x00000038, IOMUXC_BASE_ADDR + 0x4c8);
> +	writel(0x00000038, IOMUXC_BASE_ADDR + 0x4cc);
> +	writel(0x00000038, IOMUXC_BASE_ADDR + 0x4d0);
> +	writel(0x00000038, IOMUXC_BASE_ADDR + 0x4d4);
> +	writel(0x00000038, IOMUXC_BASE_ADDR + 0x4d8);
> +	/* Data */
> +	writel(0x00020000, IOMUXC_BASE_ADDR + 0x760);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x764);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x770);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x778);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x77c);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x780);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x784);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x78c);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x748);
> +
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x470);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x474);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x478);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x47c);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x480);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x484);
> +	writel(0x00000030, IOMUXC_BASE_ADDR + 0x488);
> +	writel(0x000C0030, IOMUXC_BASE_ADDR + 0x48c);
> +	/* ZQ */
> +	writel(0xa1390003, MMDC_P0_BASE_ADDR + 0x800);
> +	writel(0xa1390003, MMDC_P1_BASE_ADDR + 0x800);
> +	/* Write leveling */
> +	writel(0x0040003c, MMDC_P0_BASE_ADDR + 0x80c);
> +	writel(0x0032003e, MMDC_P0_BASE_ADDR + 0x810);
> +
> +	writel(0x42350231, MMDC_P0_BASE_ADDR + 0x83c);
> +	writel(0x021a0218, MMDC_P0_BASE_ADDR + 0x840);
> +	writel(0x4b4b4e49, MMDC_P0_BASE_ADDR + 0x848);
> +	writel(0x3f3f3035, MMDC_P0_BASE_ADDR + 0x850);
> +	/* Read data bit delay */
> +	writel(0x33333333, MMDC_P0_BASE_ADDR + 0x81c);
> +	writel(0x33333333, MMDC_P0_BASE_ADDR + 0x820);
> +	writel(0x33333333, MMDC_P0_BASE_ADDR + 0x824);
> +	writel(0x33333333, MMDC_P0_BASE_ADDR + 0x828);
> +	writel(0x33333333, MMDC_P1_BASE_ADDR + 0x81c);
> +	writel(0x33333333, MMDC_P1_BASE_ADDR + 0x820);
> +	writel(0x33333333, MMDC_P1_BASE_ADDR + 0x824);
> +	writel(0x33333333, MMDC_P1_BASE_ADDR + 0x828);
> +	/* Complete calibration by forced measurement */
> +	writel(0x00000800, MMDC_P0_BASE_ADDR + 0x8b8);
> +
> +        writel(0x0002002d, MMDC_P0_BASE_ADDR + 0x004);
> +	writel(0x00333030, MMDC_P0_BASE_ADDR + 0x008);
> +	writel(0x696d5323, MMDC_P0_BASE_ADDR + 0x00c);
> +	writel(0xb66e8c63, MMDC_P0_BASE_ADDR + 0x010);
> +	writel(0x01ff00db, MMDC_P0_BASE_ADDR + 0x014);
> +	writel(0x00001740, MMDC_P0_BASE_ADDR + 0x018);
> +	writel(0x00008000, MMDC_P0_BASE_ADDR + 0x01c);
> +	writel(0x000026d2, MMDC_P0_BASE_ADDR + 0x02c);
> +	writel(0x006d0e21, MMDC_P0_BASE_ADDR + 0x030);
> +	writel(0x00000027, MMDC_P0_BASE_ADDR + 0x040);
> +	writel(0x84190000, MMDC_P0_BASE_ADDR + 0x000);
> +	writel(0x04008032, MMDC_P0_BASE_ADDR + 0x01c);
> +	writel(0x00008033, MMDC_P0_BASE_ADDR + 0x01c);
> +	writel(0x00048031, MMDC_P0_BASE_ADDR + 0x01c);
> +	writel(0x07208030, MMDC_P0_BASE_ADDR + 0x01c);
> +	writel(0x04008040, MMDC_P0_BASE_ADDR + 0x01c);
> +	writel(0x00005800, MMDC_P0_BASE_ADDR + 0x020);
> +	writel(0x00011117, MMDC_P0_BASE_ADDR + 0x818);
> +	writel(0x00011117, MMDC_P1_BASE_ADDR + 0x818);
> +	writel(0x0002556d, MMDC_P0_BASE_ADDR + 0x004);
> +	writel(0x00011006, MMDC_P1_BASE_ADDR + 0x004);
> +	writel(0x00000000, MMDC_P0_BASE_ADDR + 0x01c);
> +}

Really I do not like this solution. First, you should accessor to set
the iomux, without using base address + offset. And of course, access to
the ram controller should be done in the same way using a C structure,
not offsets.

Then the problem is similar to the pads. I will propose we have a
general function, and the values of board specific parameters (32
against 64 bit size, calibration registers, and so on), and start the
ddr procedure. The functions here do the same on different registers.

> +u32 spl_boot_device(void)
> +{
> +	puts("Boot Device: ");
> +	switch (get_boot_device()) {
> +	case SD0_BOOT:
> +		printf("SD0\n");
> +		return BOOT_DEVICE_MMC1;
> +	case SD1_BOOT:
> +		printf("SD1\n");
> +		return BOOT_DEVICE_MMC2;
> +	case MMC_BOOT:
> +		printf("MMC\n");
> +		return BOOT_DEVICE_MMC2;
> +	case NAND_BOOT:
> +		printf("NAND\n");
> +		return BOOT_DEVICE_NAND;
> +	case UNKNOWN_BOOT:
> +	default:
> +		printf("UNKNOWN\n");
> +		return BOOT_DEVICE_NONE;
> +	}
> +}

This is also common code.

> +
> +u32 spl_boot_mode(void)
> +{
> +	switch (spl_boot_device()) {
> +	case BOOT_DEVICE_MMC1:
> +	case BOOT_DEVICE_MMC2:
> +	case BOOT_DEVICE_MMC2_2:
> +#ifdef CONFIG_SPL_FAT_SUPPORT
> +		return MMCSD_MODE_FAT;
> +#else
> +		return MMCSD_MODE_RAW;
> +#endif
> +		break;
> +        case BOOT_DEVICE_NAND:
> +	default:
> +		puts("spl: ERROR:  unsupported device\n");
> +		hang();
> +	}
> +}
> +
> +void reset_cpu(ulong addr)
> +{
> +
> +}

reset_cpu for imx should activate the watchdog, see
drivers/watchdog/imx_watchdog.c

> diff --git a/boards.cfg b/boards.cfg
> index 79d6cd8..b7d66ff 100644
> --- a/boards.cfg
> +++ b/boards.cfg
> @@ -288,6 +288,7 @@ nitrogen6s1g                 arm         armv7       nitrogen6x          boundar
>  wandboard_dl		     arm	 armv7	     wandboard		 -		mx6 wandboard:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6dl.cfg,MX6DL,DDR_MB=1024
>  wandboard_quad		     arm	 armv7	     wandboard		 -		mx6 wandboard:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6q2g.cfg,MX6Q,DDR_MB=2048
>  wandboard_solo		     arm	 armv7	     wandboard		 -		mx6 wandboard:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6s.cfg,MX6S,DDR_MB=512
> +edm_cf_imx6                  arm         armv7       edm_cf_imx6         technexion     mx6 edm_cf_imx6:IMX_CONFIG=board/technexion/edm_cf_imx6/imximage.cfg,SPL

Why CONFIG_SPL is not set into the configuration file ? Is there a
version without SPL ?

Please maintain the list sorted.

> +++ b/include/configs/edm_cf_imx6.h
> @@ -0,0 +1,140 @@
> +/*
> + * Copyright (C) 2013 Freescale Semiconductor, Inc.
> + *
> + * Configuration settings for the Wandboard.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef __CONFIG_H
> +#define __CONFIG_H
> +
> +#include <asm/arch/imx-regs.h>
> +#include <asm/imx-common/gpio.h>
> +#include <asm/sizes.h>
> +
> +#define CONFIG_MX6
> +#define CONFIG_DISPLAY_CPUINFO
> +#define CONFIG_DISPLAY_BOARDINFO
> +
> +#define MACH_TYPE_EDM_CF_IMX6		4257
> +#define CONFIG_MACH_TYPE		MACH_TYPE_EDM_CF_IMX6

if MACH_TYPE_EDM_CF_IMX6 is used only here, better:

#define CONFIG_MACH_TYPE		4257

> +
> +#define CONFIG_CMDLINE_TAG
> +#define CONFIG_SETUP_MEMORY_TAGS
> +#define CONFIG_REVISION_TAG
> +
> +/* SPL magic */

Why magic ?

> +#ifdef CONFIG_SPL
> +#define CONFIG_SPL_FRAMEWORK
> +#define CONFIG_SPL_TEXT_BASE          0x00908400

Ok - SPL goes into IRAM, that is good. Can you explain me the value of
the 0x8400 offset ?

> +#define CONFIG_SPL_PAD_TO 0x400
> +#define CONFIG_SPL_START_S_PATH     "arch/arm/cpu/armv7"
> +#define CONFIG_SPL_STACK	0x0091FFB8

Maybe better set it with some size - where is coming this value ?

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


More information about the U-Boot mailing list