[U-Boot] [PATCH 0/9] tegra2: Tidy up boot path

[U-Boot] [PATCH 0/9] tegra2: Tidy up boot path

[Simon Glass]

On Tegra2 the AVP runs the normal U-Boot code to a point, then halts and
the A9 takes over. The current Tegra2 boot path is fairly complex, since it
has a separate path and code for the Cortex-A9 and the AVP. In fact, they
can largely execute the same code path.

This series cleans up this logic and removes some parallel and un-needed
code.


Simon Glass (9):
  tegra2: Add arch_cpu_init() to fire up Cortex-A9
  tegra2: Simplify tegra_start() boot path
  arm: Move CP15 init out of cpu_init_crit()
  tegra2: Enable instruction cache
  tegra2: Remove unneeded boot code
  tegra2: Remove unneeded config option
  tegra2: Remove unused low-level Tegra2 UART code
  tegra2: Remove unneeded 'dynamic ram size' message
  tegra2: Don't use board pointer before it is set up

 arch/arm/cpu/armv7/start.S                |   36 +++++-----
 arch/arm/cpu/armv7/tegra2/Makefile        |    7 ++-
 arch/arm/cpu/armv7/tegra2/ap20.c          |   54 +++++++------
 arch/arm/cpu/armv7/tegra2/ap20.h          |   10 +--
 arch/arm/cpu/armv7/tegra2/board.c         |   35 ++++-----
 arch/arm/cpu/armv7/tegra2/config.mk       |   28 -------
 arch/arm/cpu/armv7/tegra2/lowlevel_init.S |  118 -----------------------------
 arch/arm/include/asm/u-boot-arm.h         |    3 +
 arch/arm/lib/Makefile                     |    6 ++
 board/nvidia/common/board.c               |    6 +-
 board/nvidia/common/board.h               |    1 -
 drivers/serial/Makefile                   |    1 -
 drivers/serial/serial_tegra2.c            |   77 -------------------
 drivers/serial/serial_tegra2.h            |   29 -------
 include/configs/tegra2-common.h           |    4 +-
 15 files changed, 86 insertions(+), 329 deletions(-)
 delete mode 100644 arch/arm/cpu/armv7/tegra2/config.mk
 delete mode 100644 drivers/serial/serial_tegra2.c
 delete mode 100644 drivers/serial/serial_tegra2.h

-- 
1.7.3.1

[U-Boot] [PATCH 1/9] tegra2: Add arch_cpu_init() to fire up Cortex-A9

[Simon Glass]

We want to move away from a special Tegra2 start-up, and just use
arch_cpu_init() instead. However, if we run board_init_f() from boot
we need to build it for ARMv4T, since the Tegra's AVP start-up CPU
does not support ARMv7.

The effect of this is to do the AVP init earlier, and in
arch_cpu_init(), rather that board_early_init_f().

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/tegra2/board.c |   15 +++++++++++++++
 arch/arm/lib/Makefile             |    6 ++++++
 board/nvidia/common/board.c       |    3 ---
 board/nvidia/common/board.h       |    1 -
 include/configs/tegra2-common.h   |    2 +-
 5 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/armv7/tegra2/board.c
index 9061d18..e725134 100644
--- a/arch/arm/cpu/armv7/tegra2/board.c
+++ b/arch/arm/cpu/armv7/tegra2/board.c
@@ -23,6 +23,7 @@
 
 #include <common.h>
 #include <asm/io.h>
+#include "ap20.h"
 #include <asm/arch/sys_proto.h>
 #include <asm/arch/tegra2.h>
 #include <asm/arch/pmc.h>
@@ -86,3 +87,17 @@ int checkboard(void)
 	return 0;
 }
 #endif	/* CONFIG_DISPLAY_BOARDINFO */
+
+#ifdef CONFIG_ARCH_CPU_INIT
+/*
+ * Note this function is executed by the ARM7TDMI AVP. It does not return
+ * in this case. It is also called once the A9 starts up, but does nothing in
+ * that case.
+ */
+int arch_cpu_init(void)
+{
+	/* Fire up the Cortex A9 */
+	tegra2_start();
+	return 0;
+}
+#endif
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 300c8fa..3341d11 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -23,6 +23,12 @@
 
 include $(TOPDIR)/config.mk
 
+# Tegra has an ARMv4T CPU which runs board_init_f(), so we must build this
+# file with compatible flags
+ifdef CONFIG_TEGRA2
+CFLAGS_arch/arm/lib/board.o += -march=armv4t
+endif
+
 LIB	= $(obj)lib$(ARCH).o
 LIBGCC	= $(obj)libgcc.o
 
diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
index d13537d..6af317b 100644
--- a/board/nvidia/common/board.c
+++ b/board/nvidia/common/board.c
@@ -188,9 +188,6 @@ int board_early_init_f(void)
 
 	/* Initialize periph GPIOs */
 	gpio_config_uart();
-
-	/* Init UART, scratch regs, and start CPU */
-	tegra2_start();
 	return 0;
 }
 #endif	/* EARLY_INIT */
diff --git a/board/nvidia/common/board.h b/board/nvidia/common/board.h
index 344e702..5a6b323 100644
--- a/board/nvidia/common/board.h
+++ b/board/nvidia/common/board.h
@@ -24,7 +24,6 @@
 #ifndef _BOARD_H_
 #define _BOARD_H_
 
-void tegra2_start(void);
 void gpio_config_uart(void);
 void gpio_config_mmc(void);
 int tegra2_mmc_init(int dev_index, int bus_width);
diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-common.h
index 73e0f05..3454689 100644
--- a/include/configs/tegra2-common.h
+++ b/include/configs/tegra2-common.h
@@ -32,7 +32,7 @@
 #define CONFIG_TEGRA2			/* in a NVidia Tegra2 core */
 #define CONFIG_MACH_TEGRA_GENERIC	/* which is a Tegra generic machine */
 #define CONFIG_SYS_L2CACHE_OFF		/* No L2 cache */
-
+#define CONFIG_ARCH_CPU_INIT		/* Fire up the A9 core */
 #define CONFIG_ENABLE_CORTEXA9		/* enable CPU (A9 complex) */
 
 #include <asm/arch/tegra2.h>		/* get chip and board defs */
-- 
1.7.3.1

[U-Boot] [PATCH 1/9] tegra2: Add arch_cpu_init() to fire up Cortex-A9

[Mike Frysinger]

On Thursday 13 October 2011 17:05:55 Simon Glass wrote:
> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> 
> +# Tegra has an ARMv4T CPU which runs board_init_f(), so we must build this
> +# file with compatible flags
> +ifdef CONFIG_TEGRA2
> +CFLAGS_arch/arm/lib/board.o += -march=armv4t
> +endif

all Tegra SoC's have an ARMv4T inside of the package ?  that's a bit crazy.  
isn't there a tegra-specific config.mk somewhere we could add this instead ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111013/2fdd0bc9/attachment.pgp 

[U-Boot] [PATCH 1/9] tegra2: Add arch_cpu_init() to fire up Cortex-A9

[Simon Glass]

Hi Mike,

On Thu, Oct 13, 2011 at 3:52 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 13 October 2011 17:05:55 Simon Glass wrote:
>> --- a/arch/arm/lib/Makefile
>> +++ b/arch/arm/lib/Makefile
>>
>> +# Tegra has an ARMv4T CPU which runs board_init_f(), so we must build this
>> +# file with compatible flags
>> +ifdef CONFIG_TEGRA2
>> +CFLAGS_arch/arm/lib/board.o += -march=armv4t
>> +endif
>
> all Tegra SoC's have an ARMv4T inside of the package ? ?that's a bit crazy.
> isn't there a tegra-specific config.mk somewhere we could add this instead ?
> -mike
>

Well that same line can't go into the tegra-specific config.mk as it
will not be seen when compiling arch/arm/lib.

The current code compiles the whole of U-Boot with ARMv4T which is bad
since only a few functions need it. This is one thing that the patch
series cleans up.

Regards,
Simon

[U-Boot] [PATCH 1/9] tegra2: Add arch_cpu_init() to fire up Cortex-A9

[Mike Frysinger]

On Thursday 13 October 2011 19:18:22 Simon Glass wrote:
> On Thu, Oct 13, 2011 at 3:52 PM, Mike Frysinger wrote:
> > On Thursday 13 October 2011 17:05:55 Simon Glass wrote:
> >> --- a/arch/arm/lib/Makefile
> >> +++ b/arch/arm/lib/Makefile
> >> 
> >> +# Tegra has an ARMv4T CPU which runs board_init_f(), so we must build
> >> this +# file with compatible flags
> >> +ifdef CONFIG_TEGRA2
> >> +CFLAGS_arch/arm/lib/board.o += -march=armv4t
> >> +endif
> > 
> > all Tegra SoC's have an ARMv4T inside of the package ?  that's a bit
> > crazy. isn't there a tegra-specific config.mk somewhere we could add
> > this instead ?
> 
> Well that same line can't go into the tegra-specific config.mk as it
> will not be seen when compiling arch/arm/lib.

why not ?

arch/arm/lib/Makefile includes topdir/config.mk

topdir/config.mk includes topdir/$(CPUDIR)/$(SOC)/config.mk

isn't tegra your SOC here ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111013/13153f7f/attachment.pgp 

[U-Boot] [PATCH 1/9] tegra2: Add arch_cpu_init() to fire up Cortex-A9

[Simon Glass]

Hi Mike,

On Thu, Oct 13, 2011 at 4:34 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 13 October 2011 19:18:22 Simon Glass wrote:
>> On Thu, Oct 13, 2011 at 3:52 PM, Mike Frysinger wrote:
>> > On Thursday 13 October 2011 17:05:55 Simon Glass wrote:
>> >> --- a/arch/arm/lib/Makefile
>> >> +++ b/arch/arm/lib/Makefile
>> >>
>> >> +# Tegra has an ARMv4T CPU which runs board_init_f(), so we must build
>> >> this +# file with compatible flags
>> >> +ifdef CONFIG_TEGRA2
>> >> +CFLAGS_arch/arm/lib/board.o += -march=armv4t
>> >> +endif
>> >
>> > all Tegra SoC's have an ARMv4T inside of the package ? ?that's a bit
>> > crazy. isn't there a tegra-specific config.mk somewhere we could add
>> > this instead ?
>>
>> Well that same line can't go into the tegra-specific config.mk as it
>> will not be seen when compiling arch/arm/lib.
>
> why not ?
>
> arch/arm/lib/Makefile includes topdir/config.mk
>
> topdir/config.mk includes topdir/$(CPUDIR)/$(SOC)/config.mk
>
> isn't tegra your SOC here ?

Oh I see. I missed that link - yes it should work. Thanks for pointing
it out - I will take another look.

Regards,
Simon

> -mike
>

[U-Boot] [PATCH 2/9] tegra2: Simplify tegra_start() boot path

[Simon Glass]

The Tegra2 boot path is more complicated than it needs to be. Since we want
to move to building most of U-Boot with ARMv7 and only a small part with
ARMv4T (for AVP) it should be as simple as possible.

This makes tegra2_start() into a simple function which either does AVP
init or A9 init depending on which core is running it. Both cores now
following the same init path, beginning at _start, and the special Tegra2
boot path code is no longer required.

Only two files need to be built for ARMv4T, and this is handled in the
Tegra2 CPU Makefile.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/tegra2/Makefile |    7 ++++-
 arch/arm/cpu/armv7/tegra2/ap20.c   |   54 +++++++++++++++++++----------------
 arch/arm/cpu/armv7/tegra2/ap20.h   |    3 ++
 3 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/Makefile b/arch/arm/cpu/armv7/tegra2/Makefile
index f673f03..955c3b6 100644
--- a/arch/arm/cpu/armv7/tegra2/Makefile
+++ b/arch/arm/cpu/armv7/tegra2/Makefile
@@ -23,6 +23,11 @@
 # MA 02111-1307 USA
 #
 
+# The AVP is ARMv4T architecture so we must use special compiler
+# flags for any startup files it might use.
+CFLAGS_arch/arm/cpu/armv7/tegra2/ap20.o += -march=armv4t
+CFLAGS_arch/arm/cpu/armv7/tegra2/clock.o += -march=armv4t
+
 include $(TOPDIR)/config.mk
 
 LIB	=  $(obj)lib$(SOC).o
@@ -36,7 +41,7 @@ OBJS	:= $(addprefix $(obj),$(COBJS) $(SOBJS))
 all:	 $(obj).depend $(LIB)
 
 $(LIB):	$(OBJS)
