All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/9] MIPS L2 cache support
@ 2016-09-07 17:48 Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init Paul Burton
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-07 17:48 UTC (permalink / raw)
  To: u-boot

This series introduces support for initialising & maintaining L2 caches
on MIPS systems. This allows U-Boot to function correctly on systems
where such caches are present, whereas without performing L2 maintenance
it is likely to fail with cache coherence issues when writing code or
performing DMA transfers.

Paul Burton (9):
  MIPS: Add arch-wide arch_cpu_init
  MIPS: Probe cache line sizes once during boot
  MIPS: Enable use of the instruction cache earlier
  MIPS: Preserve Config implementation-defined bits
  MIPS: If we don't need DDR for cache init, init cache first
  MIPS: Define register names for cache init
  MIPS: Map CM Global Control Registers
  MIPS: L2 cache support
  MIPS: Malta: Enable CM & L2 support

 arch/mips/Kconfig                   |  28 +++++
 arch/mips/cpu/Makefile              |   2 +
 arch/mips/cpu/cm_init.S             |  45 +++++++
 arch/mips/cpu/cpu.c                 |   8 ++
 arch/mips/cpu/start.S               |  26 +++--
 arch/mips/include/asm/cache.h       |   9 ++
 arch/mips/include/asm/cm.h          |  57 +++++++++
 arch/mips/include/asm/global_data.h |   7 ++
 arch/mips/include/asm/mipsregs.h    |   6 +
 arch/mips/include/asm/u-boot-mips.h |  12 ++
 arch/mips/lib/cache.c               | 101 +++++++++++++---
 arch/mips/lib/cache_init.S          | 226 ++++++++++++++++++++++++++++++++----
 arch/mips/mach-ath79/cpu.c          |   3 +-
 board/imgtec/malta/lowlevel_init.S  |   6 -
 14 files changed, 482 insertions(+), 54 deletions(-)
 create mode 100644 arch/mips/cpu/cm_init.S
 create mode 100644 arch/mips/include/asm/cm.h

-- 
2.9.3

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init
  2016-09-07 17:48 [U-Boot] [PATCH 0/9] MIPS L2 cache support Paul Burton
@ 2016-09-07 17:48 ` Paul Burton
  2016-09-08 10:02   ` Marek Vasut
  2016-09-07 17:48 ` [U-Boot] [PATCH 2/9] MIPS: Probe cache line sizes once during boot Paul Burton
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Paul Burton @ 2016-09-07 17:48 UTC (permalink / raw)
  To: u-boot

Add an implementation of arch_cpu_init that covers all MIPS boards, in
preparation for performing various pieces of initialisation there in
later patches.

In order to allow the ath79 code to continue performing initialisation
at this point in the boot, it's converted to use a new mach_cpu_init
function which is called from arch_cpu_init.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/Kconfig                   |  4 ++++
 arch/mips/cpu/cpu.c                 |  6 ++++++
 arch/mips/include/asm/u-boot-mips.h | 12 ++++++++++++
 arch/mips/mach-ath79/cpu.c          |  3 +--
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 21066f0..34eb404 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -67,6 +67,7 @@ config ARCH_ATH79
 	bool "Support QCA/Atheros ath79"
 	select OF_CONTROL
 	select DM
+	select MACH_CPU_INIT
 
 config MACH_PIC32
 	bool "Support Microchip PIC32"
@@ -303,6 +304,9 @@ config MIPS_L1_CACHE_SHIFT
 config DYNAMIC_IO_PORT_BASE
 	bool
 
+config MACH_CPU_INIT
+	bool
+
 endif
 
 endmenu
diff --git a/arch/mips/cpu/cpu.c b/arch/mips/cpu/cpu.c
index 391feb3..5ee80dd 100644
--- a/arch/mips/cpu/cpu.c
+++ b/arch/mips/cpu/cpu.c
@@ -35,3 +35,9 @@ void write_one_tlb(int index, u32 pagemask, u32 hi, u32 low0, u32 low1)
 	write_c0_index(index);
 	tlb_write_indexed();
 }
+
+int arch_cpu_init(void)
+{
+	mach_cpu_init();
+	return 0;
+}
diff --git a/arch/mips/include/asm/u-boot-mips.h b/arch/mips/include/asm/u-boot-mips.h
index 1f527bb..0eaf32b 100644
--- a/arch/mips/include/asm/u-boot-mips.h
+++ b/arch/mips/include/asm/u-boot-mips.h
@@ -5,4 +5,16 @@
 #ifndef _U_BOOT_MIPS_H_
 #define _U_BOOT_MIPS_H_
 
+/**
+ * mach_cpu_init() - Perform SoC/machine-specific CPU initialisation
+ *
+ * This is called from arch_cpu_init() to allow for SoC/machine-level code to
+ * perform any CPU initialisation it may require.
+ */
+#ifdef CONFIG_MACH_CPU_INIT
+void mach_cpu_init(void);
+#else
+static inline void mach_cpu_init(void) {}
+#endif
+
 #endif /* _U_BOOT_MIPS_H_ */
diff --git a/arch/mips/mach-ath79/cpu.c b/arch/mips/mach-ath79/cpu.c
index 5756a06..a556b73 100644
--- a/arch/mips/mach-ath79/cpu.c
+++ b/arch/mips/mach-ath79/cpu.c
@@ -46,7 +46,7 @@ static const struct ath79_soc_desc desc[] = {
 	{ATH79_SOC_QCA9561,     "9561", REV_ID_MAJOR_QCA9561,   0},
 };
 
-int arch_cpu_init(void)
+void mach_cpu_init(void)
 {
 	void __iomem *base;
 	enum ath79_soc_type soc = ATH79_SOC_UNKNOWN;
@@ -98,7 +98,6 @@ int arch_cpu_init(void)
 	gd->arch.soc = soc;
 	gd->arch.rev = rev;
 	gd->arch.ver = ver;
-	return 0;
 }
 
 int print_cpuinfo(void)
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 2/9] MIPS: Probe cache line sizes once during boot
  2016-09-07 17:48 [U-Boot] [PATCH 0/9] MIPS L2 cache support Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init Paul Burton
@ 2016-09-07 17:48 ` Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 3/9] MIPS: Enable use of the instruction cache earlier Paul Burton
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-07 17:48 UTC (permalink / raw)
  To: u-boot

Rather than probing the cache line sizes on every call of any cache
maintenance function, probe them once during boot & store the values in
the global data structure for later use. This will reduce the overhead
of the cache maintenance functions, which isn't a big deal yet but
becomes more important once L2 caches which may expose their properties
via coprocessor 2 or the CM are supported.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/cpu/cpu.c                 |  2 ++
 arch/mips/include/asm/cache.h       |  9 ++++++++
 arch/mips/include/asm/global_data.h |  4 ++++
 arch/mips/lib/cache.c               | 43 +++++++++++++++++++++----------------
 4 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/arch/mips/cpu/cpu.c b/arch/mips/cpu/cpu.c
index 5ee80dd..a82c3c4 100644
--- a/arch/mips/cpu/cpu.c
+++ b/arch/mips/cpu/cpu.c
@@ -8,6 +8,7 @@
 #include <common.h>
 #include <command.h>
 #include <linux/compiler.h>
+#include <asm/cache.h>
 #include <asm/mipsregs.h>
 #include <asm/reboot.h>
 
@@ -38,6 +39,7 @@ void write_one_tlb(int index, u32 pagemask, u32 hi, u32 low0, u32 low1)
 
 int arch_cpu_init(void)
 {
+	mips_cache_probe();
 	mach_cpu_init();
 	return 0;
 }
diff --git a/arch/mips/include/asm/cache.h b/arch/mips/include/asm/cache.h
index 0cea581..669c362 100644
--- a/arch/mips/include/asm/cache.h
+++ b/arch/mips/include/asm/cache.h
@@ -19,4 +19,13 @@
  */
 #define CONFIG_SYS_CACHELINE_SIZE ARCH_DMA_MINALIGN
 
+/**
+ * mips_cache_probe() - Probe the properties of the caches
+ *
+ * Call this to probe the properties such as line sizes of the caches
+ * present in the system, if any. This must be done before cache maintenance
+ * functions such as flush_cache may be called.
+ */
+void mips_cache_probe(void);
+
 #endif /* __MIPS_CACHE_H__ */
diff --git a/arch/mips/include/asm/global_data.h b/arch/mips/include/asm/global_data.h
index 37f8ed5..8533b69 100644
--- a/arch/mips/include/asm/global_data.h
+++ b/arch/mips/include/asm/global_data.h
@@ -21,6 +21,10 @@ struct arch_global_data {
 	unsigned long rev;
 	unsigned long ver;
 #endif
+#ifdef CONFIG_SYS_CACHE_SIZE_AUTO
+	unsigned short l1i_line_size;
+	unsigned short l1d_line_size;
+#endif
 };
 
 #include <asm-generic/global_data.h>
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c
index db81953..d8baf08 100644
--- a/arch/mips/lib/cache.c
+++ b/arch/mips/lib/cache.c
@@ -9,32 +9,39 @@
 #include <asm/cacheops.h>
 #include <asm/mipsregs.h>
 
-static inline unsigned long icache_line_size(void)
-{
-	unsigned long conf1, il;
+DECLARE_GLOBAL_DATA_PTR;
 
-	if (!config_enabled(CONFIG_SYS_CACHE_SIZE_AUTO))
-		return CONFIG_SYS_ICACHE_LINE_SIZE;
+void mips_cache_probe(void)
+{
+#ifdef CONFIG_SYS_CACHE_SIZE_AUTO
+	unsigned long conf1, il, dl;
 
 	conf1 = read_c0_config1();
+
 	il = (conf1 & MIPS_CONF1_IL) >> MIPS_CONF1_IL_SHF;
-	if (!il)
-		return 0;
-	return 2 << il;
+	dl = (conf1 & MIPS_CONF1_DL) >> MIPS_CONF1_DL_SHF;
+
+	gd->arch.l1i_line_size = il ? (2 << il) : 0;
+	gd->arch.l1d_line_size = dl ? (2 << dl) : 0;
+#endif
 }
 
-static inline unsigned long dcache_line_size(void)
+static inline unsigned long icache_line_size(void)
 {
-	unsigned long conf1, dl;
-
-	if (!config_enabled(CONFIG_SYS_CACHE_SIZE_AUTO))
-		return CONFIG_SYS_DCACHE_LINE_SIZE;
+#ifdef CONFIG_SYS_CACHE_SIZE_AUTO
+	return gd->arch.l1i_line_size;
+#else
+	return CONFIG_SYS_ICACHE_LINE_SIZE;
+#endif
+}
 
-	conf1 = read_c0_config1();
-	dl = (conf1 & MIPS_CONF1_DL) >> MIPS_CONF1_DL_SHF;
-	if (!dl)
-		return 0;
-	return 2 << dl;
+static inline unsigned long dcache_line_size(void)
+{
+#ifdef CONFIG_SYS_CACHE_SIZE_AUTO
+	return gd->arch.l1d_line_size;
+#else
+	return CONFIG_SYS_DCACHE_LINE_SIZE;
+#endif
 }
 
 #define cache_loop(start, end, lsize, ops...) do {			\
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 3/9] MIPS: Enable use of the instruction cache earlier
  2016-09-07 17:48 [U-Boot] [PATCH 0/9] MIPS L2 cache support Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 2/9] MIPS: Probe cache line sizes once during boot Paul Burton
