Re: [U-Boot] [PATCH 2/5] vybrid: add Freescale vybrid vf600 tower board support

Hi, Fabio,

On Fri, Apr 12, 2013 at 3:53 AM, Alison Wang b18965@freescale.com wrote:

This patch adds Freescale vybrid vf600 tower board support.

Signed-off-by: TsiChung Liew tsicliew@gmail.com Signed-off-by: Jason Jin Jason.jin@freescale.com Signed-off-by: Alison Wang b18965@freescale.com

---

board/freescale/vybrid/Makefile | 40 +++ board/freescale/vybrid/vybrid.c | 488 +++++++++++++++++++++++++++++++++

Vybrid is the SoC name and the board name is vf600, right?

Then you should add:

board/freescale/vf600

[Alison] I will modify the name.

Also, please copy Stefano Babic in your future patches, as he is the U-boot imx maintainer.

board/freescale/vybrid/vybridimage.cfg | 44 +++ boards.cfg | 2 + include/configs/vybrid.h | 284 +++++++++++++++++++ include/configs/vybrid_iram.h | 284 +++++++++++++++++++ 6 files changed, 1142 insertions(+) create mode 100644 board/freescale/vybrid/Makefile create mode 100644 board/freescale/vybrid/vybrid.c create mode 100644 board/freescale/vybrid/vybridimage.cfg create mode 100644 include/configs/vybrid.h create mode 100644 include/configs/vybrid_iram.h

This patch should be the last of the series.

You add the board support here, but ethernet, serial, esdhc support comes later in the patch series.

[Alison] Agree, I will reorganize the series.

Please add an entry to MAINTAINERS file.

[Alison] Ok, I will add.

+#ifdef CONFIG_FSL_ESDHC +struct fsl_esdhc_cfg esdhc_cfg[2] = {

•   {CONFIG_SYS_ESDHC1_BASE, 1},
  

•   {ESDHC2_BASE_ADDR, 1},
  

CONFIG_ESDHC2_BASE_ADDR ?

[Alison] Yes, I will modify.

+int dram_init(void) +{

•   setup_iomux_ddr();
  

+#ifdef CONFIG_SYS_UBOOT_IN_GPURAM

You introduced CONFIG_SYS_UBOOT_IN_GPURAM here and it would be nice to add an entry into README explaining what it means.

[Alison] This macro is used in vybrid_iram configuration. It is not necessary. I will remove it with vybrid_iram configuration.

•   gd->ram_size = 0x80000;
  

•   ddr_ctrl_init();
  

+#else

•   gd->ram_size = ddr_ctrl_init(); #endif
  

•   return 0;
  

Can't you use the standard method?

gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);

[Alison] Agree, I will use the standard method.

+#ifdef CONFIG_QUAD_SPI

CONFIG_QUAD_SPI does not exist in U-boot. Can't you re-use an existing SPI config option?

[Alison] I will modify.

+#ifdef CONFIG_FSL_ESDHC +int board_mmc_getcd(struct mmc *mmc) +{

•   int ret;
  

•   __raw_writel(0x005031ef, IOMUXC_PAD_014);       /* clk */
  

•   __raw_writel(0x005031ef, IOMUXC_PAD_015);       /* cmd */
  

•   __raw_writel(0x005031ef, IOMUXC_PAD_016);       /* dat0 */
  

•   __raw_writel(0x005031ef, IOMUXC_PAD_017);       /* dat1 */
  

•   __raw_writel(0x005031ef, IOMUXC_PAD_018);       /* dat2 */
  

•   __raw_writel(0x005031ef, IOMUXC_PAD_019);       /* dat3 */
  

This function should basically return the card detect status, please setup the IOMUX in another place.

[Alison] Ok, I will rewrite these functions.

•   ret = 1;
  

•   return ret;
  

In this board you assume that the SD card is always present, right?

You could remove the 'ret' variable.

[Alison] Yes. I will remove it.

diff --git a/board/freescale/vybrid/vybridimage.cfg b/board/freescale/vybrid/vybridimage.cfg new file mode 100644 index 0000000..5da5659 --- /dev/null +++ b/board/freescale/vybrid/vybridimage.cfg @@ -0,0 +1,44 @@ +# +# Copyright 2012-2013 Freescale Semiconductor, Inc. +# +# See file CREDITS for list of people who contributed to this # +project. +# +# This program is free software; you can redistribute it and/or # +modify it under the terms of the GNU General Public License as # +published by the Free Software Foundation; either version 2 of # the +License or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, # +but WITHOUT ANY WARRANTY; without even the implied warranty of # +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU +General Public License for more details. +# +# You should have received a copy of the GNU General Public License # +along with this program; if not write to the Free Software # +Foundation Inc. 51 Franklin Street Fifth Floor Boston, # MA +02110-1301 USA # # Refer docs/README.imxmage for more details about +how-to configure # and create imximage boot image # # The syntax is +taken as close as possible with the kwbimage

+IMAGE_VERSION 2

+# Boot Device : one of +# spi, sd (the board has no nand neither onenand)