-	$(AR) $(ARFLAGS) $@ $(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
 
 #########################################################################
 
diff --git a/arch/arm/cpu/armv7/tegra2/ap20.c b/arch/arm/cpu/armv7/tegra2/ap20.c
index 5cb4b1b..4c44bb3 100644
--- a/arch/arm/cpu/armv7/tegra2/ap20.c
+++ b/arch/arm/cpu/armv7/tegra2/ap20.c
@@ -31,7 +31,12 @@
 #include <asm/arch/scu.h>
 #include <common.h>
 
-u32 s_first_boot = 1;
+/* Returns 1 if the current CPU executing is a Cortex-A9, else 0 */
+static int ap20_cpu_is_cortexa9(void)
+{
+	u32 id = readb(NV_PA_PG_UP_BASE + PG_UP_TAG_0);
+	return id == (PG_UP_TAG_0_PID_CPU & 0xff);
+}
 
 void init_pllx(void)
 {
@@ -283,38 +288,37 @@ void init_pmc_scratch(void)
 	writel(CONFIG_SYS_BOARD_ODMDATA, &pmc->pmc_scratch20);
 }
 
-void cpu_start(void)
+void tegra2_start(void)
 {
 	struct pmux_tri_ctlr *pmt = (struct pmux_tri_ctlr *)NV_PA_APB_MISC_BASE;
 
-	/* enable JTAG */
-	writel(0xC0, &pmt->pmt_cfg_ctl);
+	/* If we are the AVP, start up the first Cortex-A9 */
+	if (!ap20_cpu_is_cortexa9()) {
+		/* enable JTAG */
+		writel(0xC0, &pmt->pmt_cfg_ctl);
 
-	if (s_first_boot) {
 		/*
-		 * Need to set this before cold-booting,
-		 *  otherwise we'll end up in an infinite loop.
-		 */
-		s_first_boot = 0;
-		cold_boot();
+		* If we are ARM7 - give it a different stack. We are about to
+		* start up the A9 which will want to use this one.
+		*/
+		asm volatile("ldr	sp, =%c0\n"
+			: : "i"(AVP_EARLY_BOOT_STACK_LIMIT));
+
+		start_cpu((u32)_start);
+		halt_avp();
+		/* not reached */
 	}
-}
 
-void tegra2_start()
-{
-	if (s_first_boot) {
-		/* Init Debug UART Port (115200 8n1) */
-		uart_init();
+	/* Init PMC scratch memory */
+	init_pmc_scratch();
 
-		/* Init PMC scratch memory */
-		init_pmc_scratch();
-	}
+	enable_scu();
 
-#ifdef CONFIG_ENABLE_CORTEXA9
-	/* take the mpcore out of reset */
-	cpu_start();
+	/* enable SMP mode and FW for CPU0, by writing to Auxiliary Ctl reg */
+	asm volatile(
+		"mrc	p15, 0, r0, c1, c0, 1\n"
+		"orr	r0, r0, #0x41\n"
+		"mcr	p15, 0, r0, c1, c0, 1\n");
 
-	/* configure cache */
-	cache_configure();
-#endif
+	/* FIXME: should have ap20's L2 disabled too? */
 }
diff --git a/arch/arm/cpu/armv7/tegra2/ap20.h b/arch/arm/cpu/armv7/tegra2/ap20.h
index 49fe340..1bb48d6 100644
--- a/arch/arm/cpu/armv7/tegra2/ap20.h
+++ b/arch/arm/cpu/armv7/tegra2/ap20.h
@@ -102,3 +102,6 @@ void uart_init(void);
 void udelay(unsigned long);
 void cold_boot(void);
 void cache_configure(void);
+
+/* This is the main entry into U-Boot, used by the Cortex-A9 */
+extern void _start(void);
-- 
1.7.3.1

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Some SOCs have do not start up with their 'main' CPU. The first U-Boot
code may then be executed with a CPU which does not have a CP15, or not a
useful one.

Here we split the initialization of CP15 into a separate call, which can
be performed later if required.

Once the main CPU is running, you should call cpu_init_cp15() to perform
this init as early as possible.

Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not
need to change, this CP15 init is still skipped in that case. The only
impact for these boards is that the cpu_init_cp15() will be available
even if it is never used on these boards.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/start.S        |   24 ++++++++++++++++++------
 arch/arm/include/asm/u-boot-arm.h |    3 +++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index db8e9d2..7fdd422 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -168,6 +168,7 @@ next:
 #endif
 	/* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
+	bl	cpu_reset_cp15
 	bl	cpu_init_crit
 #endif
 
@@ -305,17 +306,16 @@ jump_2_ram:
 _board_init_r_ofs:
 	.word board_init_r - _start
 
-
-#ifndef CONFIG_SKIP_LOWLEVEL_INIT
 /*************************************************************************
  *
- * CPU_init_critical registers
+ * cpu_reset_cp15
  *
- * setup important registers
- * setup memory timing
+ * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on unless
+ * CONFIG_SYS_ICACHE_OFF is defined.
  *
  *************************************************************************/
-cpu_init_crit:
+.globl cpu_init_cp15
+cpu_init_cp15:
 	/*
 	 * Invalidate L1 I/D
 	 */
@@ -340,7 +340,19 @@ cpu_init_crit:
 	orr	r0, r0, #0x00001000	@ set bit 12 (I) I-cache
 #endif
 	mcr	p15, 0, r0, c1, c0, 0
+	mov	pc, lr			@ back to my caller
 
+
+#ifndef CONFIG_SKIP_LOWLEVEL_INIT
+/*************************************************************************
+ *
+ * CPU_init_critical registers
+ *
+ * setup important registers
+ * setup memory timing
+ *
+ *************************************************************************/
+cpu_init_crit:
 	/*
 	 * Jump to board specific initialization...
 	 * The Mask ROM will have already initialized
diff --git a/arch/arm/include/asm/u-boot-arm.h b/arch/arm/include/asm/u-boot-arm.h
index d3308f7..4ca75f9 100644
--- a/arch/arm/include/asm/u-boot-arm.h
+++ b/arch/arm/include/asm/u-boot-arm.h
@@ -46,6 +46,9 @@ extern ulong IRQ_STACK_START_IN;	/* 8 bytes in IRQ stack */
 int	cpu_init(void);
 int	cleanup_before_linux(void);
 
+/* Set up ARMv7 MMU, caches and TLBs */
+void	cpu_init_cp15(void);
+
 /* cpu/.../arch/cpu.c */
 int	arch_cpu_init(void);
 int	arch_misc_init(void);
-- 
1.7.3.1

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Le 13/10/2011 23:05, Simon Glass a ?crit :
> Some SOCs have do not start up with their 'main' CPU. The first U-Boot
> code may then be executed with a CPU which does not have a CP15, or not a
> useful one.
>
> Here we split the initialization of CP15 into a separate call, which can
> be performed later if required.
>
> Once the main CPU is running, you should call cpu_init_cp15() to perform
> this init as early as possible.
>
> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not
> need to change, this CP15 init is still skipped in that case. The only
> impact for these boards is that the cpu_init_cp15() will be available
> even if it is never used on these boards.

I'm not too sure I understand how this is working: if you are moving 
cp15 init to later, it will still be done by the same core that would 
have done it earlier, won't it?

IOW, I would like to better understand how this boot core/main core 
business works. How does the main core start execution? At which 
address? Does it go through its reset vector? Do the cores share the 
same location for reset vectors? Etc.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Fri, Oct 21, 2011 at 1:58 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 13/10/2011 23:05, Simon Glass a ?crit :
>>
>> Some SOCs have do not start up with their 'main' CPU. The first U-Boot
>> code may then be executed with a CPU which does not have a CP15, or not a
>> useful one.
>>
>> Here we split the initialization of CP15 into a separate call, which can
>> be performed later if required.
>>
>> Once the main CPU is running, you should call cpu_init_cp15() to perform
>> this init as early as possible.
>>
>> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not
>> need to change, this CP15 init is still skipped in that case. The only
>> impact for these boards is that the cpu_init_cp15() will be available
>> even if it is never used on these boards.
>
> I'm not too sure I understand how this is working: if you are moving cp15
> init to later, it will still be done by the same core that would have done
> it earlier, won't it?
>
> IOW, I would like to better understand how this boot core/main core business
> works. How does the main core start execution? At which address? Does it go
> through its reset vector? Do the cores share the same location for reset
> vectors? Etc.

For Tegra is it like this:

The ARM7 CPU (called AVP for Audio Video Processor) starts up first.
It runs the boot ROM and then U-Boot and gets as far as
arch_cpu_init(). The AVP does not have a CP15 or a cache so cannot run
the CP15 init code. The AVP then starts up the first Cortex-A9 (an
ARMv7 architecture CPU). This CPU (the main core, if you like) starts
from the same address as the first one (i.e. the start of U-Boot). It
is as if this is the core that we really wanted to use, but it wasn't
available initially. This main core runs through arch_cpu_init() and
sails into board_init_f(). At this point no CP15 init has been done.

Basically there is now an assumption in start.S that the current CPU
has a CP15 and that it is useful to fiddle with it. By splitting out
that code into a separate function we can:

1. Avoid the AVP fiddling with CP15
2. Have the Cotex-A9 (the intended target of the code) get its CP15
init when needed

Note that all the cores have a unified view of memory, peripherals, etc.

Regards,
Simon

>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Le 21/10/2011 23:18, Simon Glass a ?crit :
> Hi Albert,
>
> On Fri, Oct 21, 2011 at 1:58 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net>  wrote:
>> Le 13/10/2011 23:05, Simon Glass a ?crit :
>>>
>>> Some SOCs have do not start up with their 'main' CPU. The first U-Boot
>>> code may then be executed with a CPU which does not have a CP15, or not a
>>> useful one.
>>>
>>> Here we split the initialization of CP15 into a separate call, which can
>>> be performed later if required.
>>>
>>> Once the main CPU is running, you should call cpu_init_cp15() to perform
>>> this init as early as possible.
>>>
>>> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not
>>> need to change, this CP15 init is still skipped in that case. The only
>>> impact for these boards is that the cpu_init_cp15() will be available
>>> even if it is never used on these boards.
>>
>> I'm not too sure I understand how this is working: if you are moving cp15
>> init to later, it will still be done by the same core that would have done
>> it earlier, won't it?
>>
>> IOW, I would like to better understand how this boot core/main core business
>> works. How does the main core start execution? At which address? Does it go
>> through its reset vector? Do the cores share the same location for reset
>> vectors? Etc.
>
> For Tegra is it like this:
>
> The ARM7 CPU (called AVP for Audio Video Processor) starts up first.
> It runs the boot ROM and then U-Boot and gets as far as
> arch_cpu_init(). The AVP does not have a CP15 or a cache so cannot run
> the CP15 init code. The AVP then starts up the first Cortex-A9 (an
> ARMv7 architecture CPU). This CPU (the main core, if you like) starts
> from the same address as the first one (i.e. the start of U-Boot). It
> is as if this is the core that we really wanted to use, but it wasn't
> available initially. This main core runs through arch_cpu_init() and
> sails into board_init_f(). At this point no CP15 init has been done.

Thanks. So what this amounts to is, both cores will run the same binary, 
and I assume the AVP will shut itself off once the A9 runs. But what I 
don't get is, if A9 goes through the same sequence of code as AVP, then 
it will execute cp15 where is is not, won't it?

Anyway: your patch moves cp15 init far enough that the AVP won't execute 
it, but the A9 will.

Only, what will happen when another multiple-core ARM SoC gets U-Boot 
support but the location you chose for cp15 init is inadequate for it? 
Shall we move cp15 init again, and where?

I'd prefer the cp15 init to stay where it is but execute only for A9, 
for instance by checking some core identification register.

> Basically there is now an assumption in start.S that the current CPU
> has a CP15 and that it is useful to fiddle with it. By splitting out
> that code into a separate function we can:
>
> 1. Avoid the AVP fiddling with CP15
> 2. Have the Cotex-A9 (the intended target of the code) get its CP15
> init when needed
>
> Note that all the cores have a unified view of memory, peripherals, etc.
>
> Regards,
> Simon
>
>>
>> Amicalement,
>> --
>> Albert.
>>
>
>


Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Fri, Oct 21, 2011 at 2:43 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 21/10/2011 23:18, Simon Glass a ?crit :
>>
>> Hi Albert,
>>
>> On Fri, Oct 21, 2011 at 1:58 PM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net> ?wrote:
>>>
>>> Le 13/10/2011 23:05, Simon Glass a ?crit :
>>>>
>>>> Some SOCs have do not start up with their 'main' CPU. The first U-Boot
>>>> code may then be executed with a CPU which does not have a CP15, or not
>>>> a
>>>> useful one.
>>>>
>>>> Here we split the initialization of CP15 into a separate call, which can
>>>> be performed later if required.
>>>>
>>>> Once the main CPU is running, you should call cpu_init_cp15() to perform
>>>> this init as early as possible.
>>>>
>>>> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not
>>>> need to change, this CP15 init is still skipped in that case. The only
>>>> impact for these boards is that the cpu_init_cp15() will be available
>>>> even if it is never used on these boards.
>>>
>>> I'm not too sure I understand how this is working: if you are moving cp15
>>> init to later, it will still be done by the same core that would have
>>> done
>>> it earlier, won't it?
>>>
>>> IOW, I would like to better understand how this boot core/main core
>>> business
>>> works. How does the main core start execution? At which address? Does it
>>> go
>>> through its reset vector? Do the cores share the same location for reset
>>> vectors? Etc.
>>
>> For Tegra is it like this:
>>
>> The ARM7 CPU (called AVP for Audio Video Processor) starts up first.
>> It runs the boot ROM and then U-Boot and gets as far as
>> arch_cpu_init(). The AVP does not have a CP15 or a cache so cannot run
>> the CP15 init code. The AVP then starts up the first Cortex-A9 (an
>> ARMv7 architecture CPU). This CPU (the main core, if you like) starts
>> from the same address as the first one (i.e. the start of U-Boot). It
>> is as if this is the core that we really wanted to use, but it wasn't
>> available initially. This main core runs through arch_cpu_init() and
>> sails into board_init_f(). At this point no CP15 init has been done.
>
> Thanks. So what this amounts to is, both cores will run the same binary, and
> I assume the AVP will shut itself off once the A9 runs. But what I don't get

Yes

> is, if A9 goes through the same sequence of code as AVP, then it will
> execute cp15 where is is not, won't it?

On Tegra we have CONFIG_SKIP_LOWLEVEL_INIT set to avoid this. That
much is already supported by U-Boot.

>
> Anyway: your patch moves cp15 init far enough that the AVP won't execute it,
> but the A9 will.

Well, on Tegra we call the cp15 init directly when it is safe to do so, later.

>
> Only, what will happen when another multiple-core ARM SoC gets U-Boot
> support but the location you chose for cp15 init is inadequate for it? Shall
> we move cp15 init again, and where?

If CONFIG_SKIP_LOWLEVEL_INIT is not defined, then the low level init
will operate exactly as now. My patch effectively just allows you to
have CONFIG_SKIP_LOWLEVEL_INIT but later call part of that lowlevel
init. In my view, the cp15 init should not be lumped in with the
'memory init' or whatever else it expected to be done in the lowlevel
init.

>
> I'd prefer the cp15 init to stay where it is but execute only for A9, for
> instance by checking some core identification register.

Well I actually haven't moved it! It is just that previously it was
impossible to call cp15_init from anywhere later.

What you say can be done, it would involve some assembler though and
would need to be another CONFIG option. Was trying to avoid adding new
assembler.

Regards,
Simon

>
>> Basically there is now an assumption in start.S that the current CPU
>> has a CP15 and that it is useful to fiddle with it. By splitting out
>> that code into a separate function we can:
>>
>> 1. Avoid the AVP fiddling with CP15
>> 2. Have the Cotex-A9 (the intended target of the code) get its CP15
>> init when needed
>>
>> Note that all the cores have a unified view of memory, peripherals, etc.
>>
>> Regards,
>> Simon
>>
>>>
>>> Amicalement,
>>> --
>>> Albert.
>>>
>>
>>
>
>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Le 21/10/2011 23:54, Simon Glass a ?crit :
> Hi Albert,
>
> On Fri, Oct 21, 2011 at 2:43 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net>  wrote:
>> Le 21/10/2011 23:18, Simon Glass a ?crit :
>>>
>>> Hi Albert,
>>>
>>> On Fri, Oct 21, 2011 at 1:58 PM, Albert ARIBAUD
>>> <albert.u.boot@aribaud.net>    wrote:
>>>>
>>>> Le 13/10/2011 23:05, Simon Glass a ?crit :
>>>>>
>>>>> Some SOCs have do not start up with their 'main' CPU. The first U-Boot
>>>>> code may then be executed with a CPU which does not have a CP15, or not
>>>>> a
>>>>> useful one.
>>>>>
>>>>> Here we split the initialization of CP15 into a separate call, which can
>>>>> be performed later if required.
>>>>>
>>>>> Once the main CPU is running, you should call cpu_init_cp15() to perform
>>>>> this init as early as possible.
>>>>>
>>>>> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not
>>>>> need to change, this CP15 init is still skipped in that case. The only
>>>>> impact for these boards is that the cpu_init_cp15() will be available
>>>>> even if it is never used on these boards.
>>>>
>>>> I'm not too sure I understand how this is working: if you are moving cp15
>>>> init to later, it will still be done by the same core that would have
>>>> done
>>>> it earlier, won't it?
>>>>
>>>> IOW, I would like to better understand how this boot core/main core
>>>> business
>>>> works. How does the main core start execution? At which address? Does it
>>>> go
>>>> through its reset vector? Do the cores share the same location for reset
>>>> vectors? Etc.
>>>
>>> For Tegra is it like this:
>>>
>>> The ARM7 CPU (called AVP for Audio Video Processor) starts up first.
>>> It runs the boot ROM and then U-Boot and gets as far as
>>> arch_cpu_init(). The AVP does not have a CP15 or a cache so cannot run
>>> the CP15 init code. The AVP then starts up the first Cortex-A9 (an
>>> ARMv7 architecture CPU). This CPU (the main core, if you like) starts
>>> from the same address as the first one (i.e. the start of U-Boot). It
>>> is as if this is the core that we really wanted to use, but it wasn't
>>> available initially. This main core runs through arch_cpu_init() and
>>> sails into board_init_f(). At this point no CP15 init has been done.
>>
>> Thanks. So what this amounts to is, both cores will run the same binary, and
>> I assume the AVP will shut itself off once the A9 runs. But what I don't get
>
> Yes
>
>> is, if A9 goes through the same sequence of code as AVP, then it will
>> execute cp15 where is is not, won't it?
>
> On Tegra we have CONFIG_SKIP_LOWLEVEL_INIT set to avoid this. That
> much is already supported by U-Boot.

I still don't get it: if you have lowlevel init skipped for one core, it 
will be for the other as well.

>> Anyway: your patch moves cp15 init far enough that the AVP won't execute it,
>> but the A9 will.
>
> Well, on Tegra we call the cp15 init directly when it is safe to do so, later.
>
>>
>> Only, what will happen when another multiple-core ARM SoC gets U-Boot
>> support but the location you chose for cp15 init is inadequate for it? Shall
>> we move cp15 init again, and where?
>
> If CONFIG_SKIP_LOWLEVEL_INIT is not defined, then the low level init
> will operate exactly as now. My patch effectively just allows you to
> have CONFIG_SKIP_LOWLEVEL_INIT but later call part of that lowlevel
> init. In my view, the cp15 init should not be lumped in with the
> 'memory init' or whatever else it expected to be done in the lowlevel
> init.

I think I get the point: due to the fact that both cores run the same 
starup code path, and due to the fact that they run it one after the 
other and share a lot of devices, at mots one of them shoud perform low 
level inits (e.g. RAM init), and some low-level inits can only be 
performed by one of them (e.g., cp15).

>> I'd prefer the cp15 init to stay where it is but execute only for A9, for
>> instance by checking some core identification register.
>
> Well I actually haven't moved it! It is just that previously it was
> impossible to call cp15_init from anywhere later.

It is moved, in that it belongs to low level init... of A9.

> What you say can be done, it would involve some assembler though and
> would need to be another CONFIG option. Was trying to avoid adding new
> assembler.

Low level init is about assembler and, well, low level. :)

But I don't see why there should be another CONFIG option. IIUC, you 
should be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low 
level init code under it, you would do the equivalent of a switch, with 
one branch for AVM (and DDR init etc) and one branch for A9 (with cp15 
init etc).

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Fri, Oct 21, 2011 at 3:24 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 21/10/2011 23:54, Simon Glass a ?crit :
>>
>> Hi Albert,
>>
>> On Fri, Oct 21, 2011 at 2:43 PM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net> ?wrote:
>>>
>>> Le 21/10/2011 23:18, Simon Glass a ?crit :
>>>>
>>>> Hi Albert,
>>>>
>>>> On Fri, Oct 21, 2011 at 1:58 PM, Albert ARIBAUD
>>>> <albert.u.boot@aribaud.net> ? ?wrote:
>>>>>
>>>>> Le 13/10/2011 23:05, Simon Glass a ?crit :
>>>>>>
>>>>>> Some SOCs have do not start up with their 'main' CPU. The first U-Boot
>>>>>> code may then be executed with a CPU which does not have a CP15, or
>>>>>> not
>>>>>> a
>>>>>> useful one.
>>>>>>
>>>>>> Here we split the initialization of CP15 into a separate call, which
>>>>>> can
>>>>>> be performed later if required.
>>>>>>
>>>>>> Once the main CPU is running, you should call cpu_init_cp15() to
>>>>>> perform
>>>>>> this init as early as possible.
>>>>>>
>>>>>> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should
>>>>>> not
>>>>>> need to change, this CP15 init is still skipped in that case. The only
>>>>>> impact for these boards is that the cpu_init_cp15() will be available
>>>>>> even if it is never used on these boards.
>>>>>
>>>>> I'm not too sure I understand how this is working: if you are moving
>>>>> cp15
>>>>> init to later, it will still be done by the same core that would have
>>>>> done
>>>>> it earlier, won't it?
>>>>>
>>>>> IOW, I would like to better understand how this boot core/main core
>>>>> business
>>>>> works. How does the main core start execution? At which address? Does
>>>>> it
>>>>> go
>>>>> through its reset vector? Do the cores share the same location for
>>>>> reset
>>>>> vectors? Etc.
>>>>
>>>> For Tegra is it like this:
>>>>
>>>> The ARM7 CPU (called AVP for Audio Video Processor) starts up first.
>>>> It runs the boot ROM and then U-Boot and gets as far as
>>>> arch_cpu_init(). The AVP does not have a CP15 or a cache so cannot run
>>>> the CP15 init code. The AVP then starts up the first Cortex-A9 (an
>>>> ARMv7 architecture CPU). This CPU (the main core, if you like) starts
>>>> from the same address as the first one (i.e. the start of U-Boot). It
>>>> is as if this is the core that we really wanted to use, but it wasn't
>>>> available initially. This main core runs through arch_cpu_init() and
>>>> sails into board_init_f(). At this point no CP15 init has been done.
>>>
>>> Thanks. So what this amounts to is, both cores will run the same binary,
>>> and
>>> I assume the AVP will shut itself off once the A9 runs. But what I don't
>>> get
>>
>> Yes
>>
>>> is, if A9 goes through the same sequence of code as AVP, then it will
>>> execute cp15 where is is not, won't it?
>>
>> On Tegra we have CONFIG_SKIP_LOWLEVEL_INIT set to avoid this. That
>> much is already supported by U-Boot.
>
> I still don't get it: if you have lowlevel init skipped for one core, it
> will be for the other as well.

OK so let's run through the U-Boot code in order for each of the two CPUs:

First AVP:
_start
skips lowlevel init (so no CP15)
board_init_f
arch_cpu_init
(at this point it starts up the A9 and halts)

A9:
_start
skips lowlevel init (so no CP15)
board_init_f
arch_cpu_init
cpu_init_cp15

The call to cpu_init_cp15() is performed in arch_cpu_init() when we
detect that we are the A9.

>
>>> Anyway: your patch moves cp15 init far enough that the AVP won't execute
>>> it,
>>> but the A9 will.
>>
>> Well, on Tegra we call the cp15 init directly when it is safe to do so,
>> later.
>>
>>>
>>> Only, what will happen when another multiple-core ARM SoC gets U-Boot
>>> support but the location you chose for cp15 init is inadequate for it?
>>> Shall
>>> we move cp15 init again, and where?
>>
>> If CONFIG_SKIP_LOWLEVEL_INIT is not defined, then the low level init
>> will operate exactly as now. My patch effectively just allows you to
>> have CONFIG_SKIP_LOWLEVEL_INIT but later call part of that lowlevel
>> init. In my view, the cp15 init should not be lumped in with the
>> 'memory init' or whatever else it expected to be done in the lowlevel
>> init.
>
> I think I get the point: due to the fact that both cores run the same starup
> code path, and due to the fact that they run it one after the other and
> share a lot of devices, at mots one of them shoud perform low level inits
> (e.g. RAM init), and some low-level inits can only be performed by one of
> them (e.g., cp15).

Well on Tegra the RAM init is down by the boot ROM. There is no low
level init bar the cp15 stuff.

>
>>> I'd prefer the cp15 init to stay where it is but execute only for A9, for
>>> instance by checking some core identification register.
>>
>> Well I actually haven't moved it! It is just that previously it was
>> impossible to call cp15_init from anywhere later.
>
> It is moved, in that it belongs to low level init... of A9.

OK, I see - you mean moved in order if not in source code file.

>
>> What you say can be done, it would involve some assembler though and
>> would need to be another CONFIG option. Was trying to avoid adding new
>> assembler.
>
> Low level init is about assembler and, well, low level. :)

