On 02/10/2014 04:25 PM, Tom Rini wrote:
On Fri, Feb 07, 2014 at 06:23:13PM -0500, Murali Karicheri wrote:
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.
Globally, only use
/*
* Multiline comments like this
*/
I'll fix this.
+++ b/arch/arm/cpu/armv7/keystone/Makefile
@@ -0,0 +1,18 @@
+#
+# (C) Copyright 2012-2014
+# Texas Instruments Incorporated, <www.ti.com>
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+obj-y += aemif.o
+obj-y += init.o
+obj-y += psc.o
+obj-y += clock.o
+obj-y += cmd_clock.o
+obj-y += cmd_mon.o
+obj-y += msmc.o
+obj-y += lowlevel_init.o
+obj-$(CONFIG_SPL_BUILD) += spl.o
+obj-y += ddr3.o
Mix of space and tab, just tab it all out to line up please.
Will fix.
+++ b/arch/arm/cpu/armv7/keystone/aemif.c
@@ -0,0 +1,79 @@
+/*
+ * Keystone2: Asynchronous EMIF Configuration
+ *
+ * (C) Copyright 2012-2014
+ * Texas Instruments Incorporated, <www.ti.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/clock.h>
+
+#ifdef CONFIG_SOC_K2HK
+#define ASYNC_EMIF_BASE K2HK_ASYNC_EMIF_CNTRL_BASE
+#endif
So, does Keystone 1 have this in a different location?
That is not for Keystone1. K2HK is for Hawking and Kepler SoC.
The ASYNC_EMIF_BASE may be different for other SOC of the Keystone2 family.
+#define ASYNC_EMIF_CONFIG(cs) (ASYNC_EMIF_BASE+0x10+(cs)*4)
+#define ASYNC_EMIF_ONENAND_CONTROL (ASYNC_EMIF_BASE+0x5c)
+#define ASYNC_EMIF_NAND_CONTROL (ASYNC_EMIF_BASE+0x60)
+#define ASYNC_EMIF_WAITCYCLE_CONFIG (ASYNC_EMIF_BASE+0x4)
Register access is done over structs not define of base + offset, please
rework.
This style was taken form Davinci code. Shall we rework it?
+#define CONFIG_SELECT_STROBE(v) ((v) ? 1 << 31 : 0)
+#define CONFIG_EXTEND_WAIT(v) ((v) ? 1 << 30 : 0)
+#define CONFIG_WR_SETUP(v) (((v) & 0x0f) << 26)
+#define CONFIG_WR_STROBE(v) (((v) & 0x3f) << 20)
+#define CONFIG_WR_HOLD(v) (((v) & 0x07) << 17)
+#define CONFIG_RD_SETUP(v) (((v) & 0x0f) << 13)
+#define CONFIG_RD_STROBE(v) (((v) & 0x3f) << 7)
+#define CONFIG_RD_HOLD(v) (((v) & 0x07) << 4)
+#define CONFIG_TURN_AROUND(v) (((v) & 0x03) << 2)
+#define CONFIG_WIDTH(v) (((v) & 0x03) << 0)
To avoid confusion please do AEMIF_CONFIG_... or AEMIF_CFG_... or maybe
just CFG_...
Agree and will change.
+++ b/arch/arm/cpu/armv7/keystone/clock-k2hk.c
[snip]
+ prediv = (tmp & 0x3f) + 1;
+ mult = (((tmp & 0x7f000) >> 6) | (pllctl_reg_read(pll,
+ mult) & 0x3f)) + 1;
+ output_div = ((pllctl_reg_read(pll, secctl) >> 19) &
+ 0xf) + 1;
Please define magic numbers, check globally.
OK
+ return ret;
+}
+
+
+
+
+unsigned long clk_get_rate(unsigned int clk)
Extra empty lines.
Will remove.
+++ b/arch/arm/cpu/armv7/keystone/clock.c
[snip]
+static void pll_delay(unsigned int loop_count)
+{
+ while (loop_count--)
+ asm(" NOP");
+}
If we cannot use udelay yet use sdelay.
+#include "clock-k2hk.c"
Please use function prototypes, header files and then build the right
SoC clock file itself.
The idea was to have common clock.h for all Keystone2 family and include
in to
it a specific for each SoC include file. Are you asking to remove that
specific include
from the clock.h?
+++ b/arch/arm/cpu/armv7/keystone/cmd_clock.c
We just added a generic
I'll check it out.
@@ -0,0 +1,139 @@
+/*
+ * keystone2: commands for clocks
+ *
+ * (C) Copyright 2012-2014
+ * Texas Instruments Incorporated, <www.ti.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <command.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/psc_defs.h>
+
+static u32 atoui(char *pstr)
+{
+ u32 res = 0;
+
+ for (; *pstr != 0; pstr++) {
+ if (*pstr < '0' || *pstr > '9')
+ break;
+
+ res = (res * 10) + (*pstr - '0');
+ }
+
+ return res;
+}
+
+struct pll_init_data cmd_pll_data = {
+ .pll = MAIN_PLL,
+ .pll_m = 16,
+ .pll_d = 1,
+ .pll_od = 2,
+};
+
+int do_pll_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+ if (argc != 5)
+ goto pll_cmd_usage;
+
+ if (strncmp(argv[1], "pa", 2) == 0)
+ cmd_pll_data.pll = PASS_PLL;
+ else if (strncmp(argv[1], "arm", 3) == 0)
+ cmd_pll_data.pll = TETRIS_PLL;
+ else if (strncmp(argv[1], "ddr3a", 5) == 0)
+ cmd_pll_data.pll = DDR3A_PLL;
+ else if (strncmp(argv[1], "ddr3b", 5) == 0)
+ cmd_pll_data.pll = DDR3B_PLL;
+ else
+ goto pll_cmd_usage;
+
+ cmd_pll_data.pll_m = atoui(argv[2]);
+ cmd_pll_data.pll_d = atoui(argv[3]);
+ cmd_pll_data.pll_od = atoui(argv[4]);
Why not simple_strtoul(argv[n], NULL, 10) ?
+
+ printf("Trying to set pll %d; mult %d; div %d; OD %d\n",
+ cmd_pll_data.pll, cmd_pll_data.pll_m,
+ cmd_pll_data.pll_d, cmd_pll_data.pll_od);
+ init_pll(&cmd_pll_data);
+
+ return 0;
+
+pll_cmd_usage:
+ return cmd_usage(cmdtp);
+}
+
+U_BOOT_CMD(
+ pllset, 5, 0, do_pll_cmd,
+ "set pll multiplier and pre divider",
+ "<pa|arm|ddr3a|ddr3b> <mult> <div> <OD>\n"
+);
Wolfgang, if we add another command that takes decimal not hexidecimal
inputs, you're going to hit me with a ruler, aren't you? Even for a
clock command?
+U_BOOT_CMD(
+ getclk, 2, 0, do_getclk_cmd,
+ "get clock rate",
+ "<clk index>\n"
+ "See the 'enum clk_e' in the k2hk clock.h for clk indexes\n"
+);
This would fit with the generic clk command and 'dump' which we just
added.
+++ b/arch/arm/cpu/armv7/keystone/config.mk
@@ -0,0 +1,14 @@
+# (C) Copyright 2012-2014
+# Texas Instruments Incorporated, <www.ti.com>
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float
+
+# =========================================================================
+#
+# Supply options according to compiler version
+#
+# =========================================================================
+PLATFORM_RELFLAGS +=$(call cc-option,-mshort-load-bytes,\
+ $(call cc-option,-malignment-traps,))
You need to do this as:
PF_CPPFLAGS_SLB := ...
PLATFORM_RELFALGS += $(PF_CPPFLAGS_SLB)
The way you have it causes an evaluation per file and the above is a one
time evaluation.
diff --git a/arch/arm/cpu/armv7/keystone/ddr3.c
b/arch/arm/cpu/armv7/keystone/ddr3.c
new file mode 100644
index 0000000..4875db7
--- /dev/null
+++ b/arch/arm/cpu/armv7/keystone/ddr3.c
Part of me wonders just how close/far this is from the omap-common
EMIF/DDR code. Some parts look pretty familiar (and makes me wonder if
you don't have some corner-case bugs around not setting shadow registers
and initially setting some of the values to max, then setting them
optimally due to which bits cause a refresh cycle).
This code is based on AVV test code and is very specific to SOC DDR3
controller,
which as I know is different from OMAP.
+++ b/arch/arm/cpu/armv7/keystone/lowlevel_init.S
Is there a reason you don't use arch/arm/cpu/armv7/lowlevel_init.S and
the s_init calling sequence?
+++ b/board/ti/k2hk_evm/board.c
[snip]
+/* Byte swap the 32-bit data if the device is BE */
+int cpu_to_bus(u32 *ptr, u32 length)
+{
+ u32 i;
+
+ if (device_big_endian)
+ for (i = 0; i < length; i++, ptr++)
+ *ptr = __swab32(*ptr);
+
+ return 0;
+}
cpu_to_be32() ?
+int board_init(void)
+{
+ gd->bd->bi_arch_number = -1;
Just don't set it?
+void enable_caches(void)
+{
+#ifndef CONFIG_SYS_DCACHE_OFF
+ /* Enable D-cache. I-cache is already enabled in start.S */
+ dcache_enable();
+#endif
+}
This belongs in one of the arch/arm/cpu/armv7/keystone/ files
+++ b/drivers/i2c/keystone_i2c.c
@@ -0,0 +1,372 @@
+/*
+ * TI Keystone i2c driver
+ * Based on TI DaVinci (TMS320DM644x) I2C driver.
And how different is it really? Can we not reuse the davinci driver?
It is reworked a little bit and also supports multiple ports, while
davinci driver
supports only one.
I'll check out other comment as well.
Thanks,
Vitaly
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
