[U-Boot] [U-Boot PATCH v2 08/12] k2hk: add support for k2hk SOC and EVM

Tom Rini
2014-02-25T22:11:43Z

On Thu, Feb 20, 2014 at 12:55:10PM -0500, Murali Karicheri wrote:

> From: Vitaly Andrianov <vitalya@ti.com>
>
> k2hk EVM is based on Texas Instruments Keystone2 Hawking/Kepler
> SoC. Keystone2 SoC has ARM v7 Cortex-A15 MPCore processor. Please
> refer the ti/k2hk_evm/README for details on the board, build and other
> information.
>
> This patch add support for keystone architecture and k2hk evm.
>
> Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
> Signed-off-by: Sandeep Nair <sandeep_n@ti.com>
[snip]
> diff --git a/Makefile b/Makefile
> index 47a03e3..ea2a387 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -491,6 +491,23 @@ $(obj)u-boot.spr: $(obj)u-boot.img $(obj)spl/u-boot-spl.bin
> --pad-to=$(CONFIG_SPL_PAD_TO) --gap-fill=0xff $@
> cat $(obj)u-boot.img >> $@
>
> +$(obj)u-boot-spi.gph: $(obj)u-boot.img $(obj)spl/u-boot-spl.bin
> + $(obj)tools/mkimage -A $(ARCH) -T gpimage -C none \
> + -a $(CONFIG_SPL_TEXT_BASE) -e $(CONFIG_SPL_TEXT_BASE) \
> + -n SPL -d $(obj)spl/u-boot-spl.bin $(obj)spl/u-boot-spl.gph
> + $(OBJCOPY) ${OBJCFLAGS} -I binary \
> + --pad-to=$(CONFIG_SPL_PAD_TO) --gap-fill=0 -O binary \
> + $(obj)spl/u-boot-spl.gph $(obj)spl/u-boot-spl-pad.gph
> + cat $(obj)spl/u-boot-spl-pad.gph $(obj)u-boot.img > $@
> +
> +$(obj)u-boot-nand.gph: $(obj)u-boot.bin
> + $(obj)tools/mkimage -A $(ARCH) -T gpimage -C none \
> + -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_TEXT_BASE) \
> + -n U-Boot -d $(obj)u-boot.bin $(obj)gph-u-boot.bin
> + @dd if=/dev/zero of=$(obj)zero.bin bs=8 count=1 2>/dev/null
> + @cat $(obj)gph-u-boot.bin $(obj)zero.bin > $@
> + @rm $(obj)zero.bin

First, these need re-writing for Kbuild. Second, we don't ever use
u-boot-nand.gph or talk about it in the README, do we?