Yes but it's yuck. Part of the clean-up is to remove most of the
assembler - really very little is needed.

>
> But I don't see why there should be another CONFIG option. IIUC, you should
> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level init
> code under it, you would do the equivalent of a switch, with one branch for
> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc).

Yes I can, but I need to be able to call cp15_init. And I can't do
that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is
compiled out! So I need to move that cp15_init code outside the
CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do,
honest!

Regards,
Simon

>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Le 22/10/2011 07:05, Simon Glass a ?crit :

>>> Well I actually haven't moved it! It is just that previously it was
>>> impossible to call cp15_init from anywhere later.
>>
>> It is moved, in that it belongs to low level init... of A9.
>
> OK, I see - you mean moved in order if not in source code file.

Yes. In the code, though, it belongs to low-level init.

>>> What you say can be done, it would involve some assembler though and
>>> would need to be another CONFIG option. Was trying to avoid adding new
>>> assembler.
>>
>> Low level init is about assembler and, well, low level. :)
>
> Yes but it's yuck. Part of the clean-up is to remove most of the
> assembler - really very little is needed.

I don't think "yuck" is a valid reasoned argument against assembly 
language :) but I assume what you mainly mean is 'hard to understand and 
replaceable by easy to understand C code'. I agree to the first part, 
and to the second one for the general case, but not for the startup path 
where, precisely, we don't have a working C run-time and most of the 
understanding requires knowledge of hardware, not only general 
programming knowledge. Past that, C is ok -- but then beware: some C++ 
guy could chime in and contend that C is "yuck" by your own standards. :)

>> But I don't see why there should be another CONFIG option. IIUC, you should
>> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level init
>> code under it, you would do the equivalent of a switch, with one branch for
>> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc).
>
> Yes I can, but I need to be able to call cp15_init. And I can't do
> that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is
> compiled out! So I need to move that cp15_init code outside the
> CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do,
> honest!

I understand, and I think the approach here is wrong, because you *do* 
want low-level init in the A9 case, and thus you should not have 
CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init 
to be empty, and the AVP and A9 must follow the same startup path. And 
all this it not necessarily required for all armv7 platforms, but some 
of them will want lowlevel init, and some not. Considering all this...

For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option 
which start.S would use to compile out the low level init code 
definition (like CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls 
to it (UNline CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a 
SoC-specific version of lowlevel_init.

In the longer term, I would be more in favor of a weak definition system 
with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind 
of fond of it) for lowlevel init -- but that will require looking into 
asm weak symbol handling and may require splitting of the assembly code 
function by function.

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Sat, Oct 22, 2011 at 12:56 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 22/10/2011 07:05, Simon Glass a ?crit :
>
>>>> Well I actually haven't moved it! It is just that previously it was
>>>> impossible to call cp15_init from anywhere later.
>>>
>>> It is moved, in that it belongs to low level init... of A9.
>>
>> OK, I see - you mean moved in order if not in source code file.
>
> Yes. In the code, though, it belongs to low-level init.

Arguably this could go in a library like arch/arm/cpu/armv7/lib/cache.S

>
>>>> What you say can be done, it would involve some assembler though and
>>>> would need to be another CONFIG option. Was trying to avoid adding new
>>>> assembler.
>>>
>>> Low level init is about assembler and, well, low level. :)
>>
>> Yes but it's yuck. Part of the clean-up is to remove most of the
>> assembler - really very little is needed.
>
> I don't think "yuck" is a valid reasoned argument against assembly language
> :) but I assume what you mainly mean is 'hard to understand and replaceable
> by easy to understand C code'. I agree to the first part, and to the second
> one for the general case, but not for the startup path where, precisely, we
> don't have a working C run-time and most of the understanding requires
> knowledge of hardware, not only general programming knowledge. Past that, C

I'm not arguing for the cache init stuff to move to C, just trying to
avoid any creeping growth of assembler when C can do it.

> is ok -- but then beware: some C++ guy could chime in and contend that C is
> "yuck" by your own standards. :)

The C++ guy who finds himself in U-Boot is probably lost :-)

>
>>> But I don't see why there should be another CONFIG option. IIUC, you
>>> should
>>> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level
>>> init
>>> code under it, you would do the equivalent of a switch, with one branch
>>> for
>>> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc).
>>
>> Yes I can, but I need to be able to call cp15_init. And I can't do
>> that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is
>> compiled out! So I need to move that cp15_init code outside the
>> CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do,
>> honest!
>
> I understand, and I think the approach here is wrong, because you *do* want
> low-level init in the A9 case, and thus you should not have
> CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init to
> be empty, and the AVP and A9 must follow the same startup path. And all this
> it not necessarily required for all armv7 platforms, but some of them will
> want lowlevel init, and some not. Considering all this...
>
> For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option which
> start.S would use to compile out the low level init code definition (like
> CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls to it (UNline
> CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a SoC-specific
> version of lowlevel_init.

If I understand you correctly, this is the opposite of what I want. It
is basically what we have now.

All ARMv7 chips are going to turn off MMU, caches, flush TLBs, and the
like - this is an architecture feature, not an SOC one. We have
conflated ARM arch init with SOC init as you say, but the solution is
not to force the SOC to copy code from start.S just so that it can do
ARM arch init later.

And I'd argue for LOWLEVEL_INIT being about setting up the memory so
we can run code, and in particular relocate it later. The cp15_init
stuff should arguably be in arch_cpu_init() (or perhaps a new
arch_init()) as the first thing called from board_init_f() in my view.

So how about I create a patch to move the cp15_init() code into
arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later?

>
> In the longer term, I would be more in favor of a weak definition system
> with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind of
> fond of it) for lowlevel init -- but that will require looking into asm weak
> symbol handling and may require splitting of the assembly code function by
> function.

I think you are trying to avoid the

#ifdef CONFIG_ARCH_CPU_INIT
   arch_cpu_init,
#endif
#ifdef ...
   some_other_thing
#endif

in the board_init_f() function table. Yes I agree it would tidy things
up, although I wonder if Graeme's initcall thing isn't better again.
It has the same confusion level coefficient (only the linker decides
what code is included) with a bit more flexibility. Then each arch /
board / cpu / soc / squirrel can decide on its own init and put it in
its own file. C only please :-)

Regards,
Simon

>
>> Regards,
>> Simon
>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Sat, Oct 22, 2011 at 9:13 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Albert,
>
> On Sat, Oct 22, 2011 at 12:56 AM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>> Le 22/10/2011 07:05, Simon Glass a ?crit :
>>
>>>>> Well I actually haven't moved it! It is just that previously it was
>>>>> impossible to call cp15_init from anywhere later.
>>>>
>>>> It is moved, in that it belongs to low level init... of A9.
>>>
>>> OK, I see - you mean moved in order if not in source code file.
>>
>> Yes. In the code, though, it belongs to low-level init.
>
> Arguably this could go in a library like arch/arm/cpu/armv7/lib/cache.S
>
>>
>>>>> What you say can be done, it would involve some assembler though and
>>>>> would need to be another CONFIG option. Was trying to avoid adding new
>>>>> assembler.
>>>>
>>>> Low level init is about assembler and, well, low level. :)
>>>
>>> Yes but it's yuck. Part of the clean-up is to remove most of the
>>> assembler - really very little is needed.
>>
>> I don't think "yuck" is a valid reasoned argument against assembly language
>> :) but I assume what you mainly mean is 'hard to understand and replaceable
>> by easy to understand C code'. I agree to the first part, and to the second
>> one for the general case, but not for the startup path where, precisely, we
>> don't have a working C run-time and most of the understanding requires
>> knowledge of hardware, not only general programming knowledge. Past that, C
>
> I'm not arguing for the cache init stuff to move to C, just trying to
> avoid any creeping growth of assembler when C can do it.
>
>> is ok -- but then beware: some C++ guy could chime in and contend that C is
>> "yuck" by your own standards. :)
>
> The C++ guy who finds himself in U-Boot is probably lost :-)
>
>>
>>>> But I don't see why there should be another CONFIG option. IIUC, you
>>>> should
>>>> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level
>>>> init
>>>> code under it, you would do the equivalent of a switch, with one branch
>>>> for
>>>> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc).
>>>
>>> Yes I can, but I need to be able to call cp15_init. And I can't do
>>> that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is
>>> compiled out! So I need to move that cp15_init code outside the
>>> CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do,
>>> honest!
>>
>> I understand, and I think the approach here is wrong, because you *do* want
>> low-level init in the A9 case, and thus you should not have
>> CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init to
>> be empty, and the AVP and A9 must follow the same startup path. And all this
>> it not necessarily required for all armv7 platforms, but some of them will
>> want lowlevel init, and some not. Considering all this...
>>
>> For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option which
>> start.S would use to compile out the low level init code definition (like
>> CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls to it (UNline
>> CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a SoC-specific
>> version of lowlevel_init.
>
> If I understand you correctly, this is the opposite of what I want. It
> is basically what we have now.
>
> All ARMv7 chips are going to turn off MMU, caches, flush TLBs, and the
> like - this is an architecture feature, not an SOC one. We have
> conflated ARM arch init with SOC init as you say, but the solution is
> not to force the SOC to copy code from start.S just so that it can do
> ARM arch init later.
>
> And I'd argue for LOWLEVEL_INIT being about setting up the memory so
> we can run code, and in particular relocate it later. The cp15_init
> stuff should arguably be in arch_cpu_init() (or perhaps a new
> arch_init()) as the first thing called from board_init_f() in my view.
>
> So how about I create a patch to move the cp15_init() code into
> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later?

I had a look at this, and it is a bit of a pain since the SPL builds
all bring in files by name from random directories around U-Boot,
including arch/arm/cpu/armv7/start.S. If I move code out of that file
into lib.S or cp15.S then I need to fix up various SPL builds. But it
seems to me that it would be better for arch/arm/cpu/armv7/Makefile
(another other Makefiles) to specify what object files are needed for
an SPL build, rather than putting it in *_spl/Makefile.

Therefore I have left this along, and would like proceed as previously!

I will send an updated patch series when MAKEALL finishes.

Regards,
Simon

>
>>
>> In the longer term, I would be more in favor of a weak definition system
>> with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind of
>> fond of it) for lowlevel init -- but that will require looking into asm weak
>> symbol handling and may require splitting of the assembly code function by
>> function.
>
> I think you are trying to avoid the
>
> #ifdef CONFIG_ARCH_CPU_INIT
> ? arch_cpu_init,
> #endif
> #ifdef ...
> ? some_other_thing
> #endif
>
> in the board_init_f() function table. Yes I agree it would tidy things
> up, although I wonder if Graeme's initcall thing isn't better again.
> It has the same confusion level coefficient (only the linker decides
> what code is included) with a bit more flexibility. Then each arch /
> board / cpu / soc / squirrel can decide on its own init and put it in
> its own file. C only please :-)
>
> Regards,
> Simon
>
>>
>>> Regards,
>>> Simon
>>
>> Amicalement,
>> --
>> Albert.
>>
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Le 22/10/2011 18:13, Simon Glass a ?crit :
> Hi Albert,
>
> On Sat, Oct 22, 2011 at 12:56 AM, Albert ARIBAUD
> <albert.u.boot@aribaud.net>  wrote:
>> Le 22/10/2011 07:05, Simon Glass a ?crit :
>>
>>>>> Well I actually haven't moved it! It is just that previously it was
>>>>> impossible to call cp15_init from anywhere later.
>>>>
>>>> It is moved, in that it belongs to low level init... of A9.
>>>
>>> OK, I see - you mean moved in order if not in source code file.
>>
>> Yes. In the code, though, it belongs to low-level init.
>
> Arguably this could go in a library like arch/arm/cpu/armv7/lib/cache.S
>
>>
>>>>> What you say can be done, it would involve some assembler though and
>>>>> would need to be another CONFIG option. Was trying to avoid adding new
>>>>> assembler.
>>>>
>>>> Low level init is about assembler and, well, low level. :)
>>>
>>> Yes but it's yuck. Part of the clean-up is to remove most of the
>>> assembler - really very little is needed.
>>
>> I don't think "yuck" is a valid reasoned argument against assembly language
>> :) but I assume what you mainly mean is 'hard to understand and replaceable
>> by easy to understand C code'. I agree to the first part, and to the second
>> one for the general case, but not for the startup path where, precisely, we
>> don't have a working C run-time and most of the understanding requires
>> knowledge of hardware, not only general programming knowledge. Past that, C
>
> I'm not arguing for the cache init stuff to move to C, just trying to
> avoid any creeping growth of assembler when C can do it.
>
>> is ok -- but then beware: some C++ guy could chime in and contend that C is
>> "yuck" by your own standards. :)
>
> The C++ guy who finds himself in U-Boot is probably lost :-)
>
>>
>>>> But I don't see why there should be another CONFIG option. IIUC, you
>>>> should
>>>> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level
>>>> init
>>>> code under it, you would do the equivalent of a switch, with one branch
>>>> for
>>>> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc).
>>>
>>> Yes I can, but I need to be able to call cp15_init. And I can't do
>>> that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is
>>> compiled out! So I need to move that cp15_init code outside the
>>> CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do,
>>> honest!
>>
>> I understand, and I think the approach here is wrong, because you *do* want
>> low-level init in the A9 case, and thus you should not have
>> CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init to
>> be empty, and the AVP and A9 must follow the same startup path. And all this
>> it not necessarily required for all armv7 platforms, but some of them will
>> want lowlevel init, and some not. Considering all this...
>>
>> For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option which
>> start.S would use to compile out the low level init code definition (like
>> CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls to it (UNline
>> CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a SoC-specific
>> version of lowlevel_init.
>
> If I understand you correctly, this is the opposite of what I want. It
> is basically what we have now.
>
> All ARMv7 chips are going to turn off MMU, caches, flush TLBs, and the
> like - this is an architecture feature, not an SOC one. We have
> conflated ARM arch init with SOC init as you say, but the solution is
> not to force the SOC to copy code from start.S just so that it can do
> ARM arch init later.

Yes, all ARMv7 CPUs will turn off caches etc, and thus, they all will 
want to do the cp15 init where it is now. Only the Tegra wants has 
issues with it so far.

> And I'd argue for LOWLEVEL_INIT being about setting up the memory so
> we can run code, and in particular relocate it later. The cp15_init
> stuff should arguably be in arch_cpu_init() (or perhaps a new
> arch_init()) as the first thing called from board_init_f() in my view.

I disagree that cp15 could be set later: if we don't set it at low level 
init, we cannot be sure whether data caches are off or on, and if on, 
what they contain already.

> So how about I create a patch to move the cp15_init() code into
> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later?

I don't mind moving it to armv7/lib. I do mind the "later".

>> In the longer term, I would be more in favor of a weak definition system
>> with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind of
>> fond of it) for lowlevel init -- but that will require looking into asm weak
>> symbol handling and may require splitting of the assembly code function by
>> function.
>
> I think you are trying to avoid the
>
> #ifdef CONFIG_ARCH_CPU_INIT
>     arch_cpu_init,
> #endif
> #ifdef ...
>     some_other_thing
> #endif
>
> in the board_init_f() function table. Yes I agree it would tidy things
> up, although I wonder if Graeme's initcall thing isn't better again.
> It has the same confusion level coefficient (only the linker decides
> what code is included) with a bit more flexibility. Then each arch /
> board / cpu / soc / squirrel can decide on its own init and put it in
> its own file. C only please :-)