@ 2016-09-07 17:48 ` Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 4/9] MIPS: Preserve Config implementation-defined bits Paul Burton
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-07 17:48 UTC (permalink / raw)
  To: u-boot

Enable use of the instruction cache immediately after it has been
initialised. This will only take effect if U-Boot was linked to run from
kseg0 rather than kseg1, but when this is the case the data cache
initialisation code will run cached & thus significantly faster.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/cpu/start.S      |  8 --------
 arch/mips/lib/cache_init.S | 12 ++++++++++++
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/mips/cpu/start.S b/arch/mips/cpu/start.S
index fc6dd66..827a544 100644
--- a/arch/mips/cpu/start.S
+++ b/arch/mips/cpu/start.S
@@ -12,10 +12,6 @@
 #include <asm/regdef.h>
 #include <asm/mipsregs.h>
 
-#ifndef CONFIG_SYS_MIPS_CACHE_MODE
-#define CONFIG_SYS_MIPS_CACHE_MODE CONF_CM_CACHABLE_NONCOHERENT
-#endif
-
 #ifndef CONFIG_SYS_INIT_SP_ADDR
 #define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_SDRAM_BASE + \
 				CONFIG_SYS_INIT_SP_OFFSET)
@@ -154,10 +150,6 @@ reset:
 	PTR_LA	t9, mips_cache_reset
 	jalr	t9
 	 nop
-
-	/* ... and enable them */
-	li	t0, CONFIG_SYS_MIPS_CACHE_MODE
-	mtc0	t0, CP0_CONFIG
 #endif
 
 	/* Set up temporary stack */
diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S
index bc8ab27..c3fb249 100644
--- a/arch/mips/lib/cache_init.S
+++ b/arch/mips/lib/cache_init.S
@@ -172,6 +172,18 @@ LEAF(mips_cache_reset)
 	cache_loop	t0, t1, t8, INDEX_STORE_TAG_I
 #endif
 
+	/* Enable use of the I-cache by setting Config.K0 */
+	mfc0		t0, CP0_CONFIG
+	li		t1, CONFIG_SYS_MIPS_CACHE_MODE
+#if __mips_isa_rev >= 2
+	ins		t0, t1, 0, 3
+#else
+	ori		t0, t0, CONF_CM_CMASK
+	xori		t0, t0, CONF_CM_CMASK
+	or		t0, t0, t1
+#endif
+	mtc0		t0, CP0_CONFIG
+
 	/*
 	 * then initialize D-cache.
 	 */
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/9] MIPS: Preserve Config implementation-defined bits
  2016-09-07 17:48 [U-Boot] [PATCH 0/9] MIPS L2 cache support Paul Burton
                   ` (2 preceding siblings ...)
  2016-09-07 17:48 ` [U-Boot] [PATCH 3/9] MIPS: Enable use of the instruction cache earlier Paul Burton
@ 2016-09-07 17:48 ` Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 5/9] MIPS: If we don't need DDR for cache init, init cache first Paul Burton
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-07 17:48 UTC (permalink / raw)
  To: u-boot

The coprocessor 0 Config register includes 9 implementation defined
bits, which in some processors do things like enable write combining or
other functionality. We ought not to wipe them to 0 during boot. Rather
than doing so, preserve their value & only clear the bits standardised
by the MIPS architecture.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/cpu/start.S            | 5 +++--
 arch/mips/include/asm/mipsregs.h | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/mips/cpu/start.S b/arch/mips/cpu/start.S
index 827a544..6aec430 100644
--- a/arch/mips/cpu/start.S
+++ b/arch/mips/cpu/start.S
@@ -123,8 +123,9 @@ reset:
 	mtc0	zero, CP0_COMPARE
 
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	/* CONFIG0 register */
-	li	t0, CONF_CM_UNCACHED
+	mfc0	t0, CP0_CONFIG
+	and	t0, t0, MIPS_CONF_IMPL
+	or	t0, t0, CONF_CM_UNCACHED
 	mtc0	t0, CP0_CONFIG
 #endif
 
diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 3185dc7..cd4f952 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -450,6 +450,7 @@
 #define MIPS_CONF_MT_FTLB	(_ULCAST_(4) <<  7)
 #define MIPS_CONF_AR		(_ULCAST_(7) << 10)
 #define MIPS_CONF_AT		(_ULCAST_(3) << 13)
+#define MIPS_CONF_IMPL		(_ULCAST_(0x1ff) << 16)
 #define MIPS_CONF_M		(_ULCAST_(1) << 31)
 
 /*
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 5/9] MIPS: If we don't need DDR for cache init, init cache first
  2016-09-07 17:48 [U-Boot] [PATCH 0/9] MIPS L2 cache support Paul Burton
                   ` (3 preceding siblings ...)
  2016-09-07 17:48 ` [U-Boot] [PATCH 4/9] MIPS: Preserve Config implementation-defined bits Paul Burton
@ 2016-09-07 17:48 ` Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 6/9] MIPS: Define register names for cache init Paul Burton
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-07 17:48 UTC (permalink / raw)
  To: u-boot

On systems where cache initialisation doesn't require zeroed memory (ie.
systems where CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD is not defined)
perform cache initialisation prior to lowlevel_init & DDR
initialisation. This allows for DDR initialisation code to run cached &
thus significantly faster.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/cpu/start.S | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/mips/cpu/start.S b/arch/mips/cpu/start.S
index 6aec430..6f1d219 100644
--- a/arch/mips/cpu/start.S
+++ b/arch/mips/cpu/start.S
@@ -142,15 +142,24 @@ reset:
 	PTR_L	gp, 0(ra)
 
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
+# ifdef CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD
 	/* Initialize any external memory */
 	PTR_LA	t9, lowlevel_init
 	jalr	t9
 	 nop
+# endif
 
 	/* Initialize caches... */
 	PTR_LA	t9, mips_cache_reset
 	jalr	t9
 	 nop
+
+# ifndef CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD
+	/* Initialize any external memory */
+	PTR_LA	t9, lowlevel_init
+	jalr	t9
+	 nop
+# endif
 #endif
 
 	/* Set up temporary stack */
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 6/9] MIPS: Define register names for cache init
  2016-09-07 17:48 [U-Boot] [PATCH 0/9] MIPS L2 cache support Paul Burton
                   ` (4 preceding siblings ...)
  2016-09-07 17:48 ` [U-Boot] [PATCH 5/9] MIPS: If we don't need DDR for cache init, init cache first Paul Burton
@ 2016-09-07 17:48 ` Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 7/9] MIPS: Map CM Global Control Registers Paul Burton
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-07 17:48 UTC (permalink / raw)
  To: u-boot

Define names for registers holding cache sizes throughout
mips_cache_reset, in order to make the code easier to read & allow for
changing register assignments more easily.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/lib/cache_init.S | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S
index c3fb249..2df3a82 100644
--- a/arch/mips/lib/cache_init.S
+++ b/arch/mips/lib/cache_init.S
@@ -98,19 +98,23 @@
  * RETURNS: N/A
  *
  */
+#define R_IC_SIZE	t2
+#define R_IC_LINE	t8
+#define R_DC_SIZE	t3
+#define R_DC_LINE	t9
 LEAF(mips_cache_reset)
 #ifndef CONFIG_SYS_CACHE_SIZE_AUTO
-	li	t2, CONFIG_SYS_ICACHE_SIZE
-	li	t8, CONFIG_SYS_ICACHE_LINE_SIZE
+	li	R_IC_SIZE, CONFIG_SYS_ICACHE_SIZE
+	li	R_IC_LINE, CONFIG_SYS_ICACHE_LINE_SIZE
 #else
-	l1_info	t2, t8, MIPS_CONF1_IA_SHF
+	l1_info	R_IC_SIZE, R_IC_LINE, MIPS_CONF1_IA_SHF
 #endif
 
 #ifndef CONFIG_SYS_CACHE_SIZE_AUTO
-	li	t3, CONFIG_SYS_DCACHE_SIZE
-	li	t9, CONFIG_SYS_DCACHE_LINE_SIZE
+	li	R_DC_SIZE, CONFIG_SYS_DCACHE_SIZE
+	li	R_DC_LINE, CONFIG_SYS_DCACHE_LINE_SIZE
 #else
-	l1_info	t3, t9, MIPS_CONF1_DA_SHF
+	l1_info	R_DC_SIZE, R_DC_LINE, MIPS_CONF1_DA_SHF
 #endif
 
 #ifdef CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD
@@ -123,9 +127,9 @@ LEAF(mips_cache_reset)
 	li	v0, CONFIG_SYS_DCACHE_SIZE
 #endif
 #else
-	move	v0, t2
-	sltu	t1, t2, t3
-	movn	v0, t3, t1
+	move	v0, R_IC_SIZE
+	sltu	t1, R_IC_SIZE, R_DC_SIZE
+	movn	v0, R_DC_SIZE, t1
 #endif
 	/*
 	 * Now clear that much memory starting from zero.
@@ -158,18 +162,18 @@ LEAF(mips_cache_reset)
 	/*
 	 * Initialize the I-cache first,
 	 */
-	blez		t2, 1f
+	blez		R_IC_SIZE, 1f
 	PTR_LI		t0, INDEX_BASE
-	PTR_ADDU	t1, t0, t2
+	PTR_ADDU	t1, t0, R_IC_SIZE
 	/* clear tag to invalidate */
-	cache_loop	t0, t1, t8, INDEX_STORE_TAG_I
+	cache_loop	t0, t1, R_IC_LINE, INDEX_STORE_TAG_I
 #ifdef CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD
 	/* fill once, so data field parity is correct */
 	PTR_LI		t0, INDEX_BASE
-	cache_loop	t0, t1, t8, FILL
+	cache_loop	t0, t1, R_IC_LINE, FILL
 	/* invalidate again - prudent but not strictly neccessary */
 	PTR_LI		t0, INDEX_BASE
-	cache_loop	t0, t1, t8, INDEX_STORE_TAG_I
+	cache_loop	t0, t1, R_IC_LINE, INDEX_STORE_TAG_I
 #endif
 
 	/* Enable use of the I-cache by setting Config.K0 */
@@ -187,20 +191,20 @@ LEAF(mips_cache_reset)
 	/*
 	 * then initialize D-cache.
 	 */
-1:	blez		t3, 3f
+1:	blez		R_DC_SIZE, 3f
 	PTR_LI		t0, INDEX_BASE
-	PTR_ADDU	t1, t0, t3
+	PTR_ADDU	t1, t0, R_DC_SIZE
 	/* clear all tags */
-	cache_loop	t0, t1, t9, INDEX_STORE_TAG_D
+	cache_loop	t0, t1, R_DC_LINE, INDEX_STORE_TAG_D
 #ifdef CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD
 	/* load from each line (in cached space) */
 	PTR_LI		t0, INDEX_BASE
 2:	LONG_L		zero, 0(t0)
-	PTR_ADDU	t0, t9
+	PTR_ADDU	t0, R_DC_LINE
 	bne		t0, t1, 2b
 	/* clear all tags */
 	PTR_LI		t0, INDEX_BASE
-	cache_loop	t0, t1, t9, INDEX_STORE_TAG_D
+	cache_loop	t0, t1, R_DC_LINE, INDEX_STORE_TAG_D
 #endif
 
 3:	jr	ra
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 7/9] MIPS: Map CM Global Control Registers
  2016-09-07 17:48 [U-Boot] [PATCH 0/9] MIPS L2 cache support Paul Burton
                   ` (5 preceding siblings ...)
  2016-09-07 17:48 ` [U-Boot] [PATCH 6/9] MIPS: Define register names for cache init Paul Burton
@ 2016-09-07 17:48 ` Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 8/9] MIPS: L2 cache support Paul Burton
  2016-09-07 17:48 ` [U-Boot] [PATCH 9/9] MIPS: Malta: Enable CM & L2 support Paul Burton
  8 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-07 17:48 UTC (permalink / raw)
  To: u-boot

Map the Global Control Registers (GCRs) provided by the MIPS Coherence
Manager (CM) in preparation for using some of them in later patches.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/Kconfig          | 16 ++++++++++++++++
 arch/mips/cpu/Makefile     |  2 ++
 arch/mips/cpu/cm_init.S    | 45 +++++++++++++++++++++++++++++++++++++++++++++
 arch/mips/cpu/start.S      |  6 ++++++
 arch/mips/include/asm/cm.h | 19 +++++++++++++++++++
 5 files changed, 88 insertions(+)
 create mode 100644 arch/mips/cpu/cm_init.S
 create mode 100644 arch/mips/include/asm/cm.h

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 34eb404..732d129 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -307,6 +307,22 @@ config DYNAMIC_IO_PORT_BASE
 config MACH_CPU_INIT
 	bool
 
+config MIPS_CM
+	bool
+	help
+	  Select this if your system contains a MIPS Coherence Manager and you
+	  wish U-Boot to configure it or make use of it to retrieve system
+	  information such as cache configuration.
+
+config MIPS_CM_BASE
+	hex
+	default 0x1fbf8000
+	help
+	  The physical base address at which to map the MIPS Coherence Manager
+	  Global Configuration Registers (GCRs). This should be set such that
+	  the GCRs occupy a region of the physical address space which is
+	  otherwise unused, or at minimum that software doesn't need to access.
+
 endif
 
 endmenu
diff --git a/arch/mips/cpu/Makefile b/arch/mips/cpu/Makefile
index fc6b455..429fd3a 100644
--- a/arch/mips/cpu/Makefile
+++ b/arch/mips/cpu/Makefile
@@ -7,3 +7,5 @@ extra-y	= start.o
 obj-y += time.o
 obj-y += interrupts.o
 obj-y += cpu.o
+
+obj-$(CONFIG_MIPS_CM)	+= cm_init.o
diff --git a/arch/mips/cpu/cm_init.S b/arch/mips/cpu/cm_init.S
new file mode 100644
index 0000000..ddcaa49
--- /dev/null
+++ b/arch/mips/cpu/cm_init.S
@@ -0,0 +1,45 @@
+/*
+ * MIPS Coherence Manager (CM) Initialisation
+ *
+ * Copyright (c) 2016 Imagination Technologies Ltd.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <asm/addrspace.h>
+#include <asm/asm.h>
+#include <asm/cm.h>
+#include <asm/mipsregs.h>
+#include <asm/regdef.h>
+
+LEAF(mips_cm_map)
+	/* Config3 must exist for a CM to be present */
+	mfc0		t0, CP0_CONFIG, 1
+	bgez		t0, 2f
+	mfc0		t0, CP0_CONFIG, 2
+	bgez		t0, 2f
+
+	/* Check Config3.CMGCR to determine CM presence */
+	mfc0		t0, CP0_CONFIG, 3
+	and		t0, t0, MIPS_CONF3_CMGCR
+	beqz		t0, 2f
+
+	/* Find the current physical GCR base address */
+1:	MFC0		t0, CP0_CMGCRBASE
+	PTR_SLL		t0, t0, 4
+
+	/* If the GCRs are where we want, we're done */
+	PTR_LI		t1, CONFIG_MIPS_CM_BASE
+	beq		t0, t1, 2f
+
+	/* Move the GCRs to our configured base address */
+	PTR_LI		t2, CKSEG1
+	PTR_ADDU	t0, t0, t2
+	sw		zero, GCR_BASE_UPPER(t0)
+	sw		t1, GCR_BASE(t0)
+
+	/* Re-check the GCR base */
+	b		1b
+
+2:	jr		ra
+	END(mips_cm_map)
diff --git a/arch/mips/cpu/start.S b/arch/mips/cpu/start.S
index 6f1d219..c157d03 100644
--- a/arch/mips/cpu/start.S
+++ b/arch/mips/cpu/start.S
@@ -141,6 +141,12 @@ reset:
 1:
 	PTR_L	gp, 0(ra)
 
+#ifdef CONFIG_MIPS_CM
+	PTR_LA	t9, mips_cm_map
+	jalr	t9
+	 nop
+#endif
+
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
 # ifdef CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD
 	/* Initialize any external memory */
diff --git a/arch/mips/include/asm/cm.h b/arch/mips/include/asm/cm.h
new file mode 100644
index 0000000..0261733
--- /dev/null
+++ b/arch/mips/include/asm/cm.h
@@ -0,0 +1,19 @@
+/*
+ * MIPS Coherence Manager (CM) Register Definitions
+ *
+ * Copyright (c) 2016 Imagination Technologies Ltd.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#ifndef __MIPS_ASM_CM_H__
+#define __MIPS_ASM_CM_H__
+
+/* Global Control Register (GCR) offsets */
+#define GCR_BASE			0x0008
+#define GCR_BASE_UPPER			0x000c
+#define GCR_REV				0x0030
+
+/* GCR_REV CM versions */
+#define GCR_REV_CM3			0x0800
+
+#endif /* __MIPS_ASM_CM_H__ */
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 8/9] MIPS: L2 cache support
  2016-09-07 17:48 [U-Boot] [PATCH 0/9] MIPS L2 cache support Paul Burton
                   ` (6 preceding siblings ...)
  2016-09-07 17:48 ` [U-Boot] [PATCH 7/9] MIPS: Map CM Global Control Registers Paul Burton
@ 2016-09-07 17:48 ` Paul Burton
  2016-09-08 10:04   ` Marek Vasut
  2016-09-07 17:48 ` [U-Boot] [PATCH 9/9] MIPS: Malta: Enable CM & L2 support Paul Burton
  8 siblings, 1 reply; 20+ messages in thread
