[U-Boot] [PATCH V3 12/18] arm: mx6: add support for Compulab cm-fx6 CoM
Igor Grinberg
grinberg at compulab.co.il
Wed Aug 13 14:55:20 CEST 2014
Hi Nikita,
Several comments below in addition to Simon's.
On 08/11/14 19:22, Nikita Kiryanov wrote:
> Add initial support for Compulab CM-FX6 CoM.
> Support includes MMC, SPI flash, and SPL with dynamic DRAM detection.
>
> Cc: Igor Grinberg <grinberg at compulab.co.il>
> Cc: Stefano Babic <sbabic at denx.de>
> Cc: Tom Rini <trini at ti.com>
> Cc: Marek Vasut <marex at denx.de>
> Acked-by: Marek Vasut <marex at denx.de>
> Signed-off-by: Nikita Kiryanov <nikita at compulab.co.il>
> ---
> Changes in V3:
> - Remove CONFIG_SYS_TEXT_BASE from config file to not clash with the
> one supplied by imx6_spl.h
>
> Changes in V2:
> - Remove unnecessary line removal from arch/arm/cpu/armv7/mx6/ddr.c
> - Move probe_mmdc_config() code straight to dram_init()
> - Use imx6_spl.h
> - Use imx_ddr_size()
> NOTE: the correctness of this patch now depends on https://patchwork.ozlabs.org/patch/376095/
>
> board/compulab/cm_fx6/Makefile | 12 ++
> board/compulab/cm_fx6/cm_fx6.c | 98 +++++++++
> board/compulab/cm_fx6/common.c | 83 ++++++++
> board/compulab/cm_fx6/common.h | 36 ++++
> board/compulab/cm_fx6/imximage.cfg | 8 +
> board/compulab/cm_fx6/spl.c | 400 +++++++++++++++++++++++++++++++++++++
> boards.cfg | 2 +
> include/configs/cm_fx6.h | 211 +++++++++++++++++++
> 8 files changed, 850 insertions(+)
> create mode 100644 board/compulab/cm_fx6/Makefile
> create mode 100644 board/compulab/cm_fx6/cm_fx6.c
> create mode 100644 board/compulab/cm_fx6/common.c
> create mode 100644 board/compulab/cm_fx6/common.h
> create mode 100644 board/compulab/cm_fx6/imximage.cfg
> create mode 100644 board/compulab/cm_fx6/spl.c
> create mode 100644 include/configs/cm_fx6.h
>
As Simon already said, if Kconfig is already in and boards.cfg
is already out, we should adjust the patch set to the new world.
[...]
> diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
> new file mode 100644
> index 0000000..47d17bb
> --- /dev/null
> +++ b/board/compulab/cm_fx6/cm_fx6.c
> @@ -0,0 +1,98 @@
[...]
> +#ifdef CONFIG_FSL_ESDHC
> +int board_mmc_init(bd_t *bis)
> +{
> + int i;
> +
> + cm_fx6_set_usdhc_iomux();
> + for (i = 0; i < CONFIG_SYS_FSL_USDHC_NUM; i++) {
> + usdhc_cfg[i].sdhc_clk = mxc_get_clock(usdhc_clk[i]);
> + usdhc_cfg[i].max_bus_width = 4;
> + fsl_esdhc_initialize(bis, &usdhc_cfg[i]);
> + enable_usdhc_clk(1, i);
Why does the board code needs to handle the mmc clocks?
Can't this be handled in the common code?
> + }
> +
> + return 0;
> +}
> +#endif
> +
> +int board_init(void)
> +{
> + gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
> + return 0;
> +}
> +
> +int checkboard(void)
> +{
> + puts("Board: CM-FX6\n");
> + return 0;
> +}
> +
> +static ulong bank1_size;
> +static ulong bank2_size;
> +
> +void dram_init_banksize(void)
> +{
> + gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
> + gd->bd->bi_dram[0].size = bank1_size;
> + gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
> + gd->bd->bi_dram[1].size = bank2_size;
> +}
> +
> +int dram_init(void)
> +{
> + gd->ram_size = imx_ddr_size();
> + switch (gd->ram_size) {
> + case 0x10000000: /* DDR_16BIT_256MB */
> + bank1_size = 0x10000000;
> + bank2_size = 0;
> + break;
> + case 0x20000000: /* DDR_32BIT_512MB */
> + bank1_size = 0x20000000;
> + bank2_size = 0;
> + break;
> + case 0x40000000:
> + if (is_cpu_type(MXC_CPU_MX6SOLO)) { /* DDR_32BIT_1GB */
> + bank1_size = 0x20000000;
> + bank2_size = 0x20000000;
> + } else { /* DDR_64BIT_1GB */
> + bank1_size = 0x40000000;
> + bank2_size = 0;
You don't need to init bank2_size to zero in all above cases.
> + }
> + break;
> + case 0x80000000: /* DDR_64BIT_2GB */
> + bank1_size = 0x40000000;
> + bank2_size = 0x40000000;
> + break;
> + case 0xF0000000: /* DDR_64BIT_4GB */
> + bank1_size = 0x70000000;
> + bank2_size = 0x7FF00000;
> + gd->ram_size -= 0x100000;
> + break;
> + default:
> + printf("ERROR: Unsupported DRAM size 0x%lx\n", gd->ram_size);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +u32 get_board_rev(void)
> +{
> + return 100;
Hmmm... cl_eeprom_get_board_rev()?
> +}
[...]
> diff --git a/board/compulab/cm_fx6/common.h b/board/compulab/cm_fx6/common.h
> new file mode 100644
> index 0000000..05eab34
> --- /dev/null
> +++ b/board/compulab/cm_fx6/common.h
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright (C) 2014, Compulab Ltd - http://compulab.co.il/
> + *
> + * Author: Nikita Kiryanov <nikita at compulab.co.il>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <asm/arch/mx6-pins.h>
> +#include <asm/arch/clock.h>
> +
> +#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 CM_FX6_ECSPI_BUS0_CS0 IMX_GPIO_NR(2, 30)
> +#define CM_FX6_GREEN_LED IMX_GPIO_NR(2, 31)
> +
> +#if defined(CONFIG_FSL_ESDHC)
> +#include <fsl_esdhc.h>
> +
> +static __maybe_unused struct fsl_esdhc_cfg usdhc_cfg[3] = {
> + {USDHC1_BASE_ADDR},
> + {USDHC2_BASE_ADDR},
> + {USDHC3_BASE_ADDR},
> +};
> +
> +static __maybe_unused enum mxc_clock usdhc_clk[3] = {
> + MXC_ESDHC_CLK,
> + MXC_ESDHC2_CLK,
> + MXC_ESDHC3_CLK,
> +};
Why do you need the above structures defined here in .h file?
Can they live in .c file that is using them?
I think cm_fx6.c is the appropriate place for these.
> +#endif
[...]
> diff --git a/board/compulab/cm_fx6/spl.c b/board/compulab/cm_fx6/spl.c
> new file mode 100644
> index 0000000..9f9e5f8
> --- /dev/null
> +++ b/board/compulab/cm_fx6/spl.c
> @@ -0,0 +1,400 @@
> +/*
> + * SPL specific code for Compulab CM-FX6 board
> + *
> + * Copyright (C) 2014, Compulab Ltd - http://compulab.co.il/
> + *
> + * Author: Nikita Kiryanov <nikita at compulab.co.il>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <spl.h>
> +#include <asm/io.h>
> +#include <asm/gpio.h>
> +#include <asm/arch/mx6-ddr.h>
> +#include <asm/arch/clock.h>
> +#include <asm/arch/sys_proto.h>
> +#include <asm/imx-common/iomux-v3.h>
> +#include <fsl_esdhc.h>
> +#include "common.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +enum ddr_config {
> + DDR_16BIT_256MB,
> + DDR_32BIT_512MB,
> + DDR_32BIT_1GB,
> + DDR_64BIT_1GB,
> + DDR_64BIT_2GB,
> + DDR_64BIT_4GB,
> + DDR_UNKNOWN,
> +};
> +
> +static void spl_mx6s_dram_setup_iomux(void)
> +{
> + struct mx6sdl_iomux_ddr_regs ddr_iomux;
> + struct mx6sdl_iomux_grp_regs grp_iomux;
> +
> + ddr_iomux.dram_sdqs0 = 0x00000038;
> + ddr_iomux.dram_sdqs1 = 0x00000038;
> + ddr_iomux.dram_sdqs2 = 0x00000038;
> + ddr_iomux.dram_sdqs3 = 0x00000038;
> + ddr_iomux.dram_sdqs4 = 0x00000038;
> + ddr_iomux.dram_sdqs5 = 0x00000038;
> + ddr_iomux.dram_sdqs6 = 0x00000038;
> + ddr_iomux.dram_sdqs7 = 0x00000038;
> + ddr_iomux.dram_dqm0 = 0x00000038;
> + ddr_iomux.dram_dqm1 = 0x00000038;
> + ddr_iomux.dram_dqm2 = 0x00000038;
> + ddr_iomux.dram_dqm3 = 0x00000038;
> + ddr_iomux.dram_dqm4 = 0x00000038;
> + ddr_iomux.dram_dqm5 = 0x00000038;
> + ddr_iomux.dram_dqm6 = 0x00000038;
> + ddr_iomux.dram_dqm7 = 0x00000038;
> + ddr_iomux.dram_cas = 0x00000038;
> + ddr_iomux.dram_ras = 0x00000038;
> + ddr_iomux.dram_sdclk_0 = 0x00000038;
> + ddr_iomux.dram_sdclk_1 = 0x00000038;
> + ddr_iomux.dram_sdcke0 = 0x00003000;
> + ddr_iomux.dram_sdcke1 = 0x00003000;
> + /*
> + * Below DRAM_RESET[DDR_SEL] = 0 which is incorrect according to
> + * Freescale QRM, but this is exactly the value used by the automatic
> + * calibration script and it works also in all our tests, so we leave
> + * it as is at this point.
> + */
> + ddr_iomux.dram_reset = 0x00000038;
> + ddr_iomux.dram_sdba2 = 0x00000000;
> + ddr_iomux.dram_sdodt0 = 0x00000038;
> + ddr_iomux.dram_sdodt1 = 0x00000038;
> + grp_iomux.grp_b0ds = 0x00000038;
> + grp_iomux.grp_b1ds = 0x00000038;
> + grp_iomux.grp_b2ds = 0x00000038;
> + grp_iomux.grp_b3ds = 0x00000038;
> + grp_iomux.grp_b4ds = 0x00000038;
> + grp_iomux.grp_b5ds = 0x00000038;
> + grp_iomux.grp_b6ds = 0x00000038;
> + grp_iomux.grp_b7ds = 0x00000038;
> + grp_iomux.grp_addds = 0x00000038;
> + grp_iomux.grp_ddrmode_ctl = 0x00020000;
> + grp_iomux.grp_ddrpke = 0x00000000;
> + grp_iomux.grp_ddrmode = 0x00020000;
> + grp_iomux.grp_ctlds = 0x00000038;
> + grp_iomux.grp_ddr_type = 0x000C0000;
Hmmm... Can we do the above initializations statically?
I mean, define the structures outside of the function and initialize
them statically, and then only pass the structures to the below
function. Moreover, this way, you will not need this function at all,
but call the below from cm_fx6_spl_dram_init().
> + mx6sdl_dram_iocfg(64, &ddr_iomux, &grp_iomux);
> +}
> +
> +static void spl_mx6q_dram_setup_iomux(void)
> +{
> + struct mx6dq_iomux_ddr_regs ddr_iomux;
> + struct mx6dq_iomux_grp_regs grp_iomux;
> +
> + ddr_iomux.dram_sdqs0 = 0x00000038;
> + ddr_iomux.dram_sdqs1 = 0x00000038;
> + ddr_iomux.dram_sdqs2 = 0x00000038;
> + ddr_iomux.dram_sdqs3 = 0x00000038;
> + ddr_iomux.dram_sdqs4 = 0x00000038;
> + ddr_iomux.dram_sdqs5 = 0x00000038;
> + ddr_iomux.dram_sdqs6 = 0x00000038;
> + ddr_iomux.dram_sdqs7 = 0x00000038;
> + ddr_iomux.dram_dqm0 = 0x00000038;
> + ddr_iomux.dram_dqm1 = 0x00000038;
> + ddr_iomux.dram_dqm2 = 0x00000038;
> + ddr_iomux.dram_dqm3 = 0x00000038;
> + ddr_iomux.dram_dqm4 = 0x00000038;
> + ddr_iomux.dram_dqm5 = 0x00000038;
> + ddr_iomux.dram_dqm6 = 0x00000038;
> + ddr_iomux.dram_dqm7 = 0x00000038;
> + ddr_iomux.dram_cas = 0x00000038;
> + ddr_iomux.dram_ras = 0x00000038;
> + ddr_iomux.dram_sdclk_0 = 0x00000038;
> + ddr_iomux.dram_sdclk_1 = 0x00000038;
> + ddr_iomux.dram_sdcke0 = 0x00003000;
> + ddr_iomux.dram_sdcke1 = 0x00003000;
> + /*
> + * Below DRAM_RESET[DDR_SEL] = 0 which is incorrect according to
> + * Freescale QRM, but this is exactly the value used by the automatic
> + * calibration script and it works also in all our tests, so we leave
> + * it as is at this point.
> + */
> + ddr_iomux.dram_reset = 0x00000038;
> + ddr_iomux.dram_sdba2 = 0x00000000;
> + ddr_iomux.dram_sdodt0 = 0x00000038;
> + ddr_iomux.dram_sdodt1 = 0x00000038;
> + grp_iomux.grp_b0ds = 0x00000038;
> + grp_iomux.grp_b1ds = 0x00000038;
> + grp_iomux.grp_b2ds = 0x00000038;
> + grp_iomux.grp_b3ds = 0x00000038;
> + grp_iomux.grp_b4ds = 0x00000038;
> + grp_iomux.grp_b5ds = 0x00000038;
> + grp_iomux.grp_b6ds = 0x00000038;
> + grp_iomux.grp_b7ds = 0x00000038;
> + grp_iomux.grp_addds = 0x00000038;
> + grp_iomux.grp_ddrmode_ctl = 0x00020000;
> + grp_iomux.grp_ddrpke = 0x00000000;
> + grp_iomux.grp_ddrmode = 0x00020000;
> + grp_iomux.grp_ctlds = 0x00000038;
> + grp_iomux.grp_ddr_type = 0x000C0000;
same here?
> + mx6dq_dram_iocfg(64, &ddr_iomux, &grp_iomux);
> +}
> +
> +static void spl_mx6s_dram_init(enum ddr_config dram_config, int reset)
I think we have bool type in U-Boot, right?
If so, it would be more descriptive to have bool reset.
> +{
> + struct mx6_mmdc_calibration calib;
> + struct mx6_ddr_sysinfo sysinfo;
> + struct mx6_ddr3_cfg ddr3_cfg;
> +
> + if (reset)
> + ((struct mmdc_p_regs *)MX6_MMDC_P0_MDCTL)->mdmisc = 2;
> +
> + calib.p0_mpwldectrl0 = 0x005B0061;
> + calib.p0_mpwldectrl1 = 0x004F0055;
> + calib.p0_mpdgctrl0 = 0x0314030C;
> + calib.p0_mpdgctrl1 = 0x025C0268;
> + calib.p0_mprddlctl = 0x42464646;
> + calib.p0_mpwrdlctl = 0x36322C34;
> + ddr3_cfg.mem_speed = 800;
> + ddr3_cfg.density = 4;
> + ddr3_cfg.rowaddr = 14;
> + ddr3_cfg.coladdr = 10;
> + ddr3_cfg.pagesz = 2;
> + ddr3_cfg.trcd = 1800;
> + ddr3_cfg.trcmin = 5200;
> + ddr3_cfg.trasmin = 3600;
> + ddr3_cfg.SRT = 0;
> + sysinfo.cs1_mirror = 1;
> + sysinfo.cs_density = 16;
> + sysinfo.bi_on = 1;
> + sysinfo.rtt_nom = 1;
> + sysinfo.rtt_wr = 0;
> + sysinfo.ralat = 5;
> + sysinfo.walat = 1;
> + sysinfo.mif3_mode = 3;
> + sysinfo.rst_to_cke = 0x23;
> + sysinfo.sde_to_rst = 0x10;
static init here?
> + switch (dram_config) {
> + case DDR_16BIT_256MB:
> + sysinfo.dsize = 0;
> + sysinfo.ncs = 1;
> + break;
> + case DDR_32BIT_512MB:
> + sysinfo.dsize = 1;
> + sysinfo.ncs = 1;
> + break;
> + case DDR_32BIT_1GB:
> + sysinfo.dsize = 1;
> + sysinfo.ncs = 2;
> + break;
> + default:
> + puts("Tried to setup invalid DDR configuration\n");
> + hang();
> + }
> +
> + mx6_dram_cfg(&sysinfo, &calib, &ddr3_cfg);
> + udelay(100);
> +}
> +
> +static void spl_mx6q_dram_init(enum ddr_config dram_config, int reset)
> +{
> + struct mx6_mmdc_calibration calib;
> + struct mx6_ddr_sysinfo sysinfo;
> + struct mx6_ddr3_cfg ddr3_cfg;
> +
> + if (reset)
> + ((struct mmdc_p_regs *)MX6_MMDC_P0_MDCTL)->mdmisc = 2;
> +
> + calib.p0_mpwldectrl0 = 0x00630068;
> + calib.p0_mpwldectrl1 = 0x0068005D;
> + calib.p0_mpdgctrl0 = 0x04140428;
> + calib.p0_mpdgctrl1 = 0x037C037C;
> + calib.p0_mprddlctl = 0x3C30303A;
> + calib.p0_mpwrdlctl = 0x3A344038;
> + calib.p1_mpwldectrl0 = 0x0035004C;
> + calib.p1_mpwldectrl1 = 0x00170026;
> + calib.p1_mpdgctrl0 = 0x0374037C;
> + calib.p1_mpdgctrl1 = 0x0350032C;
> + calib.p1_mprddlctl = 0x30322A3C;
> + calib.p1_mpwrdlctl = 0x48304A3E;
> + ddr3_cfg.mem_speed = 1066;
> + ddr3_cfg.density = 4;
> + ddr3_cfg.rowaddr = 14;
> + ddr3_cfg.coladdr = 10;
> + ddr3_cfg.pagesz = 2;
> + ddr3_cfg.trcd = 1324;
> + ddr3_cfg.trcmin = 59500;
> + ddr3_cfg.trasmin = 9750;
> + ddr3_cfg.SRT = 0;
> + sysinfo.cs_density = 16;
> + sysinfo.cs1_mirror = 1;
> + sysinfo.bi_on = 1;
> + sysinfo.rtt_nom = 1;
> + sysinfo.rtt_wr = 0;
> + sysinfo.ralat = 5;
> + sysinfo.walat = 1;
> + sysinfo.mif3_mode = 3;
> + sysinfo.rst_to_cke = 0x23;
> + sysinfo.sde_to_rst = 0x10;
and here?
> + switch (dram_config) {
> + case DDR_16BIT_256MB:
> + sysinfo.dsize = 0;
> + sysinfo.ncs = 1;
> + break;
> + case DDR_32BIT_512MB:
> + sysinfo.dsize = 1;
> + sysinfo.ncs = 1;
> + break;
> + case DDR_64BIT_1GB:
> + sysinfo.dsize = 2;
> + sysinfo.ncs = 1;
> + break;
> + case DDR_64BIT_2GB:
> + sysinfo.dsize = 2;
> + sysinfo.ncs = 2;
> + break;
> + case DDR_64BIT_4GB:
> + sysinfo.dsize = 2;
> + sysinfo.ncs = 2;
> + ddr3_cfg.rowaddr = 15;
> + break;
> + default:
> + puts("Tried to setup invalid DDR configuration\n");
> + hang();
> + }
> +
> + mx6_dram_cfg(&sysinfo, &calib, &ddr3_cfg);
> + udelay(100);
> +}
> +
> +static int cm_fx6_spl_dram_init(void)
> +{
> + u32 cpurev, imxtype;
> + unsigned long bank1_size, bank2_size;
> +
> + cpurev = get_cpu_rev();
> + imxtype = (cpurev & 0xFF000) >> 12;
I would expect here at least cpu_type() usage instead of open coding.
Or may be to spare the above construct, it is worth introducing
a kind of get_cpu_type()? like:
#define get_cpu_type() (cpu_type(get_cpu_rev()))
in arch/arm/include/asm/arch-mx6/sys_proto.h
And then you can use get_imx_cpu_type() inside the switch below.
What do you think?
> +
> + switch (imxtype) {
> + case MXC_CPU_MX6SOLO:
> + spl_mx6s_dram_setup_iomux();
> +
> + spl_mx6s_dram_init(DDR_32BIT_1GB, 0);
> + bank1_size = get_ram_size((long int *)PHYS_SDRAM_1, 0x80000000);
> + if (bank1_size == 0x40000000)
> + return 0;
> +
> + if (bank1_size == 0x20000000) {
> + spl_mx6s_dram_init(DDR_32BIT_512MB, 1);
> + return 0;
> + }
> +
> + spl_mx6s_dram_init(DDR_16BIT_256MB, 1);
> + bank1_size = get_ram_size((long int *)PHYS_SDRAM_1, 0x80000000);
> + if (bank1_size == 0x10000000)
> + return 0;
> +
> + break;
> + case MXC_CPU_MX6D:
> + case MXC_CPU_MX6Q:
> + spl_mx6q_dram_setup_iomux();
> +
> + spl_mx6q_dram_init(DDR_64BIT_4GB, 0);
> + bank1_size = get_ram_size((long int *)PHYS_SDRAM_1, 0x80000000);
> + if (bank1_size == 0x80000000)
> + return 0;
> +
> + if (bank1_size == 0x40000000) {
> + bank2_size = get_ram_size((long int *)PHYS_SDRAM_2,
> + 0x80000000);
> + if (bank2_size == 0x40000000) {
> + /* Don't do a full reset here */
> + spl_mx6q_dram_init(DDR_64BIT_2GB, 0);
> + } else {
> + spl_mx6q_dram_init(DDR_64BIT_1GB, 1);
> + }
> +
> + return 0;
> + }
> +
> + spl_mx6q_dram_init(DDR_32BIT_512MB, 1);
> + bank1_size = get_ram_size((long int *)PHYS_SDRAM_1, 0x80000000);
> + if (bank1_size == 0x20000000)
> + return 0;
> +
> + spl_mx6q_dram_init(DDR_16BIT_256MB, 1);
> + bank1_size = get_ram_size((long int *)PHYS_SDRAM_1, 0x80000000);
> + if (bank1_size == 0x10000000)
> + return 0;
> +
> + break;
> + }
> +
> + return -1;
> +}
> +
> +static iomux_v3_cfg_t const uart4_pads[] = {
> + IOMUX_PADS(PAD_KEY_COL0__UART4_TX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL)),
> + IOMUX_PADS(PAD_KEY_ROW0__UART4_RX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL)),
> +};
> +
> +static void cm_fx6_setup_uart(void)
> +{
> + SETUP_IOMUX_PADS(uart4_pads);
> + enable_uart_clk(1);
> +}
> +
> +#ifdef CONFIG_SPL_SPI_SUPPORT
> +static void cm_fx6_setup_ecspi(void)
> +{
> + enable_cspi_clock(1, 0);
> + cm_fx6_set_ecspi_iomux();
It does not really meter, but it will lok better if the sequence
will be the same as for uart: mux and then clock...
> +}
> +#else
> +static void cm_fx6_setup_ecspi(void) { }
> +#endif
> +
> +void board_init_f(ulong dummy)
> +{
> + gd = &gdata;
> + enable_usdhc_clk(1, 2);
can this be done inside board_mmc_init() or even in a common location
like fsl_esdhc_initialize()?
> + arch_cpu_init();
> + timer_init();
> + cm_fx6_setup_ecspi();
> + cm_fx6_setup_uart();
> + get_clocks();
> + preloader_console_init();
> + gpio_direction_output(CM_FX6_GREEN_LED, 1);
> + if (cm_fx6_spl_dram_init()) {
> + puts("!!!ERROR!!! DRAM detection failed!!!\n");
> + hang();
Hmmm... why hang? may be reset the cpu/board and try again?
> + }
> +
> + memset(__bss_start, 0, __bss_end - __bss_start);
> + board_init_r(NULL, 0);
> +}
> +
> +void spl_board_init(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;
> +
> + if (bt_mem_ctl == 0x3 && !bt_mem_type)
> + puts("Booting from SPI flash\n");
> + else if (bt_mem_ctl == 0x4 || bt_mem_ctl == 0x5)
> + puts("Booting from MMC\n");
> + else
> + puts("Unknown boot device\n");
Can we call spl_boot_device() here instead?
> +}
> +
> +#ifdef CONFIG_SPL_MMC_SUPPORT
> +int board_mmc_init(bd_t *bis)
> +{
> + cm_fx6_set_usdhc_iomux();
> +
> + usdhc_cfg[2].sdhc_clk = mxc_get_clock(usdhc_clk[2]);
> + usdhc_cfg[2].max_bus_width = 4;
You use only one member of that array...
It is not likely to change.
Can we just define one instance on the stack and
hardcode its initialization?
This will eliminate the need for sharing the same definition
of that array between SPL and U-Boot and thus make things simpler.
> +
> + return fsl_esdhc_initialize(bis, &usdhc_cfg[2]);
> +}
> +#endif
[...]
> diff --git a/include/configs/cm_fx6.h b/include/configs/cm_fx6.h
> new file mode 100644
> index 0000000..b877b65
> --- /dev/null
> +++ b/include/configs/cm_fx6.h
> @@ -0,0 +1,211 @@
> +/*
> + * Config file for Compulab CM-FX6 board
> + *
> + * Copyright (C) 2014, Compulab Ltd - http://compulab.co.il/
> + *
> + * Author: Nikita Kiryanov <nikita at compulab.co.il>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef __CONFIG_CM_FX6_H
> +#define __CONFIG_CM_FX6_H
> +
> +#include <asm/arch/imx-regs.h>
> +#include <config_distro_defaults.h>
> +
> +#define CONFIG_SYS_L2CACHE_OFF
Hmmm... Is there a problem using cache on i.MX6 currently?
[...]
> +#define CONFIG_EXTRA_ENV_SETTINGS \
> + "kernel=uImage-cm-fx6\0" \
> + "autoload=no\0" \
> + "loadaddr=0x10800000\0" \
> + "fdtaddr=0x11000000\0" \
> + "console=ttymxc3,115200\0" \
> + "ethprime=FEC0\0" \
> + "bootscr=boot.scr\0" \
> + "bootm_low=18000000\0" \
> + "video_hdmi=mxcfb0:dev=hdmi,1920x1080M-32 at 50,if=RGB32\0" \
> + "video_dvi=mxcfb0:dev=dvi,1280x800M-32 at 50,if=RGB32\0" \
> + "fdtfile=cm-fx6.dtb\0" \
> + "doboot=bootm ${loadaddr}\0" \
> + "loadfdt=false\0" \
> + "setboottypez=setenv kernel zImage-cm-fx6;" \
> + "setenv doboot bootz ${loadaddr} - ${fdtaddr};" \
> + "setenv loadfdt true;\0" \
> + "setboottypem=setenv kernel uImage-cm-fx6;" \
> + "setenv doboot bootm ${loadaddr};" \
> + "setenv loadfdt false;\0"\
> + "run_eboot=echo Starting EBOOT ...; "\
> + "mmc dev ${mmcdev} && " \
> + "mmc rescan && mmc read 10042000 a 400 && go 10042000\0" \
> + "mmcdev=2\0" \
> + "mmcroot=/dev/mmcblk0p2 rw rootwait\0" \
> + "loadmmcbootscript=fatload mmc ${mmcdev} ${loadaddr} ${bootscr}\0" \
Can we switch to use load instead of fatload?
> + "mmcbootscript=echo Running bootscript from mmc ...; "\
> + "source ${loadaddr}\0" \
> + "mmcargs=setenv bootargs console=${console} " \
> + "root=${mmcroot} " \
> + "${video}\0" \
> + "mmcloadkernel=fatload mmc ${mmcdev} ${loadaddr} ${kernel}\0" \
> + "mmcloadfdt=fatload mmc ${mmcdev} ${fdtaddr} ${fdtfile}\0" \
> + "mmcboot=echo Booting from mmc ...; " \
> + "run mmcargs; " \
> + "run doboot\0" \
> + "nandroot=/dev/mtdblock4 rw\0" \
> + "nandrootfstype=ubifs\0" \
> + "nandargs=setenv bootargs console=${console} " \
> + "root=${nandroot} " \
> + "rootfstype=${nandrootfstype} " \
> + "${video}\0" \
> + "nandloadfdt=nand read ${fdtaddr} 780000 80000;\0" \
> + "nandboot=echo Booting from nand ...; " \
> + "run nandargs; " \
> + "nand read ${loadaddr} 0 780000; " \
> + "if ${loadfdt}; then " \
> + "run nandloadfdt;" \
> + "fi; " \
> + "run doboot\0" \
> + "boot=mmc dev ${mmcdev}; " \
> + "if mmc rescan; then " \
> + "if run loadmmcbootscript; then " \
> + "run mmcbootscript;" \
> + "else " \
> + "if run mmcloadkernel; then " \
> + "if ${loadfdt}; then " \
> + "run mmcloadfdt;" \
> + "fi;" \
> + "run mmcboot;" \
> + "fi;" \
> + "fi;" \
> + "fi;"
> +
> +#define CONFIG_BOOTCOMMAND \
> + "run setboottypem; run boot"
> +
[...]
> +
> +/* Boot */
> +#define CONFIG_ZERO_BOOTDELAY_CHECK
> +#define CONFIG_LOADADDR 0x10800000
> +#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR
> +#define CONFIG_CMDLINE_TAG /* enable passing of ATAGs */
> +#define CONFIG_SYS_BOOTMAPSZ (8 << 20)
> +#define CONFIG_SETUP_MEMORY_TAGS
> +#define CONFIG_INITRD_TAG
CONFIG_REVISION_TAG ?
--
Regards,
Igor.
More information about the U-Boot
mailing list