[U-Boot] [PATCH v2 5/6] ARM64: hikey: hi6220: Add u-boot support for the 96boards CE HiKey board.

Peter Griffin peter.griffin at linaro.org
Thu Jul 16 02:41:33 CEST 2015


Hi Rob,

On Fri, 10 Jul 2015, Rob Herring wrote:

> On Wed, Jul 8, 2015 at 10:57 AM, Peter Griffin <peter.griffin at linaro.org> wrote:
> > HiKey is the first 96boards consumer edition compliant board. It features a hi6220
> > SoC which has eight ARM A53 cpu's.
> >
> > This initial port adds support for: -
> > 1) Serial
> > 2) eMMC / sd card
> 
> s/sd/SD/

Will fix in v3

> 
> > 3) USB
> > 4) GPIO
> >
> > It has been tested with Arm Trusted Firmware running u-boot as the BL33 executable.
> >
> > Notes:
> >
> > eMMC has been tested with basic reading of eMMC partition intto DDR. I have not
> 
> s/intto/into/

Will fix in v3

> 
> > tested writing / erasing. Due to lack of clock control it won't be
> > running in the most performant high speed mode.
> >
> > SD card slot has been tested for reading and booting kernels into DDR.
> > It is also currently used for saving the u-boot enviroment.
> 
> s/enviroment/environment/

Will fix in v3.

> 
> > USB has been tested with ASIX networking adapter to tftpboot kernels
> > into DDR. On v2015.07-rc2 dhcp now works, and also usb mass storage
> 
> s/usb/USB/