From: Paul Burton @ 2016-09-07 17:48 UTC (permalink / raw)
  To: u-boot

This patch adds support for initialising & maintaining L2 caches on MIPS
systems. The L2 cache configuration may be advertised through either
coprocessor 0 or the MIPS Coherence Manager depending upon the system,
and support for both is included.

If the L2 can be bypassed then we bypass it early in boot & initialise
the L1 caches first, such that we can start making use of the L1
instruction cache as early as possible. Otherwise we initialise the L2
first such that the L1s have no opportunity to generate access to the
uninitialised L2.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/Kconfig                   |   6 ++
 arch/mips/include/asm/cm.h          |  38 ++++++++
 arch/mips/include/asm/global_data.h |   3 +
 arch/mips/include/asm/mipsregs.h    |   5 +
 arch/mips/lib/cache.c               |  62 ++++++++++++-
 arch/mips/lib/cache_init.S          | 180 +++++++++++++++++++++++++++++++++++-
 6 files changed, 288 insertions(+), 6 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 732d129..8f97727 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -301,6 +301,12 @@ config MIPS_L1_CACHE_SHIFT
 	default "4" if MIPS_L1_CACHE_SHIFT_4
 	default "5"
 
+config MIPS_L2_CACHE
+	bool
+	help
+	  Select this if your system includes an L2 cache and you want U-Boot
+	  to initialise & maintain it.
+
 config DYNAMIC_IO_PORT_BASE
 	bool
 
diff --git a/arch/mips/include/asm/cm.h b/arch/mips/include/asm/cm.h
index 0261733..62ecef2 100644
--- a/arch/mips/include/asm/cm.h
+++ b/arch/mips/include/asm/cm.h
@@ -12,8 +12,46 @@
 #define GCR_BASE			0x0008
 #define GCR_BASE_UPPER			0x000c
 #define GCR_REV				0x0030
+#define GCR_L2_CONFIG			0x0130
+#define GCR_L2_TAG_ADDR			0x0600
+#define GCR_L2_TAG_ADDR_UPPER		0x0604
+#define GCR_L2_TAG_STATE		0x0608
+#define GCR_L2_TAG_STATE_UPPER		0x060c
+#define GCR_L2_DATA			0x0610
+#define GCR_L2_DATA_UPPER		0x0614
 
 /* GCR_REV CM versions */
 #define GCR_REV_CM3			0x0800
 
+/* GCR_L2_CONFIG fields */
+#define GCR_L2_CONFIG_ASSOC_SHIFT	0
+#define GCR_L2_CONFIG_ASSOC_BITS	8
+#define GCR_L2_CONFIG_LINESZ_SHIFT	8
+#define GCR_L2_CONFIG_LINESZ_BITS	4
+#define GCR_L2_CONFIG_SETSZ_SHIFT	12
+#define GCR_L2_CONFIG_SETSZ_BITS	4
+#define GCR_L2_CONFIG_BYPASS		(1 << 20)
+
+#ifndef __ASSEMBLY__
+
+#include <asm/io.h>
+
+static inline void *mips_cm_base(void)
+{
+	return (void *)CKSEG1ADDR(CONFIG_MIPS_CM_BASE);
+}
+
+static inline unsigned long mips_cm_l2_line_size(void)
+{
+	unsigned long l2conf, line_sz;
+
+	l2conf = __raw_readl(mips_cm_base() + GCR_L2_CONFIG);
+
+	line_sz = l2conf >> GCR_L2_CONFIG_LINESZ_SHIFT;
+	line_sz &= GENMASK(GCR_L2_CONFIG_LINESZ_BITS - 1, 0);
+	return line_sz ? (2 << line_sz) : 0;
+}
+
+#endif /* !__ASSEMBLY__ */
+
 #endif /* __MIPS_ASM_CM_H__ */
diff --git a/arch/mips/include/asm/global_data.h b/arch/mips/include/asm/global_data.h
index 8533b69..0078bbe 100644
--- a/arch/mips/include/asm/global_data.h
+++ b/arch/mips/include/asm/global_data.h
@@ -25,6 +25,9 @@ struct arch_global_data {
 	unsigned short l1i_line_size;
 	unsigned short l1d_line_size;
 #endif
+#ifdef CONFIG_MIPS_L2_CACHE
+	unsigned short l2_line_size;
+#endif
 };
 
 #include <asm-generic/global_data.h>
diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index cd4f952..b4c2dff 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -485,9 +485,13 @@
 #define MIPS_CONF1_TLBS_SIZE    (6)
 #define MIPS_CONF1_TLBS         (_ULCAST_(63) << MIPS_CONF1_TLBS_SHIFT)
 
+#define MIPS_CONF2_SA_SHF	0
 #define MIPS_CONF2_SA		(_ULCAST_(15) << 0)
+#define MIPS_CONF2_SL_SHF	4
 #define MIPS_CONF2_SL		(_ULCAST_(15) << 4)
+#define MIPS_CONF2_SS_SHF	8
 #define MIPS_CONF2_SS		(_ULCAST_(15) << 8)
+#define MIPS_CONF2_L2B		(_ULCAST_(1) << 12)
 #define MIPS_CONF2_SU		(_ULCAST_(15) << 12)
 #define MIPS_CONF2_TA		(_ULCAST_(15) << 16)
 #define MIPS_CONF2_TL		(_ULCAST_(15) << 20)
@@ -551,6 +555,7 @@
 #define MIPS_CONF5_MVH		(_ULCAST_(1) << 5)
 #define MIPS_CONF5_FRE		(_ULCAST_(1) << 8)
 #define MIPS_CONF5_UFE		(_ULCAST_(1) << 9)
+#define MIPS_CONF5_L2C		(_ULCAST_(1) << 10)
 #define MIPS_CONF5_MSAEN	(_ULCAST_(1) << 27)
 #define MIPS_CONF5_EVA		(_ULCAST_(1) << 28)
 #define MIPS_CONF5_CV		(_ULCAST_(1) << 29)
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c
index d8baf08..bd14ba6 100644
--- a/arch/mips/lib/cache.c
+++ b/arch/mips/lib/cache.c
@@ -7,10 +7,44 @@
 
 #include <common.h>
 #include <asm/cacheops.h>
+#include <asm/cm.h>
 #include <asm/mipsregs.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static void probe_l2(void)
