[U-Boot] [PATCH v4] Add initial support for Wandboard dual lite and solo.

From: Fabio Estevam fabio.estevam@freescale.com
Add initial support for Wandboard.
Wandboard is a development board that has two variants: one version based on mx6 dual lite and another one based on mx6 solo.
For details on the features of Wandboard, please refer to: http://www.wandboard.org/
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v3: - Rebased after fixing nitrogen DDR size: http://marc.info/?l=u-boot&m=136321557726201&w=2
Changes since v2: - Also added support to the mx6solo variant. - Rebased against u-boot-imx
Changes since v1: - Use get_ram_size - Remove ARP_TIMEOUT - Remove unneeded "1u" MAINTAINERS | 1 + arch/arm/include/asm/arch-mx6/mx6dl_pins.h | 3 + board/wandboard/Makefile | 29 ++++ board/wandboard/wandboard.c | 181 ++++++++++++++++++++++++ boards.cfg | 2 + include/configs/wandboard.h | 207 ++++++++++++++++++++++++++++ 6 files changed, 423 insertions(+) create mode 100644 board/wandboard/Makefile create mode 100644 board/wandboard/wandboard.c create mode 100644 include/configs/wandboard.h
diff --git a/MAINTAINERS b/MAINTAINERS index f490d62..791afce 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -664,6 +664,7 @@ Fabio Estevam fabio.estevam@freescale.com mx53smd i.MX53 mx6qsabresd i.MX6Q mx6qsabreauto i.MX6Q + wandboard i.MX6DL/S
Daniel Gorsulowski daniel.gorsulowski@esd.eu
diff --git a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h index 9494e41..9846f1b 100644 --- a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h +++ b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h @@ -50,6 +50,8 @@ #define NO_MUX_I 0 #define NO_PAD_I 0 enum { + MX6_PAD_CSI0_DAT10__UART1_TXD = IOMUX_PAD(0x0360, 0x004C, 3, 0x0000, 0, 0), + MX6_PAD_CSI0_DAT11__UART1_RXD = IOMUX_PAD(0x0364, 0x0050, 3, 0x08FC, 1, 0), MX6_PAD_DI0_DISP_CLK__IPU1_DI0_DISP_CLK = IOMUX_PAD(0x03B0, 0x009C, 0, 0x0000, 0, PAD_CTL_DSE_120ohm), MX6_PAD_DI0_PIN15__IPU1_DI0_PIN15 = IOMUX_PAD(0x03B4, 0x00A0, 0, 0x0000, 0, PAD_CTL_DSE_120ohm), MX6_PAD_DI0_PIN2__IPU1_DI0_PIN2 = IOMUX_PAD(0x03B8, 0x00A4, 0, 0x0000, 0, PAD_CTL_DSE_120ohm), @@ -90,6 +92,7 @@ enum { MX6_PAD_EIM_D27__UART2_RXD = IOMUX_PAD(0x0540, 0x0170, 4, 0x0904, 1, 0), MX6_PAD_EIM_D28__I2C1_SDA = IOMUX_PAD(0x0544, 0x0174, 1 | IOMUX_CONFIG_SION, 0x086C, 1, 0), MX6_PAD_EIM_D28__GPIO_3_28 = IOMUX_PAD(0x0544, 0x0174, 5, 0x0000, 0, 0), + MX6_PAD_EIM_D29__GPIO_3_29 = IOMUX_PAD(0x0548, 0x0178, 5, 0x0000, 0, 0), MX6_PAD_ENET_MDC__ENET_MDC = IOMUX_PAD(0x05B8, 0x01E8, 1, 0x0000, 0, 0), MX6_PAD_ENET_MDIO__ENET_MDIO = IOMUX_PAD(0x05BC, 0x01EC, 1, 0x0810, 0, 0), MX6_PAD_ENET_REF_CLK__ENET_TX_CLK = IOMUX_PAD(0x05C0, 0x01F0, 1, 0x0000, 0, 0), diff --git a/board/wandboard/Makefile b/board/wandboard/Makefile new file mode 100644 index 0000000..014ea6c --- /dev/null +++ b/board/wandboard/Makefile @@ -0,0 +1,29 @@ +# +# (C) Copyright 2013 Freescale Semiconductor, Inc. +# +# 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. +# + +include $(TOPDIR)/config.mk + +LIB = $(obj)lib$(BOARD).o + +COBJS := wandboard.o + +SRCS := $(COBJS:.o=.c) +OBJS := $(addprefix $(obj),$(COBJS)) + +$(LIB): $(obj).depend $(OBJS) + $(call cmd_link_o_target, $(OBJS)) + +######################################################################### + +# defines $(obj).depend target +include $(SRCTREE)/rules.mk + +sinclude $(obj).depend + +######################################################################### diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c new file mode 100644 index 0000000..79366cd --- /dev/null +++ b/board/wandboard/wandboard.c @@ -0,0 +1,181 @@ +/* + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * + * Author: Fabio Estevam fabio.estevam@freescale.com + * + * 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. + */ + +#include <asm/arch/clock.h> +#include <asm/arch/iomux.h> +#include <asm/arch/imx-regs.h> +#include <asm/arch/mx6-pins.h> +#include <asm/arch/sys_proto.h> +#include <asm/gpio.h> +#include <asm/imx-common/iomux-v3.h> +#include <asm/io.h> +#include <asm/sizes.h> +#include <common.h> +#include <fsl_esdhc.h> +#include <mmc.h> +#include <miiphy.h> +#include <netdev.h> + +DECLARE_GLOBAL_DATA_PTR; + +#define UART_PAD_CTRL (PAD_CTL_PKE | PAD_CTL_PUE | \ + 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_PKE | PAD_CTL_PUE | \ + 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_PKE | PAD_CTL_PUE | \ + PAD_CTL_PUS_100K_UP | PAD_CTL_SPEED_MED | \ + PAD_CTL_DSE_40ohm | PAD_CTL_HYS) + +#define ETH_PHY_RESET IMX_GPIO_NR(3, 29) + +int dram_init(void) +{ + gd->ram_size = CONFIG_DDR_MB * SZ_1M; + + return 0; +} + +static iomux_v3_cfg_t const uart1_pads[] = { + MX6_PAD_CSI0_DAT10__UART1_TXD | MUX_PAD_CTRL(UART_PAD_CTRL), + MX6_PAD_CSI0_DAT11__UART1_RXD | MUX_PAD_CTRL(UART_PAD_CTRL), +}; + +static iomux_v3_cfg_t const usdhc3_pads[] = { + MX6_PAD_SD3_CLK__USDHC3_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD3_CMD__USDHC3_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD3_DAT0__USDHC3_DAT0 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD3_DAT1__USDHC3_DAT1 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD3_DAT2__USDHC3_DAT2 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD3_DAT3__USDHC3_DAT3 | MUX_PAD_CTRL(USDHC_PAD_CTRL), +}; + +static iomux_v3_cfg_t const enet_pads[] = { + MX6_PAD_ENET_MDIO__ENET_MDIO | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_ENET_MDC__ENET_MDC | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_TXC__ENET_RGMII_TXC | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_TD0__ENET_RGMII_TD0 | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_TD1__ENET_RGMII_TD1 | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_TD2__ENET_RGMII_TD2 | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_TD3__ENET_RGMII_TD3 | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_TX_CTL__RGMII_TX_CTL | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_ENET_REF_CLK__ENET_TX_CLK | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_RXC__ENET_RGMII_RXC | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_RD0__ENET_RGMII_RD0 | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_RD1__ENET_RGMII_RD1 | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_RD2__ENET_RGMII_RD2 | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_RD3__ENET_RGMII_RD3 | MUX_PAD_CTRL(ENET_PAD_CTRL), + MX6_PAD_RGMII_RX_CTL__RGMII_RX_CTL | MUX_PAD_CTRL(ENET_PAD_CTRL), + /* AR8031 PHY Reset */ + MX6_PAD_EIM_D29__GPIO_3_29 | MUX_PAD_CTRL(NO_PAD_CTRL), +}; + +static void setup_iomux_uart(void) +{ + imx_iomux_v3_setup_multiple_pads(uart1_pads, ARRAY_SIZE(uart1_pads)); +} + +static void setup_iomux_enet(void) +{ + imx_iomux_v3_setup_multiple_pads(enet_pads, ARRAY_SIZE(enet_pads)); + + /* Reset AR8031 PHY */ + gpio_direction_output(ETH_PHY_RESET, 0); + udelay(500); + gpio_set_value(ETH_PHY_RESET, 1); +} + +static struct fsl_esdhc_cfg usdhc_cfg[1] = { + {USDHC3_BASE_ADDR}, +}; + +int board_mmc_init(bd_t *bis) +{ + imx_iomux_v3_setup_multiple_pads(usdhc3_pads, ARRAY_SIZE(usdhc3_pads)); + + usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); + return fsl_esdhc_initialize(bis, &usdhc_cfg[0]); +} + +static int mx6_rgmii_rework(struct phy_device *phydev) +{ + unsigned short val; + + /* To enable AR8031 ouput a 125MHz clk from CLK_25M */ + phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x7); + phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016); + phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007); + + val = phy_read(phydev, MDIO_DEVAD_NONE, 0xe); + val &= 0xffe3; + val |= 0x18; + phy_write(phydev, MDIO_DEVAD_NONE, 0xe, val); + + /* introduce tx clock delay */ + phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x5); + val = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e); + val |= 0x0100; + phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, val); + + return 0; +} + +int board_phy_config(struct phy_device *phydev) +{ + mx6_rgmii_rework(phydev); + + if (phydev->drv->config) + phydev->drv->config(phydev); + + return 0; +} + +int board_eth_init(bd_t *bis) +{ + int ret; + + setup_iomux_enet(); + + ret = cpu_eth_init(bis); + if (ret) + printf("FEC MXC: %s:failed\n", __func__); + + return 0; +} + +int board_early_init_f(void) +{ + setup_iomux_uart(); + return 0; +} + +int board_init(void) +{ + /* address of boot parameters */ + gd->bd->bi_boot_params = PHYS_SDRAM + 0x100; + + return 0; +} + +u32 get_board_rev(void) +{ + return 0x61011; +} + +int checkboard(void) +{ + puts("Board: Wandboard\n"); + + return 0; +} diff --git a/boards.cfg b/boards.cfg index 84f96e0..9fd4bd6 100644 --- a/boards.cfg +++ b/boards.cfg @@ -261,6 +261,8 @@ nitrogen6q arm armv7 nitrogen6x boundar nitrogen6q2g arm armv7 nitrogen6x boundary mx6 nitrogen6x:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6q2g.cfg,MX6Q,DDR_MB=2048 nitrogen6s arm armv7 nitrogen6x boundary mx6 nitrogen6x:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6s.cfg,MX6S,DDR_MB=512 nitrogen6s1g arm armv7 nitrogen6x boundary mx6 nitrogen6x:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6s1g.cfg,MX6S,DDR_MB=1024 +wandboard_dl arm armv7 wandboard - mx6 wandboard:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6dl.cfg,MX6DL,DDR_MB=1024 +wandboard_solo arm armv7 wandboard - mx6 wandboard:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6s.cfg,MX6S,DDR_MB=512 cm_t35 arm armv7 cm_t35 - omap3 omap3_overo arm armv7 overo - omap3 omap3_pandora arm armv7 pandora - omap3 diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h new file mode 100644 index 0000000..e14fe6e --- /dev/null +++ b/include/configs/wandboard.h @@ -0,0 +1,207 @@ +/* + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * + * Configuration settings for the Wandboard. + * + * 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. + */ + +#ifndef __CONFIG_H +#define __CONFIG_H + +#define CONFIG_MX6 +#define CONFIG_DISPLAY_CPUINFO +#define CONFIG_DISPLAY_BOARDINFO + +#define CONFIG_MACH_TYPE 4412 + +#include <asm/arch/imx-regs.h> +#include <asm/imx-common/gpio.h> + +#define CONFIG_CMDLINE_TAG +#define CONFIG_SETUP_MEMORY_TAGS +#define CONFIG_INITRD_TAG +#define CONFIG_REVISION_TAG + +/* Size of malloc() pool */ +#define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024) + +#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_MXC_GPIO + +#define CONFIG_MXC_UART +#define CONFIG_MXC_UART_BASE UART1_BASE + +/* allow to overwrite serial and ethaddr */ +#define CONFIG_ENV_OVERWRITE +#define CONFIG_CONS_INDEX 1 +#define CONFIG_BAUDRATE 115200 + +/* Command definition */ +#include <config_cmd_default.h> + +#undef CONFIG_CMD_IMLS + +#define CONFIG_BOOTDELAY 1 + +#define CONFIG_PREBOOT "" + +#define CONFIG_LOADADDR 0x12000000 +#define CONFIG_SYS_TEXT_BASE 0x17800000 + +/* MMC Configuration */ +#define CONFIG_FSL_ESDHC +#define CONFIG_FSL_USDHC +#define CONFIG_SYS_FSL_ESDHC_ADDR 0 +#define CONFIG_SYS_FSL_USDHC_NUM 2 + +#define CONFIG_MMC +#define CONFIG_CMD_MMC +#define CONFIG_GENERIC_MMC +#define CONFIG_BOUNCE_BUFFER +#define CONFIG_CMD_EXT2 +#define CONFIG_CMD_FAT +#define CONFIG_DOS_PARTITION + +/* Ethernet Configuration */ +#define CONFIG_CMD_PING +#define CONFIG_CMD_DHCP +#define CONFIG_CMD_MII +#define CONFIG_CMD_NET +#define CONFIG_FEC_MXC +#define CONFIG_MII +#define IMX_FEC_BASE ENET_BASE_ADDR +#define CONFIG_FEC_XCV_TYPE RGMII +#define CONFIG_ETHPRIME "FEC" +#define CONFIG_FEC_MXC_PHYADDR 1 +#define CONFIG_PHYLIB +#define CONFIG_PHY_ATHEROS + +#define CONFIG_EXTRA_ENV_SETTINGS \ + "script=boot.scr\0" \ + "uimage=uImage\0" \ + "console=ttymxc0\0" \ + "fdt_high=0xffffffff\0" \ + "initrd_high=0xffffffff\0" \ + "fdt_file=imx6dl-wandboard.dtb\0" \ + "fdt_addr=0x11000000\0" \ + "boot_fdt=try\0" \ + "ip_dyn=yes\0" \ + "mmcdev=0\0" \ + "mmcpart=2\0" \ + "mmcroot=/dev/mmcblk0p3 rootwait rw\0" \ + "mmcargs=setenv bootargs console=${console},${baudrate} " \ + "root=${mmcroot}\0" \ + "loadbootscript=" \ + "fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${script};\0" \ + "bootscript=echo Running bootscript from mmc ...; " \ + "source\0" \ + "loaduimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \ + "loadfdt=fatload mmc ${mmcdev}:${mmcpart} ${fdt_addr} ${fdt_file}\0" \ + "mmcboot=echo Booting from mmc ...; " \ + "run mmcargs; " \ + "if test ${boot_fdt} = yes || test ${boot_fdt} = try; then " \ + "if run loadfdt; then " \ + "bootm ${loadaddr} - ${fdt_addr}; " \ + "else " \ + "if test ${boot_fdt} = try; then " \ + "bootm; " \ + "else " \ + "echo WARN: Cannot load the DT; " \ + "fi; " \ + "fi; " \ + "else " \ + "bootm; " \ + "fi;\0" \ + "netargs=setenv bootargs console=${console},${baudrate} " \ + "root=/dev/nfs " \ + "ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \ + "netboot=echo Booting from net ...; " \ + "run netargs; " \ + "if test ${ip_dyn} = yes; then " \ + "setenv get_cmd dhcp; " \ + "else " \ + "setenv get_cmd tftp; " \ + "fi; " \ + "${get_cmd} ${uimage}; " \ + "if test ${boot_fdt} = yes || test ${boot_fdt} = try; then " \ + "if ${get_cmd} ${fdt_addr} ${fdt_file}; then " \ + "bootm ${loadaddr} - ${fdt_addr}; " \ + "else " \ + "if test ${boot_fdt} = try; then " \ + "bootm; " \ + "else " \ + "echo WARN: Cannot load the DT; " \ + "fi; " \ + "fi; " \ + "else " \ + "bootm; " \ + "fi;\0" + +#define CONFIG_BOOTCOMMAND \ + "mmc dev ${mmcdev};" \ + "mmc dev ${mmcdev}; if mmc rescan; then " \ + "if run loadbootscript; then " \ + "run bootscript; " \ + "else " \ + "if run loaduimage; then " \ + "run mmcboot; " \ + "else run netboot; " \ + "fi; " \ + "fi; " \ + "else run netboot; fi" + +/* Miscellaneous configurable options */ +#define CONFIG_SYS_LONGHELP +#define CONFIG_SYS_HUSH_PARSER +#define CONFIG_SYS_PROMPT "=> " +#define CONFIG_AUTO_COMPLETE +#define CONFIG_SYS_CBSIZE 256 + +/* Print Buffer Size */ +#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16) +#define CONFIG_SYS_MAXARGS 16 +#define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE + +#define CONFIG_SYS_MEMTEST_START 0x10000000 +#define CONFIG_SYS_MEMTEST_END 0x10010000 +#define CONFIG_SYS_MEMTEST_SCRATCH 0x10800000 + +#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR +#define CONFIG_SYS_HZ 1000 + +#define CONFIG_CMDLINE_EDITING + +/* Physical Memory Map */ +#define CONFIG_NR_DRAM_BANKS 1 +#define PHYS_SDRAM MMDC0_ARB_BASE_ADDR + +#define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM +#define CONFIG_SYS_INIT_RAM_ADDR IRAM_BASE_ADDR +#define CONFIG_SYS_INIT_RAM_SIZE IRAM_SIZE + +#define CONFIG_SYS_INIT_SP_OFFSET \ + (CONFIG_SYS_INIT_RAM_SIZE - GENERATED_GBL_DATA_SIZE) +#define CONFIG_SYS_INIT_SP_ADDR \ + (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET) + +/* FLASH and environment organization */ +#define CONFIG_SYS_NO_FLASH + +#define CONFIG_ENV_SIZE (8 * 1024) + +#define CONFIG_ENV_IS_IN_MMC +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) +#define CONFIG_SYS_MMC_ENV_DEV 0 + +#define CONFIG_OF_LIBFDT +#define CONFIG_CMD_BOOTZ + +#ifndef CONFIG_SYS_DCACHE_OFF +#define CONFIG_CMD_CACHE +#endif + +#endif /* __CONFIG_H * */