AFAIU, initcall does not have an override mechanism by which a later 
module can replace an initcall symbol already defined; two modules 
defining the same function name as initcall will just end up causing a 
linker error.

> Regards,
> Simon
>
>>
>>> Regards,
>>> Simon
>>
>> Amicalement,
>> --
>> Albert.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Mon, Oct 24, 2011 at 1:04 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 22/10/2011 18:13, Simon Glass a ?crit :
>>
>> Hi Albert,
>>
>> On Sat, Oct 22, 2011 at 12:56 AM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net> ?wrote:
>>>
>>> Le 22/10/2011 07:05, Simon Glass a ?crit :
>>>
>>>>>> Well I actually haven't moved it! It is just that previously it was
>>>>>> impossible to call cp15_init from anywhere later.
>>>>>
>>>>> It is moved, in that it belongs to low level init... of A9.
>>>>
>>>> OK, I see - you mean moved in order if not in source code file.
>>>
>>> Yes. In the code, though, it belongs to low-level init.
>>
>> Arguably this could go in a library like arch/arm/cpu/armv7/lib/cache.S
>>
>>>
>>>>>> What you say can be done, it would involve some assembler though and
>>>>>> would need to be another CONFIG option. Was trying to avoid adding new
>>>>>> assembler.
>>>>>
>>>>> Low level init is about assembler and, well, low level. :)
>>>>
>>>> Yes but it's yuck. Part of the clean-up is to remove most of the
>>>> assembler - really very little is needed.
>>>
>>> I don't think "yuck" is a valid reasoned argument against assembly
>>> language
>>> :) but I assume what you mainly mean is 'hard to understand and
>>> replaceable
>>> by easy to understand C code'. I agree to the first part, and to the
>>> second
>>> one for the general case, but not for the startup path where, precisely,
>>> we
>>> don't have a working C run-time and most of the understanding requires
>>> knowledge of hardware, not only general programming knowledge. Past that,
>>> C
>>
>> I'm not arguing for the cache init stuff to move to C, just trying to
>> avoid any creeping growth of assembler when C can do it.
>>
>>> is ok -- but then beware: some C++ guy could chime in and contend that C
>>> is
>>> "yuck" by your own standards. :)
>>
>> The C++ guy who finds himself in U-Boot is probably lost :-)
>>
>>>
>>>>> But I don't see why there should be another CONFIG option. IIUC, you
>>>>> should
>>>>> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level
>>>>> init
>>>>> code under it, you would do the equivalent of a switch, with one branch
>>>>> for
>>>>> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc).
>>>>
>>>> Yes I can, but I need to be able to call cp15_init. And I can't do
>>>> that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is
>>>> compiled out! So I need to move that cp15_init code outside the
>>>> CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do,
>>>> honest!
>>>
>>> I understand, and I think the approach here is wrong, because you *do*
>>> want
>>> low-level init in the A9 case, and thus you should not have
>>> CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init
>>> to
>>> be empty, and the AVP and A9 must follow the same startup path. And all
>>> this
>>> it not necessarily required for all armv7 platforms, but some of them
>>> will
>>> want lowlevel init, and some not. Considering all this...
>>>
>>> For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option which
>>> start.S would use to compile out the low level init code definition (like
>>> CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls to it (UNline
>>> CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a SoC-specific
>>> version of lowlevel_init.
>>
>> If I understand you correctly, this is the opposite of what I want. It
>> is basically what we have now.
>>
>> All ARMv7 chips are going to turn off MMU, caches, flush TLBs, and the
>> like - this is an architecture feature, not an SOC one. We have
>> conflated ARM arch init with SOC init as you say, but the solution is
>> not to force the SOC to copy code from start.S just so that it can do
>> ARM arch init later.
>
> Yes, all ARMv7 CPUs will turn off caches etc, and thus, they all will want
> to do the cp15 init where it is now. Only the Tegra wants has issues with it
> so far.

Yes

>
>> And I'd argue for LOWLEVEL_INIT being about setting up the memory so
>> we can run code, and in particular relocate it later. The cp15_init
>> stuff should arguably be in arch_cpu_init() (or perhaps a new
>> arch_init()) as the first thing called from board_init_f() in my view.
>
> I disagree that cp15 could be set later: if we don't set it at low level
> init, we cannot be sure whether data caches are off or on, and if on, what
> they contain already.

Well on Tegra the AVP cannot execute this code. There is no danger in
this case since it doesn't have caches, etc. We execute the code as
soon as practical on the A9.

>
>> So how about I create a patch to move the cp15_init() code into
>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later?
>
> I don't mind moving it to armv7/lib. I do mind the "later".

OK, but the AVP cannot execute this code, whereas the A9 needs to
execute it on start-up.

We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that
this option exists suggests that platforms are permitted to do it. We
don't need to do any memory init, and the CP15 init can be called in
arch_cpu_init() once we establish that we are an A9 and not an AVP.
Does that sound ok?

The only problem I can see with this is if the data cache is on. This
could happen if you load U-Boot with (say) tftp and jump to it. Well
'don't do that!'. We also hope that the D-cache has at least been
flushed or there is nothing we can do. One fix for this (prior to the
refactor you are planning and mentioned in your earlier email) is
perhaps to implement a run-time check for the existence of CP15. Is
that better or worse?

Regards,
Simon

>
>>> In the longer term, I would be more in favor of a weak definition system
>>> with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind of
>>> fond of it) for lowlevel init -- but that will require looking into asm
>>> weak
>>> symbol handling and may require splitting of the assembly code function
>>> by
>>> function.
>>
>> I think you are trying to avoid the
>>
>> #ifdef CONFIG_ARCH_CPU_INIT
>> ? ?arch_cpu_init,
>> #endif
>> #ifdef ...
>> ? ?some_other_thing
>> #endif
>>
>> in the board_init_f() function table. Yes I agree it would tidy things
>> up, although I wonder if Graeme's initcall thing isn't better again.
>> It has the same confusion level coefficient (only the linker decides
>> what code is included) with a bit more flexibility. Then each arch /
>> board / cpu / soc / squirrel can decide on its own init and put it in
>> its own file. C only please :-)
>
> AFAIU, initcall does not have an override mechanism by which a later module
> can replace an initcall symbol already defined; two modules defining the
> same function name as initcall will just end up causing a linker error.
>
>> Regards,
>> Simon
>>
>>>
>>>> Regards,
>>>> Simon
>>>
>>> Amicalement,
>>> --
>>> Albert.
>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Le 24/10/2011 22:14, Simon Glass a ?crit :

>>> So how about I create a patch to move the cp15_init() code into
>>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later?
>>
>> I don't mind moving it to armv7/lib. I do mind the "later".
>
> OK, but the AVP cannot execute this code, whereas the A9 needs to
> execute it on start-up.

Yes, and that's the real problem: if you did not have that AVP running 
the same code path as the A9, you wouldn't even have though of moving 
cp15 init elsewhere.

> We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that
> this option exists suggests that platforms are permitted to do it. We
> don't need to do any memory init, and the CP15 init can be called in
> arch_cpu_init() once we establish that we are an A9 and not an AVP.
> Does that sound ok?

Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is 
not "I'm running this code on two CPUs", it is "those inits have already 
been done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded 
by SPL, so SPL has done the inits already.

And I also agree that we should do the cp15 init as soon as we have 
checked that we're the A9, not the AVP.

But this check has to be done during low level init, based on some 
immediate identification, not based on where in the code we are.

> The only problem I can see with this is if the data cache is on. This
> could happen if you load U-Boot with (say) tftp and jump to it. Well
> 'don't do that!'. We also hope that the D-cache has at least been
> flushed or there is nothing we can do. One fix for this (prior to the
> refactor you are planning and mentioned in your earlier email) is
> perhaps to implement a run-time check for the existence of CP15. Is
> that better or worse?

According to ARM, coprocessors 14 and 15 should always be supported from 
ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra 
specification available, I'd like to have a look at it.

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Mon, Oct 24, 2011 at 2:21 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 24/10/2011 22:14, Simon Glass a ?crit :
>
>>>> So how about I create a patch to move the cp15_init() code into
>>>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later?
>>>
>>> I don't mind moving it to armv7/lib. I do mind the "later".
>>
>> OK, but the AVP cannot execute this code, whereas the A9 needs to
>> execute it on start-up.
>
> Yes, and that's the real problem: if you did not have that AVP running the
> same code path as the A9, you wouldn't even have though of moving cp15 init
> elsewhere.

What is your point here? I am trying to get to a simple
easy-to-understand execution path in a fairly complex system. So far
the model is that the AVP starts up U-Boot until arch_cpu_init(), then
halts and the A9 takes over and runs the same code again. It is very
easy to understand and debug. It is easy to set breakpoints in startup
code on either core. It is possible to even put printf() debugging on
either core with the pre-console support. It has a lot of advantages
over more complex schemes. It removes a CONFIG_TEGRA2 from start.S
too!

>
>> We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that
>> this option exists suggests that platforms are permitted to do it. We
>> don't need to do any memory init, and the CP15 init can be called in
>> arch_cpu_init() once we establish that we are an A9 and not an AVP.
>> Does that sound ok?
>
> Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is not
> "I'm running this code on two CPUs", it is "those inits have already been
> done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded by SPL,
> so SPL has done the inits already.

I thought it pre-dated SPL, but ok.

>
> And I also agree that we should do the cp15 init as soon as we have checked
> that we're the A9, not the AVP.
>
> But this check has to be done during low level init, based on some immediate
> identification, not based on where in the code we are.

There is no facility in there for this at present. Are you asking for
a new call to find out at run-time whether the CP15 init should be
called? If so, then I can fairly easily do that.

>> The only problem I can see with this is if the data cache is on. This
>> could happen if you load U-Boot with (say) tftp and jump to it. Well
>> 'don't do that!'. We also hope that the D-cache has at least been
>> flushed or there is nothing we can do. One fix for this (prior to the
>> refactor you are planning and mentioned in your earlier email) is
>> perhaps to implement a run-time check for the existence of CP15. Is
>> that better or worse?
>
> According to ARM, coprocessors 14 and 15 should always be supported from
> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra
> specification available, I'd like to have a look at it.

Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU
init on the AVP and see what happens.

Regards,
Simon

>
>> Regards,
>> Simon
>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Hi Simon,

Le 25/10/2011 07:02, Simon Glass a ?crit :
> Hi Albert,
>
> On Mon, Oct 24, 2011 at 2:21 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net>  wrote:
>> Le 24/10/2011 22:14, Simon Glass a ?crit :
>>
>>>>> So how about I create a patch to move the cp15_init() code into
>>>>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later?
>>>>
>>>> I don't mind moving it to armv7/lib. I do mind the "later".
>>>
>>> OK, but the AVP cannot execute this code, whereas the A9 needs to
>>> execute it on start-up.
>>
>> Yes, and that's the real problem: if you did not have that AVP running the
>> same code path as the A9, you wouldn't even have though of moving cp15 init
>> elsewhere.
>
> What is your point here? I am trying to get to a simple
> easy-to-understand execution path in a fairly complex system. So far
> the model is that the AVP starts up U-Boot until arch_cpu_init(), then
> halts and the A9 takes over and runs the same code again. It is very
> easy to understand and debug. It is easy to set breakpoints in startup
> code on either core. It is possible to even put printf() debugging on
> either core with the pre-console support. It has a lot of advantages
> over more complex schemes. It removes a CONFIG_TEGRA2 from start.S
> too!

My point is precisely to keep the execution path simple and reliable for 
as many ARM architectures as possible, and I consider that placing parts 
of the low level initialization at different times for different cores 
is not simple and possibly not reliable, as it makes memory state 
uncertain during part of the start.S path for the A9.

I do understand that to you, this platform you're working on is easy to 
understand and debug, but then, you've got some experience with it, and 
it all makes sens to you, but it might not to other ARM developser who 
don't have access to it.