+{
+#ifdef CONFIG_MIPS_L2_CACHE
+	unsigned long conf2, sl;
+	bool l2c = false;
+
+	if (!(read_c0_config1() & MIPS_CONF_M))
+		return;
+
+	conf2 = read_c0_config2();
+
+	if (__mips_isa_rev >= 6) {
+		l2c = conf2 & MIPS_CONF_M;
+		if (l2c)
+			l2c = read_c0_config3() & MIPS_CONF_M;
+		if (l2c)
+			l2c = read_c0_config4() & MIPS_CONF_M;
+		if (l2c)
+			l2c = read_c0_config5() & MIPS_CONF5_L2C;
+	}
+
+	if (l2c && config_enabled(CONFIG_MIPS_CM)) {
+		gd->arch.l2_line_size = mips_cm_l2_line_size();
+	} else if (l2c) {
+		/* We don't know how to retrieve L2 config on this system */
+		BUG();
+	} else {
+		sl = (conf2 & MIPS_CONF2_SL) >> MIPS_CONF2_SL_SHF;
+		gd->arch.l2_line_size = sl ? (2 << sl) : 0;
+	}
+#endif
+}
+
 void mips_cache_probe(void)
 {
 #ifdef CONFIG_SYS_CACHE_SIZE_AUTO
@@ -24,6 +58,7 @@ void mips_cache_probe(void)
 	gd->arch.l1i_line_size = il ? (2 << il) : 0;
 	gd->arch.l1d_line_size = dl ? (2 << dl) : 0;
 #endif
+	probe_l2();
 }
 
 static inline unsigned long icache_line_size(void)
@@ -44,6 +79,15 @@ static inline unsigned long dcache_line_size(void)
 #endif
 }
 
+static inline unsigned long scache_line_size(void)
+{
+#ifdef CONFIG_MIPS_L2_CACHE
+	return gd->arch.l2_line_size;
+#else
+	return 0;
+#endif
+}
+
 #define cache_loop(start, end, lsize, ops...) do {			\
 	const void *addr = (const void *)(start & ~(lsize - 1));	\
 	const void *aend = (const void *)((end - 1) & ~(lsize - 1));	\