Dear Fabio Estevam,
In message 1363228354-29534-1-git-send-email-festevam@gmail.com you wrote:
From: Fabio Estevam fabio.estevam@freescale.com
Add initial support for Wandboard.
Subject and commit message are redundant, resulting in text like this:
Add initial support for Wandboard dual lite and solo.
Add initial support for Wandboard.
Please remove the second line.
MAINTAINERS | 1 + arch/arm/include/asm/arch-mx6/mx6dl_pins.h | 3 + board/wandboard/Makefile | 29 ++++ board/wandboard/wandboard.c | 181 ++++++++++++++++++++++++ boards.cfg | 2 + include/configs/wandboard.h | 207 ++++++++++++++++++++++++++++ 6 files changed, 423 insertions(+) create mode 100644 board/wandboard/Makefile create mode 100644 board/wandboard/wandboard.c create mode 100644 include/configs/wandboard.h
The patch does not apply cleanly against current u-boot-imx#master; there are conflicts for boards.cfg
It would be nice if there was some board/wandboard/README that describes how to install a new U-Boot image on a running system.
+u32 get_board_rev(void) +{
- return 0x61011;
+}
Umm... This is very ugly. Where is this magic number coming froim, and what does it mean? It makes no sense to me to hardcode the board revision number into the U-Boot image??
+int checkboard(void) +{
- puts("Board: Wandboard\n");
Should we also add information about DL / Solo here? It would probably make sense...
...
+#define CONFIG_MACH_TYPE 4412
Should this not rather be:
#define MACH_TYPE_WANDBOARD 4412 #define CONFIG_MACH_TYPE MACH_TYPE_WANDBOARD
so we can remove this when machtypes gets updated?
Is there only a single MACH_TYPE for the DL and Solo variants?
+#include <asm/arch/imx-regs.h> +#include <asm/imx-common/gpio.h>
Please move includes to the start of the file.
+/* Size of malloc() pool */ +#define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024)
Why do we need that much?
+#define CONFIG_BOOTDELAY 1
Is there any reason for not chosing the more standard 5 second delay?
+#define CONFIG_PREBOOT ""
Omit this?
+#define CONFIG_EXTRA_ENV_SETTINGS \
- "script=boot.scr\0" \
- "uimage=uImage\0" \
- "console=ttymxc0\0" \
- "fdt_high=0xffffffff\0" \
- "initrd_high=0xffffffff\0" \
- "fdt_file=imx6dl-wandboard.dtb\0" \
Should this not be adjusted for the Solo variant?
+#define CONFIG_SYS_MEMTEST_START 0x10000000 +#define CONFIG_SYS_MEMTEST_END 0x10010000
This makes no sense. Please see doc/README.memory-test
I can confirm that the code boots on a wanboard_dl system, but it does not find the environment as used by the original Technixion port. Is this intentional?
Can we please remove the "Reset cause: WDOG" line in production mode?
Thanks.
Reviewed-by: Wolfgang Denk wd@denx.de Tested-by: Wolfgang Denk wd@denx.de
Best regards,
Wolfgang Denk

Hi Wolfgang,
On Thu, Mar 14, 2013 at 9:31 AM, Wolfgang Denk wd@denx.de wrote:
Dear Fabio Estevam,
In message 1363228354-29534-1-git-send-email-festevam@gmail.com you wrote:
From: Fabio Estevam fabio.estevam@freescale.com
Add initial support for Wandboard.
Subject and commit message are redundant, resulting in text like this:
Add initial support for Wandboard dual lite and solo. Add initial support for Wandboard.
Please remove the second line.
Thanks for the review. I will address your comments and send v5 after the nitrogen patches are applied into u-boot.imx.
+#define CONFIG_BOOTDELAY 1
Is there any reason for not chosing the more standard 5 second delay?
Ok, so let's go with 3 seconds then ;-)
I can confirm that the code boots on a wanboard_dl system, but it does not find the environment as used by the original Technixion port. Is this intentional?
To be honest I haven't really checked the environment settings used in the original Technixion port.
Hopefully this is not a problem.
Can we please remove the "Reset cause: WDOG" line in production mode?
Do you mean the change below?
--- a/arch/arm/imx-common/cpu.c +++ b/arch/arm/imx-common/cpu.c @@ -148,7 +148,7 @@ int print_cpuinfo(void) (cpurev & 0x000F0) >> 4, (cpurev & 0x0000F) >> 0, mxc_get_clock(MXC_ARM_CLK) / 1000000); - printf("Reset cause: %s\n", get_reset_cause()); + debug("Reset cause: %s\n", get_reset_cause()); return 0; } #endif
Since this is common code I can address it separately with other patch. Just let me know if this is OK.
Regards,
Fabio Estevam