+BOOT_FROM sd

+# Device Configuration Data (DCD) +# +# Each entry must have the format: +# Addr-type Address Value +# +# where: +# Addr-type register length (1,2 or 4 bytes) +# Address absolute address of the register +# value value to be stored in the register

+# Setting IOMUXC

This file is suited for imx. If you don't need this for Vybrid, please remove it.

[Alison] I will investigate.

diff --git a/boards.cfg b/boards.cfg index 7725a15..2700a76 100644 --- a/boards.cfg +++ b/boards.cfg @@ -244,6 +244,8 @@ am335x_evm_uart5 arm armv7 am335x ti am335x_evm_usbspl arm armv7 am335x ti am33xx am335x_evm:SERIAL1,CONS_INDEX=1,SPL_USBETH_SUPPORT pcm051 arm armv7 pcm051 phytec am33xx pcm051 highbank arm armv7 highbank - highbank +vybrid arm armv7 vybrid freescale vybrid vybrid:VYBRID_CONFIG=board/freescale/vybrid/vybridimage.cfg +vybrid_iram arm armv7 vybrid freescale vybrid vybrid_iram:VYBRID_CONFIG=board/freescale/vybrid/vybridimage.cfg

Looks like you don't need vybridimage.cfg.

mx51_efikamx arm armv7 mx51_efikamx genesi mx5 mx51_efikamx:MACH_TYPE=MACH_TYPE_MX51_EFIKAMX,IMX_CONFIG=board/genesi/mx51_efikamx/imximage_mx.cfg mx51_efikasb arm armv7 mx51_efikamx genesi mx5 mx51_efikamx:MACH_TYPE=MACH_TYPE_MX51_EFIKASB,IMX_CONFIG=board/genesi/mx51_efikamx/imximage_sb.cfg mx51evk arm armv7 mx51evk freescale mx5 mx51evk:IMX_CONFIG=board/freescale/mx51evk/imximage.cfg diff --git a/include/configs/vybrid.h b/include/configs/vybrid.h new file mode 100644 index 0000000..d915bd0 --- /dev/null +++ b/include/configs/vybrid.h @@ -0,0 +1,284 @@ +/*

• • Copyright 2012-2013 Freescale Semiconductor, Inc.
• • Configuration settings for the vybrid Board

Since Vybrid is the SoC name, I would expect the

[Alison] Yes, I will modify.

• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License as
• • published by the Free Software Foundation; either version 2 of
• • the License, or (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • You should have received a copy of the GNU General Public License
• • along with this program; if not, write to the Free Software
• • Foundation, Inc., 59 Temple Place, Suite 330, Boston,
• • MA 02111-1307 USA
• */

+#ifndef __CONFIG_H +#define __CONFIG_H

• /* High Level Configuration Options */

+#define CONFIG_VYBRID

+#define CONFIG_SYS_VYBRID_HCLK 24000000 +#define CONFIG_SYS_VYBRID_CLK32 32768 +#define CONFIG_DISPLAY_CPUINFO +#define CONFIG_DISPLAY_BOARDINFO +#define CONFIG_SYS_IPG

+#define CONFIG_SYS_ICACHE_OFF +#define CONFIG_SYS_CACHELINE_SIZE 64

+#include <asm/arch/vybrid-regs.h>

Move the include to the start of the file.

[Alison] Ok.

+/*

• • Disabled for now due to build problems under Debian and a

+significant

• • increase in the final file size: 144260 vs. 109536 Bytes.
• */

Please remove this comment. It does not apply here.

[Alison] Ok.

+#define CONFIG_CMDLINE_TAG /* enable passing of ATAGs */ +#undef CONFIG_SETUP_MEMORY_TAGS +#undef CONFIG_INITRD_TAG

+#undef CONFIG_OF_LIBFDT

+#define CONFIG_MACH_TYPE MACH_TYPE_VYBRID_VF6XX +/* Size of malloc() pool */ +#define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 2 * 1024 * 1024)

+#define CONFIG_BOARD_LATE_INIT

+/* Hardware drivers */ +#define CONFIG_VYBRID_UART +#define CONFIG_VYBRID_UART_BASE UART0_BASE +#define CONFIG_VYBRID_GPIO

+/* allow to overwrite serial and ethaddr */ #define +CONFIG_ENV_OVERWRITE +#define CONFIG_SYS_UART_PORT (1) +#define CONFIG_BAUDRATE 115200 +#define CONFIG_SYS_BAUDRATE_TABLE {9600, 19200, 38400, 57600, 115200}

+/* Command definition */ +#include <config_cmd_default.h>

+#define CONFIG_CMD_BDI /* bdinfo */ +#define CONFIG_CMD_BOOTD +#define CONFIG_CMD_CONSOLE /* coninfo */ +#define CONFIG_CMD_DHCP +#define CONFIG_CMD_ELF +#define CONFIG_CMD_MEMORY /* md mm nm mw cp cmp crc base loop mtest */ +#define CONFIG_CMD_MISC +#define CONFIG_CMD_MII +#define CONFIG_CMD_NET +#undef CONFIG_CMD_NFS /* NFS support */