@@ -60,12 +104,13 @@ void flush_cache(ulong start_addr, ulong size)
 {
 	unsigned long ilsize = icache_line_size();
 	unsigned long dlsize = dcache_line_size();
+	unsigned long slsize = scache_line_size();
 
 	/* aend will be miscalculated when size is zero, so we return here */
 	if (size == 0)
 		return;
 
-	if (ilsize == dlsize) {
+	if ((ilsize == dlsize) && !slsize) {
 		/* flush I-cache & D-cache simultaneously */
 		cache_loop(start_addr, start_addr + size, ilsize,
 			   HIT_WRITEBACK_INV_D, HIT_INVALIDATE_I);
@@ -75,6 +120,11 @@ void flush_cache(ulong start_addr, ulong size)
 	/* flush D-cache */
 	cache_loop(start_addr, start_addr + size, dlsize, HIT_WRITEBACK_INV_D);
 
+	/* flush L2 cache */
+	if (slsize)
+		cache_loop(start_addr, start_addr + size, slsize,
+			   HIT_WRITEBACK_INV_SD);
+
 	/* flush I-cache */
 	cache_loop(start_addr, start_addr + size, ilsize, HIT_INVALIDATE_I);
 }
@@ -82,21 +132,31 @@ void flush_cache(ulong start_addr, ulong size)
 void flush_dcache_range(ulong start_addr, ulong stop)
 {
 	unsigned long lsize = dcache_line_size();
+	unsigned long slsize = scache_line_size();
 
 	/* aend will be miscalculated when size is zero, so we return here */
 	if (start_addr == stop)
 		return;
 
 	cache_loop(start_addr, stop, lsize, HIT_WRITEBACK_INV_D);
+
+	/* flush L2 cache */
+	if (slsize)
+		cache_loop(start_addr, stop, slsize, HIT_WRITEBACK_INV_SD);
 }
 
 void invalidate_dcache_range(ulong start_addr, ulong stop)
 {
 	unsigned long lsize = dcache_line_size();
+	unsigned long slsize = scache_line_size();
 
 	/* aend will be miscalculated when size is zero, so we return here */
 	if (start_addr == stop)
 		return;
 
+	/* invalidate L2 cache */
+	if (slsize)
+		cache_loop(start_addr, stop, slsize, HIT_INVALIDATE_SD);
+
 	cache_loop(start_addr, stop, lsize, HIT_INVALIDATE_D);
 }
diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S
index 2df3a82..599a85f 100644
--- a/arch/mips/lib/cache_init.S
+++ b/arch/mips/lib/cache_init.S
@@ -13,6 +13,7 @@
 #include <asm/mipsregs.h>
 #include <asm/addrspace.h>
 #include <asm/cacheops.h>
+#include <asm/cm.h>
 
 #ifndef CONFIG_SYS_MIPS_CACHE_MODE
 #define CONFIG_SYS_MIPS_CACHE_MODE CONF_CM_CACHABLE_NONCOHERENT
@@ -95,14 +96,135 @@
  * with good parity is available. This routine will initialise an area of
  * memory starting at location zero to be used as a source of parity.
  *
+ * Note that this function does not follow the standard calling convention &
+ * may clobber typically callee-saved registers.
+ *
  * RETURNS: N/A
  *
  */
-#define R_IC_SIZE	t2
-#define R_IC_LINE	t8
-#define R_DC_SIZE	t3
-#define R_DC_LINE	t9
+#define R_RETURN	s0
+#define R_IC_SIZE	s1
+#define R_IC_LINE	s2
+#define R_DC_SIZE	s3
+#define R_DC_LINE	s4
+#define R_L2_SIZE	s5
+#define R_L2_LINE	s6
+#define R_L2_BYPASSED	s7
+#define R_L2_L2C	t8
 LEAF(mips_cache_reset)
+	move	R_RETURN, ra
+
+#ifdef CONFIG_MIPS_L2_CACHE
+	/*
+	 * For there to be an L2 present, Config2 must be present. If it isn't
+	 * then we proceed knowing there's no L2 cache.
+	 */
+	move	R_L2_SIZE, zero
+	move	R_L2_LINE, zero
+	move	R_L2_BYPASSED, zero
+	move	R_L2_L2C, zero
+	mfc0	t0, CP0_CONFIG, 1
+	bgez	t0, l2_probe_done
+
+	/*
+	 * From MIPSr6 onwards the L2 cache configuration might not be reported
+	 * by Config2. The Config5.L2C bit indicates whether this is the case,
+	 * and if it is then we need knowledge of where else to look. For cores
+	 * from Imagination Technologies this is a CM GCR.
+	 */
+# if __mips_isa_rev >= 6
+	/* Check that Config5 exists */
+	mfc0	t0, CP0_CONFIG, 2
+	bgez	t0, l2_probe_cop0
+	mfc0	t0, CP0_CONFIG, 3
+	bgez	t0, l2_probe_cop0
+	mfc0	t0, CP0_CONFIG, 4
+	bgez	t0, l2_probe_cop0
+
+	/* Check Config5.L2C is set */
+	mfc0	t0, CP0_CONFIG, 5
+	and	R_L2_L2C, t0, MIPS_CONF5_L2C
+	beqz	R_L2_L2C, l2_probe_cop0
+
+	/* Config5.L2C is set */
+#  ifdef CONFIG_MIPS_CM
+	/* The CM will provide L2 configuration */
+	PTR_LI	t0, CKSEG1ADDR(CONFIG_MIPS_CM_BASE)
+	lw	t1, GCR_L2_CONFIG(t0)
+	bgez	t1, l2_probe_done
+
+	ext	R_L2_LINE, t1, \
+		GCR_L2_CONFIG_LINESZ_SHIFT, GCR_L2_CONFIG_LINESZ_BITS
+	beqz	R_L2_LINE, l2_probe_done
+	li	t2, 2
+	sllv	R_L2_LINE, t2, R_L2_LINE
+
+	ext	t2, t1, GCR_L2_CONFIG_ASSOC_SHIFT, GCR_L2_CONFIG_ASSOC_BITS
+	addiu	t2, t2, 1
+	mul	R_L2_SIZE, R_L2_LINE, t2
+
+	ext	t2, t1, GCR_L2_CONFIG_SETSZ_SHIFT, GCR_L2_CONFIG_SETSZ_BITS
+	sllv	R_L2_SIZE, R_L2_SIZE, t2
+	li	t2, 64
+	mul	R_L2_SIZE, R_L2_SIZE, t2
+
+	/* Bypass the L2 cache so that we can init the L1s early */
+	or	t1, t1, GCR_L2_CONFIG_BYPASS
+	sw	t1, GCR_L2_CONFIG(t0)
+	sync
+	li	R_L2_BYPASSED, 1
+
+	/* Zero the L2 tag registers */
+	sw	zero, GCR_L2_TAG_ADDR(t0)
+	sw	zero, GCR_L2_TAG_ADDR_UPPER(t0)
+	sw	zero, GCR_L2_TAG_STATE(t0)
+	sw	zero, GCR_L2_TAG_STATE_UPPER(t0)
+	sw	zero, GCR_L2_DATA(t0)
+	sw	zero, GCR_L2_DATA_UPPER(t0)
+	sync
+#  else
+	/* We don't know how to retrieve L2 configuration on this system */
+#  endif
+	b	l2_probe_done
+# endif
+
+	/*
+	 * For pre-r6 systems, or r6 systems with Config5.L2C==0, probe the L2
+	 * cache configuration from the cop0 Config2 register.
+	 */
+l2_probe_cop0:
+	mfc0	t0, CP0_CONFIG, 2
+
+	srl	R_L2_LINE, t0, MIPS_CONF2_SL_SHF
+	andi	R_L2_LINE, R_L2_LINE, MIPS_CONF2_SL >> MIPS_CONF2_SL_SHF
+	beqz	R_L2_LINE, l2_probe_done
+	li	t1, 2
+	sllv	R_L2_LINE, t1, R_L2_LINE
+
+	srl	t1, t0, MIPS_CONF2_SA_SHF
+	andi	t1, t1, MIPS_CONF2_SA >> MIPS_CONF2_SA_SHF
+	addiu	t1, t1, 1
+	mul	R_L2_SIZE, R_L2_LINE, t1
+
+	srl	t1, t0, MIPS_CONF2_SS_SHF
+	andi	t1, t1, MIPS_CONF2_SS >> MIPS_CONF2_SS_SHF
+	sllv	R_L2_SIZE, R_L2_SIZE, t1
+	li	t1, 64
+	mul	R_L2_SIZE, R_L2_SIZE, t1
+
+	/* Attempt to bypass the L2 so that we can init the L1s early */
+	or	t0, t0, MIPS_CONF2_L2B
+	mtc0	t0, CP0_CONFIG, 2
+	ehb
+	mfc0	t0, CP0_CONFIG, 2
+	and	R_L2_BYPASSED, t0, MIPS_CONF2_L2B
+
+	/* Zero the L2 tag registers */
+	mtc0	zero, CP0_TAGLO, 4
+	ehb
+l2_probe_done:
+#endif
+
 #ifndef CONFIG_SYS_CACHE_SIZE_AUTO
 	li	R_IC_SIZE, CONFIG_SYS_ICACHE_SIZE
 	li	R_IC_LINE, CONFIG_SYS_ICACHE_LINE_SIZE
@@ -142,11 +264,33 @@ LEAF(mips_cache_reset)
 
 #endif /* CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD */
 
+#ifdef CONFIG_MIPS_L2_CACHE
+	/*
+	 * If the L2 is bypassed, init the L1 first so that we can execute the
+	 * rest of the cache initialisation using the L1 instruction cache.
+	 */
+	bnez		R_L2_BYPASSED, l1_init
+
+l2_init:
+	PTR_LI		t0, INDEX_BASE
+	PTR_ADDU	t1, t0, R_L2_SIZE
+1:	cache		INDEX_STORE_TAG_SD, 0(t0)
+	PTR_ADDU	t0, t0, R_L2_LINE
+	bne		t0, t1, 1b
+
+	/*
+	 * If the L2 was bypassed then we already initialised the L1s before
+	 * the L2, so we are now done.
+	 */
+	bnez		R_L2_BYPASSED, return
+#endif
+
 	/*
 	 * The TagLo registers used depend upon the CPU implementation, but the
 	 * architecture requires that it is safe for software to write to both
 	 * TagLo selects 0 & 2 covering supported cases.
 	 */
+l1_init:
 	mtc0		zero, CP0_TAGLO
 	mtc0		zero, CP0_TAGLO, 2
 
@@ -206,8 +350,34 @@ LEAF(mips_cache_reset)
 	PTR_LI		t0, INDEX_BASE
 	cache_loop	t0, t1, R_DC_LINE, INDEX_STORE_TAG_D
 #endif
+3:
+
+#ifdef CONFIG_MIPS_L2_CACHE
+	/* If the L2 isn't bypassed then we're done */
+	beqz		R_L2_BYPASSED, return
 
-3:	jr	ra
+	/* The L2 is bypassed - un-bypass it then initialise it */
+# if __mips_isa_rev >= 6
+	beqz		R_L2_L2C, 1f
+
+	li		t0, CKSEG1ADDR(CONFIG_MIPS_CM_BASE)
+	lw		t1, GCR_L2_CONFIG(t0)
+	xor		t1, t1, GCR_L2_CONFIG_BYPASS
+	sw		t1, GCR_L2_CONFIG(t0)
+	sync
+	ehb
+	b		l2_init
+# endif
+1:
+	mfc0		t0, CP0_CONFIG, 2
+	xor		t0, t0, MIPS_CONF2_L2B
+	mtc0		t0, CP0_CONFIG, 2
+	ehb
+	b		l2_init
+#endif
+
+return:
+	jr	ra
 	END(mips_cache_reset)
 
 /*
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 9/9] MIPS: Malta: Enable CM & L2 support
  2016-09-07 17:48 [U-Boot] [PATCH 0/9] MIPS L2 cache support Paul Burton
                   ` (7 preceding siblings ...)
  2016-09-07 17:48 ` [U-Boot] [PATCH 8/9] MIPS: L2 cache support Paul Burton
@ 2016-09-07 17:48 ` Paul Burton
  8 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-07 17:48 UTC (permalink / raw)
  To: u-boot

Enable support for the MIPS Coherence Manager & L2 caches on the MIPS
Malta board, removing the need for us to attempt to bypass the L2 during
boot (which would fail with recent CPUs that expose L2 config via the CM
anyway).

Signed-off-by: Paul Burton <paul.burton@imgtec.com>

---

 arch/mips/Kconfig                  | 2 ++
 board/imgtec/malta/lowlevel_init.S | 6 ------
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 8f97727..4edb258 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -26,6 +26,8 @@ config TARGET_MALTA
 	select DM
 	select DM_SERIAL
 	select DYNAMIC_IO_PORT_BASE
+	select MIPS_CM
+	select MIPS_L2_CACHE
 	select OF_CONTROL
 	select OF_ISA_BUS
 	select SUPPORTS_BIG_ENDIAN
diff --git a/board/imgtec/malta/lowlevel_init.S b/board/imgtec/malta/lowlevel_init.S
index 3d48cdc..6df4d9f 100644
--- a/board/imgtec/malta/lowlevel_init.S
+++ b/board/imgtec/malta/lowlevel_init.S
@@ -28,12 +28,6 @@
 
 	.globl	lowlevel_init
 lowlevel_init:
-	/* disable any L2 cache for now */
-	sync
-	mfc0	t0, CP0_CONFIG, 2
-	ori	t0, t0, 0x1 << 12
-	mtc0	t0, CP0_CONFIG, 2
-
 	/* detect the core card */
 	PTR_LI	t0, CKSEG1ADDR(MALTA_REVISION)
 	lw	t0, 0(t0)
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init
  2016-09-07 17:48 ` [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init Paul Burton
@ 2016-09-08 10:02   ` Marek Vasut
  2016-09-08 10:36     ` Paul Burton
  0 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2016-09-08 10:02 UTC (permalink / raw)
  To: u-boot

On 09/07/2016 07:48 PM, Paul Burton wrote:
> Add an implementation of arch_cpu_init that covers all MIPS boards, in
> preparation for performing various pieces of initialisation there in
> later patches.
> 
> In order to allow the ath79 code to continue performing initialisation
> at this point in the boot, it's converted to use a new mach_cpu_init
> function which is called from arch_cpu_init.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>

[...]

Hi!

> diff --git a/arch/mips/include/asm/u-boot-mips.h b/arch/mips/include/asm/u-boot-mips.h
> index 1f527bb..0eaf32b 100644
> --- a/arch/mips/include/asm/u-boot-mips.h
> +++ b/arch/mips/include/asm/u-boot-mips.h
> @@ -5,4 +5,16 @@
>  #ifndef _U_BOOT_MIPS_H_
>  #define _U_BOOT_MIPS_H_
>  
> +/**
> + * mach_cpu_init() - Perform SoC/machine-specific CPU initialisation
> + *
> + * This is called from arch_cpu_init() to allow for SoC/machine-level code to
> + * perform any CPU initialisation it may require.

Just call this function from arch_cpu_init() in various places instead
of replacing arch_cpu_init() with it all over the place. Also rename it
to arch_cpu_init_common() to make it more obvious what this does .

> + */
> +#ifdef CONFIG_MACH_CPU_INIT
> +void mach_cpu_init(void);
> +#else
> +static inline void mach_cpu_init(void) {}

Implement this as __weak int and you can nuke the ifdefery .

> +#endif
> +
>  #endif /* _U_BOOT_MIPS_H_ */
> diff --git a/arch/mips/mach-ath79/cpu.c b/arch/mips/mach-ath79/cpu.c
> index 5756a06..a556b73 100644
> --- a/arch/mips/mach-ath79/cpu.c
> +++ b/arch/mips/mach-ath79/cpu.c
> @@ -46,7 +46,7 @@ static const struct ath79_soc_desc desc[] = {
>  	{ATH79_SOC_QCA9561,     "9561", REV_ID_MAJOR_QCA9561,   0},
>  };
>  
> -int arch_cpu_init(void)
> +void mach_cpu_init(void)

See above.

>  {
>  	void __iomem *base;
>  	enum ath79_soc_type soc = ATH79_SOC_UNKNOWN;
> @@ -98,7 +98,6 @@ int arch_cpu_init(void)
>  	gd->arch.soc = soc;
>  	gd->arch.rev = rev;
>  	gd->arch.ver = ver;
> -	return 0;

That's a nope, keep the return value.

>  }
>  
>  int print_cpuinfo(void)
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 8/9] MIPS: L2 cache support
  2016-09-07 17:48 ` [U-Boot] [PATCH 8/9] MIPS: L2 cache support Paul Burton
@ 2016-09-08 10:04   ` Marek Vasut
  2016-09-08 10:31     ` Paul Burton
  0 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2016-09-08 10:04 UTC (permalink / raw)
  To: u-boot

On 09/07/2016 07:48 PM, Paul Burton wrote:
> This patch adds support for initialising & maintaining L2 caches on MIPS
> systems. The L2 cache configuration may be advertised through either
> coprocessor 0 or the MIPS Coherence Manager depending upon the system,
> and support for both is included.
> 
> If the L2 can be bypassed then we bypass it early in boot & initialise
> the L1 caches first, such that we can start making use of the L1
> instruction cache as early as possible. Otherwise we initialise the L2
> first such that the L1s have no opportunity to generate access to the
> uninitialised L2.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---
> 
>  arch/mips/Kconfig                   |   6 ++
>  arch/mips/include/asm/cm.h          |  38 ++++++++
>  arch/mips/include/asm/global_data.h |   3 +
>  arch/mips/include/asm/mipsregs.h    |   5 +
>  arch/mips/lib/cache.c               |  62 ++++++++++++-
>  arch/mips/lib/cache_init.S          | 180 +++++++++++++++++++++++++++++++++++-
>  6 files changed, 288 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 732d129..8f97727 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -301,6 +301,12 @@ config MIPS_L1_CACHE_SHIFT
>  	default "4" if MIPS_L1_CACHE_SHIFT_4
>  	default "5"
>  
> +config MIPS_L2_CACHE
> +	bool
> +	help
> +	  Select this if your system includes an L2 cache and you want U-Boot
> +	  to initialise & maintain it.
> +
>  config DYNAMIC_IO_PORT_BASE
>  	bool
>  
> diff --git a/arch/mips/include/asm/cm.h b/arch/mips/include/asm/cm.h
> index 0261733..62ecef2 100644
> --- a/arch/mips/include/asm/cm.h
> +++ b/arch/mips/include/asm/cm.h
> @@ -12,8 +12,46 @@
>  #define GCR_BASE			0x0008
>  #define GCR_BASE_UPPER			0x000c
>  #define GCR_REV				0x0030
> +#define GCR_L2_CONFIG			0x0130
> +#define GCR_L2_TAG_ADDR			0x0600
> +#define GCR_L2_TAG_ADDR_UPPER		0x0604
> +#define GCR_L2_TAG_STATE		0x0608
> +#define GCR_L2_TAG_STATE_UPPER		0x060c
> +#define GCR_L2_DATA			0x0610
> +#define GCR_L2_DATA_UPPER		0x0614
>  
>  /* GCR_REV CM versions */
>  #define GCR_REV_CM3			0x0800
>  
> +/* GCR_L2_CONFIG fields */
> +#define GCR_L2_CONFIG_ASSOC_SHIFT	0
> +#define GCR_L2_CONFIG_ASSOC_BITS	8
> +#define GCR_L2_CONFIG_LINESZ_SHIFT	8
> +#define GCR_L2_CONFIG_LINESZ_BITS	4
> +#define GCR_L2_CONFIG_SETSZ_SHIFT	12
> +#define GCR_L2_CONFIG_SETSZ_BITS	4
> +#define GCR_L2_CONFIG_BYPASS		(1 << 20)
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <asm/io.h>
> +
> +static inline void *mips_cm_base(void)
> +{
> +	return (void *)CKSEG1ADDR(CONFIG_MIPS_CM_BASE);
> +}
> +
> +static inline unsigned long mips_cm_l2_line_size(void)
> +{
> +	unsigned long l2conf, line_sz;
> +
> +	l2conf = __raw_readl(mips_cm_base() + GCR_L2_CONFIG);
> +
> +	line_sz = l2conf >> GCR_L2_CONFIG_LINESZ_SHIFT;
> +	line_sz &= GENMASK(GCR_L2_CONFIG_LINESZ_BITS - 1, 0);
> +	return line_sz ? (2 << line_sz) : 0;
> +}

Drop the inline stuff, the compiler can decide that itself, especially
on static functions. The inline is only a hint anyway.

[...]


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 8/9] MIPS: L2 cache support
  2016-09-08 10:04   ` Marek Vasut
@ 2016-09-08 10:31     ` Paul Burton
  2016-09-08 10:39       ` Paul Burton
  2016-09-08 10:40       ` Marek Vasut
  0 siblings, 2 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-08 10:31 UTC (permalink / raw)
  To: u-boot

On 08/09/16 11:04, Marek Vasut wrote:
> On 09/07/2016 07:48 PM, Paul Burton wrote:
>> This patch adds support for initialising & maintaining L2 caches on MIPS
>> systems. The L2 cache configuration may be advertised through either
>> coprocessor 0 or the MIPS Coherence Manager depending upon the system,
>> and support for both is included.
>>
>> If the L2 can be bypassed then we bypass it early in boot & initialise
>> the L1 caches first, such that we can start making use of the L1
>> instruction cache as early as possible. Otherwise we initialise the L2
>> first such that the L1s have no opportunity to generate access to the
>> uninitialised L2.
>>
>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>> ---
>>
>>  arch/mips/Kconfig                   |   6 ++
>>  arch/mips/include/asm/cm.h          |  38 ++++++++
>>  arch/mips/include/asm/global_data.h |   3 +
>>  arch/mips/include/asm/mipsregs.h    |   5 +
>>  arch/mips/lib/cache.c               |  62 ++++++++++++-
>>  arch/mips/lib/cache_init.S          | 180 +++++++++++++++++++++++++++++++++++-
>>  6 files changed, 288 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>> index 732d129..8f97727 100644
>> --- a/arch/mips/Kconfig
>> +++ b/arch/mips/Kconfig
>> @@ -301,6 +301,12 @@ config MIPS_L1_CACHE_SHIFT
>>  	default "4" if MIPS_L1_CACHE_SHIFT_4
>>  	default "5"
>>  
>> +config MIPS_L2_CACHE
>> +	bool
>> +	help
>> +	  Select this if your system includes an L2 cache and you want U-Boot
>> +	  to initialise & maintain it.
>> +
>>  config DYNAMIC_IO_PORT_BASE
>>  	bool
>>  
>> diff --git a/arch/mips/include/asm/cm.h b/arch/mips/include/asm/cm.h
>> index 0261733..62ecef2 100644
>> --- a/arch/mips/include/asm/cm.h
>> +++ b/arch/mips/include/asm/cm.h
>> @@ -12,8 +12,46 @@
>>  #define GCR_BASE			0x0008
>>  #define GCR_BASE_UPPER			0x000c
>>  #define GCR_REV				0x0030
>> +#define GCR_L2_CONFIG			0x0130
>> +#define GCR_L2_TAG_ADDR			0x0600
>> +#define GCR_L2_TAG_ADDR_UPPER		0x0604
>> +#define GCR_L2_TAG_STATE		0x0608
>> +#define GCR_L2_TAG_STATE_UPPER		0x060c
>> +#define GCR_L2_DATA			0x0610
>> +#define GCR_L2_DATA_UPPER		0x0614
>>  
>>  /* GCR_REV CM versions */
>>  #define GCR_REV_CM3			0x0800
>>  
>> +/* GCR_L2_CONFIG fields */
>> +#define GCR_L2_CONFIG_ASSOC_SHIFT	0
>> +#define GCR_L2_CONFIG_ASSOC_BITS	8
>> +#define GCR_L2_CONFIG_LINESZ_SHIFT	8
>> +#define GCR_L2_CONFIG_LINESZ_BITS	4
>> +#define GCR_L2_CONFIG_SETSZ_SHIFT	12
>> +#define GCR_L2_CONFIG_SETSZ_BITS	4
>> +#define GCR_L2_CONFIG_BYPASS		(1 << 20)
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +#include <asm/io.h>
>> +
>> +static inline void *mips_cm_base(void)
>> +{
>> +	return (void *)CKSEG1ADDR(CONFIG_MIPS_CM_BASE);
>> +}
>> +
>> +static inline unsigned long mips_cm_l2_line_size(void)
>> +{
>> +	unsigned long l2conf, line_sz;
>> +
>> +	l2conf = __raw_readl(mips_cm_base() + GCR_L2_CONFIG);
>> +
>> +	line_sz = l2conf >> GCR_L2_CONFIG_LINESZ_SHIFT;
>> +	line_sz &= GENMASK(GCR_L2_CONFIG_LINESZ_BITS - 1, 0);
>> +	return line_sz ? (2 << line_sz) : 0;
>> +}
> 
> Drop the inline stuff, the compiler can decide that itself, especially
> on static functions. The inline is only a hint anyway.
> 
> [...]

Hi Marek,

Nope - inline does have an impact for functions in headers. If it's not
there & a file includes a header but doesn't use *all* of the functions
in it then the compiler will warn about the functions being unused. With
inline that isn't the case. Thus "static inline" is used quite
extensively in many of the headers under include/.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init
  2016-09-08 10:02   ` Marek Vasut
@ 2016-09-08 10:36     ` Paul Burton
  2016-09-08 10:47       ` Marek Vasut
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Burton @ 2016-09-08 10:36 UTC (permalink / raw)
  To: u-boot

On 08/09/16 11:02, Marek Vasut wrote:
> On 09/07/2016 07:48 PM, Paul Burton wrote:
>> Add an implementation of arch_cpu_init that covers all MIPS boards, in
>> preparation for performing various pieces of initialisation there in
>> later patches.
>>
>> In order to allow the ath79 code to continue performing initialisation
>> at this point in the boot, it's converted to use a new mach_cpu_init
>> function which is called from arch_cpu_init.
>>
>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> 
> [...]
> 
> Hi!
> 
>> diff --git a/arch/mips/include/asm/u-boot-mips.h b/arch/mips/include/asm/u-boot-mips.h
>> index 1f527bb..0eaf32b 100644
>> --- a/arch/mips/include/asm/u-boot-mips.h
>> +++ b/arch/mips/include/asm/u-boot-mips.h
>> @@ -5,4 +5,16 @@
>>  #ifndef _U_BOOT_MIPS_H_
>>  #define _U_BOOT_MIPS_H_
>>  
>> +/**
>> + * mach_cpu_init() - Perform SoC/machine-specific CPU initialisation
>> + *
>> + * This is called from arch_cpu_init() to allow for SoC/machine-level code to
>> + * perform any CPU initialisation it may require.
> 
> Just call this function from arch_cpu_init() in various places instead
> of replacing arch_cpu_init() with it all over the place. Also rename it
> to arch_cpu_init_common() to make it more obvious what this does .

Hi Marek,

That's "backwards" compared with what this code actually does -
arch_cpu_init becomes the common function (ie. is arch-wide as in the
patch subject) and mach_cpu_init is for use by machines/SoCs - ie. code
under arch/mips/mach-*.

>> + */
>> +#ifdef CONFIG_MACH_CPU_INIT
>> +void mach_cpu_init(void);
>> +#else
>> +static inline void mach_cpu_init(void) {}
> 
> Implement this as __weak int and you can nuke the ifdefery .

I could make it weak, but then we don't let the compiler optimise it out
entirely for builds that don't need it (ie. everything except ath79).

I'd say that since the #ifdefery here is confined to this one case in
the header (which is a pretty common approach) it's ugliness is kept
minimal & its cost on binaries & runtime is as low as it can be at zero.

Daniel - do you have a preference?

> 
>> +#endif
>> +
>>  #endif /* _U_BOOT_MIPS_H_ */
>> diff --git a/arch/mips/mach-ath79/cpu.c b/arch/mips/mach-ath79/cpu.c
>> index 5756a06..a556b73 100644
>> --- a/arch/mips/mach-ath79/cpu.c
>> +++ b/arch/mips/mach-ath79/cpu.c
>> @@ -46,7 +46,7 @@ static const struct ath79_soc_desc desc[] = {
>>  	{ATH79_SOC_QCA9561,     "9561", REV_ID_MAJOR_QCA9561,   0},
>>  };
>>  
>> -int arch_cpu_init(void)
>> +void mach_cpu_init(void)
> 
> See above.
> 
>>  {
>>  	void __iomem *base;
>>  	enum ath79_soc_type soc = ATH79_SOC_UNKNOWN;
>> @@ -98,7 +98,6 @@ int arch_cpu_init(void)
>>  	gd->arch.soc = soc;
>>  	gd->arch.rev = rev;
>>  	gd->arch.ver = ver;
>> -	return 0;
> 
> That's a nope, keep the return value.

The implementation always returns zero, so I see no point.

Thanks,
    Paul

> 
>>  }
>>  
>>  int print_cpuinfo(void)
>>
> 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 8/9] MIPS: L2 cache support
  2016-09-08 10:31     ` Paul Burton
@ 2016-09-08 10:39       ` Paul Burton
  2016-09-08 10:40       ` Marek Vasut
  1 sibling, 0 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-08 10:39 UTC (permalink / raw)
  To: u-boot

On 08/09/16 11:31, Paul Burton wrote:
> Nope - inline does have an impact for functions in headers. If it's not
> there & a file includes a header but doesn't use *all* of the functions
> in it then the compiler will warn about the functions being unused.

Or rather, I should say a vaguely modern gcc will warn about unused
functions.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 8/9] MIPS: L2 cache support
  2016-09-08 10:31     ` Paul Burton
  2016-09-08 10:39       ` Paul Burton