On Thu, Mar 14, 2013 at 02:01:20PM -0300, Fabio Estevam wrote:
Hi Wolfgang,
On Thu, Mar 14, 2013 at 9:31 AM, Wolfgang Denk wd@denx.de wrote:
Dear Fabio Estevam,
In message 1363228354-29534-1-git-send-email-festevam@gmail.com you wrote:
From: Fabio Estevam fabio.estevam@freescale.com
Add initial support for Wandboard.
Subject and commit message are redundant, resulting in text like this:
Add initial support for Wandboard dual lite and solo. Add initial support for Wandboard.
Please remove the second line.
Thanks for the review. I will address your comments and send v5 after the nitrogen patches are applied into u-boot.imx.
+#define CONFIG_BOOTDELAY 1
Is there any reason for not chosing the more standard 5 second delay?
Ok, so let's go with 3 seconds then ;-)
There's a pretty even distribution of 1 3 and 5 second delays (with a few 10s, 2s and 6s). If they want 1, let them have 1, it's not hard to break into, you have U-Boot starting + 1sec.

Dear Tom,
In message 20130314173648.GX23324@bill-the-cat you wrote:
Is there any reason for not chosing the more standard 5 second delay?
Ok, so let's go with 3 seconds then ;-)
There's a pretty even distribution of 1 3 and 5 second delays (with a few 10s, 2s and 6s). If they want 1, let them have 1, it's not hard to break into, you have U-Boot starting + 1sec.
The question is who "they" are. My vote is for 5 :-)
A quick AWK oneliner in include/configs finds this statistics:
Value Count -1: 49 0: 12 1: 59 2: 30 3: 150 5: 175 6: 14 10: 38 20: 1
[And this does not take into account that for example amcc-common.h which defines a value of 5 is included by 28 other board config files...]
Best regards,
Wolfgang Denk

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 03/14/2013 04:36 PM, Wolfgang Denk wrote:
Dear Tom,
In message 20130314173648.GX23324@bill-the-cat you wrote:
Is there any reason for not chosing the more standard 5 second delay?
Ok, so let's go with 3 seconds then ;-)
There's a pretty even distribution of 1 3 and 5 second delays (with a few 10s, 2s and 6s). If they want 1, let them have 1, it's not hard to break into, you have U-Boot starting + 1sec.
The question is who "they" are. My vote is for 5 :-)
A quick AWK oneliner in include/configs finds this statistics:
OK, I forgot to sub -1 from 1, oops. So it's not very even, but...
Value Count -1: 49 0: 12 1: 59 2: 30 3: 150 5: 175 6: 14 10: 38 20: 1
We have a lot of variation and no "standard" aside from "board maintainer picks". While I'd strongly question anyone who did > 10 I think this is a board maintainer decision. Now, if you've convinced Fabio that 5 or 3 just makes more sense than 1 for all the right reasons, great. But "I think 1 is too short" isn't a good one. 1 is plenty of time to get a keypress in and stop the boot given that you have roughly from u-boot starting to press any key, not just when the countdown is going. Unless you have a bug in your timer code :)
- -- Tom