Besides, what I suggest doing (keeping cp15 access where it is, but 
subject it to the condition that it's not the AVP trying to execute it) 
does not preclude doing early printf() etc.

>>> We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that
>>> this option exists suggests that platforms are permitted to do it. We
>>> don't need to do any memory init, and the CP15 init can be called in
>>> arch_cpu_init() once we establish that we are an A9 and not an AVP.
>>> Does that sound ok?
>>
>> Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is not
>> "I'm running this code on two CPUs", it is "those inits have already been
>> done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded by SPL,
>> so SPL has done the inits already.
>
> I thought it pre-dated SPL, but ok.

It may have, and before or after SPL, it was/is immensely useful as a 
debugging aid. As such, it nearly disappeared at one point, but with 
SPL, it has received a non-debugging, functional, utility.

>> And I also agree that we should do the cp15 init as soon as we have checked
>> that we're the A9, not the AVP.
>>
>> But this check has to be done during low level init, based on some immediate
>> identification, not based on where in the code we are.
>
> There is no facility in there for this at present. Are you asking for
> a new call to find out at run-time whether the CP15 init should be
> called? If so, then I can fairly easily do that.

Is there really no register that the core can access which would tell it 
which core it actually is? No version/revision register either?

>>> The only problem I can see with this is if the data cache is on. This
>>> could happen if you load U-Boot with (say) tftp and jump to it. Well
>>> 'don't do that!'. We also hope that the D-cache has at least been
>>> flushed or there is nothing we can do. One fix for this (prior to the
>>> refactor you are planning and mentioned in your earlier email) is
>>> perhaps to implement a run-time check for the existence of CP15. Is
>>> that better or worse?
>>
>> According to ARM, coprocessors 14 and 15 should always be supported from
>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra
>> specification available, I'd like to have a look at it.
>
> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU
> init on the AVP and see what happens.

Please do check this. It may be that you can actually write to it from 
AVP even though it has no or not all expected effect; and if that is 
confirmed, then you don't have to separate the boot paths any more,

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Mon, Oct 24, 2011 at 11:41 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Simon,
>
> Le 25/10/2011 07:02, Simon Glass a ?crit :
>>
>> Hi Albert,
>>
>> On Mon, Oct 24, 2011 at 2:21 PM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net> ?wrote:
>>>
>>> Le 24/10/2011 22:14, Simon Glass a ?crit :
>>>
>>>>>> So how about I create a patch to move the cp15_init() code into
>>>>>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later?
>>>>>
>>>>> I don't mind moving it to armv7/lib. I do mind the "later".
>>>>
>>>> OK, but the AVP cannot execute this code, whereas the A9 needs to
>>>> execute it on start-up.
>>>
>>> Yes, and that's the real problem: if you did not have that AVP running
>>> the
>>> same code path as the A9, you wouldn't even have though of moving cp15
>>> init
>>> elsewhere.
>>
>> What is your point here? I am trying to get to a simple
>> easy-to-understand execution path in a fairly complex system. So far
>> the model is that the AVP starts up U-Boot until arch_cpu_init(), then
>> halts and the A9 takes over and runs the same code again. It is very
>> easy to understand and debug. It is easy to set breakpoints in startup
>> code on either core. It is possible to even put printf() debugging on
>> either core with the pre-console support. It has a lot of advantages
>> over more complex schemes. It removes a CONFIG_TEGRA2 from start.S
>> too!
>
> My point is precisely to keep the execution path simple and reliable for as
> many ARM architectures as possible, and I consider that placing parts of the
> low level initialization at different times for different cores is not
> simple and possibly not reliable, as it makes memory state uncertain during
> part of the start.S path for the A9.

Certainly if we can manage it so much the better.

The memory is set up by the boot ROM and the A9 has just come out of reset.

>
> I do understand that to you, this platform you're working on is easy to
> understand and debug, but then, you've got some experience with it, and it
> all makes sens to you, but it might not to other ARM developser who don't
> have access to it.
>
> Besides, what I suggest doing (keeping cp15 access where it is, but subject
> it to the condition that it's not the AVP trying to execute it) does not
> preclude doing early printf() etc.

But it does involve some extremely early soc-specific code. With my
current patch set there is none of this in start.S (the #ifdef TEGRA2
currently there is removed).

>
>>>> We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that
>>>> this option exists suggests that platforms are permitted to do it. We
>>>> don't need to do any memory init, and the CP15 init can be called in
>>>> arch_cpu_init() once we establish that we are an A9 and not an AVP.
>>>> Does that sound ok?
>>>
>>> Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is
>>> not
>>> "I'm running this code on two CPUs", it is "those inits have already been
>>> done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded by
>>> SPL,
>>> so SPL has done the inits already.
>>
>> I thought it pre-dated SPL, but ok.
>
> It may have, and before or after SPL, it was/is immensely useful as a
> debugging aid. As such, it nearly disappeared at one point, but with SPL, it
> has received a non-debugging, functional, utility.
>
>>> And I also agree that we should do the cp15 init as soon as we have
>>> checked
>>> that we're the A9, not the AVP.
>>>
>>> But this check has to be done during low level init, based on some
>>> immediate
>>> identification, not based on where in the code we are.
>>
>> There is no facility in there for this at present. Are you asking for
>> a new call to find out at run-time whether the CP15 init should be
>> called? If so, then I can fairly easily do that.
>
> Is there really no register that the core can access which would tell it
> which core it actually is? No version/revision register either?

Yes of course - this is the C function ap20_cpu_is_cortexa9(),
currently used from C code.

But please think about what you actually want in start.S. Do you
really want an #ifdef TEGRA2 with a check of a SOC register, followed
by a branch around the cp15_init code?

>
>>>> The only problem I can see with this is if the data cache is on. This
>>>> could happen if you load U-Boot with (say) tftp and jump to it. Well
>>>> 'don't do that!'. We also hope that the D-cache has at least been
>>>> flushed or there is nothing we can do. One fix for this (prior to the
>>>> refactor you are planning and mentioned in your earlier email) is
>>>> perhaps to implement a run-time check for the existence of CP15. Is
>>>> that better or worse?
>>>
>>> According to ARM, coprocessors 14 and 15 should always be supported from
>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra
>>> specification available, I'd like to have a look at it.
>>
>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU
>> init on the AVP and see what happens.
>
> Please do check this. It may be that you can actually write to it from AVP
> even though it has no or not all expected effect; and if that is confirmed,
> then you don't have to separate the boot paths any more,

Yes I'm tempted to go with this even if the ARM ARM is silent on the
matter. I will try it out today.

Regards,
Simon

> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Tue, Oct 25, 2011 at 6:44 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Albert,
>
> On Mon, Oct 24, 2011 at 11:41 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>> Hi Simon,
>>
>> Le 25/10/2011 07:02, Simon Glass a ?crit :
>>>
>>> Hi Albert,
>>>
>>> On Mon, Oct 24, 2011 at 2:21 PM, Albert ARIBAUD
>>> <albert.u.boot@aribaud.net> ?wrote:
>>>>
>>>> Le 24/10/2011 22:14, Simon Glass a ?crit :
>>>>
>>>>>>> So how about I create a patch to move the cp15_init() code into
>>>>>>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later?
>>>>>>
>>>>>> I don't mind moving it to armv7/lib. I do mind the "later".
>>>>>
>>>>> OK, but the AVP cannot execute this code, whereas the A9 needs to
>>>>> execute it on start-up.
>>>>
>>>> Yes, and that's the real problem: if you did not have that AVP running
>>>> the
>>>> same code path as the A9, you wouldn't even have though of moving cp15
>>>> init
>>>> elsewhere.
>>>
>>> What is your point here? I am trying to get to a simple
>>> easy-to-understand execution path in a fairly complex system. So far
>>> the model is that the AVP starts up U-Boot until arch_cpu_init(), then
>>> halts and the A9 takes over and runs the same code again. It is very
>>> easy to understand and debug. It is easy to set breakpoints in startup
>>> code on either core. It is possible to even put printf() debugging on
>>> either core with the pre-console support. It has a lot of advantages
>>> over more complex schemes. It removes a CONFIG_TEGRA2 from start.S
>>> too!
>>
>> My point is precisely to keep the execution path simple and reliable for as
>> many ARM architectures as possible, and I consider that placing parts of the
>> low level initialization at different times for different cores is not
>> simple and possibly not reliable, as it makes memory state uncertain during
>> part of the start.S path for the A9.
>
> Certainly if we can manage it so much the better.
>
> The memory is set up by the boot ROM and the A9 has just come out of reset.
>
>>
>> I do understand that to you, this platform you're working on is easy to
>> understand and debug, but then, you've got some experience with it, and it
>> all makes sens to you, but it might not to other ARM developser who don't
>> have access to it.
>>
>> Besides, what I suggest doing (keeping cp15 access where it is, but subject
>> it to the condition that it's not the AVP trying to execute it) does not
>> preclude doing early printf() etc.
>
> But it does involve some extremely early soc-specific code. With my
> current patch set there is none of this in start.S (the #ifdef TEGRA2
> currently there is removed).
>
>>
>>>>> We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that
>>>>> this option exists suggests that platforms are permitted to do it. We
>>>>> don't need to do any memory init, and the CP15 init can be called in
>>>>> arch_cpu_init() once we establish that we are an A9 and not an AVP.
>>>>> Does that sound ok?
>>>>
>>>> Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is
>>>> not
>>>> "I'm running this code on two CPUs", it is "those inits have already been
>>>> done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded by
>>>> SPL,
>>>> so SPL has done the inits already.
>>>
>>> I thought it pre-dated SPL, but ok.
>>
>> It may have, and before or after SPL, it was/is immensely useful as a
>> debugging aid. As such, it nearly disappeared at one point, but with SPL, it
>> has received a non-debugging, functional, utility.
>>
>>>> And I also agree that we should do the cp15 init as soon as we have
>>>> checked
>>>> that we're the A9, not the AVP.
>>>>
>>>> But this check has to be done during low level init, based on some
>>>> immediate
>>>> identification, not based on where in the code we are.
>>>
>>> There is no facility in there for this at present. Are you asking for
>>> a new call to find out at run-time whether the CP15 init should be
>>> called? If so, then I can fairly easily do that.
>>
>> Is there really no register that the core can access which would tell it
>> which core it actually is? No version/revision register either?
>
> Yes of course - this is the C function ap20_cpu_is_cortexa9(),
> currently used from C code.
>
> But please think about what you actually want in start.S. Do you
> really want an #ifdef TEGRA2 with a check of a SOC register, followed
> by a branch around the cp15_init code?

^^^^
This is the bit we need to sort out.

>
>>
>>>>> The only problem I can see with this is if the data cache is on. This
>>>>> could happen if you load U-Boot with (say) tftp and jump to it. Well
>>>>> 'don't do that!'. We also hope that the D-cache has at least been
>>>>> flushed or there is nothing we can do. One fix for this (prior to the
>>>>> refactor you are planning and mentioned in your earlier email) is
>>>>> perhaps to implement a run-time check for the existence of CP15. Is
>>>>> that better or worse?
>>>>
>>>> According to ARM, coprocessors 14 and 15 should always be supported from
>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra
>>>> specification available, I'd like to have a look at it.
>>>
>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU
>>> init on the AVP and see what happens.
>>
>> Please do check this. It may be that you can actually write to it from AVP
>> even though it has no or not all expected effect; and if that is confirmed,
>> then you don't have to separate the boot paths any more,
>
> Yes I'm tempted to go with this even if the ARM ARM is silent on the
> matter. I will try it out today.

Silly me of course it doesn't work - since if the CP15 doesn't have
those features then you get undef instruction exception. Please see
^^^ above!

Regards,
Simon

>
> Regards,
> Simon
>
>> Amicalement,
>> --
>> Albert.
>>
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Hi Simon,

Le 25/10/2011 23:16, Simon Glass a ?crit :

>> But please think about what you actually want in start.S. Do you
>> really want an #ifdef TEGRA2 with a check of a SOC register, followed
>> by a branch around the cp15_init code?
>
> ^^^^
> This is the bit we need to sort out.

Indeed, but I dont want a '#ifdef TEGRA2'. In the worst case I want a 
'if (!core_is_avp())' by way of an identification register, and in the 
best case, I just want cp15 to be set unconditionally.

>>>>> According to ARM, coprocessors 14 and 15 should always be supported from
>>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra
>>>>> specification available, I'd like to have a look at it.
>>>>
>>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU
>>>> init on the AVP and see what happens.
>>>
>>> Please do check this. It may be that you can actually write to it from AVP
>>> even though it has no or not all expected effect; and if that is confirmed,
>>> then you don't have to separate the boot paths any more,
>>
>> Yes I'm tempted to go with this even if the ARM ARM is silent on the
>> matter. I will try it out today.
>
> Silly me of course it doesn't work - since if the CP15 doesn't have
> those features then you get undef instruction exception. Please see
> ^^^ above!

Are you stating a reasoning here or the results of an actual test? What 
causes an undefined instruction exception is the absence of a 
coprocessor to handle the instruction, not the presence of a coprocessor 
which incidentally cannot perform all of what"s being asked.

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Thu, Oct 27, 2011 at 1:05 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Simon,
>
> Le 25/10/2011 23:16, Simon Glass a ?crit :
>
>>> But please think about what you actually want in start.S. Do you
>>> really want an #ifdef TEGRA2 with a check of a SOC register, followed
>>> by a branch around the cp15_init code?
>>
>> ^^^^
>> This is the bit we need to sort out.
>
> Indeed, but I dont want a '#ifdef TEGRA2'. In the worst case I want a 'if
> (!core_is_avp())' by way of an identification register, and in the best
> case, I just want cp15 to be set unconditionally.

You already have #ifdef TEGRA2 (look at the top of start.S). That is
what I am trying to remove.

How can I call core_is_avp() on an SOC that isn't Tegra? Can you
please write a little code fragment that I should use in start.S, so
that I can understand what is required here.

>
>>>>>> According to ARM, coprocessors 14 and 15 should always be supported
>>>>>> from
>>>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra
>>>>>> specification available, I'd like to have a look at it.
>>>>>
>>>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU
>>>>> init on the AVP and see what happens.
>>>>
>>>> Please do check this. It may be that you can actually write to it from
>>>> AVP
>>>> even though it has no or not all expected effect; and if that is
>>>> confirmed,
>>>> then you don't have to separate the boot paths any more,
>>>
>>> Yes I'm tempted to go with this even if the ARM ARM is silent on the
>>> matter. I will try it out today.
>>
>> Silly me of course it doesn't work - since if the CP15 doesn't have
>> those features then you get undef instruction exception. Please see
>> ^^^ above!
>
> Are you stating a reasoning here or the results of an actual test? What
> causes an undefined instruction exception is the absence of a coprocessor to
> handle the instruction, not the presence of a coprocessor which incidentally
> cannot perform all of what"s being asked.

Yes I did test it - it hangs. Would you like me to spend some time to
debug exactly why? I suspect it is an undef instruction, since
executing cache instructions on an ARM7TDMI is dodgy, but I could
check if it would help. But let's assume for now we can't do that!

Regards,
Simon

>
>> Regards,
>> Simon
>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Le 27/10/2011 22:17, Simon Glass a ?crit :
> Hi Albert,
>
> On Thu, Oct 27, 2011 at 1:05 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net>  wrote:
>> Hi Simon,
>>
>> Le 25/10/2011 23:16, Simon Glass a ?crit :
>>
>>>> But please think about what you actually want in start.S. Do you
>>>> really want an #ifdef TEGRA2 with a check of a SOC register, followed
>>>> by a branch around the cp15_init code?
>>>
>>> ^^^^
>>> This is the bit we need to sort out.
>>
>> Indeed, but I dont want a '#ifdef TEGRA2'. In the worst case I want a 'if
>> (!core_is_avp())' by way of an identification register, and in the best
>> case, I just want cp15 to be set unconditionally.
>
> You already have #ifdef TEGRA2 (look at the top of start.S). That is
> what I am trying to remove.
>
> How can I call core_is_avp() on an SOC that isn't Tegra? Can you
> please write a little code fragment that I should use in start.S, so
> that I can understand what is required here.
>
>>
>>>>>>> According to ARM, coprocessors 14 and 15 should always be supported
>>>>>>> from
>>>>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra
>>>>>>> specification available, I'd like to have a look at it.
>>>>>>
>>>>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU
>>>>>> init on the AVP and see what happens.
>>>>>
>>>>> Please do check this. It may be that you can actually write to it from
>>>>> AVP
>>>>> even though it has no or not all expected effect; and if that is
>>>>> confirmed,
>>>>> then you don't have to separate the boot paths any more,
>>>>
>>>> Yes I'm tempted to go with this even if the ARM ARM is silent on the
>>>> matter. I will try it out today.
>>>
>>> Silly me of course it doesn't work - since if the CP15 doesn't have
>>> those features then you get undef instruction exception. Please see
>>> ^^^ above!
>>
>> Are you stating a reasoning here or the results of an actual test? What
>> causes an undefined instruction exception is the absence of a coprocessor to
>> handle the instruction, not the presence of a coprocessor which incidentally
>> cannot perform all of what"s being asked.
>
> Yes I did test it - it hangs. Would you like me to spend some time to
> debug exactly why? I suspect it is an undef instruction, since
> executing cache instructions on an ARM7TDMI is dodgy, but I could
> check if it would help. But let's assume for now we can't do that!

What was the test exactly? Depending on which register of CP15 you try 
to access, things could be different. Can you test MRC 
p15,0,<Rt>,c0,c0,0 ? That's the MIDR and it should be always accessible, 
plus it should have a unique value for each type of core, so it could be 
used to not write cp15 for caches etc from the AVP.

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Thu, Oct 27, 2011 at 3:39 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 27/10/2011 22:17, Simon Glass a ?crit :
>>
>> Hi Albert,
>>
>> On Thu, Oct 27, 2011 at 1:05 PM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net> ?wrote:
>>>
>>> Hi Simon,
>>>
>>> Le 25/10/2011 23:16, Simon Glass a ?crit :
>>>
>>>>> But please think about what you actually want in start.S. Do you
>>>>> really want an #ifdef TEGRA2 with a check of a SOC register, followed
>>>>> by a branch around the cp15_init code?
>>>>
>>>> ^^^^
>>>> This is the bit we need to sort out.
>>>
>>> Indeed, but I dont want a '#ifdef TEGRA2'. In the worst case I want a 'if
>>> (!core_is_avp())' by way of an identification register, and in the best
>>> case, I just want cp15 to be set unconditionally.
>>
>> You already have #ifdef TEGRA2 (look at the top of start.S). That is
>> what I am trying to remove.
>>
>> How can I call core_is_avp() on an SOC that isn't Tegra? Can you
>> please write a little code fragment that I should use in start.S, so
>> that I can understand what is required here.
>>
>>>
>>>>>>>> According to ARM, coprocessors 14 and 15 should always be supported
>>>>>>>> from
>>>>>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra
>>>>>>>> specification available, I'd like to have a look at it.
>>>>>>>
>>>>>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU
>>>>>>> init on the AVP and see what happens.
>>>>>>
>>>>>> Please do check this. It may be that you can actually write to it from
>>>>>> AVP
>>>>>> even though it has no or not all expected effect; and if that is
>>>>>> confirmed,
>>>>>> then you don't have to separate the boot paths any more,
>>>>>
>>>>> Yes I'm tempted to go with this even if the ARM ARM is silent on the
>>>>> matter. I will try it out today.
>>>>
>>>> Silly me of course it doesn't work - since if the CP15 doesn't have
>>>> those features then you get undef instruction exception. Please see
>>>> ^^^ above!
>>>
>>> Are you stating a reasoning here or the results of an actual test? What
>>> causes an undefined instruction exception is the absence of a coprocessor
>>> to
>>> handle the instruction, not the presence of a coprocessor which
>>> incidentally
>>> cannot perform all of what"s being asked.
>>
>> Yes I did test it - it hangs. Would you like me to spend some time to
>> debug exactly why? I suspect it is an undef instruction, since
>> executing cache instructions on an ARM7TDMI is dodgy, but I could
>> check if it would help. But let's assume for now we can't do that!
>
> What was the test exactly? Depending on which register of CP15 you try to
> access, things could be different. Can you test MRC p15,0,<Rt>,c0,c0,0 ?
> That's the MIDR and it should be always accessible, plus it should have a
> unique value for each type of core, so it could be used to not write cp15
> for caches etc from the AVP.

I'm sure that is fine. So do you want me to add a check that the ID
shows architecture ARMv7, and then skip the cache code if not? It
should be safe enough since it is a clear signal that something is not
right. It would also handle the case where an Cortex-A7 (with a cache)
is attached.

Regards,
Simon

>
>> Regards,
>> Simon
>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Thu, Oct 27, 2011 at 3:46 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Albert,
>
> On Thu, Oct 27, 2011 at 3:39 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>> Le 27/10/2011 22:17, Simon Glass a ?crit :
>>>
>>> Hi Albert,
>>>
>>> On Thu, Oct 27, 2011 at 1:05 PM, Albert ARIBAUD
>>> <albert.u.boot@aribaud.net> ?wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> Le 25/10/2011 23:16, Simon Glass a ?crit :
>>>>
>>>>>> But please think about what you actually want in start.S. Do you
>>>>>> really want an #ifdef TEGRA2 with a check of a SOC register, followed
>>>>>> by a branch around the cp15_init code?
>>>>>
>>>>> ^^^^
>>>>> This is the bit we need to sort out.
>>>>
>>>> Indeed, but I dont want a '#ifdef TEGRA2'. In the worst case I want a 'if
>>>> (!core_is_avp())' by way of an identification register, and in the best
>>>> case, I just want cp15 to be set unconditionally.
>>>
>>> You already have #ifdef TEGRA2 (look at the top of start.S). That is
>>> what I am trying to remove.
>>>
>>> How can I call core_is_avp() on an SOC that isn't Tegra? Can you
>>> please write a little code fragment that I should use in start.S, so
>>> that I can understand what is required here.
>>>
>>>>
>>>>>>>>> According to ARM, coprocessors 14 and 15 should always be supported
>>>>>>>>> from
>>>>>>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra
>>>>>>>>> specification available, I'd like to have a look at it.
>>>>>>>>
>>>>>>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU
>>>>>>>> init on the AVP and see what happens.
>>>>>>>
>>>>>>> Please do check this. It may be that you can actually write to it from
>>>>>>> AVP
>>>>>>> even though it has no or not all expected effect; and if that is
>>>>>>> confirmed,
>>>>>>> then you don't have to separate the boot paths any more,
>>>>>>
>>>>>> Yes I'm tempted to go with this even if the ARM ARM is silent on the
>>>>>> matter. I will try it out today.
>>>>>
>>>>> Silly me of course it doesn't work - since if the CP15 doesn't have
>>>>> those features then you get undef instruction exception. Please see
>>>>> ^^^ above!
>>>>
>>>> Are you stating a reasoning here or the results of an actual test? What
>>>> causes an undefined instruction exception is the absence of a coprocessor
>>>> to
>>>> handle the instruction, not the presence of a coprocessor which
>>>> incidentally
>>>> cannot perform all of what"s being asked.
>>>
>>> Yes I did test it - it hangs. Would you like me to spend some time to
>>> debug exactly why? I suspect it is an undef instruction, since
>>> executing cache instructions on an ARM7TDMI is dodgy, but I could
>>> check if it would help. But let's assume for now we can't do that!
>>
>> What was the test exactly? Depending on which register of CP15 you try to
>> access, things could be different. Can you test MRC p15,0,<Rt>,c0,c0,0 ?
>> That's the MIDR and it should be always accessible, plus it should have a
>> unique value for each type of core, so it could be used to not write cp15
>> for caches etc from the AVP.

The test was

 	mrc	p15, 0, r0, c0, c0, 0	@ get ID register
 	and	r0, r0, #0xf0000	@ get architecture
 	cmp	r0, #0xf0000		@ check for > ARMv6
 	movne	pc, lr			@ else skip cache init

Unfortunately I think it is a plain ARM7TDMI with no CP15.

Regards,
Simon

>
> I'm sure that is fine. So do you want me to add a check that the ID
> shows architecture ARMv7, and then skip the cache code if not? It
> should be safe enough since it is a clear signal that something is not
> right. It would also handle the case where an Cortex-A7 (with a cache)
> is attached.
>
> Regards,
> Simon
>
>>
>>> Regards,
>>> Simon
>>
>> Amicalement,
>> --
>> Albert.
>>
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Le 28/10/2011 03:43, Simon Glass a ?crit :

> The test was
>
>   	mrc	p15, 0, r0, c0, c0, 0	@ get ID register
>   	and	r0, r0, #0xf0000	@ get architecture
>   	cmp	r0, #0xf0000		@ check for>  ARMv6
>   	movne	pc, lr			@ else skip cache init
>
> Unfortunately I think it is a plain ARM7TDMI with no CP15.

What about other fields in r0 right after mrc?

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Thu, Oct 27, 2011 at 10:09 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 28/10/2011 03:43, Simon Glass a ?crit :
>
>> The test was
>>
>> ? ? ? ?mrc ? ? p15, 0, r0, c0, c0, 0 ? @ get ID register
>> ? ? ? ?and ? ? r0, r0, #0xf0000 ? ? ? ?@ get architecture
>> ? ? ? ?cmp ? ? r0, #0xf0000 ? ? ? ? ? ?@ check for> ?ARMv6
>> ? ? ? ?movne ? pc, lr ? ? ? ? ? ? ? ? ?@ else skip cache init
>>
>> Unfortunately I think it is a plain ARM7TDMI with no CP15.
>
> What about other fields in r0 right after mrc?

I don't really understand that sentence, sorry.

The ARM7TDMI does not have a CP15 and aborts if I try to access it.
Just in case there is something odd going on I checked with DSTREAM /
RVdebug and it definitely doesn't have a CP15. [as Ford Prefect would
say, I counted them twice]

The simplest thing I have been able to think of that does not involve
exceptions, differing instruction behaviour, doing the init later or
putting in some Tegra-specific code is to check for the existence of
the Q bit in the CPSR (actually APSR on ARMv7). This does seem to work
and I have verified both in my old 1996 ARM ARM DDI 0100B and the
ARMv7-A one (DDI 0406B) that from an architecture point of view this
should work. The Q bit is RAZ on ARMv4T.

I believe this will cope with the Cortex-A7 / A-15 combinations and
possibly even Cortex-R4 / A-15 although I have not tested this. I
suppose we can deal with this when it becomes an issue.

So I have redone this one patch with that in mind, and adjusted the
series slightly to fit with this. I will resend it when it completes
MAKEALL.

I hope that this resolves the matter, but if not(!), I would very much
appreciate it if you could send through some actual pseudo code
showing what you are looking for, to avoid any confusion.

Thanks,
Simon

>
>> Regards,
>> Simon
>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Hi Simon,

Le 29/10/2011 02:36, Simon Glass a ?crit :
> Hi Albert,
>
> On Thu, Oct 27, 2011 at 10:09 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net>  wrote:
>> Le 28/10/2011 03:43, Simon Glass a ?crit :
>>
>>> The test was
>>>
>>>         mrc     p15, 0, r0, c0, c0, 0   @ get ID register
>>>         and     r0, r0, #0xf0000        @ get architecture
>>>         cmp     r0, #0xf0000            @ check for>    ARMv6
>>>         movne   pc, lr                  @ else skip cache init
>>>
>>> Unfortunately I think it is a plain ARM7TDMI with no CP15.
>>
>> What about other fields in r0 right after mrc?
>
> I don't really understand that sentence, sorry.
>
> The ARM7TDMI does not have a CP15 and aborts if I try to access it.
> Just in case there is something odd going on I checked with DSTREAM /
> RVdebug and it definitely doesn't have a CP15. [as Ford Prefect would
> say, I counted them twice]

Ok, so debug tools do not show cp15. But tools can be tailored to what 
tool makes think is needed -- I could tell about some debugging tools 
that will not let me see all I want a core because the debug designers 
had finite resources and what I wanted was not a priority to them.

OTOH, according to ARM, ARM7DTMI is an ARMv4T architecture, and indeed 
cp15 is mandatory only for ARMv6 and up, but ARM also states cp15 
support was a de facto standard already for ARMv4.

So I am left with the question: would the Tegra2 AVP be the only ARM 
implementation supported by U-Boot that does not have a cp15? That is 
possible, but I want direct testimony from Nividia.

This is why I asked about the Tegra2 TRM, or whatever Nvidia calls it, 
in case it would explicitely state if AVP has cp15 support or not. 
Failing that, I'd be ok with experimenting but through the AVP, not 
through debugging tools -- encoding a cp15 MIDR read in the U-Boot 
startup code, step through it with the debugger and see if it causes an 
UND or not, and if not, what is the hex value of r0 -- maybe that is 
exactly what you did, but I am not 100% sure it is, hence my insistence.

I am especially surprised that a recent core be synthesized without a 
means for run-time core identification, especially in a design with two 
ARM cores.

> The simplest thing I have been able to think of that does not involve
> exceptions, differing instruction behaviour, doing the init later or
> putting in some Tegra-specific code is to check for the existence of
> the Q bit in the CPSR (actually APSR on ARMv7). This does seem to work
> and I have verified both in my old 1996 ARM ARM DDI 0100B and the
> ARMv7-A one (DDI 0406B) that from an architecture point of view this
> should work. The Q bit is RAZ on ARMv4T.

This could hep if we really cannot access the Main ID Register on the AVP.

> I believe this will cope with the Cortex-A7 / A-15 combinations and
> possibly even Cortex-R4 / A-15 although I have not tested this. I
> suppose we can deal with this when it becomes an issue.
>
> So I have redone this one patch with that in mind, and adjusted the
> series slightly to fit with this. I will resend it when it completes
> MAKEALL.
>
> I hope that this resolves the matter, but if not(!), I would very much
> appreciate it if you could send through some actual pseudo code
> showing what you are looking for, to avoid any confusion.

Well, I just want to see if the MIDR is accessible and what its value 
is, so I want the AVP to execute

	mrc	p15, 0, r0, c0, c0, 0

The ending 0 is what selects MIDR rather than other cp15 registers -- 
other values can cause UND (and I would gladly understand that AVP goes 
UND for reading cp15 CTR for instance).

The simplest test would be to insert the exact instruction above in the 
reset sequence in start.S right after SVC32 switch, debug the reset 
execution path, see if the mrc above goes UND or else check r0's 
contents after mrc is done.

> Thanks,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Sun, Oct 30, 2011 at 3:16 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Simon,
>
> Le 29/10/2011 02:36, Simon Glass a ?crit :
>>
>> Hi Albert,
>>
>> On Thu, Oct 27, 2011 at 10:09 PM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net> ?wrote:
>>>
>>> Le 28/10/2011 03:43, Simon Glass a ?crit :
>>>
>>>> The test was
>>>>
>>>> ? ? ? ?mrc ? ? p15, 0, r0, c0, c0, 0 ? @ get ID register
>>>> ? ? ? ?and ? ? r0, r0, #0xf0000 ? ? ? ?@ get architecture
>>>> ? ? ? ?cmp ? ? r0, #0xf0000 ? ? ? ? ? ?@ check for> ? ?ARMv6
>>>> ? ? ? ?movne ? pc, lr ? ? ? ? ? ? ? ? ?@ else skip cache init
>>>>
>>>> Unfortunately I think it is a plain ARM7TDMI with no CP15.
>>>
>>> What about other fields in r0 right after mrc?
>>
>> I don't really understand that sentence, sorry.
>>
>> The ARM7TDMI does not have a CP15 and aborts if I try to access it.
>> Just in case there is something odd going on I checked with DSTREAM /
>> RVdebug and it definitely doesn't have a CP15. [as Ford Prefect would
>> say, I counted them twice]
>
> Ok, so debug tools do not show cp15. But tools can be tailored to what tool
> makes think is needed -- I could tell about some debugging tools that will
> not let me see all I want a core because the debug designers had finite
> resources and what I wanted was not a priority to them.