@ 2016-09-08 10:40       ` Marek Vasut
  2016-09-08 11:24         ` Paul Burton
  1 sibling, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2016-09-08 10:40 UTC (permalink / raw)
  To: u-boot

On 09/08/2016 12:31 PM, Paul Burton wrote:
> On 08/09/16 11:04, Marek Vasut wrote:
>> On 09/07/2016 07:48 PM, Paul Burton wrote:
>>> This patch adds support for initialising & maintaining L2 caches on MIPS
>>> systems. The L2 cache configuration may be advertised through either
>>> coprocessor 0 or the MIPS Coherence Manager depending upon the system,
>>> and support for both is included.
>>>
>>> If the L2 can be bypassed then we bypass it early in boot & initialise
>>> the L1 caches first, such that we can start making use of the L1
>>> instruction cache as early as possible. Otherwise we initialise the L2
>>> first such that the L1s have no opportunity to generate access to the
>>> uninitialised L2.
>>>
>>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>>> ---
>>>
>>>  arch/mips/Kconfig                   |   6 ++
>>>  arch/mips/include/asm/cm.h          |  38 ++++++++
>>>  arch/mips/include/asm/global_data.h |   3 +
>>>  arch/mips/include/asm/mipsregs.h    |   5 +
>>>  arch/mips/lib/cache.c               |  62 ++++++++++++-
>>>  arch/mips/lib/cache_init.S          | 180 +++++++++++++++++++++++++++++++++++-
>>>  6 files changed, 288 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>>> index 732d129..8f97727 100644
>>> --- a/arch/mips/Kconfig
>>> +++ b/arch/mips/Kconfig
>>> @@ -301,6 +301,12 @@ config MIPS_L1_CACHE_SHIFT
>>>  	default "4" if MIPS_L1_CACHE_SHIFT_4
>>>  	default "5"
>>>  
>>> +config MIPS_L2_CACHE
>>> +	bool
>>> +	help
>>> +	  Select this if your system includes an L2 cache and you want U-Boot
>>> +	  to initialise & maintain it.
>>> +
>>>  config DYNAMIC_IO_PORT_BASE
>>>  	bool
>>>  
>>> diff --git a/arch/mips/include/asm/cm.h b/arch/mips/include/asm/cm.h
>>> index 0261733..62ecef2 100644
>>> --- a/arch/mips/include/asm/cm.h
>>> +++ b/arch/mips/include/asm/cm.h
>>> @@ -12,8 +12,46 @@
>>>  #define GCR_BASE			0x0008
>>>  #define GCR_BASE_UPPER			0x000c
>>>  #define GCR_REV				0x0030
>>> +#define GCR_L2_CONFIG			0x0130
>>> +#define GCR_L2_TAG_ADDR			0x0600
>>> +#define GCR_L2_TAG_ADDR_UPPER		0x0604
>>> +#define GCR_L2_TAG_STATE		0x0608
>>> +#define GCR_L2_TAG_STATE_UPPER		0x060c
>>> +#define GCR_L2_DATA			0x0610
>>> +#define GCR_L2_DATA_UPPER		0x0614
>>>  
>>>  /* GCR_REV CM versions */
>>>  #define GCR_REV_CM3			0x0800
>>>  
>>> +/* GCR_L2_CONFIG fields */
>>> +#define GCR_L2_CONFIG_ASSOC_SHIFT	0
>>> +#define GCR_L2_CONFIG_ASSOC_BITS	8
>>> +#define GCR_L2_CONFIG_LINESZ_SHIFT	8
>>> +#define GCR_L2_CONFIG_LINESZ_BITS	4
>>> +#define GCR_L2_CONFIG_SETSZ_SHIFT	12
>>> +#define GCR_L2_CONFIG_SETSZ_BITS	4
>>> +#define GCR_L2_CONFIG_BYPASS		(1 << 20)
>>> +
>>> +#ifndef __ASSEMBLY__
>>> +
>>> +#include <asm/io.h>
>>> +
>>> +static inline void *mips_cm_base(void)
>>> +{
>>> +	return (void *)CKSEG1ADDR(CONFIG_MIPS_CM_BASE);
>>> +}
>>> +
>>> +static inline unsigned long mips_cm_l2_line_size(void)
>>> +{
>>> +	unsigned long l2conf, line_sz;
>>> +
>>> +	l2conf = __raw_readl(mips_cm_base() + GCR_L2_CONFIG);
>>> +
>>> +	line_sz = l2conf >> GCR_L2_CONFIG_LINESZ_SHIFT;
>>> +	line_sz &= GENMASK(GCR_L2_CONFIG_LINESZ_BITS - 1, 0);
>>> +	return line_sz ? (2 << line_sz) : 0;
>>> +}
>>
>> Drop the inline stuff, the compiler can decide that itself, especially
>> on static functions. The inline is only a hint anyway.
>>
>> [...]
> 
> Hi Marek,
> 
> Nope - inline does have an impact for functions in headers. If it's not
> there & a file includes a header but doesn't use *all* of the functions
> in it then the compiler will warn about the functions being unused. With
> inline that isn't the case. Thus "static inline" is used quite
> extensively in many of the headers under include/.