On 03/14/2013 04:01:59 PM, Tom Rini wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 03/14/2013 04:36 PM, Wolfgang Denk wrote:
Dear Tom,
In message 20130314173648.GX23324@bill-the-cat you wrote:
Is there any reason for not chosing the more standard 5 second delay?
Ok, so let's go with 3 seconds then ;-)
There's a pretty even distribution of 1 3 and 5 second delays (with a few 10s, 2s and 6s). If they want 1, let them have 1, it's not hard to break into, you have U-Boot starting + 1sec.
The question is who "they" are. My vote is for 5 :-)
A quick AWK oneliner in include/configs finds this statistics:
OK, I forgot to sub -1 from 1, oops. So it's not very even, but...
Value Count -1: 49 0: 12 1: 59 2: 30 3: 150 5: 175 6: 14 10: 38 20: 1
We have a lot of variation and no "standard" aside from "board maintainer picks". While I'd strongly question anyone who did > 10 I think this is a board maintainer decision.
It looks like a lot of the 10s are Freescale reference boards... I don't think it was a conscious decision there, just something that got copied and pasted. We generally use 3 seconds internally in our board farm.
As for the default, does it make sense to autoboot at all? What are the odds that the default configuration works as is (e.g. network configuration or images in flash)? And personally, I find autoboot quite annoying during development (except for booting a secondary flash bank from a locked primary bank) -- it just adds risk of booting the new kernel before it's ready (or adds time if I have to wait until the new kernel's ready before I reboot).
Now, if you've convinced Fabio that 5 or 3 just makes more sense than 1 for all the right reasons, great. But "I think 1 is too short" isn't a good one. 1 is plenty of time to get a keypress in and stop the boot given that you have roughly from u-boot starting to press any key, not just when the countdown is going. Unless you have a bug in your timer code :)
It's barely enough time if I'm watching for it, ready to pounce (and network lag doesn't strike at the wrong moment) -- but otherwise it's an extra reboot (or jamming Ctrl-C at a not-working network load until it finally listens) if I'm not expecting it (new environment, or someone else used the board last, etc).
-Scott

Dear Fabio,
In message CAOMZO5AVbVjXhHvMmZYRmroBWZYXb_-_4eNiQVTUjOOXD=iDhg@mail.gmail.com you wrote:
Is there any reason for not chosing the more standard 5 second delay?
Ok, so let's go with 3 seconds then ;-)
Why not 5? For development, 1 or 3 is often quite short. I've seen enough cases where connecting to a serial port (for example, through one of thos @#&^!*y USB devices) eats a major part of that time. And for production mode when boot times are important you don;t want such a dealy anyway, not even with 1 or with 3 seconds.
I can confirm that the code boots on a wanboard_dl system, but it does not find the environment as used by the original Technixion port. Is this intentional?
Sorry for my typo. The company name is Technexion.
To be honest I haven't really checked the environment settings used in the original Technixion port.
Hopefully this is not a problem.
Not really a problem. I just wondered if there was a good reason to chose a different location for the environment. BTW - would it not make sense to enable redundant environment?
Actually I don;t think it is worth keepin the old settings, but that's just my opinion. Other users might be surprised to lose their whole environment settings when updating from the vendor provided version to mainline, so at least a warning should go to the board specific README.
Can we please remove the "Reset cause: WDOG" line in production mode?
Do you mean the change below?
--- a/arch/arm/imx-common/cpu.c +++ b/arch/arm/imx-common/cpu.c @@ -148,7 +148,7 @@ int print_cpuinfo(void) (cpurev & 0x000F0) >> 4, (cpurev & 0x0000F) >> 0, mxc_get_clock(MXC_ARM_CLK) / 1000000);
printf("Reset cause: %s\n", get_reset_cause());
debug("Reset cause: %s\n", get_reset_cause()); return 0;
} #endif
Since this is common code I can address it separately with other patch. Just let me know if this is OK.
Indeed this is common code, I see it now. So yes, if we change this, it should be done as separate patch. I think debug() makes a lot of sense here to reduce the output at boot time to a reasonable minimum, but then - is there another way for the user to inquire for this information. If not, should we add it to the bdinfo output?
Best regards,
Wolfgang Denk