Ye of little faith...

>
> OTOH, according to ARM, ARM7DTMI is an ARMv4T architecture, and indeed cp15
> is mandatory only for ARMv6 and up, but ARM also states cp15 support was a
> de facto standard already for ARMv4.

Yes it is de facto when you have a core with that support - typically
an MMU and caches. But in that case it would have three digits, as in
ARM720T, rather than ARM7TDMI. It is definitely an ARM7TDMI (actually
-S) core in the datasheet, and there is no external CP15 block shown,
etc. (Although I can see a cache!)

>
> So I am left with the question: would the Tegra2 AVP be the only ARM
> implementation supported by U-Boot that does not have a cp15? That is
> possible, but I want direct testimony from Nividia.

No there are lots and lots of ARM7TDMIs out there, just not that many
that run U-Boot - and Linux is hard without an MMU. But even in the
U-Boot tree we have the s3c44b0 cpu which seems to be a plain
ARM7TDMI.

In terms of direct testimony, two engineers are on this thread and may
wish to chime in. The datasheet is pretty clear to me...

>
> This is why I asked about the Tegra2 TRM, or whatever Nvidia calls it, in
> case it would explicitely state if AVP has cp15 support or not. Failing

See above, it clearly states ARM7TDMI-S.

> that, I'd be ok with experimenting but through the AVP, not through
> debugging tools -- encoding a cp15 MIDR read in the U-Boot startup code,
> step through it with the debugger and see if it causes an UND or not, and if
> not, what is the hex value of r0 -- maybe that is exactly what you did, but
> I am not 100% sure it is, hence my insistence.

OK I have done this, and yes the AVP definitely takes the undef
exception when you execute:

mrc     p15, 0, r0, c0, c0, 0

>
> I am especially surprised that a recent core be synthesized without a means
> for run-time core identification, especially in a design with two ARM cores.

The -S means synthesized. It does have its own method of
identification as I mentioned. There is a Tegra-specific register that
I can read.

>
>> The simplest thing I have been able to think of that does not involve
>> exceptions, differing instruction behaviour, doing the init later or
>> putting in some Tegra-specific code is to check for the existence of
>> the Q bit in the CPSR (actually APSR on ARMv7). This does seem to work
>> and I have verified both in my old 1996 ARM ARM DDI 0100B and the
>> ARMv7-A one (DDI 0406B) that from an architecture point of view this
>> should work. The Q bit is RAZ on ARMv4T.
>
> This could hep if we really cannot access the Main ID Register on the AVP.

OK, well I will send a patch set up with this change.

>
>> I believe this will cope with the Cortex-A7 / A-15 combinations and
>> possibly even Cortex-R4 / A-15 although I have not tested this. I
>> suppose we can deal with this when it becomes an issue.
>>
>> So I have redone this one patch with that in mind, and adjusted the
>> series slightly to fit with this. I will resend it when it completes
>> MAKEALL.
>>
>> I hope that this resolves the matter, but if not(!), I would very much
>> appreciate it if you could send through some actual pseudo code
>> showing what you are looking for, to avoid any confusion.
>
> Well, I just want to see if the MIDR is accessible and what its value is, so
> I want the AVP to execute
>
> ? ? ? ?mrc ? ? p15, 0, r0, c0, c0, 0
>
> The ending 0 is what selects MIDR rather than other cp15 registers -- other
> values can cause UND (and I would gladly understand that AVP goes UND for
> reading cp15 CTR for instance).
>
> The simplest test would be to insert the exact instruction above in the
> reset sequence in start.S right after SVC32 switch, debug the reset
> execution path, see if the mrc above goes UND or else check r0's contents
> after mrc is done.

OK see above, I have done this. What I meant was for you to provide a
code sequence that you want in start.S. Hopefully my patch will fit
the bill.

I am very happy for this to be rewritten/changed down the track. But
for now, and assuming you don't want to call a function to find out
whether or not CP15 exists, I have created something which seems to
solve the problem. There are many other problems to solve. I feel that
this one has had its fair share of attention!

Regards,
Simon

>
>> Thanks,
>> Simon
>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Tom Warren]

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert.