Uh, I didn't notice these functions were placed in a header file. Is
that really necessary at all or can you move them to a C file ?

> Thanks,
>     Paul
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init
  2016-09-08 10:36     ` Paul Burton
@ 2016-09-08 10:47       ` Marek Vasut
  2016-09-08 11:21         ` Paul Burton
  0 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2016-09-08 10:47 UTC (permalink / raw)
  To: u-boot

On 09/08/2016 12:36 PM, Paul Burton wrote:
> On 08/09/16 11:02, Marek Vasut wrote:
>> On 09/07/2016 07:48 PM, Paul Burton wrote:
>>> Add an implementation of arch_cpu_init that covers all MIPS boards, in
>>> preparation for performing various pieces of initialisation there in
>>> later patches.
>>>
>>> In order to allow the ath79 code to continue performing initialisation
>>> at this point in the boot, it's converted to use a new mach_cpu_init
>>> function which is called from arch_cpu_init.
>>>
>>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>>
>> [...]
>>
>> Hi!
>>
>>> diff --git a/arch/mips/include/asm/u-boot-mips.h b/arch/mips/include/asm/u-boot-mips.h
>>> index 1f527bb..0eaf32b 100644
>>> --- a/arch/mips/include/asm/u-boot-mips.h
>>> +++ b/arch/mips/include/asm/u-boot-mips.h
>>> @@ -5,4 +5,16 @@
>>>  #ifndef _U_BOOT_MIPS_H_
>>>  #define _U_BOOT_MIPS_H_
>>>  
>>> +/**
>>> + * mach_cpu_init() - Perform SoC/machine-specific CPU initialisation
>>> + *
>>> + * This is called from arch_cpu_init() to allow for SoC/machine-level code to
>>> + * perform any CPU initialisation it may require.
>>
>> Just call this function from arch_cpu_init() in various places instead
>> of replacing arch_cpu_init() with it all over the place. Also rename it
>> to arch_cpu_init_common() to make it more obvious what this does .
> 
> Hi Marek,

Hi,

> That's "backwards" compared with what this code actually does -
> arch_cpu_init becomes the common function (ie. is arch-wide as in the
> patch subject) and mach_cpu_init is for use by machines/SoCs - ie. code
> under arch/mips/mach-*.

For machines, we already have board_cpu_init() , for SoCs we have
arch_cpu_init() . Reading through the patchset, I understand the
purpose, but then the content of mach_cpu_init() looks like some
common arch init bit to me.

>>> + */
>>> +#ifdef CONFIG_MACH_CPU_INIT
>>> +void mach_cpu_init(void);
>>> +#else
>>> +static inline void mach_cpu_init(void) {}
>>
>> Implement this as __weak int and you can nuke the ifdefery .
> 
> I could make it weak, but then we don't let the compiler optimise it out
> entirely for builds that don't need it (ie. everything except ath79).

Did you try it ?

> I'd say that since the #ifdefery here is confined to this one case in
> the header (which is a pretty common approach) it's ugliness is kept
> minimal & its cost on binaries & runtime is as low as it can be at zero.

Even if it costs 4 bytes more, one less ifdef is one good ifdef.

> Daniel - do you have a preference?
> 
>>
>>> +#endif
>>> +
>>>  #endif /* _U_BOOT_MIPS_H_ */
>>> diff --git a/arch/mips/mach-ath79/cpu.c b/arch/mips/mach-ath79/cpu.c
>>> index 5756a06..a556b73 100644
>>> --- a/arch/mips/mach-ath79/cpu.c
>>> +++ b/arch/mips/mach-ath79/cpu.c
>>> @@ -46,7 +46,7 @@ static const struct ath79_soc_desc desc[] = {
>>>  	{ATH79_SOC_QCA9561,     "9561", REV_ID_MAJOR_QCA9561,   0},
>>>  };
>>>  
>>> -int arch_cpu_init(void)
>>> +void mach_cpu_init(void)
>>
>> See above.
>>
>>>  {
>>>  	void __iomem *base;
>>>  	enum ath79_soc_type soc = ATH79_SOC_UNKNOWN;
>>> @@ -98,7 +98,6 @@ int arch_cpu_init(void)
>>>  	gd->arch.soc = soc;
>>>  	gd->arch.rev = rev;
>>>  	gd->arch.ver = ver;
>>> -	return 0;
>>
>> That's a nope, keep the return value.
> 
> The implementation always returns zero, so I see no point.

It does for now, but arch_cpu_init() can fail, so I see a point in
propagating return values.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init
  2016-09-08 10:47       ` Marek Vasut
@ 2016-09-08 11:21         ` Paul Burton
  2016-09-08 11:54           ` Marek Vasut
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Burton @ 2016-09-08 11:21 UTC (permalink / raw)
  To: u-boot



On 08/09/16 11:47, Marek Vasut wrote:
> On 09/08/2016 12:36 PM, Paul Burton wrote:
>> On 08/09/16 11:02, Marek Vasut wrote:
>>> On 09/07/2016 07:48 PM, Paul Burton wrote:
>>>> Add an implementation of arch_cpu_init that covers all MIPS boards, in
>>>> preparation for performing various pieces of initialisation there in
>>>> later patches.
>>>>
>>>> In order to allow the ath79 code to continue performing initialisation
>>>> at this point in the boot, it's converted to use a new mach_cpu_init
>>>> function which is called from arch_cpu_init.
>>>>
>>>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>>>
>>> [...]
>>>
>>> Hi!
>>>
>>>> diff --git a/arch/mips/include/asm/u-boot-mips.h b/arch/mips/include/asm/u-boot-mips.h
>>>> index 1f527bb..0eaf32b 100644
>>>> --- a/arch/mips/include/asm/u-boot-mips.h
>>>> +++ b/arch/mips/include/asm/u-boot-mips.h
>>>> @@ -5,4 +5,16 @@
>>>>  #ifndef _U_BOOT_MIPS_H_
>>>>  #define _U_BOOT_MIPS_H_
>>>>  
>>>> +/**
>>>> + * mach_cpu_init() - Perform SoC/machine-specific CPU initialisation
>>>> + *
>>>> + * This is called from arch_cpu_init() to allow for SoC/machine-level code to
>>>> + * perform any CPU initialisation it may require.
>>>
>>> Just call this function from arch_cpu_init() in various places instead
>>> of replacing arch_cpu_init() with it all over the place. Also rename it
>>> to arch_cpu_init_common() to make it more obvious what this does .
>>
>> Hi Marek,
> 
> Hi,
> 
>> That's "backwards" compared with what this code actually does -
>> arch_cpu_init becomes the common function (ie. is arch-wide as in the
>> patch subject) and mach_cpu_init is for use by machines/SoCs - ie. code
>> under arch/mips/mach-*.
> 
> For machines, we already have board_cpu_init() , for SoCs we have
> arch_cpu_init() . Reading through the patchset, I understand the
> purpose, but then the content of mach_cpu_init() looks like some
> common arch init bit to me.

Hi Marek,

The content of the only implementation of mach_cpu_init is some
ath79-specific chip ID stuff. I'm not sure how you think that's "common
arch init"?

Right now without this patch you're right - arch_cpu_init is used by
SoCs (well, one SoC - ath79). This patch changes that so that
arch_cpu_init is used by the arch code & mach_cpu_init is used by the
SoC/machine code under arch/mips/mach-*. That seems like the most
logical way to handle it to me.

I get that arch_cpu_init is also used by SoCs on some other
architectures (arm & x86 seemingly), but not on all - there's precedent
for an arch-wide implementation in arc, avr32, blackfin, nios2 or xtensa.

> 
>>>> + */
>>>> +#ifdef CONFIG_MACH_CPU_INIT
>>>> +void mach_cpu_init(void);
>>>> +#else
>>>> +static inline void mach_cpu_init(void) {}
>>>
>>> Implement this as __weak int and you can nuke the ifdefery .
>>
>> I could make it weak, but then we don't let the compiler optimise it out
>> entirely for builds that don't need it (ie. everything except ath79).
> 
> Did you try it ?

I didn't, but the compiler will have to emit the call as it won't be
known whether there's an implementation of the function until link time
so it will obviously have a small cost (unless LTO is used). Having said
that I see the U-Boot wiki seems to indicate that weak is preferred so
I'm ok with changing it.

Thanks,
    Paul

> 
>> I'd say that since the #ifdefery here is confined to this one case in
>> the header (which is a pretty common approach) it's ugliness is kept
>> minimal & its cost on binaries & runtime is as low as it can be at zero.
> 
> Even if it costs 4 bytes more, one less ifdef is one good ifdef.
> 
>> Daniel - do you have a preference?
>>
>>>
>>>> +#endif
>>>> +
>>>>  #endif /* _U_BOOT_MIPS_H_ */
>>>> diff --git a/arch/mips/mach-ath79/cpu.c b/arch/mips/mach-ath79/cpu.c
>>>> index 5756a06..a556b73 100644
>>>> --- a/arch/mips/mach-ath79/cpu.c
>>>> +++ b/arch/mips/mach-ath79/cpu.c
>>>> @@ -46,7 +46,7 @@ static const struct ath79_soc_desc desc[] = {
>>>>  	{ATH79_SOC_QCA9561,     "9561", REV_ID_MAJOR_QCA9561,   0},
>>>>  };
>>>>  
>>>> -int arch_cpu_init(void)
>>>> +void mach_cpu_init(void)
>>>
>>> See above.
>>>
>>>>  {
>>>>  	void __iomem *base;
>>>>  	enum ath79_soc_type soc = ATH79_SOC_UNKNOWN;
>>>> @@ -98,7 +98,6 @@ int arch_cpu_init(void)
>>>>  	gd->arch.soc = soc;
>>>>  	gd->arch.rev = rev;
>>>>  	gd->arch.ver = ver;
>>>> -	return 0;
>>>
>>> That's a nope, keep the return value.
>>
>> The implementation always returns zero, so I see no point.
> 
> It does for now, but arch_cpu_init() can fail, so I see a point in
> propagating return values.
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 8/9] MIPS: L2 cache support
  2016-09-08 10:40       ` Marek Vasut
@ 2016-09-08 11:24         ` Paul Burton
  0 siblings, 0 replies; 20+ messages in thread