Do you need to undef it? Just do not include it, right?

+#define CONFIG_CMD_PING +#undef CONFIG_CMD_NAND

Same here.

+#undef CONFIG_CMD_DATE

Same here.

[Alison] Ok, I will remove them.

+#undef CONFIG_CMD_IMI /* iminfo */ +#undef CONFIG_CMD_IMLS +#undef CONFIG_CMD_LOADB /* loadb */ +#undef CONFIG_CMD_LOADS /* loads */

+#define CONFIG_SYS_TEXT_BASE 0x3f000800

+#define CONFIG_MMC +#ifdef CONFIG_MMC +#define CONFIG_SYS_ESDHC1_BASE ESDHC2_BASE_ADDR +#define CONFIG_SYS_ESDHC2_BASE ESDHC2_BASE_ADDR +#define CONFIG_FSL_ESDHC +#define CONFIG_SYS_FSL_ESDHC_ADDR 0 +#define CONFIG_SYS_FSL_ESDHC_NUM 1

+#define CONFIG_SYS_FSL_ERRATUM_ESDHC135 #define +CONFIG_SYS_FSL_ERRATUM_ESDHC111 #define +CONFIG_SYS_FSL_ERRATUM_ESDHC_A001

+#define CONFIG_CMD_MMC +#define CONFIG_GENERIC_MMC +#define CONFIG_CMD_FAT +#define CONFIG_DOS_PARTITION +#endif

+/*

• • NAND FLASH
• */

+#ifdef CONFIG_CMD_NAND +#define CONFIG_MTD_NAND_FSL_NFC_SWECC 1 #define CONFIG_JFFS2_NAND +#define CONFIG_NAND_FSL_NFC +#define CONFIG_SYS_NAND_BASE 0x400E0000 +#define CONFIG_SYS_MAX_NAND_DEVICE 1 +#define NAND_MAX_CHIPS CONFIG_SYS_MAX_NAND_DEVICE +#define CONFIG_SYS_NAND_SELECT_DEVICE +#define CONFIG_SYS_64BIT_VSPRINTF /* needed for nand_util.c */ +#endif

+#define CONFIG_QUAD_SPI

+/* Network configuration */ +#define CONFIG_MCFFEC +#ifdef CONFIG_MCFFEC +# define CONFIG_MII 1 +# define CONFIG_MII_INIT 1 +# define CONFIG_SYS_DISCOVER_PHY +# define CONFIG_SYS_RX_ETH_BUFFER 8 +# define CONFIG_SYS_FAULT_ECHO_LINK_DOWN

+# define CONFIG_SYS_FEC0_PINMUX 0 +# define CONFIG_SYS_FEC1_PINMUX 0 +# define CONFIG_SYS_FEC0_IOBASE MACNET0_BASE_ADDR +# define CONFIG_SYS_FEC1_IOBASE MACNET1_BASE_ADDR +# define CONFIG_SYS_FEC0_MIIBASE MACNET0_BASE_ADDR +# define CONFIG_SYS_FEC1_MIIBASE MACNET0_BASE_ADDR +# define MCFFEC_TOUT_LOOP 50000 +# undef CONFIG_HAS_ETH1

+# define CONFIG_ETHADDR 00:e0:0c:bc:e5:60 +# define CONFIG_ETH1ADDR 00:e0:0c:bc:e5:61 +# define CONFIG_ETHPRIME "FEC0" +# define CONFIG_IPADDR 10.81.67.175 +# define CONFIG_NETMASK 255.255.252.0 +# define CONFIG_SERVERIP 10.81.64.153 +# define CONFIG_GATEWAYIP 10.81.67.254 +# define CONFIG_OVERWRITE_ETHADDR_ONCE

No hardcoded IP addresses/MAC addresses, please.

[Alison] Agree. What is the general configuration way for these default settings?

+/* If CONFIG_SYS_DISCOVER_PHY is not defined - hardcoded */ +# ifndef CONFIG_SYS_DISCOVER_PHY +# define FECDUPLEX FULL +# define FECSPEED _100BASET +# else +# ifndef CONFIG_SYS_FAULT_ECHO_LINK_DOWN +# define CONFIG_SYS_FAULT_ECHO_LINK_DOWN +# endif +# endif /* CONFIG_SYS_DISCOVER_PHY */ +#endif

+#define CONFIG_BOOTDELAY 3 +#define CONFIG_ETHPRIME "FEC0" +#define CONFIG_LOADADDR 0x80010000 /* loadaddr env var */ +#define CONFIG_ARP_TIMEOUT 200UL

Is this really needed?

diff --git a/include/configs/vybrid_iram.h b/include/configs/vybrid_iram.h new file mode 100644 index 0000000..f88050e --- /dev/null +++ b/include/configs/vybrid_iram.h

Do we really need two configs? Can't we have just one?

[Alison] Agree. We really need only one config. Although vybrid_iram is another configuration for running in OCRAM-gfxRAM, it is not necessary. So I will remove it.

Thanks!

Best Regards, Alison Wang