On Mon, Oct 31, 2011 at 3:54 PM, Tom Warren <TWarren@nvidia.com> wrote:
> From one of our HW engineers:
>
> The AVP does not have a CP15.
> As this thread speculates, it is an ARM7TDMI which is a basic ARM V4 core with no ARM co-processors or MMU.
>
> Also, ARM app note 99 (DAI0099C_core_type_rev_id.pdf) says the same thing.
>
> HTH,
>
> Tom
>
>> -----Original Message-----
>> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
>> Sent: Monday, October 31, 2011 2:44 PM
>> To: Albert ARIBAUD
>> Cc: U-Boot Mailing List; Tom Warren; Stephen Warren
>> Subject: Re: [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()
>>
>> Hi Albert,
>>
>> On Sun, Oct 30, 2011 at 3:16 AM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net> wrote:
>> > Hi Simon,
>> >
>> > Le 29/10/2011 02:36, Simon Glass a ?crit :
>> >>
>> >> Hi Albert,
>> >>
>> >> On Thu, Oct 27, 2011 at 10:09 PM, Albert ARIBAUD
>> >> <albert.u.boot@aribaud.net> ?wrote:
>> >>>
>> >>> Le 28/10/2011 03:43, Simon Glass a ?crit :
>> >>>
>> >>>> The test was
>> >>>>
>> >>>> ? ? ? ?mrc ? ? p15, 0, r0, c0, c0, 0 ? @ get ID register
>> >>>> ? ? ? ?and ? ? r0, r0, #0xf0000 ? ? ? ?@ get architecture
>> >>>> ? ? ? ?cmp ? ? r0, #0xf0000 ? ? ? ? ? ?@ check for> ? ?ARMv6
>> >>>> ? ? ? ?movne ? pc, lr ? ? ? ? ? ? ? ? ?@ else skip cache init
>> >>>>
>> >>>> Unfortunately I think it is a plain ARM7TDMI with no CP15.
>> >>>
>> >>> What about other fields in r0 right after mrc?
>> >>
>> >> I don't really understand that sentence, sorry.
>> >>
>> >> The ARM7TDMI does not have a CP15 and aborts if I try to access it.
>> >> Just in case there is something odd going on I checked with DSTREAM /
>> >> RVdebug and it definitely doesn't have a CP15. [as Ford Prefect would
>> >> say, I counted them twice]
>> >
>> > Ok, so debug tools do not show cp15. But tools can be tailored to what
>> tool
>> > makes think is needed -- I could tell about some debugging tools that will
>> > not let me see all I want a core because the debug designers had finite
>> > resources and what I wanted was not a priority to them.
>>
>> Ye of little faith...
>>
>> >
>> > OTOH, according to ARM, ARM7DTMI is an ARMv4T architecture, and indeed
>> cp15
>> > is mandatory only for ARMv6 and up, but ARM also states cp15 support was a
>> > de facto standard already for ARMv4.
>>
>> Yes it is de facto when you have a core with that support - typically
>> an MMU and caches. But in that case it would have three digits, as in
>> ARM720T, rather than ARM7TDMI. It is definitely an ARM7TDMI (actually
>> -S) core in the datasheet, and there is no external CP15 block shown,
>> etc. (Although I can see a cache!)
>>
>> >
>> > So I am left with the question: would the Tegra2 AVP be the only ARM
>> > implementation supported by U-Boot that does not have a cp15? That is
>> > possible, but I want direct testimony from Nividia.
>>
>> No there are lots and lots of ARM7TDMIs out there, just not that many
>> that run U-Boot - and Linux is hard without an MMU. But even in the
>> U-Boot tree we have the s3c44b0 cpu which seems to be a plain
>> ARM7TDMI.
>>
>> In terms of direct testimony, two engineers are on this thread and may
>> wish to chime in. The datasheet is pretty clear to me...
>>
>> >
>> > This is why I asked about the Tegra2 TRM, or whatever Nvidia calls it, in
>> > case it would explicitely state if AVP has cp15 support or not. Failing
>>
>> See above, it clearly states ARM7TDMI-S.
>>
>> > that, I'd be ok with experimenting but through the AVP, not through
>> > debugging tools -- encoding a cp15 MIDR read in the U-Boot startup code,
>> > step through it with the debugger and see if it causes an UND or not, and
>> if
>> > not, what is the hex value of r0 -- maybe that is exactly what you did,
>> but
>> > I am not 100% sure it is, hence my insistence.
>>
>> OK I have done this, and yes the AVP definitely takes the undef
>> exception when you execute:
>>
>> mrc ? ? p15, 0, r0, c0, c0, 0
>>
>> >
>> > I am especially surprised that a recent core be synthesized without a
>> means
>> > for run-time core identification, especially in a design with two ARM
>> cores.
>>
>> The -S means synthesized. It does have its own method of
>> identification as I mentioned. There is a Tegra-specific register that
>> I can read.
>>
>> >
>> >> The simplest thing I have been able to think of that does not involve
>> >> exceptions, differing instruction behaviour, doing the init later or
>> >> putting in some Tegra-specific code is to check for the existence of
>> >> the Q bit in the CPSR (actually APSR on ARMv7). This does seem to work
>> >> and I have verified both in my old 1996 ARM ARM DDI 0100B and the
>> >> ARMv7-A one (DDI 0406B) that from an architecture point of view this
>> >> should work. The Q bit is RAZ on ARMv4T.
>> >
>> > This could hep if we really cannot access the Main ID Register on the AVP.
>>
>> OK, well I will send a patch set up with this change.
>>
>> >
>> >> I believe this will cope with the Cortex-A7 / A-15 combinations and
>> >> possibly even Cortex-R4 / A-15 although I have not tested this. I
>> >> suppose we can deal with this when it becomes an issue.
>> >>
>> >> So I have redone this one patch with that in mind, and adjusted the
>> >> series slightly to fit with this. I will resend it when it completes
>> >> MAKEALL.
>> >>
>> >> I hope that this resolves the matter, but if not(!), I would very much
>> >> appreciate it if you could send through some actual pseudo code
>> >> showing what you are looking for, to avoid any confusion.
>> >
>> > Well, I just want to see if the MIDR is accessible and what its value is,
>> so
>> > I want the AVP to execute
>> >
>> > ? ? ? ?mrc ? ? p15, 0, r0, c0, c0, 0
>> >
>> > The ending 0 is what selects MIDR rather than other cp15 registers --
>> other
>> > values can cause UND (and I would gladly understand that AVP goes UND for
>> > reading cp15 CTR for instance).
>> >
>> > The simplest test would be to insert the exact instruction above in the
>> > reset sequence in start.S right after SVC32 switch, debug the reset
>> > execution path, see if the mrc above goes UND or else check r0's contents
>> > after mrc is done.
>>
>> OK see above, I have done this. What I meant was for you to provide a
>> code sequence that you want in start.S. Hopefully my patch will fit
>> the bill.
>>
>> I am very happy for this to be rewritten/changed down the track. But
>> for now, and assuming you don't want to call a function to find out
>> whether or not CP15 exists, I have created something which seems to
>> solve the problem. There are many other problems to solve. I feel that
>> this one has had its fair share of attention!

Have you had a chance to look at the revised patch that I sent?

Regards,
Simon

>>
>> Regards,
>> Simon
>>
>> >
>> >> Thanks,
>> >> Simon
>> >
>> > Amicalement,
>> > --
>> > Albert.
>> >
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information. ?Any unauthorized review, use, disclosure or distribution
> is prohibited. ?If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
>

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Hi Simon et al.,

Thanks to you and others for their patience. Let me summarize the thread:

- ARM7TDMI definitely lacks a cp15, as AN99 explicitely states;

- ARM provides no other, reliable, way to identify an ARM7TDMI reliably;

- Identification through a manufacturer-specific method just defeats the 
goal.

This makes it impossible to support ARM7TDMI (as well as others listed 
in AM99)  with a start.S code that contains cp15 instructions.

OTOH, I've gone through a number of ARM TRMs for cores that have 
caches/MMU and they all state that these are disabled on reset. I will 
also assume that when U-Boot starts indirectly through SPL or any other 
mechanism, and if this mechanism uses caches/MMU, then it will disable 
them properly before passing control to U-Boot, just like U-Boot 
disables caches etc before passing control to e.g. Linux.

I understand that this initialization is done so that the the cache/MMU 
is in a correct state, and this must still be ensured, but I now think 
that ensuring this is the burden of whatever passed control to the start 
label (this includes true resets, branching from SPL or any other IPL, 
and --ugh-- branching to start from inside U-Boot).

Based on this, I revise my opinion that the caches and MMU cleaning code 
in start.S should be kept there, and actually I now consider that they 
should be removed entirely.

However, removing cp15 inits would mean a *third* revision of your 
patchset, plus it could cause unexpected issues which I don't want to 
happen at this point of the release cycle.

Therefore, I'll go back to your patch as posted on oct 13th.

Apologies for the long discussion.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Sat, Nov 5, 2011 at 1:09 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Simon et al.,
>
> Thanks to you and others for their patience. Let me summarize the thread:
>
> - ARM7TDMI definitely lacks a cp15, as AN99 explicitely states;
>
> - ARM provides no other, reliable, way to identify an ARM7TDMI reliably;
>
> - Identification through a manufacturer-specific method just defeats the
> goal.
>
> This makes it impossible to support ARM7TDMI (as well as others listed in
> AM99) ?with a start.S code that contains cp15 instructions.
>
> OTOH, I've gone through a number of ARM TRMs for cores that have caches/MMU
> and they all state that these are disabled on reset. I will also assume that
> when U-Boot starts indirectly through SPL or any other mechanism, and if
> this mechanism uses caches/MMU, then it will disable them properly before
> passing control to U-Boot, just like U-Boot disables caches etc before
> passing control to e.g. Linux.
>
> I understand that this initialization is done so that the the cache/MMU is
> in a correct state, and this must still be ensured, but I now think that
> ensuring this is the burden of whatever passed control to the start label
> (this includes true resets, branching from SPL or any other IPL, and --ugh--
> branching to start from inside U-Boot).
>
> Based on this, I revise my opinion that the caches and MMU cleaning code in
> start.S should be kept there, and actually I now consider that they should
> be removed entirely.
>
> However, removing cp15 inits would mean a *third* revision of your patchset,
> plus it could cause unexpected issues which I don't want to happen at this
> point of the release cycle.

Yes - I did something similar at v2 where I moved it into a library.
Best left alone for now. I would prefer to continue this discussion if
needed later when we have got this series committed.

>
> Therefore, I'll go back to your patch as posted on oct 13th.
>
> Apologies for the long discussion.
>
> Amicalement,
> --
> Albert.
>

OK, it will at least need a rebase - I will take another look.

Also please note that my series is based on Stephen Warren's MMC
series (1/4 went in via the MMC tree, the other three are ready for
your to pick up) and also Stephen's GPIO patch which is still pending.
Please pick up Stephen's patches first.

Regards,
Simon

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Hi Simon,

Le 28/10/2011 00:46, Simon Glass a ?crit :
> Hi Albert,

>>>>>> Yes I'm tempted to go with this even if the ARM ARM is silent on the
>>>>>> matter. I will try it out today.
>>>>>
>>>>> Silly me of course it doesn't work - since if the CP15 doesn't have
>>>>> those features then you get undef instruction exception. Please see
>>>>> ^^^ above!
>>>>
>>>> Are you stating a reasoning here or the results of an actual test? What
>>>> causes an undefined instruction exception is the absence of a coprocessor
>>>> to
>>>> handle the instruction, not the presence of a coprocessor which
>>>> incidentally
>>>> cannot perform all of what"s being asked.
>>>
>>> Yes I did test it - it hangs. Would you like me to spend some time to
>>> debug exactly why? I suspect it is an undef instruction, since
>>> executing cache instructions on an ARM7TDMI is dodgy, but I could
>>> check if it would help. But let's assume for now we can't do that!
>>
>> What was the test exactly? Depending on which register of CP15 you try to
>> access, things could be different. Can you test MRC p15,0,<Rt>,c0,c0,0 ?
>> That's the MIDR and it should be always accessible, plus it should have a
>> unique value for each type of core, so it could be used to not write cp15
>> for caches etc from the AVP.
>
> I'm sure that is fine. So do you want me to add a check that the ID
> shows architecture ARMv7, and then skip the cache code if not?
> It should be safe enough since it is a clear signal that something is not
> right. It would also handle the case where an Cortex-A7 (with a cache)
> is attached.

That's the general idea of what I am asking for: test for actual 
conditions when needed rather than assume conditions at build time.

However I don't want to test for ARMv7 architecture, because there is no 
reason for *all* ARMv7 cores to fail handling current cp15 init.

What I want here is to test for NVidia Tegra2 AVP (using the 
Implementer, Variant, Primary part number and possibly even Revision 
fields of MIDR), since you have demonstrated it to not handle the 
current cp15 init.

Plus, run-time testing on MIDR could later on allow us to reduce ARM 
init code path to a single, common start.S for generic/common ARM init 
code, and specific code would be 'switched' into, based on MIDR -- that 
could allow for multiple board support in a single binary.

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Albert ARIBAUD]

Hi Simon,

Going back to this version of the patch as indicated, I notice something 
weird:

Le 13/10/2011 23:05, Simon Glass a ?crit :
> Some SOCs have do not start up with their 'main' CPU. The first U-Boot
> code may then be executed with a CPU which does not have a CP15, or not a
> useful one.
>
> Here we split the initialization of CP15 into a separate call, which can
> be performed later if required.
>
> Once the main CPU is running, you should call cpu_init_cp15() to perform
> this init as early as possible.
>
> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not
> need to change, this CP15 init is still skipped in that case. The only
> impact for these boards is that the cpu_init_cp15() will be available
> even if it is never used on these boards.
>
> Signed-off-by: Simon Glass<sjg@chromium.org>
> ---
>   arch/arm/cpu/armv7/start.S        |   24 ++++++++++++++++++------
>   arch/arm/include/asm/u-boot-arm.h |    3 +++
>   2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index db8e9d2..7fdd422 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -168,6 +168,7 @@ next:
>   #endif
>   	/* the mask ROM code should have PLL and others stable */
>   #ifndef CONFIG_SKIP_LOWLEVEL_INIT
> +	bl	cpu_reset_cp15
>   	bl	cpu_init_crit
>   #endif
>
> @@ -305,17 +306,16 @@ jump_2_ram:
>   _board_init_r_ofs:
>   	.word board_init_r - _start
>
> -
> -#ifndef CONFIG_SKIP_LOWLEVEL_INIT
>   /*************************************************************************
>    *
> - * CPU_init_critical registers
> + * cpu_reset_cp15
>    *
> - * setup important registers
> - * setup memory timing
> + * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on unless
> + * CONFIG_SYS_ICACHE_OFF is defined.
>    *
>    *************************************************************************/
> -cpu_init_crit:
> +.globl cpu_init_cp15
> +cpu_init_cp15:

Seems like half this patch says 'cpu_reset_cp15' and then the second 
half says 'cpu_init_cp15' (I prefer 'cpu_init_cp15').

>   	/*
>   	 * Invalidate L1 I/D
>   	 */
> @@ -340,7 +340,19 @@ cpu_init_crit:
>   	orr	r0, r0, #0x00001000	@ set bit 12 (I) I-cache
>   #endif
>   	mcr	p15, 0, r0, c1, c0, 0
> +	mov	pc, lr			@ back to my caller
>
> +
> +#ifndef CONFIG_SKIP_LOWLEVEL_INIT
> +/*************************************************************************
> + *
> + * CPU_init_critical registers
> + *
> + * setup important registers
> + * setup memory timing
> + *
> + *************************************************************************/
> +cpu_init_crit:
>   	/*
>   	 * Jump to board specific initialization...
>   	 * The Mask ROM will have already initialized
> diff --git a/arch/arm/include/asm/u-boot-arm.h b/arch/arm/include/asm/u-boot-arm.h
> index d3308f7..4ca75f9 100644
> --- a/arch/arm/include/asm/u-boot-arm.h
> +++ b/arch/arm/include/asm/u-boot-arm.h
> @@ -46,6 +46,9 @@ extern ulong IRQ_STACK_START_IN;	/* 8 bytes in IRQ stack */
>   int	cpu_init(void);
>   int	cleanup_before_linux(void);
>
> +/* Set up ARMv7 MMU, caches and TLBs */
> +void	cpu_init_cp15(void);
> +
>   /* cpu/.../arch/cpu.c */
>   int	arch_cpu_init(void);
>   int	arch_misc_init(void);

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit()

[Simon Glass]

Hi Albert,

On Sat, Nov 5, 2011 at 1:17 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Simon,
>
> Going back to this version of the patch as indicated, I notice something
> weird:
>
> Le 13/10/2011 23:05, Simon Glass a ?crit :
>>
>> Some SOCs have do not start up with their 'main' CPU. The first U-Boot
>> code may then be executed with a CPU which does not have a CP15, or not a
>> useful one.
>>
>> Here we split the initialization of CP15 into a separate call, which can
>> be performed later if required.
>>
>> Once the main CPU is running, you should call cpu_init_cp15() to perform
>> this init as early as possible.
>>
>> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not
>> need to change, this CP15 init is still skipped in that case. The only
>> impact for these boards is that the cpu_init_cp15() will be available
>> even if it is never used on these boards.
>>
>> Signed-off-by: Simon Glass<sjg@chromium.org>
>> ---
>> ?arch/arm/cpu/armv7/start.S ? ? ? ?| ? 24 ++++++++++++++++++------
>> ?arch/arm/include/asm/u-boot-arm.h | ? ?3 +++
>> ?2 files changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>> index db8e9d2..7fdd422 100644
>> --- a/arch/arm/cpu/armv7/start.S
>> +++ b/arch/arm/cpu/armv7/start.S
>> @@ -168,6 +168,7 @@ next:
>> ?#endif
>> ? ? ? ?/* the mask ROM code should have PLL and others stable */
>> ?#ifndef CONFIG_SKIP_LOWLEVEL_INIT
>> + ? ? ? bl ? ? ?cpu_reset_cp15
>> ? ? ? ?bl ? ? ?cpu_init_crit
>> ?#endif
>>
>> @@ -305,17 +306,16 @@ jump_2_ram:
>> ?_board_init_r_ofs:
>> ? ? ? ?.word board_init_r - _start
>>
>> -
>> -#ifndef CONFIG_SKIP_LOWLEVEL_INIT
>>
>> ?/*************************************************************************
>> ? *
>> - * CPU_init_critical registers
>> + * cpu_reset_cp15
>> ? *
>> - * setup important registers
>> - * setup memory timing
>> + * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on
>> unless
>> + * CONFIG_SYS_ICACHE_OFF is defined.
>> ? *
>>
>> *************************************************************************/
>> -cpu_init_crit:
>> +.globl cpu_init_cp15
>> +cpu_init_cp15:
>
> Seems like half this patch says 'cpu_reset_cp15' and then the second half
> says 'cpu_init_cp15' (I prefer 'cpu_init_cp15').

Given this and the activity in the past 3 weeks I have done a new
serial v4 with a rebase and this fixed. The only change I made to the
series was in this one patch to correct the first call to
cpu_init_cp15 and update the comment. But I suggest you pick up this
new series to avoid conflicts.

Regards,
Simon

>
>> ? ? ? ?/*
>> ? ? ? ? * Invalidate L1 I/D
>> ? ? ? ? */
>> @@ -340,7 +340,19 @@ cpu_init_crit:
>> ? ? ? ?orr ? ? r0, r0, #0x00001000 ? ? @ set bit 12 (I) I-cache
>> ?#endif
>> ? ? ? ?mcr ? ? p15, 0, r0, c1, c0, 0
>> + ? ? ? mov ? ? pc, lr ? ? ? ? ? ? ? ? ?@ back to my caller
>>
>> +
>> +#ifndef CONFIG_SKIP_LOWLEVEL_INIT
>>
>> +/*************************************************************************
>> + *
>> + * CPU_init_critical registers
>> + *
>> + * setup important registers
>> + * setup memory timing
>> + *
>> +
>> *************************************************************************/
>> +cpu_init_crit:
>> ? ? ? ?/*
>> ? ? ? ? * Jump to board specific initialization...
>> ? ? ? ? * The Mask ROM will have already initialized
>> diff --git a/arch/arm/include/asm/u-boot-arm.h
>> b/arch/arm/include/asm/u-boot-arm.h
>> index d3308f7..4ca75f9 100644
>> --- a/arch/arm/include/asm/u-boot-arm.h
>> +++ b/arch/arm/include/asm/u-boot-arm.h
>> @@ -46,6 +46,9 @@ extern ulong IRQ_STACK_START_IN; ? ? ?/* 8 bytes in IRQ
>> stack */
>> ?int ? cpu_init(void);
>> ?int ? cleanup_before_linux(void);
>>
>> +/* Set up ARMv7 MMU, caches and TLBs */
>> +void ? cpu_init_cp15(void);
>> +
>> ?/* cpu/.../arch/cpu.c */
>> ?int ? arch_cpu_init(void);
>> ?int ? arch_misc_init(void);
>
> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH 4/9] tegra2: Enable instruction cache