On 14/03/2013 21:24, Wolfgang Denk wrote:
Dear Fabio,
Hi Wolfgang, hi Fabio,
Can we please remove the "Reset cause: WDOG" line in production mode?
Do you mean the change below?
--- a/arch/arm/imx-common/cpu.c +++ b/arch/arm/imx-common/cpu.c @@ -148,7 +148,7 @@ int print_cpuinfo(void) (cpurev & 0x000F0) >> 4, (cpurev & 0x0000F) >> 0, mxc_get_clock(MXC_ARM_CLK) / 1000000);
printf("Reset cause: %s\n", get_reset_cause());
debug("Reset cause: %s\n", get_reset_cause()); return 0;
} #endif
Since this is common code I can address it separately with other patch. Just let me know if this is OK.
Indeed this is common code, I see it now. So yes, if we change this, it should be done as separate patch. I think debug() makes a lot of sense here to reduce the output at boot time to a reasonable minimum, but then - is there another way for the user to inquire for this information. If not, should we add it to the bdinfo output?
Reset cause is a very important information - we can know if the processor gets a watchdog, or if a power on happened. The last one can address to hiddden issues with the power supply.
I understand that less redundant information is better, but this is very important info and it cannot be used only if DEBUG is set. IMHO we should let it as it is - it helps us when something bad happens.
Best regards, Stefano Babic