Will fix in v3
> 
> > is enumerated correctly.
> >
> > GPIO has been tested using gpio toggle GPIO4_1-3 to flash LEDs.
> >
> > Basic SoC datasheet can be found here: -
> > https://github.com/96boards/documentation/blob/master/hikey/
> > Hi6220V100_Multi-Mode_Application_Processor_Function_Description.pdf
> >
> > Board schematic can be found here: -
> > https://github.com/96boards/documentation/blob/master/hikey/
> > 96Boards-Hikey-Rev-A1.pdf
> >
> > Signed-off-by: Peter Griffin <peter.griffin at linaro.org>
> > ---
> >  arch/arm/Kconfig               |   8 +
> >  board/hisilicon/hikey/Kconfig  |  15 ++
> >  board/hisilicon/hikey/Makefile |   8 +
> >  board/hisilicon/hikey/hikey.c  | 415 +++++++++++++++++++++++++++++++++++++++++
> >  configs/hikey_defconfig        |   5 +
> >  include/configs/hikey.h        | 168 +++++++++++++++++
> >  6 files changed, 619 insertions(+)
> >  create mode 100644 board/hisilicon/hikey/Kconfig
> >  create mode 100644 board/hisilicon/hikey/Makefile
> >  create mode 100644 board/hisilicon/hikey/hikey.c
> >  create mode 100644 configs/hikey_defconfig
> >  create mode 100644 include/configs/hikey.h
> 
> [...]
> 
> > +
> > +int misc_init_r(void)
> > +{
> > +       init_usb_and_picophy();
> 
> Can board_usb_init or usb_lowlevel_init be used here?

Umm I wasn't aware of those functions. It looks like usb_lowlevel_init() is used 
by the host controller driver. Other host controllers like ohci / ehci and xhci call
board_usb_init() from their usb_lowlevel_init function, but not dwc2.

So I can make use of board_usb_init() if I update dwc2.c generic code to call it
from usb_lowlevel_init(). I'll do this in V3 and hopefully nobody has any objections.

> 
> > +
> > +       return 0;
> > +}
> > +
> > +int board_init(void)
> > +{
> > +       gd->flags = 0;
> > +
> > +       icache_enable();
> 
> The enable_caches call in board_r.c should do this for you.

Ok, will remove in v3.

> 
> > +
> > +       return 0;
> > +}
> > +
> > +#ifdef CONFIG_GENERIC_MMC
> > +
> > +static int init_dwmmc(void)
> > +{
> > +       int ret;
> > +
> > +#ifdef CONFIG_DWMMC
> > +       /* mmc0 pinmux and clocks are already configured by ATF */
> > +       ret = hi6220_dwmci_add_port(0, HI6220_MMC0_BASE, 8);
> > +
> > +       if (ret)
> > +               printf("%s: Error adding eMMC port\n", __func__);
> > +
> > +       /* take mmc1 (sd slot) out of reset, configure clocks and pinmuxing */
> > +
> > +       mmc1_init_pll();
> > +       mmc1_reset_clk();
> > +       mmc1_setup_pinmux();
> > +
> > +       ret |= hi6220_dwmci_add_port(1, HI6220_MMC1_BASE, 4);
> > +
> > +       if (ret)
> > +               printf("%s: Error adding SD port\n", __func__);
> > +#endif
> > +       return ret;
> > +}
> > +
> > +int board_mmc_init(bd_t *bis)
> > +{
> > +       int ret;
> > +
> > +       /* init the pmussi ip */
> > +       hi6220_pmussi_init();
> > +
> > +       /* init the hi6553 pmic */
> > +       hikey_hi6553_init();
> > +
> > +       /* add the eMMC and sd ports */
> > +       ret = init_dwmmc();
> > +
> > +       if (ret)
> > +               debug("init_dwmmc failed\n");
> > +
> > +       return ret;
> > +}
> > +#endif
> > +
> > +int dram_init(void)
> > +{
> > +       gd->ram_size = PHYS_SDRAM_1_SIZE;
> > +       return 0;
> > +}
> > +
> > +void dram_init_banksize(void)
> > +{
> > +       gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
> > +       gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
> > +}
> > +
> > +/* Use the Watchdog to cause reset */
> > +void reset_cpu(ulong addr)
> > +{
> > +       /* TODO program the watchdog */
> > +}
> > diff --git a/configs/hikey_defconfig b/configs/hikey_defconfig
> > new file mode 100644
> > index 0000000..50baf22
> > --- /dev/null
> > +++ b/configs/hikey_defconfig
> > @@ -0,0 +1,5 @@
> > +# 96boards HiKey
> > +CONFIG_ARM=y
> > +CONFIG_TARGET_HIKEY=y
> > +CONFIG_SHOW_BOOT_PROGRESS=y
> > +CONFIG_NET=y
> > diff --git a/include/configs/hikey.h b/include/configs/hikey.h
> > new file mode 100644
> > index 0000000..303b857
> > --- /dev/null
> > +++ b/include/configs/hikey.h
> > @@ -0,0 +1,168 @@
> > +/*
> > + * (C) Copyright 2015 Linaro
> > + *
> > + * Peter Griffin <peter.griffin at linaro.org>
> > + *
> > + * Configuration for HiKey 96boards CE. Parts were derived from other ARM
> > + * configurations.
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +#ifndef __HIKEY_AEMV8A_H
> > +#define __HIKEY_AEMV8A_H
> 
> Drop the AEMV8A.

Ah, I thought I'd got rid of all that, but cleary missed this one.
Will remove in V3

> 
> > +
> > +/* We use generic board for hikey */
> > +#define CONFIG_SYS_GENERIC_BOARD
> > +
> > +#define CONFIG_REMAKE_ELF
> > +
> > +#define CONFIG_SUPPORT_RAW_INITRD
> > +
> > +/* Cache Definitions */
> > +#define CONFIG_SYS_DCACHE_OFF
> > +
> > +#define CONFIG_IDENT_STRING            "hikey"
> > +
> > +/* Flat Device Tree Definitions */
> > +#define CONFIG_OF_LIBFDT
> > +
> > +/* Physical Memory Map */
> > +
> > +/* CONFIG_SYS_TEXT_BASE needs to align with where ATF loads bl33.bin */
> > +#define CONFIG_SYS_TEXT_BASE           0x35000000
> > +
> > +#define CONFIG_NR_DRAM_BANKS           1
> > +#define PHYS_SDRAM_1                   0x00000000
> > +
> > +/* 1008 MB (the last 16Mb are secured for TrustZone by ATF*/
> > +#define PHYS_SDRAM_1_SIZE              0x3f000000
> > +#define CONFIG_SYS_SDRAM_BASE          PHYS_SDRAM_1
> > +
> > +#define CONFIG_SYS_INIT_RAM_SIZE       0x1000
> > +
> > +#define CONFIG_SYS_INIT_SP_ADDR         (CONFIG_SYS_SDRAM_BASE + 0x7fff0)
> > +
> > +#define CONFIG_SYS_LOAD_ADDR           (CONFIG_SYS_SDRAM_BASE + 0x80000)
> > +
> > +/* Generic Timer Definitions */
> > +#define COUNTER_FREQUENCY              (19000000)
> > +
> > +/* Generic Interrupt Controller Definitions */
> > +#define GICD_BASE                      (0xf6801000)
> > +#define GICC_BASE                      (0xf6802000)
> > +
> > +/* Size of malloc() pool */
> > +#define CONFIG_SYS_MALLOC_LEN          (CONFIG_ENV_SIZE + (8 << 20))
> > +
> > +/* PL011 Serial Configuration */
> > +#define CONFIG_PL011_SERIAL
> > +
> > +#define CONFIG_PL011_CLOCK             19200000
> > +#define CONFIG_PL01x_PORTS             {(void *)CONFIG_SYS_SERIAL0}
> > +#define CONFIG_CONS_INDEX              0
> > +
> > +#define CONFIG_BAUDRATE                        115200
> > +#define CONFIG_SYS_SERIAL0             0xF8015000
> 
> Just do:
> 
> #define CONFIG_PL01x_PORTS             {(void *)0xf8015000}