From: Paul Burton @ 2016-09-08 11:24 UTC (permalink / raw)
  To: u-boot

On 08/09/16 11:40, Marek Vasut wrote:
> On 09/08/2016 12:31 PM, Paul Burton wrote:
>> On 08/09/16 11:04, Marek Vasut wrote:
>>> On 09/07/2016 07:48 PM, Paul Burton wrote:
>>>> This patch adds support for initialising & maintaining L2 caches on MIPS
>>>> systems. The L2 cache configuration may be advertised through either
>>>> coprocessor 0 or the MIPS Coherence Manager depending upon the system,
>>>> and support for both is included.
>>>>
>>>> If the L2 can be bypassed then we bypass it early in boot & initialise
>>>> the L1 caches first, such that we can start making use of the L1
>>>> instruction cache as early as possible. Otherwise we initialise the L2
>>>> first such that the L1s have no opportunity to generate access to the
>>>> uninitialised L2.
>>>>
>>>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>>>> ---
>>>>
>>>>  arch/mips/Kconfig                   |   6 ++
>>>>  arch/mips/include/asm/cm.h          |  38 ++++++++
>>>>  arch/mips/include/asm/global_data.h |   3 +
>>>>  arch/mips/include/asm/mipsregs.h    |   5 +
>>>>  arch/mips/lib/cache.c               |  62 ++++++++++++-
>>>>  arch/mips/lib/cache_init.S          | 180 +++++++++++++++++++++++++++++++++++-
>>>>  6 files changed, 288 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>>>> index 732d129..8f97727 100644
>>>> --- a/arch/mips/Kconfig
>>>> +++ b/arch/mips/Kconfig
>>>> @@ -301,6 +301,12 @@ config MIPS_L1_CACHE_SHIFT
>>>>  	default "4" if MIPS_L1_CACHE_SHIFT_4
>>>>  	default "5"
>>>>  
>>>> +config MIPS_L2_CACHE
>>>> +	bool
>>>> +	help
>>>> +	  Select this if your system includes an L2 cache and you want U-Boot
>>>> +	  to initialise & maintain it.
>>>> +
>>>>  config DYNAMIC_IO_PORT_BASE
>>>>  	bool
>>>>  
>>>> diff --git a/arch/mips/include/asm/cm.h b/arch/mips/include/asm/cm.h
>>>> index 0261733..62ecef2 100644
>>>> --- a/arch/mips/include/asm/cm.h
>>>> +++ b/arch/mips/include/asm/cm.h
>>>> @@ -12,8 +12,46 @@
>>>>  #define GCR_BASE			0x0008
>>>>  #define GCR_BASE_UPPER			0x000c
>>>>  #define GCR_REV				0x0030
>>>> +#define GCR_L2_CONFIG			0x0130
>>>> +#define GCR_L2_TAG_ADDR			0x0600
>>>> +#define GCR_L2_TAG_ADDR_UPPER		0x0604
>>>> +#define GCR_L2_TAG_STATE		0x0608
>>>> +#define GCR_L2_TAG_STATE_UPPER		0x060c
>>>> +#define GCR_L2_DATA			0x0610
>>>> +#define GCR_L2_DATA_UPPER		0x0614
>>>>  
>>>>  /* GCR_REV CM versions */
>>>>  #define GCR_REV_CM3			0x0800
>>>>  
>>>> +/* GCR_L2_CONFIG fields */
>>>> +#define GCR_L2_CONFIG_ASSOC_SHIFT	0
>>>> +#define GCR_L2_CONFIG_ASSOC_BITS	8
>>>> +#define GCR_L2_CONFIG_LINESZ_SHIFT	8
>>>> +#define GCR_L2_CONFIG_LINESZ_BITS	4
>>>> +#define GCR_L2_CONFIG_SETSZ_SHIFT	12
>>>> +#define GCR_L2_CONFIG_SETSZ_BITS	4
>>>> +#define GCR_L2_CONFIG_BYPASS		(1 << 20)
>>>> +
>>>> +#ifndef __ASSEMBLY__
>>>> +
>>>> +#include <asm/io.h>
>>>> +
>>>> +static inline void *mips_cm_base(void)
>>>> +{
>>>> +	return (void *)CKSEG1ADDR(CONFIG_MIPS_CM_BASE);
>>>> +}
>>>> +
>>>> +static inline unsigned long mips_cm_l2_line_size(void)
>>>> +{
>>>> +	unsigned long l2conf, line_sz;
>>>> +
>>>> +	l2conf = __raw_readl(mips_cm_base() + GCR_L2_CONFIG);
>>>> +
>>>> +	line_sz = l2conf >> GCR_L2_CONFIG_LINESZ_SHIFT;
>>>> +	line_sz &= GENMASK(GCR_L2_CONFIG_LINESZ_BITS - 1, 0);
>>>> +	return line_sz ? (2 << line_sz) : 0;
>>>> +}
>>>
>>> Drop the inline stuff, the compiler can decide that itself, especially
>>> on static functions. The inline is only a hint anyway.
>>>
>>> [...]
>>
>> Hi Marek,
>>
>> Nope - inline does have an impact for functions in headers. If it's not
>> there & a file includes a header but doesn't use *all* of the functions
>> in it then the compiler will warn about the functions being unused. With
>> inline that isn't the case. Thus "static inline" is used quite
>> extensively in many of the headers under include/.
> 
> Uh, I didn't notice these functions were placed in a header file. Is
> that really necessary at all or can you move them to a C file ?

I could move them, but then the compiler wouldn't get an opportunity to
inline these short functions. I don't see any downside to keeping these
in the same place as the registers they access are defined.

Thanks,
    Paul

> 
>> Thanks,
>>     Paul
>>
> 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init
  2016-09-08 11:21         ` Paul Burton
@ 2016-09-08 11:54           ` Marek Vasut
  0 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2016-09-08 11:54 UTC (permalink / raw)
  To: u-boot

On 09/08/2016 01:21 PM, Paul Burton wrote:
> 
> 
> On 08/09/16 11:47, Marek Vasut wrote:
>> On 09/08/2016 12:36 PM, Paul Burton wrote:
>>> On 08/09/16 11:02, Marek Vasut wrote:
>>>> On 09/07/2016 07:48 PM, Paul Burton wrote:
>>>>> Add an implementation of arch_cpu_init that covers all MIPS boards, in
>>>>> preparation for performing various pieces of initialisation there in
>>>>> later patches.
>>>>>
>>>>> In order to allow the ath79 code to continue performing initialisation
>>>>> at this point in the boot, it's converted to use a new mach_cpu_init
>>>>> function which is called from arch_cpu_init.
>>>>>
>>>>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>>>>
>>>> [...]
>>>>
>>>> Hi!
>>>>
>>>>> diff --git a/arch/mips/include/asm/u-boot-mips.h b/arch/mips/include/asm/u-boot-mips.h
>>>>> index 1f527bb..0eaf32b 100644
>>>>> --- a/arch/mips/include/asm/u-boot-mips.h
>>>>> +++ b/arch/mips/include/asm/u-boot-mips.h
>>>>> @@ -5,4 +5,16 @@
>>>>>  #ifndef _U_BOOT_MIPS_H_
>>>>>  #define _U_BOOT_MIPS_H_
>>>>>  
>>>>> +/**
>>>>> + * mach_cpu_init() - Perform SoC/machine-specific CPU initialisation
>>>>> + *
>>>>> + * This is called from arch_cpu_init() to allow for SoC/machine-level code to
>>>>> + * perform any CPU initialisation it may require.
>>>>
>>>> Just call this function from arch_cpu_init() in various places instead
>>>> of replacing arch_cpu_init() with it all over the place. Also rename it
>>>> to arch_cpu_init_common() to make it more obvious what this does .
>>>
>>> Hi Marek,
>>
>> Hi,
>>
>>> That's "backwards" compared with what this code actually does -
>>> arch_cpu_init becomes the common function (ie. is arch-wide as in the
>>> patch subject) and mach_cpu_init is for use by machines/SoCs - ie. code
>>> under arch/mips/mach-*.
>>
>> For machines, we already have board_cpu_init() , for SoCs we have
>> arch_cpu_init() . Reading through the patchset, I understand the
>> purpose, but then the content of mach_cpu_init() looks like some
>> common arch init bit to me.
> 
> Hi Marek,
> 
> The content of the only implementation of mach_cpu_init is some
> ath79-specific chip ID stuff. I'm not sure how you think that's "common
> arch init"?
> 
> Right now without this patch you're right - arch_cpu_init is used by
> SoCs (well, one SoC - ath79). This patch changes that so that
> arch_cpu_init is used by the arch code & mach_cpu_init is used by the
> SoC/machine code under arch/mips/mach-*. That seems like the most
> logical way to handle it to me.

It also introduces discrepancy with ARM and other architectures though.

> I get that arch_cpu_init is also used by SoCs on some other
> architectures (arm & x86 seemingly), but not on all - there's precedent
> for an arch-wide implementation in arc, avr32, blackfin, nios2 or xtensa.

Most of which are dead/unmaintained though.

Seems like this might need some further cleanup/clarification then.

>>
>>>>> + */
>>>>> +#ifdef CONFIG_MACH_CPU_INIT
>>>>> +void mach_cpu_init(void);
>>>>> +#else
>>>>> +static inline void mach_cpu_init(void) {}
>>>>
>>>> Implement this as __weak int and you can nuke the ifdefery .
>>>
>>> I could make it weak, but then we don't let the compiler optimise it out
>>> entirely for builds that don't need it (ie. everything except ath79).
>>
>> Did you try it ?
> 
> I didn't, but the compiler will have to emit the call as it won't be
> known whether there's an implementation of the function until link time
> so it will obviously have a small cost (unless LTO is used). Having said
> that I see the U-Boot wiki seems to indicate that weak is preferred so
> I'm ok with changing it.

btw if you want to go ahead with adding the mach_cpu_init(), you might
want to consider adding it into the board_f.c initcalls , so other
arches can convert to it.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-09-08 11:54 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-07 17:48 [U-Boot] [PATCH 0/9] MIPS L2 cache support Paul Burton
2016-09-07 17:48 ` [U-Boot] [PATCH 1/9] MIPS: Add arch-wide arch_cpu_init Paul Burton
2016-09-08 10:02   ` Marek Vasut
2016-09-08 10:36     ` Paul Burton
2016-09-08 10:47       ` Marek Vasut
2016-09-08 11:21         ` Paul Burton
2016-09-08 11:54           ` Marek Vasut
2016-09-07 17:48 ` [U-Boot] [PATCH 2/9] MIPS: Probe cache line sizes once during boot Paul Burton
2016-09-07 17:48 ` [U-Boot] [PATCH 3/9] MIPS: Enable use of the instruction cache earlier Paul Burton
2016-09-07 17:48 ` [U-Boot] [PATCH 4/9] MIPS: Preserve Config implementation-defined bits Paul Burton
2016-09-07 17:48 ` [U-Boot] [PATCH 5/9] MIPS: If we don't need DDR for cache init, init cache first Paul Burton
2016-09-07 17:48 ` [U-Boot] [PATCH 6/9] MIPS: Define register names for cache init Paul Burton
2016-09-07 17:48 ` [U-Boot] [PATCH 7/9] MIPS: Map CM Global Control Registers Paul Burton
2016-09-07 17:48 ` [U-Boot] [PATCH 8/9] MIPS: L2 cache support Paul Burton
2016-09-08 10:04   ` Marek Vasut
2016-09-08 10:31     ` Paul Burton
2016-09-08 10:39       ` Paul Burton
2016-09-08 10:40       ` Marek Vasut
2016-09-08 11:24         ` Paul Burton
2016-09-07 17:48 ` [U-Boot] [PATCH 9/9] MIPS: Malta: Enable CM & L2 support Paul Burton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.