[Simon Glass]

Since low-level init is skipped, the instruction cache is never enabled on
Tegra2. This explicitly calls this initialization as soon as the A9 is
initialized.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 board/nvidia/common/board.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
index 6af317b..0db95ff 100644
--- a/board/nvidia/common/board.c
+++ b/board/nvidia/common/board.c
@@ -177,6 +177,9 @@ int board_mmc_init(bd_t *bd)
 #ifdef CONFIG_BOARD_EARLY_INIT_F
 int board_early_init_f(void)
 {
+	/* We didn't do this init in start.S, so do it now */
+	cpu_init_cp15();
+
 	/* Initialize essential common plls */
 	clock_early_init();
 
-- 
1.7.3.1

[U-Boot] [PATCH 5/9] tegra2: Remove unneeded boot code

[Simon Glass]

Since we have cache support built in we can remove Tegra's existing cache
initialization code amd other related dead code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/start.S                |   12 ---
 arch/arm/cpu/armv7/tegra2/ap20.h          |    7 +--
 arch/arm/cpu/armv7/tegra2/board.c         |    8 --
 arch/arm/cpu/armv7/tegra2/config.mk       |   28 -------
 arch/arm/cpu/armv7/tegra2/lowlevel_init.S |  118 -----------------------------
 5 files changed, 1 insertions(+), 172 deletions(-)
 delete mode 100644 arch/arm/cpu/armv7/tegra2/config.mk

diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 7fdd422..fa79d73 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -81,18 +81,6 @@ _end_vect:
 _TEXT_BASE:
 	.word	CONFIG_SYS_TEXT_BASE
 
-#ifdef CONFIG_TEGRA2
-/*
- * Tegra2 uses 2 separate CPUs - the AVP (ARM7TDMI) and the CPU (dual A9s).
- * U-Boot runs on the AVP first, setting things up for the CPU (PLLs,
- * muxes, clocks, clamps, etc.). Then the AVP halts, and expects the CPU
- * to pick up its reset vector, which points here.
- */
-.globl _armboot_start
-_armboot_start:
-	.word _start
-#endif
-
 /*
  * These are defined in the board-specific linker script.
  */
diff --git a/arch/arm/cpu/armv7/tegra2/ap20.h b/arch/arm/cpu/armv7/tegra2/ap20.h
index 1bb48d6..a4b4d73 100644
--- a/arch/arm/cpu/armv7/tegra2/ap20.h
+++ b/arch/arm/cpu/armv7/tegra2/ap20.h
@@ -95,13 +95,8 @@
 #define HALT_COP_EVENT_IRQ_1		(1 << 11)
 #define HALT_COP_EVENT_FIQ_1		(1 << 9)
 
-/* Prototypes */
-
+/* Start up the tegra2 SOC */
 void tegra2_start(void);
-void uart_init(void);
-void udelay(unsigned long);
-void cold_boot(void);
-void cache_configure(void);
 
 /* This is the main entry into U-Boot, used by the Cortex-A9 */
 extern void _start(void);
diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/armv7/tegra2/board.c
index e725134..9fc4f1b 100644
--- a/arch/arm/cpu/armv7/tegra2/board.c
+++ b/arch/arm/cpu/armv7/tegra2/board.c
@@ -55,14 +55,6 @@ unsigned int query_sdram_size(void)
 	}
 }
 
-void s_init(void)
-{
-#ifndef CONFIG_ICACHE_OFF
-	icache_enable();
-#endif
-	invalidate_dcache();
-}
-
 int dram_init(void)
 {
 	unsigned long rs;
diff --git a/arch/arm/cpu/armv7/tegra2/config.mk b/arch/arm/cpu/armv7/tegra2/config.mk
deleted file mode 100644
index 96c0795..0000000
--- a/arch/arm/cpu/armv7/tegra2/config.mk
+++ /dev/null
@@ -1,28 +0,0 @@
-#
-# (C) Copyright 2010,2011
-# NVIDIA Corporation <www.nvidia.com>
-#
-# (C) Copyright 2002
-# Gary Jennejohn, DENX Software Engineering, <garyj@denx.de>
-#
-# 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., 59 Temple Place, Suite 330, Boston,
-# MA 02111-1307 USA
-#
-
-# Use ARMv4 for Tegra2 - initial code runs on the AVP, which is an ARM7TDI.
-PLATFORM_CPPFLAGS += -march=armv4
diff --git a/arch/arm/cpu/armv7/tegra2/lowlevel_init.S b/arch/arm/cpu/armv7/tegra2/lowlevel_init.S
index f24a2ff..6b86647 100644
--- a/arch/arm/cpu/armv7/tegra2/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/tegra2/lowlevel_init.S
@@ -26,14 +26,6 @@
 #include <config.h>
 #include <version.h>
 
-
-_TEXT_BASE:
-	.word	CONFIG_SYS_TEXT_BASE	@ sdram load addr from config file
-
-.global invalidate_dcache
-invalidate_dcache:
-	mov pc, lr
-
 	.align	5
 .global reset_cpu
 reset_cpu:
@@ -47,113 +39,3 @@ _loop_forever:
 	b	_loop_forever
 rstctl:
 	.word	PRM_RSTCTRL
-
-.globl lowlevel_init
-lowlevel_init:
-	ldr	sp, SRAM_STACK
-	str	ip, [sp]
-	mov	ip, lr
-	bl	s_init				@ go setup pll, mux & memory
-	ldr	ip, [sp]
-	mov	lr, ip
-
-	mov	pc, lr				@ back to arch calling code
-
-
-.globl startup_cpu
-startup_cpu:
-	@ Initialize the AVP, clocks, and memory controller
-	@ SDRAM is guaranteed to be on at this point
-
-	ldr     r0, =cold_boot			@ R0 = reset vector for CPU
-	bl      start_cpu			@ start the CPU
-
-	@ Transfer control to the AVP code
-	bl      halt_avp
-
-	@ Should never get here
-_loop_forever2:
-	b	_loop_forever2
-
-.globl cache_configure
-cache_configure:
-	stmdb	r13!,{r14}
-	@ invalidate instruction cache
-	mov	r1, #0
-	mcr	p15, 0, r1, c7, c5, 0
-
-	@ invalidate the i&d tlb entries
-	mcr	p15, 0, r1, c8, c5, 0
-	mcr	p15, 0, r1, c8, c6, 0
-
-	@ enable instruction cache
-	mrc	p15, 0, r1, c1, c0, 0
-	orr	r1, r1, #(1<<12)
-	mcr	p15, 0, r1, c1, c0, 0
-
-	bl	enable_scu
-
-	@ enable SMP mode and FW for CPU0, by writing to Auxiliary Ctl reg
-	mrc	p15, 0, r0, c1, c0, 1
-	orr	r0, r0, #0x41
-	mcr	p15, 0, r0, c1, c0, 1
-
-	@ Now flush the Dcache
-	mov	r0, #0
-	@ 256 cache lines
-	mov	r1, #256
-
-invalidate_loop:
-	add	r1, r1, #-1
-	mov	r0, r1, lsl #5
-	@ invalidate d-cache using line (way0)
-	mcr	p15, 0, r0, c7, c6, 2
-
-	orr	r2, r0, #(1<<30)
-	@ invalidate d-cache using line (way1)
-	mcr	p15, 0, r2, c7, c6, 2
-
-	orr	r2, r0, #(2<<30)
-	@ invalidate d-cache using line (way2)
-	mcr	p15, 0, r2, c7, c6, 2
-
-	orr	r2, r0, #(3<<30)
-	@ invalidate d-cache using line (way3)
-	mcr	p15, 0, r2, c7, c6, 2
-	cmp	r1, #0
-	bne	invalidate_loop
-
-	@ FIXME: should have ap20's L2 disabled too?
-invalidate_done:
-	ldmia	r13!,{pc}
-
-.globl cold_boot
-cold_boot:
-	msr	cpsr_c, #0xD3
-	@ Check current processor: CPU or AVP?
-	@  If CPU, go to CPU boot code, else continue on AVP path
-
-	ldr	r0, =NV_PA_PG_UP_BASE
-	ldr	r1, [r0]
-	ldr	r2, =PG_UP_TAG_AVP
-
-	@ are we the CPU?
-	ldr	sp, CPU_STACK
-	cmp	r1, r2
-	@ yep, we are the CPU
-	bne	_armboot_start
-
-	@ AVP initialization follows this path
-	ldr	sp, AVP_STACK
-	@ Init AVP and start CPU
-	b	startup_cpu
-
-	@ the literal pools origin
-	.ltorg
-
-SRAM_STACK:
-	.word LOW_LEVEL_SRAM_STACK
-AVP_STACK:
-	.word EARLY_AVP_STACK
-CPU_STACK:
-	.word EARLY_CPU_STACK
-- 
1.7.3.1

[U-Boot] [PATCH 6/9] tegra2: Remove unneeded config option

[Simon Glass]

CONFIG_ENABLE_CORTEXA9 and CONFIG_SKIP_RELOCATE_UBOOT are not needed,
so remove them.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 include/configs/tegra2-common.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-common.h
index 3454689..4d59eda 100644
--- a/include/configs/tegra2-common.h
+++ b/include/configs/tegra2-common.h
@@ -33,7 +33,6 @@
 #define CONFIG_MACH_TEGRA_GENERIC	/* which is a Tegra generic machine */
 #define CONFIG_SYS_L2CACHE_OFF		/* No L2 cache */
 #define CONFIG_ARCH_CPU_INIT		/* Fire up the A9 core */
-#define CONFIG_ENABLE_CORTEXA9		/* enable CPU (A9 complex) */
 
 #include <asm/arch/tegra2.h>		/* get chip and board defs */
 
@@ -43,7 +42,6 @@
 #define CONFIG_DISPLAY_CPUINFO
 #define CONFIG_DISPLAY_BOARDINFO
 
-#define CONFIG_SKIP_RELOCATE_UBOOT
 #define CONFIG_SKIP_LOWLEVEL_INIT
 
 #define CONFIG_CMDLINE_TAG		/* enable passing of ATAGs */
-- 
1.7.3.1

[U-Boot] [PATCH 7/9] tegra2: Remove unused low-level Tegra2 UART code

[Simon Glass]

This was used by the AVP in early boot but is no longer used. Unless we
plan to enable it somehow it is not needed. In any case we should try
to use the ns16550 driver instead as it has the same code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 drivers/serial/Makefile        |    1 -
 drivers/serial/serial_tegra2.c |   77 ----------------------------------------
 drivers/serial/serial_tegra2.h |   29 ---------------
 3 files changed, 0 insertions(+), 107 deletions(-)
 delete mode 100644 drivers/serial/serial_tegra2.c
 delete mode 100644 drivers/serial/serial_tegra2.h

diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 1dcc1c7..d2915f5 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -54,7 +54,6 @@ COBJS-$(CONFIG_S3C24X0_SERIAL) += serial_s3c24x0.o
 COBJS-$(CONFIG_S3C44B0_SERIAL) += serial_s3c44b0.o
 COBJS-$(CONFIG_XILINX_UARTLITE) += serial_xuartlite.o
 COBJS-$(CONFIG_SCIF_CONSOLE) += serial_sh.o
-COBJS-$(CONFIG_TEGRA2) += serial_tegra2.o
 
 ifndef CONFIG_SPL_BUILD
 COBJS-$(CONFIG_USB_TTY) += usbtty.o
diff --git a/drivers/serial/serial_tegra2.c b/drivers/serial/serial_tegra2.c
deleted file mode 100644
index 8ff34ea..0000000
--- a/drivers/serial/serial_tegra2.c
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- *  (C) Copyright 2010,2011
- *  NVIDIA Corporation <www.nvidia.com>
- *
- * 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., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#include <common.h>
-#include <ns16550.h>
-#include <asm/io.h>
-#include <asm/arch/tegra2.h>
-#include "serial_tegra2.h"
-
-static void setup_uart(struct uart_ctlr *u)
-{
-	u32 reg;
-
-	/* Prepare the divisor value */
-	reg = NVRM_PLLP_FIXED_FREQ_KHZ * 1000 / NV_DEFAULT_DEBUG_BAUD / 16;
-
-	/* Set up UART parameters */
-	writel(UART_LCR_DLAB, &u->uart_lcr);
-	writel(reg, &u->uart_thr_dlab_0);
-	writel(0, &u->uart_ier_dlab_0);
-	writel(0, &u->uart_lcr);			/* clear DLAB */
-	writel((UART_FCR_TRIGGER_3 | UART_FCR_FIFO_EN | \
-		UART_FCR_CLEAR_XMIT | UART_FCR_CLEAR_RCVR), &u->uart_iir_fcr);
-	writel(0, &u->uart_ier_dlab_0);
-	writel(UART_LCR_WLS_8, &u->uart_lcr);	/* 8N1 */
-	writel(UART_MCR_RTS, &u->uart_mcr);
-	writel(0, &u->uart_msr);
-	writel(0, &u->uart_spr);
-	writel(0, &u->uart_irda_csr);
-	writel(0, &u->uart_asr);
-	writel((UART_FCR_TRIGGER_3 | UART_FCR_FIFO_EN), &u->uart_iir_fcr);
-
-	/* Flush any old characters out of the RX FIFO */
-	reg = readl(&u->uart_lsr);
-
-	while (reg & UART_LSR_DR) {
-		reg = readl(&u->uart_thr_dlab_0);
-		reg = readl(&u->uart_lsr);
-	}
-}
-
-/*
- * Routine: uart_init
- * Description: init the UART clocks, muxes, and baudrate/parity/etc.
- */
-void uart_init(void)
-{
-	struct uart_ctlr *uart = (struct uart_ctlr *)NV_PA_APB_UARTD_BASE;
-#if defined(CONFIG_TEGRA2_ENABLE_UARTD)
-	setup_uart(uart);
-#endif	/* CONFIG_TEGRA2_ENABLE_UARTD */
-#if defined(CONFIG_TEGRA2_ENABLE_UARTA)
-	uart = (struct uart_ctlr *)NV_PA_APB_UARTA_BASE;
-
-	setup_uart(uart);
-#endif	/* CONFIG_TEGRA2_ENABLE_UARTA */
-}
diff --git a/drivers/serial/serial_tegra2.h b/drivers/serial/serial_tegra2.h
deleted file mode 100644
index 5704800..0000000
--- a/drivers/serial/serial_tegra2.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- *  (C) Copyright 2010,2011
- *  NVIDIA Corporation <www.nvidia.com>
- *
- * 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., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#ifndef _SERIAL_TEGRA_H_
-#define _SERIAL_TEGRA_H_
-
-#include <asm/arch/uart.h>
-
-#endif /* _SERIAL_TEGRA_H_ */
-- 
1.7.3.1

[U-Boot] [PATCH 8/9] tegra2: Remove unneeded 'dynamic ram size' message

[Simon Glass]

This message is not required, since it is followed by an 'official' U-Boot
message.

U-Boot 2011.03-00048-gd7cb0d3 (May 11 2011 - 17:17:23)

TEGRA2
Board: NVIDIA Seaboard
dynamic ram_size = 1073741824
DRAM:  1 GiB

becomes:

TEGRA2
Board: NVIDIA Seaboard
DRAM:  1 GiB

This is a separate commit since it changes behavior.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/tegra2/board.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/armv7/tegra2/board.c
index 9fc4f1b..04c1683 100644
--- a/arch/arm/cpu/armv7/tegra2/board.c
+++ b/arch/arm/cpu/armv7/tegra2/board.c
@@ -65,10 +65,8 @@ int dram_init(void)
 
 	/* Now check it dynamically */
 	rs = get_ram_size(CONFIG_SYS_SDRAM_BASE, gd->ram_size);
-	if (rs) {
-		printf("dynamic ram_size = %lu\n", rs);
+	if (rs)
 		gd->bd->bi_dram[0].size = gd->ram_size = rs;
-	}
 	return 0;
 }
 
-- 
1.7.3.1

[U-Boot] [PATCH 9/9] tegra2: Don't use board pointer before it is set up

[Simon Glass]

In board_init_f() the gd->bd pointer is not valid when dram_init() is called.
This only avoids dying because DRAM is at zero on Tegra2. The common ARM
routine sets up the banks in the same way anyway, so we can just remove this
code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/tegra2/board.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/armv7/tegra2/board.c
index 04c1683..91e0a6f 100644
--- a/arch/arm/cpu/armv7/tegra2/board.c
+++ b/arch/arm/cpu/armv7/tegra2/board.c
@@ -57,16 +57,8 @@ unsigned int query_sdram_size(void)
 
 int dram_init(void)
 {
-	unsigned long rs;
-
 	/* We do not initialise DRAM here. We just query the size */
-	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
-	gd->bd->bi_dram[0].size = gd->ram_size = query_sdram_size();
-
-	/* Now check it dynamically */
-	rs = get_ram_size(CONFIG_SYS_SDRAM_BASE, gd->ram_size);
-	if (rs)
-		gd->bd->bi_dram[0].size = gd->ram_size = rs;
+	gd->ram_size = query_sdram_size();
 	return 0;
 }
 
-- 
1.7.3.1