[snip]
> +void init_pll(const struct pll_init_data *data)
> +{
> + u32 tmp, tmp_ctl, pllm, plld, pllod, bwadj;
> +
> + pllm = data->pll_m - 1;
> + plld = (data->pll_d - 1) & PLL_DIV_MASK;
> + pllod = (data->pll_od - 1) & PLL_CLKOD_MASK;
> +
> + if (data->pll == MAIN_PLL) {
> + sdelay(210000);

Comments on the delay please.

> diff --git a/arch/arm/cpu/armv7/keystone/config.mk b/arch/arm/cpu/armv7/keystone/config.mk
> new file mode 100644
> index 0000000..1c8769c
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/keystone/config.mk
> @@ -0,0 +1,15 @@
> +# (C) Copyright 2012-2014
> +# Texas Instruments Incorporated, <www.ti.com>
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +
> +PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float

This is incorrect and not needed.

> +# =========================================================================
> +#
> +# Supply options according to compiler version
> +#
> +# =========================================================================
> +PF_CPPFLAGS_SLB +=$(call cc-option,-mshort-load-bytes,\
> + $(call cc-option,-malignment-traps,))
> +PLATFORM_RELFLAGS += $(PF_CPPFLAGS_SLB)

Also not needed.

> diff --git a/arch/arm/cpu/armv7/keystone/init.c b/arch/arm/cpu/armv7/keystone/init.c
> new file mode 100644
> index 0000000..1b7e52a
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/keystone/init.c
[snip]
> +void reset_cpu(ulong addr)
> +{
> + volatile u32 *rstctrl = (volatile u32 *)(CLOCK_BASE + 0xe8);
> + u32 tmp;
> +
> + tmp = *rstctrl & 0xffff0000;
> + *rstctrl = tmp | 0x5a69;
> +
> + *rstctrl &= 0xfffe0000;

Comments / define these please.

> +++ b/arch/arm/cpu/armv7/keystone/psc.c
[snip]
> +/*
> + * FUNCTION PURPOSE: Wait for end of transitional state
> + *
> + * DESCRIPTION: Polls pstat for the selected domain and waits for transitions
> + * to be complete.
> + *
> + * Since this is boot loader code it is *ASSUMED* that interrupts
> + * are disabled and no other core is mucking around with the psc
> + * at the same time.
> + *
> + * Returns 0 when the domain is free. Returns -1 if a timeout
> + * occurred waiting for the completion.

Rewrap these lines please, and anywhere else things are too long.

> +++ b/arch/arm/cpu/armv7/keystone/spl.c
[snip]
> +u32 spl_boot_device(void)
> +{
> +#if defined(CONFIG_SPL_SPI_LOAD)
> + return BOOT_DEVICE_SPI;
> +#else
> + puts("Unknown boot device\n");
> + hang();
> +#endif

return -EINVAL (or define and use BOOT_DEVICE_UNKNOWN) and fall back to
the common hang() case please.

> + u32 __pad0;

General form is resvN.

[snip]
> +struct davinci_emif_regs {
> + u_int32_t ercsr;

Generally we do uint32_t

[snip]
> +#define REG(addr) (*(volatile unsigned int *)(addr))
> +#define REG_P(addr) ((volatile unsigned int *)(addr))
> +
> +typedef volatile unsigned int dv_reg;
> +typedef volatile unsigned int *dv_reg_p;

Where are we using this? This looks like older davinci drivers that
aren't being proper about using structs to define the hardware and need
to be converted as part of this effort. I'm not disagreeable with
seeing that be a later part of the initial series, since you'll have to
convert davinci stuff too and I realize you might not have easy access
to the davinci parts for run time testing of the conversion so having K2
work will be important.

> +++ b/board/ti/k2hk_evm/README
[snip]

What's missing here and I'd like to see is flashing instructions.

> +Build instructions:
> +===================
> +
> +To build u-boot.bin
> + >make k2hk_evm_config
> + >make u-boot-spi.gph
> +
> +To build u-boot-spi.gph
> + >make k2hk_evm_config
> + >make u-boot-spi.gph

We need to use CONFIG_SPL_TARGET so that make all just works.

> + if (err < 0)
> + printf("error deleting linux,initrd-start\n");

Here and elsewhere, puts when we aren't using format chars.

> diff --git a/boards.cfg b/boards.cfg
> index a8336cc..1690315 100644
> --- a/boards.cfg
> +++ b/boards.cfg

Please make sure that the entry is correctly sorted, see the top of
boards.cfg.

> +++ b/include/configs/k2hk_evm.h
> @@ -0,0 +1,221 @@
> +/*
> + * Configuration header file for TI's k2hk-evm
> + *
> + * (C) Copyright 2012-2014
> + * Texas Instruments Incorporated, <www.ti.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef __CONFIG_H

Please use __CONFIG_K2HK_EVM_H

> +/* Platform type */
> +#define CONFIG_SOC_K2HK
> +#define CONFIG_K2HK_EVM

Make sure we use these.

> +
> +/* U-Boot Build Configuration */
> +#define CONFIG_SKIP_LOWLEVEL_INIT /* U-Boot is a 2nd stage loader */

So there's a level before SPL that is doing a certain amount of init we
don't want to re-do?

> +#define CONFIG_SYS_DCACHE_OFF

Really?

> +#define CONFIG_SYS_MALLOC_LEN (1024 << 10) /* 1 MiB */

This is pretty small, especially since we care about UBI.

> +#define CONFIG_SYS_MEMTEST_START CONFIG_SYS_SDRAM_BASE
> +#define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_SDRAM_BASE + 32 << 20)

Please see doc/README.memory-test

> +/* SPL SPI Loader Configuration */
> +#define CONFIG_SPL_TEXT_BASE 0x0c200000
> +#define CONFIG_SPL_PAD_TO 65536
> +#define CONFIG_SPL_MAX_SIZE (CONFIG_SPL_PAD_TO - 8)

Please explain this a bit more, esp since SPL_PAD_TO should take into
account the header size already...

> +#define CONFIG_SPL_BSS_START_ADDR (CONFIG_SPL_TEXT_BASE + \
> + CONFIG_SPL_MAX_SIZE)
> +#define CONFIG_SPL_BSS_MAX_SIZE (32 * 1024)

Do we really want SPL BSS in early ram (I don't want to get the name of
the type wrong) rather than DDR?

> +#define CONFIG_SYS_SPL_MALLOC_START (CONFIG_SPL_BSS_START_ADDR + \
> + CONFIG_SPL_BSS_MAX_SIZE)
> +#define CONFIG_SYS_SPL_MALLOC_SIZE (32 * 1024)

Same.

> +#define CONFIG_SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200 }

Please use the default table.

> +#define PART_BOOT "1024k(bootloader)ro,"
> +#define PART_PARAMS "512k(params)ro,"
> +#define PART_UBI "-(ubifs)"
> +#define MTDPARTS_DEFAULT "mtdparts=davinci_nand.0:" \
> + PART_BOOT PART_PARAMS PART_UBI

Please just set these outright.

> +/* U-Boot command configuration */
> +#include <config_cmd_default.h>
> +#undef CONFIG_CMD_BDI

Why?

> +#undef CONFIG_CMD_FLASH

Shouldn't be needed.

> +#undef CONFIG_CMD_FPGA
> +#undef CONFIG_CMD_SETGETDCR

Same.

> +#define CONFIG_SYS_PROMPT_HUSH_PS2 "> "

Not needed.

> + "fdt_high=0xffffffff\0" \

Please don't do this, set it to the top of kernel low mem.

> + "mtdparts=mtdparts=davinci_nand.0:" \
> + "1024k(bootloader)ro,512k(params)ro,522752k(ubifs)\0"

Now you're not using the mtdparts you define, this shouldn't be needed.

> +/* Linux interfacing */
> +#define CONFIG_CMDLINE_TAG
> +#define CONFIG_SETUP_MEMORY_TAGS

OF/FDT things should be down here.

> +#define LINUX_BOOT_PARAM_ADDR (CONFIG_SYS_SDRAM_BASE + 0x100)

Just use this in the code.

Thanks!

--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140225/d83ae6bf/attachment.pgp>