Ok, fixed in V3

> 
> You are probably going to want to setup multiple serial consoles
> (debug + LS header). That can come later, but I've figured out how to
> enable that if you are interested.

Yes I'm interested, please do let me know :)

> 
> > +
> > +#define CONFIG_CMD_USB
> > +#ifdef CONFIG_CMD_USB
> > +#define CONFIG_USB_DWC2
> > +#define CONFIG_USB_DWC2_REG_ADDR 0xF72C0000
> > +/*#define CONFIG_DWC2_DFLT_SPEED_FULL*/
> > +#define CONFIG_DWC2_ENABLE_DYNAMIC_FIFO
> > +
> > +#define CONFIG_USB_STORAGE
> > +#define CONFIG_USB_HOST_ETHER
> > +#define CONFIG_USB_ETHER_SMSC95XX
> > +#define CONFIG_USB_ETHER_ASIX
> > +#define CONFIG_MISC_INIT_R
> > +#endif
> > +
> > +#define CONFIG_HIKEY_GPIO
> > +#define CONFIG_DM_GPIO
> > +#define CONFIG_CMD_GPIO
> > +#define CONFIG_DM
> > +
> > +/* SD/MMC configuration */
> > +#define CONFIG_GENERIC_MMC
> > +#define CONFIG_MMC
> > +#define CONFIG_DWMMC
> > +#define CONFIG_HIKEY_DWMMC
> > +#define CONFIG_BOUNCE_BUFFER
> > +#define CONFIG_CMD_MMC
> > +
> > +#define CONFIG_FS_EXT4
> > +#define CONFIG_FS_FAT
> 
> distro config should set this.

Ah ok, indirectly yes, as distro_config sets CONFIG_CMD_FAT, and if that is set
config_fallbacks.h will enable CONFIG_FS_FAT.

I've removed both CONFIG_FS_FAT and CONFIG_FS_EXT4 in v3

> 
> > +
> > +/* Command line configuration */
> > +#define CONFIG_MENU
> > +#define CONFIG_CMD_CACHE
> > +#define CONFIG_CMD_BDI
> > +#define CONFIG_CMD_UNZIP
> > +#define CONFIG_CMD_PXE
> 
> and this.

Whoops, missed this one. I've removed in V3

> 
> > +#define CONFIG_CMD_ENV
> > +#define CONFIG_CMD_IMI
> > +#define CONFIG_CMD_LOADB
> > +#define CONFIG_CMD_MEMORY
> > +#define CONFIG_CMD_SAVEENV
> > +#define CONFIG_CMD_RUN
> > +#define CONFIG_CMD_BOOTD
> > +#define CONFIG_CMD_ECHO
> > +#define CONFIG_CMD_SOURCE
> > +
> > +#define CONFIG_MAC_PARTITION
> 
> Supporting Mac partitions, really?

Will remove in V3. It doesn't look like I can blame that on
vexpress either, so I must have thought it was a good idea ;)

 
> > +#define CONFIG_MTD_PARTITIONS
> 
> On what MTD device?

Good point, Will remove in v3.