Dear Stefano Babic,
In message 5143365F.5000207@denx.de you wrote:
Indeed this is common code, I see it now. So yes, if we change this, it should be done as separate patch. I think debug() makes a lot of sense here to reduce the output at boot time to a reasonable minimum, but then - is there another way for the user to inquire for this information. If not, should we add it to the bdinfo output?
Reset cause is a very important information - we can know if the processor gets a watchdog, or if a power on happened. The last one can address to hiddden issues with the power supply.
I understand that less redundant information is better, but this is very important info and it cannot be used only if DEBUG is set. IMHO we should let it as it is - it helps us when something bad happens.
I agree that it is important information, and there should be a way that the user can get at this information. But I see no urgent need to print this for everyboot, where it is just costing us precious boot time (yes, it's only a few milliseconds, but they are trivial to save).
That was why I suggested to add thjis information for example to the output of the "bdinfo" command.
BTW: if I type "reset", I will get a message "Reset cause: WDOG". I know why, but most users will not know thy they see a watchdog reset here, so actually this information may be more confusing than helpful in a number of cases (which is IMO one more reason to not always to display it - it saves us a number of unproductive support requests).
Best regards,
Wolfgang Denk

On 15/03/2013 18:27, Wolfgang Denk wrote:
Dear Stefano Babic,
Hi Wolfgang,
I agree that it is important information, and there should be a way that the user can get at this information. But I see no urgent need to print this for everyboot, where it is just costing us precious boot time (yes, it's only a few milliseconds, but they are trivial to save).
That was why I suggested to add thjis information for example to the output of the "bdinfo" command.
That is fine - but then it should be consistent. We have tried to have a consistent API between SOCs (I mean: iMX SOCs), that is, each SOC in the i.MX family should implement the same functions and not invent a new one, And the behavior must beconsistent, too. I know we are far away to be perfect, and some code can be further factorized (this was the reason to add imx_common).
To the specific case, each i.MX SOC implements a get_reset_cause() function:
arch/arm/cpu/arm1136/mx31/generic.c:static char *get_reset_cause(void) arch/arm/cpu/arm1136/mx31/generic.c: printf("Reset cause: %s\n",
arch/arm/cpu/arm1136/mx35/generic.c:static char *get_reset_cause(void) arch/arm/cpu/arm1136/mx35/generic.c: printf("Reset cause: %s\n", get_reset_cause());
arch/arm/cpu/arm926ejs/mx25/generic.c:static char *get_reset_cause(void) arch/arm/cpu/arm926ejs/mx25/generic.c: printf("Reset cause: %s\n\n",
arch/arm/imx-common/cpu.c:char *get_reset_cause(void) arch/arm/imx-common/cpu.c: printf("Reset cause: %s\n", get_reset_cause());
Checking this I see also that the mx53loco does not use general code (bad !):
board/freescale/mx53loco/mx53loco.c: printf("Reset cause: %s\n",
It is ok to move the information to bdinfo, but then it should not break the consistence: all i.MX should follow the same rule. This makes also easier for board maintainers to switch from a SOC to the next one, because they have to provide quite the same functions in their code (and using the same functions, too.).
BTW: if I type "reset", I will get a message "Reset cause: WDOG". I know why, but most users will not know thy they see a watchdog reset here, so actually this information may be more confusing than helpful in a number of cases (which is IMO one more reason to not always to display it - it saves us a number of unproductive support requests).
I agree with you that this should be done by a separate patchset: it is not related to the Wandboard. But if we go on this way, the same changes should be done for all i.MX, not only for i.MX5/i.MX6.
Best regards, Stefano Babic

Dear Stefano,
In message 514367ED.6070101@denx.de you wrote:
It is ok to move the information to bdinfo, but then it should not break the consistence: all i.MX should follow the same rule. This makes also easier for board maintainers to switch from a SOC to the next one, because they have to provide quite the same functions in their code (and using the same functions, too.).
100% agreement.
I agree with you that this should be done by a separate patchset: it is not related to the Wandboard. But if we go on this way, the same changes should be done for all i.MX, not only for i.MX5/i.MX6.
Indeed. It was just with this specific board that I got awre of the issue. I fully agree that we should implement a common solution that covers all these systems.
Best regards,
Wolfgang Denk

On Thu, Mar 14, 2013 at 9:31 AM, Wolfgang Denk wd@denx.de wrote:
+int checkboard(void) +{
puts("Board: Wandboard\n");
Should we also add information about DL / Solo here? It would probably make sense...
This is what we will get:
CPU: Freescale i.MX6DL rev1.1 at 792 MHz Board: Wandboard
The DL / S will appear in the CPU line, so this allows to distinguish between the two variants.
Regards,
Fabio Estevam

Dear Fabio Estevam,
In message CAOMZO5DEyD9qga7mbhaL-j2siqrAtYN-XYBwLZmRO6ch_0TPeQ@mail.gmail.com you wrote:
+int checkboard(void) +{
puts("Board: Wandboard\n");
Should we also add information about DL / Solo here? It would probably make sense...
This is what we will get:
CPU: Freescale i.MX6DL rev1.1 at 792 MHz Board: Wandboard
The DL / S will appear in the CPU line, so this allows to distinguish between the two variants.
Yes, you are right. Thanks.
Best regards,
Wolfgang Denk

Hi Wolfgang,
On Thu, Mar 14, 2013 at 9:31 AM, Wolfgang Denk wd@denx.de wrote:
+#define CONFIG_SYS_MEMTEST_START 0x10000000 +#define CONFIG_SYS_MEMTEST_END 0x10010000
This makes no sense. Please see doc/README.memory-test
This is something that always causes me confusion.
doc/README.memory-test is not available in u-boot-imx. but I looked in the archives.
What is a good and safe value, I should use here?
Regards,
Fabio Estevam

On Fri, Mar 15, 2013 at 3:21 PM, Fabio Estevam festevam@gmail.com wrote:
Hi Wolfgang,
On Thu, Mar 14, 2013 at 9:31 AM, Wolfgang Denk wd@denx.de wrote:
+#define CONFIG_SYS_MEMTEST_START 0x10000000 +#define CONFIG_SYS_MEMTEST_END 0x10010000
This makes no sense. Please see doc/README.memory-test
This is something that always causes me confusion.
doc/README.memory-test is not available in u-boot-imx. but I looked in the archives.
What is a good and safe value, I should use here?
I think that the following is safe to use:
#define CONFIG_SYS_MEMTEST_START 0x10000000 #define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START + 500 * SZ_1M)

Dear Fabio Estevam,
In message CAOMZO5D6NoTKtvx0+BPFCZHsht7fa_Ksg-VNVaQ45Tjr-XwLaA@mail.gmail.com you wrote:
+#define CONFIG_SYS_MEMTEST_START 0x10000000 +#define CONFIG_SYS_MEMTEST_END 0x10010000
This makes no sense. Please see doc/README.memory-test
This is something that always causes me confusion.
doc/README.memory-test is not available in u-boot-imx. but I looked in the archives.
It's in master by now.
What is a good and safe value, I should use here?
As explained, reasonable values for _START are just above the exception vectors - if any, and _END should be a bit below the space used for U-Boot's stack.
I have to admit that I don't understand what the i.MX6 port is doing with the memory. Normally, a warmboot should not change it's content, yet here it does. Also I don't think there are exception vectors actually used in low memory, because when I erase all this memory the system still continues to run.
Sorry, I'm afraid I'm not much of a help here.
Best regards,
Wolfgang Denk

On 14/03/2013 03:32, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
Add initial support for Wandboard.
Wandboard is a development board that has two variants: one version based on mx6 dual lite and another one based on mx6 solo.
For details on the features of Wandboard, please refer to: http://www.wandboard.org/
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Hi Fabio,
as far as I know after rereading all threads there are no open points with this patch / board. I know that it was submitted after the merge window is closed, but as it has impact nor on common iMX code neither on general U-Boot code I will merge it for 2013.04.
Regards, Stefano
participants (5)
-
Fabio Estevam
-
Scott Wood
-
Stefano Babic
-
Tom Rini
-
Wolfgang Denk