> 
> > +
> > +/* BOOTP options */
> > +#define CONFIG_BOOTP_BOOTFILESIZE
> > +
> > +#define CONFIG_CMD_NET
> > +
> > +#include <config_distro_defaults.h>
> > +
> > +/* Initial environment variables */
> > +
> > +/*
> > + * Defines where the kernel and FDT exist in NOR flash and where it will
> > + * be copied into DRAM
> > + */
> > +#define CONFIG_EXTRA_ENV_SETTINGS      \
> > +                               "kernel_name=Image\0"   \
> > +                               "kernel_addr=0x0000000\0" \
> 
> Shouldn't this be 0x80000 to avoid copying from 0x0 to 0x80000.

I've updated this. Kernel boot time is much reduced with this and also the
icache being enabled.

> 
> > +                               "fdt_name=hi6220-hikey.dtb\0" \
> > +                               "fdt_addr=0x0300000\0" \
> > +                               "max_fdt=0x100000\0" \
> 
> I don't think this is needed.

Removed in V3
> 
> > +                               "fdt_high=0xffffffffffffffff\0" \
> > +                               "initrd_high=0xffffffffffffffff\0" \
> > +
> > +/* Assume we boot with root on the first partition of a USB stick */
> > +#define CONFIG_BOOTARGS                "console=ttyAMA0,115200n8 /dev/mmcblk0p7 rw "
> 
> /dev/mmcblk0p7 doesn't look right. You mean "root=/dev/..."?

Good spot, yes your right. Plus now you highlight it the comment above also needs updating.

Will fix in V3.

> 
> > +
> > +/* Copy the kernel and FDT to DRAM memory and boot */
> > +#define CONFIG_BOOTCOMMAND     "booti $kernel_addr_r - $fdt_addr_r"
> 
> Don't you need to set these variables?
> 
> Also, don't you need to load the kernel and dtb first?

Yes, but I'm not sure quite what to make the default here. My personal
workflow is: -

 "usb start; dhcp; tftp $kernel_addr $kernel_name; tftp $fdt_addr $fdt_name;
   booti $kernel_addr - $fdt_addr"

So I could use that unless you have a better idea?

> 
> > +
> > +#define CONFIG_BOOTDELAY               2
> > +
> > +/* Preserve enviroment onto sd card */
> > +#define CONFIG_ENV_IS_IN_MMC
> > +#define CONFIG_SYS_MMC_ENV_DEV         1
> > +#define CONFIG_SYS_MMC_ENV_PART                0
> 
> Don't you have these reversed? The first MMC device is 0 and I think
> partition numbering starts at 1.

Having CONFIG_SYS_MMC_ENV_DEV 1 was deliberate, as the first device is eMMC, and 
I don't have a "official" partition to save the u-boot enviroment in.
So as not to corrupt anything folks may have flashed into eMMC from the official
builds I opted to save the u-boot env to SD card which is device 1.

However that seems to have been working by luck with ENC_PART being 0, and it was
actually corrupting the partition table of the SD card. Looking more closely I think
what I should of used is 

#define CONFIG_ENV_IS_IN_FAT
#define FAT_ENV_INTERFACE               "mmc"
#define FAT_ENV_DEVICE_AND_PART         "1:1"
#define FAT_ENV_FILE                    "uboot.env"

This then saves the enviroment on a fat formatted SD card with the filename
u-boot.env. This is what I plan on using for v3.

Maybe I should additionally request some space in the official eMMC parition 
table and then we could switch over to using that.

> 
> > +#define CONFIG_ENV_OFFSET               0x0
> > +#define CONFIG_ENV_SIZE                        0x1000
> > +#define CONFIG_ENV_OFFSET_REDUND        (CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE)
> > +#define CONFIG_SYS_REDUNDAND_ENVIRONMENT
> 
> Is redundant env necessary? It seems like this was more for raw NAND
> and shouldn't really be needed for MMC.

README file documents it as being valid for CONFIG_ENV_IS_IN_MMC, and a bunch of boards
declare it with their CONFIG_ENV_IS_IN_MMC such as omap5_uevm.h, dra7xx_evm.h,
am335x_evm.h. Whilst using managed NAND should be more reliable, I think it
is still used in case there is a power failure whilst issuing 'saveenv'.

Anyways with moving to CONFIG_ENV_IS_IN_FAT I won't need it anymore so it will be
removed in V3.

Many thanks for your thorough reviews, you've spotted some really good stuff
that has meant boot time is now a lot quicker, and I will no longer be crapping
over peoples SD card partition table :)

kind regards,

Peter.


More information about the U-Boot mailing list