All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/8] MIPS cache code cleanup
@ 2015-01-26 15:02 Paul Burton
  2015-01-26 15:02 ` [U-Boot] [PATCH 1/8] MIPS: avoid .set ISA for cache operations Paul Burton
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Paul Burton @ 2015-01-26 15:02 UTC (permalink / raw)
  To: u-boot

This series cleans up the MIPS cache code somewhat, and unifies the
mips32 & mips64 implementations of it. This is largely in preparation
for further patches adding L2 cache support. The final patch of this
series fixes a bug encountered with recent cores on Malta boards.

Paul Burton (8):
  MIPS: avoid .set ISA for cache operations
  MIPS: unify cache maintenance functions
  MIPS: unify cache initialization code
  MIPS: refactor L1 cache config reads to a macro
  MIPS: refactor cache loops to a macro
  MIPS: inline mips_init_[id]cache functions
  MIPS: allow systems to skip loads during cache init
  MIPS: clear TagLo select 2 during cache init

 arch/mips/Kconfig                                  |   6 +
 arch/mips/cpu/mips32/Makefile                      |   3 +-
 arch/mips/cpu/mips32/cpu.c                         | 119 -----------
 arch/mips/cpu/mips64/Makefile                      |   2 +-
 arch/mips/cpu/mips64/cache.S                       | 213 --------------------
 arch/mips/cpu/mips64/cpu.c                         |  58 ------
 arch/mips/include/asm/cacheops.h                   |   7 +
 arch/mips/lib/Makefile                             |   2 +
 arch/mips/lib/cache.c                              | 118 +++++++++++
 arch/mips/{cpu/mips32/cache.S => lib/cache_init.S} | 222 +++++++++------------
 10 files changed, 226 insertions(+), 524 deletions(-)
 delete mode 100644 arch/mips/cpu/mips64/cache.S
 create mode 100644 arch/mips/lib/cache.c
 rename arch/mips/{cpu/mips32/cache.S => lib/cache_init.S} (59%)

-- 
2.2.2

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

* [U-Boot] [PATCH 1/8] MIPS: avoid .set ISA for cache operations
  2015-01-26 15:02 [U-Boot] [PATCH 0/8] MIPS cache code cleanup Paul Burton
@ 2015-01-26 15:02 ` Paul Burton
  2015-01-28 20:43   ` Daniel Schwierzeck
  2015-01-26 15:02 ` [U-Boot] [PATCH 2/8] MIPS: unify cache maintenance functions Paul Burton
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Paul Burton @ 2015-01-26 15:02 UTC (permalink / raw)
  To: u-boot

As a step towards unifying the cache maintenance code for mips32 &
mips64 CPUs, stop using ".set <ISA>" directives in the more developed
mips32 version of the code. Instead, when present make use of the GCC
builtin for emitting a cache instruction. When not present, simply don't
bother with the .set directives since U-boot always builds with
-march=mips32 or higher anyway.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
 arch/mips/cpu/mips32/cache.S     | 18 +++++-------------
 arch/mips/cpu/mips32/cpu.c       | 40 +++++++++++++++-------------------------
 arch/mips/include/asm/cacheops.h |  7 +++++++
 3 files changed, 27 insertions(+), 38 deletions(-)

diff --git a/arch/mips/cpu/mips32/cache.S b/arch/mips/cpu/mips32/cache.S
index 22bd844..fb1d84b 100644
--- a/arch/mips/cpu/mips32/cache.S
+++ b/arch/mips/cpu/mips32/cache.S
@@ -22,14 +22,6 @@
 
 #define INDEX_BASE	CKSEG0
 
-	.macro	cache_op op addr
-	.set	push
-	.set	noreorder
-	.set	mips3
-	cache	\op, 0(\addr)
-	.set	pop
-	.endm
-
 	.macro	f_fill64 dst, offset, val
 	LONG_S	\val, (\offset +  0 * LONGSIZE)(\dst)
 	LONG_S	\val, (\offset +  1 * LONGSIZE)(\dst)
@@ -60,17 +52,17 @@ LEAF(mips_init_icache)
 	/* clear tag to invalidate */
 	PTR_LI		t0, INDEX_BASE
 	PTR_ADDU	t1, t0, a1
-1:	cache_op	INDEX_STORE_TAG_I t0
+1:	cache		INDEX_STORE_TAG_I, 0(t0)
 	PTR_ADDU	t0, a2
 	bne		t0, t1, 1b
 	/* fill once, so data field parity is correct */
 	PTR_LI		t0, INDEX_BASE
-2:	cache_op	FILL t0
+2:	cache		FILL, 0(t0)
 	PTR_ADDU	t0, a2
 	bne		t0, t1, 2b
 	/* invalidate again - prudent but not strictly neccessary */
 	PTR_LI		t0, INDEX_BASE
-1:	cache_op	INDEX_STORE_TAG_I t0
+1:	cache		INDEX_STORE_TAG_I, 0(t0)
 	PTR_ADDU	t0, a2
 	bne		t0, t1, 1b
 9:	jr		ra
@@ -85,7 +77,7 @@ LEAF(mips_init_dcache)
 	/* clear all tags */
 	PTR_LI		t0, INDEX_BASE
 	PTR_ADDU	t1, t0, a1
-1:	cache_op	INDEX_STORE_TAG_D t0
+1:	cache		INDEX_STORE_TAG_D, 0(t0)
 	PTR_ADDU	t0, a2
 	bne		t0, t1, 1b
 	/* load from each line (in cached space) */
@@ -95,7 +87,7 @@ LEAF(mips_init_dcache)
 	bne		t0, t1, 2b
 	/* clear all tags */
 	PTR_LI		t0, INDEX_BASE
-1:	cache_op	INDEX_STORE_TAG_D t0
+1:	cache		INDEX_STORE_TAG_D, 0(t0)
 	PTR_ADDU	t0, a2
 	bne		t0, t1, 1b
 9:	jr		ra
diff --git a/arch/mips/cpu/mips32/cpu.c b/arch/mips/cpu/mips32/cpu.c
index 278865b..3f247fb 100644
--- a/arch/mips/cpu/mips32/cpu.c
+++ b/arch/mips/cpu/mips32/cpu.c
@@ -12,16 +12,6 @@
 #include <asm/cacheops.h>
 #include <asm/reboot.h>
 
-#define cache_op(op,addr)						\
-	__asm__ __volatile__(						\
-	"	.set	push					\n"	\
-	"	.set	noreorder				\n"	\
-	"	.set	mips3\n\t				\n"	\
-	"	cache	%0, %1					\n"	\
-	"	.set	pop					\n"	\
-	:								\
-	: "i" (op), "R" (*(unsigned char *)(addr)))
-
 void __attribute__((weak)) _machine_restart(void)
 {
 }
@@ -74,20 +64,20 @@ void flush_cache(ulong start_addr, ulong size)
 {
 	unsigned long ilsize = icache_line_size();
 	unsigned long dlsize = dcache_line_size();
-	unsigned long addr, aend;
+	const volatile void *addr, *aend;
 
 	/* aend will be miscalculated when size is zero, so we return here */
 	if (size == 0)
 		return;
 
-	addr = start_addr & ~(dlsize - 1);
-	aend = (start_addr + size - 1) & ~(dlsize - 1);
+	addr = (void *)(start_addr & ~(dlsize - 1));
+	aend = (void *)((start_addr + size - 1) & ~(dlsize - 1));
 
 	if (ilsize == dlsize) {
 		/* flush I-cache & D-cache simultaneously */
 		while (1) {
-			cache_op(HIT_WRITEBACK_INV_D, addr);
-			cache_op(HIT_INVALIDATE_I, addr);
+			mips_cache(HIT_WRITEBACK_INV_D, addr);
+			mips_cache(HIT_INVALIDATE_I, addr);
 			if (addr == aend)
 				break;
 			addr += dlsize;
@@ -97,17 +87,17 @@ void flush_cache(ulong start_addr, ulong size)
 
 	/* flush D-cache */
 	while (1) {
-		cache_op(HIT_WRITEBACK_INV_D, addr);
+		mips_cache(HIT_WRITEBACK_INV_D, addr);
 		if (addr == aend)
 			break;
 		addr += dlsize;
 	}
 
 	/* flush I-cache */
-	addr = start_addr & ~(ilsize - 1);
-	aend = (start_addr + size - 1) & ~(ilsize - 1);
+	addr = (void *)(start_addr & ~(ilsize - 1));
+	aend = (void *)((start_addr + size - 1) & ~(ilsize - 1));
 	while (1) {
-		cache_op(HIT_INVALIDATE_I, addr);
+		mips_cache(HIT_INVALIDATE_I, addr);
 		if (addr == aend)
 			break;
 		addr += ilsize;
@@ -117,11 +107,11 @@ 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 addr = start_addr & ~(lsize - 1);
-	unsigned long aend = (stop - 1) & ~(lsize - 1);
+	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
+	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
 
 	while (1) {
-		cache_op(HIT_WRITEBACK_INV_D, addr);
+		mips_cache(HIT_WRITEBACK_INV_D, addr);
 		if (addr == aend)
 			break;
 		addr += lsize;
@@ -131,11 +121,11 @@ void flush_dcache_range(ulong start_addr, ulong stop)
 void invalidate_dcache_range(ulong start_addr, ulong stop)
 {
 	unsigned long lsize = dcache_line_size();
-	unsigned long addr = start_addr & ~(lsize - 1);
-	unsigned long aend = (stop - 1) & ~(lsize - 1);
+	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
+	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
 
 	while (1) {
-		cache_op(HIT_INVALIDATE_D, addr);
+		mips_cache(HIT_INVALIDATE_D, addr);
 		if (addr == aend)
 			break;
 		addr += lsize;
diff --git a/arch/mips/include/asm/cacheops.h b/arch/mips/include/asm/cacheops.h
index 6464250..809c966 100644
--- a/arch/mips/include/asm/cacheops.h
+++ b/arch/mips/include/asm/cacheops.h
@@ -11,6 +11,13 @@
 #ifndef	__ASM_CACHEOPS_H
 #define	__ASM_CACHEOPS_H
 
+#ifdef __GCC_HAVE_BUILTIN_MIPS_CACHE
+# define mips_cache __builtin_mips_cache
+#else
+# define mips_cache(op,addr) \
+	__asm__ __volatile__("cache	%0, %1" : : "i"(op), "R"(addr))
+#endif
+
 /*
  * Cache Operations available on all MIPS processors with R4000-style caches
  */
-- 
2.2.2

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

* [U-Boot] [PATCH 2/8] MIPS: unify cache maintenance functions
  2015-01-26 15:02 [U-Boot] [PATCH 0/8] MIPS cache code cleanup Paul Burton
  2015-01-26 15:02 ` [U-Boot] [PATCH 1/8] MIPS: avoid .set ISA for cache operations Paul Burton
@ 2015-01-26 15:02 ` Paul Burton
  2015-01-26 15:02 ` [U-Boot] [PATCH 3/8] MIPS: unify cache initialization code Paul Burton
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2015-01-26 15:02 UTC (permalink / raw)
  To: u-boot

Move the more developed mips32 version of the cache maintenance
functions to a common arch/mips/lib/cache.c, in order to reduce
duplication between mips32 & mips64.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
 arch/mips/cpu/mips32/cpu.c | 109 -----------------------------------------
 arch/mips/cpu/mips64/cpu.c |  58 ----------------------
 arch/mips/lib/Makefile     |   1 +
 arch/mips/lib/cache.c      | 118 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 119 insertions(+), 167 deletions(-)
 create mode 100644 arch/mips/lib/cache.c

diff --git a/arch/mips/cpu/mips32/cpu.c b/arch/mips/cpu/mips32/cpu.c
index 3f247fb..8e1cc4e 100644
--- a/arch/mips/cpu/mips32/cpu.c
+++ b/arch/mips/cpu/mips32/cpu.c
@@ -9,7 +9,6 @@
 #include <command.h>
 #include <netdev.h>
 #include <asm/mipsregs.h>
-#include <asm/cacheops.h>
 #include <asm/reboot.h>
 
 void __attribute__((weak)) _machine_restart(void)
@@ -24,114 +23,6 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return 0;
 }
 
-#ifdef CONFIG_SYS_CACHELINE_SIZE
-
-static inline unsigned long icache_line_size(void)
-{
-	return CONFIG_SYS_CACHELINE_SIZE;
-}
-
-static inline unsigned long dcache_line_size(void)
-{
-	return CONFIG_SYS_CACHELINE_SIZE;
-}
-
-#else /* !CONFIG_SYS_CACHELINE_SIZE */
-
-static inline unsigned long icache_line_size(void)
-{
-	unsigned long conf1, il;
-	conf1 = read_c0_config1();
-	il = (conf1 & MIPS_CONF1_IL) >> MIPS_CONF1_IL_SHIFT;
-	if (!il)
-		return 0;
-	return 2 << il;
-}
-
-static inline unsigned long dcache_line_size(void)
-{
-	unsigned long conf1, dl;
-	conf1 = read_c0_config1();
-	dl = (conf1 & MIPS_CONF1_DL) >> MIPS_CONF1_DL_SHIFT;
-	if (!dl)
-		return 0;
-	return 2 << dl;
-}
-
-#endif /* !CONFIG_SYS_CACHELINE_SIZE */
-
-void flush_cache(ulong start_addr, ulong size)
-{
-	unsigned long ilsize = icache_line_size();
-	unsigned long dlsize = dcache_line_size();
-	const volatile void *addr, *aend;
-
-	/* aend will be miscalculated when size is zero, so we return here */
-	if (size == 0)
-		return;
-
-	addr = (void *)(start_addr & ~(dlsize - 1));
-	aend = (void *)((start_addr + size - 1) & ~(dlsize - 1));
-
-	if (ilsize == dlsize) {
-		/* flush I-cache & D-cache simultaneously */
-		while (1) {
-			mips_cache(HIT_WRITEBACK_INV_D, addr);
-			mips_cache(HIT_INVALIDATE_I, addr);
-			if (addr == aend)
-				break;
-			addr += dlsize;
-		}
-		return;
-	}
-
-	/* flush D-cache */
-	while (1) {
-		mips_cache(HIT_WRITEBACK_INV_D, addr);
-		if (addr == aend)
-			break;
-		addr += dlsize;
-	}
-
-	/* flush I-cache */
-	addr = (void *)(start_addr & ~(ilsize - 1));
-	aend = (void *)((start_addr + size - 1) & ~(ilsize - 1));
-	while (1) {
-		mips_cache(HIT_INVALIDATE_I, addr);
-		if (addr == aend)
-			break;
-		addr += ilsize;
-	}
-}
-
-void flush_dcache_range(ulong start_addr, ulong stop)
-{
-	unsigned long lsize = dcache_line_size();
-	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
-	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
-
-	while (1) {
-		mips_cache(HIT_WRITEBACK_INV_D, addr);
-		if (addr == aend)
-			break;
-		addr += lsize;
-	}
-}
-
-void invalidate_dcache_range(ulong start_addr, ulong stop)
-{
-	unsigned long lsize = dcache_line_size();
-	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
-	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
-
-	while (1) {
-		mips_cache(HIT_INVALIDATE_D, addr);
-		if (addr == aend)
-			break;
-		addr += lsize;
-	}
-}
-
 void write_one_tlb(int index, u32 pagemask, u32 hi, u32 low0, u32 low1)
 {
 	write_c0_entrylo0(low0);
diff --git a/arch/mips/cpu/mips64/cpu.c b/arch/mips/cpu/mips64/cpu.c
index 9f45cfc..1d32705 100644
--- a/arch/mips/cpu/mips64/cpu.c
+++ b/arch/mips/cpu/mips64/cpu.c
@@ -9,19 +9,8 @@
 #include <command.h>
 #include <netdev.h>
 #include <asm/mipsregs.h>
-#include <asm/cacheops.h>
 #include <asm/reboot.h>
 
-#define cache_op(op, addr)						\
-	__asm__ __volatile__(						\
-	"	.set	push\n"						\
-	"	.set	noreorder\n"					\
-	"	.set	mips64\n"					\
-	"	cache	%0, %1\n"					\
-	"	.set	pop\n"						\
-	:								\
-	: "i" (op), "R" (*(unsigned char *)(addr)))
-
 void __attribute__((weak)) _machine_restart(void)
 {
 	fprintf(stderr, "*** reset failed ***\n");
@@ -37,53 +26,6 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return 0;
 }
 
-void flush_cache(ulong start_addr, ulong size)
-{
-	unsigned long lsize = CONFIG_SYS_CACHELINE_SIZE;
-	unsigned long addr = start_addr & ~(lsize - 1);
-	unsigned long aend = (start_addr + size - 1) & ~(lsize - 1);
-
-	/* aend will be miscalculated when size is zero, so we return here */
-	if (size == 0)
-		return;
-
-	while (1) {
-		cache_op(HIT_WRITEBACK_INV_D, addr);
-		cache_op(HIT_INVALIDATE_I, addr);
-		if (addr == aend)
-			break;
-		addr += lsize;
-	}
-}
-
-void flush_dcache_range(ulong start_addr, ulong stop)
-{
-	unsigned long lsize = CONFIG_SYS_CACHELINE_SIZE;
-	unsigned long addr = start_addr & ~(lsize - 1);
-	unsigned long aend = (stop - 1) & ~(lsize - 1);
-
-	while (1) {
-		cache_op(HIT_WRITEBACK_INV_D, addr);
-		if (addr == aend)
-			break;
-		addr += lsize;
-	}
-}
-
-void invalidate_dcache_range(ulong start_addr, ulong stop)
-{
-	unsigned long lsize = CONFIG_SYS_CACHELINE_SIZE;
-	unsigned long addr = start_addr & ~(lsize - 1);
-	unsigned long aend = (stop - 1) & ~(lsize - 1);
-
-	while (1) {
-		cache_op(HIT_INVALIDATE_D, addr);
-		if (addr == aend)
-			break;
-		addr += lsize;
-	}
-}
-
 void write_one_tlb(int index, u32 pagemask, u32 hi, u32 low0, u32 low1)
 {
 	write_c0_entrylo0(low0);
diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
index 7f9b653..d939ee6 100644
--- a/arch/mips/lib/Makefile
+++ b/arch/mips/lib/Makefile
@@ -5,6 +5,7 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
+obj-y	+= cache.o
 obj-y	+= io.o
 
 obj-$(CONFIG_CMD_BOOTM) += bootm.o
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c
new file mode 100644
index 0000000..4bf39db
--- /dev/null
+++ b/arch/mips/lib/cache.c
@@ -0,0 +1,118 @@
+/*
+ * (C) Copyright 2003
+ * Wolfgang Denk, DENX Software Engineering, <wd@denx.de>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/cacheops.h>
+#include <asm/mipsregs.h>
+
+#ifdef CONFIG_SYS_CACHELINE_SIZE
+
+static inline unsigned long icache_line_size(void)
+{
+	return CONFIG_SYS_CACHELINE_SIZE;
+}
+
+static inline unsigned long dcache_line_size(void)
+{
+	return CONFIG_SYS_CACHELINE_SIZE;
+}
+
+#else /* !CONFIG_SYS_CACHELINE_SIZE */
+
+static inline unsigned long icache_line_size(void)
+{
+	unsigned long conf1, il;
+	conf1 = read_c0_config1();
+	il = (conf1 & MIPS_CONF1_IL) >> MIPS_CONF1_IL_SHIFT;
+	if (!il)
+		return 0;
+	return 2 << il;
+}
+
+static inline unsigned long dcache_line_size(void)
+{
+	unsigned long conf1, dl;
+	conf1 = read_c0_config1();
+	dl = (conf1 & MIPS_CONF1_DL) >> MIPS_CONF1_DL_SHIFT;
+	if (!dl)
+		return 0;
+	return 2 << dl;
+}
+
+#endif /* !CONFIG_SYS_CACHELINE_SIZE */
+
+void flush_cache(ulong start_addr, ulong size)
+{
+	unsigned long ilsize = icache_line_size();
+	unsigned long dlsize = dcache_line_size();
+	const volatile void *addr, *aend;
+
+	/* aend will be miscalculated when size is zero, so we return here */
+	if (size == 0)
+		return;
+
+	addr = (void *)(start_addr & ~(dlsize - 1));
+	aend = (void *)((start_addr + size - 1) & ~(dlsize - 1));
+
+	if (ilsize == dlsize) {
+		/* flush I-cache & D-cache simultaneously */
+		while (1) {
+			mips_cache(HIT_WRITEBACK_INV_D, addr);
+			mips_cache(HIT_INVALIDATE_I, addr);
+			if (addr == aend)
+				break;
+			addr += dlsize;
+		}
+		return;
+	}
+
+	/* flush D-cache */
+	while (1) {
+		mips_cache(HIT_WRITEBACK_INV_D, addr);
+		if (addr == aend)
+			break;
+		addr += dlsize;
+	}
+
+	/* flush I-cache */
+	addr = (void *)(start_addr & ~(ilsize - 1));
+	aend = (void *)((start_addr + size - 1) & ~(ilsize - 1));
+	while (1) {
+		mips_cache(HIT_INVALIDATE_I, addr);
+		if (addr == aend)
+			break;
+		addr += ilsize;
+	}
+}
+
+void flush_dcache_range(ulong start_addr, ulong stop)
+{
+	unsigned long lsize = dcache_line_size();
+	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
+	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
+
+	while (1) {
+		mips_cache(HIT_WRITEBACK_INV_D, addr);
+		if (addr == aend)
+			break;
+		addr += lsize;
+	}
+}
+
+void invalidate_dcache_range(ulong start_addr, ulong stop)
+{
+	unsigned long lsize = dcache_line_size();
+	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
+	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
+
+	while (1) {
+		mips_cache(HIT_INVALIDATE_D, addr);
+		if (addr == aend)
+			break;
+		addr += lsize;
+	}
+}
-- 
2.2.2

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

* [U-Boot] [PATCH 3/8] MIPS: unify cache initialization code
  2015-01-26 15:02 [U-Boot] [PATCH 0/8] MIPS cache code cleanup Paul Burton
  2015-01-26 15:02 ` [U-Boot] [PATCH 1/8] MIPS: avoid .set ISA for cache operations Paul Burton
  2015-01-26 15:02 ` [U-Boot] [PATCH 2/8] MIPS: unify cache maintenance functions Paul Burton
@ 2015-01-26 15:02 ` Paul Burton
  2015-01-26 15:02 ` [U-Boot] [PATCH 4/8] MIPS: refactor L1 cache config reads to a macro Paul Burton
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2015-01-26 15:02 UTC (permalink / raw)
  To: u-boot

The mips32 & mips64 cache initialization code differs only in that the
mips32 code supports reading the cache size from coprocessor 0 registers
at runtime. Move the more developed mips32 version to a common
arch/mips/lib/cache_init.S & remove the now-redundant mips64 version in
order to reduce duplication. The temporary registers used are shuffled
slightly in order to work for both mips32 & mips64 builds. The RA
register is defined differently to suit mips32 & mips64, but will be
removed by a later commit in the series after further cleanup.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
 arch/mips/cpu/mips32/Makefile                      |   3 +-
 arch/mips/cpu/mips64/Makefile                      |   2 +-
 arch/mips/cpu/mips64/cache.S                       | 213 ---------------------
 arch/mips/lib/Makefile                             |   1 +
 arch/mips/{cpu/mips32/cache.S => lib/cache_init.S} |  20 +-
 5 files changed, 15 insertions(+), 224 deletions(-)
 delete mode 100644 arch/mips/cpu/mips64/cache.S
 rename arch/mips/{cpu/mips32/cache.S => lib/cache_init.S} (96%)

diff --git a/arch/mips/cpu/mips32/Makefile b/arch/mips/cpu/mips32/Makefile
index fa82dd3..19d316a 100644
--- a/arch/mips/cpu/mips32/Makefile
+++ b/arch/mips/cpu/mips32/Makefile
@@ -6,7 +6,6 @@
 #
 
 extra-y	= start.o
-obj-y	= cache.o
-obj-y	+= cpu.o interrupts.o time.o
+obj-y	= cpu.o interrupts.o time.o
 
 obj-$(CONFIG_SOC_AU1X00) += au1x00/
diff --git a/arch/mips/cpu/mips64/Makefile b/arch/mips/cpu/mips64/Makefile
index 899c319..cb4db9c 100644
--- a/arch/mips/cpu/mips64/Makefile
+++ b/arch/mips/cpu/mips64/Makefile
@@ -6,4 +6,4 @@
 #
 
 extra-y	= start.o
-obj-y	= cpu.o interrupts.o time.o cache.o
+obj-y	= cpu.o interrupts.o time.o
diff --git a/arch/mips/cpu/mips64/cache.S b/arch/mips/cpu/mips64/cache.S
deleted file mode 100644
index 36d8688..0000000
--- a/arch/mips/cpu/mips64/cache.S
+++ /dev/null
@@ -1,213 +0,0 @@
-/*
- *  Cache-handling routined for MIPS CPUs
- *
- *  Copyright (c) 2003	Wolfgang Denk <wd@denx.de>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#include <asm-offsets.h>
-#include <config.h>
-#include <asm/asm.h>
-#include <asm/regdef.h>
-#include <asm/mipsregs.h>
-#include <asm/addrspace.h>
-#include <asm/cacheops.h>
-
-#define RA		t9
-
-/*
- * 16kB is the maximum size of instruction and data caches on MIPS 4K,
- * 64kB is on 4KE, 24K, 5K, etc. Set bigger size for convenience.
- *
- * Note that the above size is the maximum size of primary cache. U-Boot
- * doesn't have L2 cache support for now.
- */
-#define MIPS_MAX_CACHE_SIZE	0x10000
-
-#define INDEX_BASE	CKSEG0
-
-	.macro	cache_op op addr
-	.set	push
-	.set	noreorder
-	.set	mips3
-	cache	\op, 0(\addr)
-	.set	pop
-	.endm
-
-	.macro	f_fill64 dst, offset, val
-	LONG_S	\val, (\offset +  0 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset +  1 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset +  2 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset +  3 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset +  4 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset +  5 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset +  6 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset +  7 * LONGSIZE)(\dst)
-#if LONGSIZE == 4
-	LONG_S	\val, (\offset +  8 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset +  9 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset + 10 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset + 11 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset + 12 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset + 13 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset + 14 * LONGSIZE)(\dst)
-	LONG_S	\val, (\offset + 15 * LONGSIZE)(\dst)
-#endif
-	.endm
-
-/*
- * mips_init_icache(uint PRId, ulong icache_size, unchar icache_linesz)
- */
-LEAF(mips_init_icache)
-	blez		a1, 9f
-	mtc0		zero, CP0_TAGLO
-	/* clear tag to invalidate */
-	PTR_LI		t0, INDEX_BASE
-	PTR_ADDU	t1, t0, a1
-1:	cache_op	INDEX_STORE_TAG_I t0
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 1b
-	/* fill once, so data field parity is correct */
-	PTR_LI		t0, INDEX_BASE
-2:	cache_op	FILL t0
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 2b
-	/* invalidate again - prudent but not strictly neccessary */
-	PTR_LI		t0, INDEX_BASE
-1:	cache_op	INDEX_STORE_TAG_I t0
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 1b
-9:	jr		ra
-	END(mips_init_icache)
-
-/*
- * mips_init_dcache(uint PRId, ulong dcache_size, unchar dcache_linesz)
- */
-LEAF(mips_init_dcache)
-	blez		a1, 9f
-	mtc0		zero, CP0_TAGLO
-	/* clear all tags */
-	PTR_LI		t0, INDEX_BASE
-	PTR_ADDU	t1, t0, a1
-1:	cache_op	INDEX_STORE_TAG_D t0
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 1b
-	/* load from each line (in cached space) */
-	PTR_LI		t0, INDEX_BASE
-2:	LONG_L		zero, 0(t0)
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 2b
-	/* clear all tags */
-	PTR_LI		t0, INDEX_BASE
-1:	cache_op	INDEX_STORE_TAG_D t0
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 1b
-9:	jr		ra
-	END(mips_init_dcache)
-
-/*
- * mips_cache_reset - low level initialisation of the primary caches
- *
- * This routine initialises the primary caches to ensure that they have good
- * parity.  It must be called by the ROM before any cached locations are used
- * to prevent the possibility of data with bad parity being written to memory.
- *
- * To initialise the instruction cache it is essential that a source of data
- * 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.
- *
- * RETURNS: N/A
- *
- */
-NESTED(mips_cache_reset, 0, ra)
-	move	RA, ra
-	li	t2, CONFIG_SYS_ICACHE_SIZE
-	li	t3, CONFIG_SYS_DCACHE_SIZE
-	li	t8, CONFIG_SYS_CACHELINE_SIZE
-
-	li	v0, MIPS_MAX_CACHE_SIZE
-
-	/*
-	 * Now clear that much memory starting from zero.
-	 */
-	PTR_LI		a0, CKSEG1
-	PTR_ADDU	a1, a0, v0
-2:	PTR_ADDIU	a0, 64
-	f_fill64	a0, -64, zero
-	bne		a0, a1, 2b
-
-	/*
-	 * The caches are probably in an indeterminate state,
-	 * so we force good parity into them by doing an
-	 * invalidate, load/fill, invalidate for each line.
-	 */
-
-	/*
-	 * Assume bottom of RAM will generate good parity for the cache.
-	 */
-
-	/*
-	 * Initialize the I-cache first,
-	 */
-	move	a1, t2
-	move	a2, t8
-	PTR_LA	v1, mips_init_icache
-	jalr	v1
-
-	/*
-	 * then initialize D-cache.
-	 */
-	move	a1, t3
-	move	a2, t8
-	PTR_LA	v1, mips_init_dcache
-	jalr	v1
-
-	jr	RA
-	END(mips_cache_reset)
-
-/*
- * dcache_status - get cache status
- *
- * RETURNS: 0 - cache disabled; 1 - cache enabled
- *
- */
-LEAF(dcache_status)
-	mfc0	t0, CP0_CONFIG
-	li	t1, CONF_CM_UNCACHED
-	andi	t0, t0, CONF_CM_CMASK
-	move	v0, zero
-	beq	t0, t1, 2f
-	li	v0, 1
-2:	jr	ra
-	END(dcache_status)
-
-/*
- * dcache_disable - disable cache
- *
- * RETURNS: N/A
- *
- */
-LEAF(dcache_disable)
-	mfc0	t0, CP0_CONFIG
-	li	t1, -8
-	and	t0, t0, t1
-	ori	t0, t0, CONF_CM_UNCACHED
-	mtc0	t0, CP0_CONFIG
-	jr	ra
-	END(dcache_disable)
-
-/*
- * dcache_enable - enable cache
- *
- * RETURNS: N/A
- *
- */
-LEAF(dcache_enable)
-	mfc0	t0, CP0_CONFIG
-	ori	t0, CONF_CM_CMASK
-	xori	t0, CONF_CM_CMASK
-	ori	t0, CONF_CM_CACHABLE_NONCOHERENT
-	mtc0	t0, CP0_CONFIG
-	jr	ra
-	END(dcache_enable)
diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
index d939ee6..ac536da 100644
--- a/arch/mips/lib/Makefile
+++ b/arch/mips/lib/Makefile
@@ -6,6 +6,7 @@
 #
 
 obj-y	+= cache.o
+obj-y	+= cache_init.o
 obj-y	+= io.o
 
 obj-$(CONFIG_CMD_BOOTM) += bootm.o
diff --git a/arch/mips/cpu/mips32/cache.S b/arch/mips/lib/cache_init.S
similarity index 96%
rename from arch/mips/cpu/mips32/cache.S
rename to arch/mips/lib/cache_init.S
index fb1d84b..6c02bf9 100644
--- a/arch/mips/cpu/mips32/cache.S
+++ b/arch/mips/lib/cache_init.S
@@ -18,7 +18,11 @@
 #define CONFIG_SYS_MIPS_CACHE_MODE CONF_CM_CACHABLE_NONCOHERENT
 #endif
 
-#define RA		t9
+#ifdef CONFIG_64BIT
+# define RA		ta3
+#else
+# define RA		t7
+#endif
 
 #define INDEX_BASE	CKSEG0
 
@@ -117,7 +121,7 @@ NESTED(mips_cache_reset, 0, ra)
 #endif
 
 #ifdef CONFIG_SYS_CACHELINE_SIZE
-	li	t7, CONFIG_SYS_CACHELINE_SIZE
+	li	t9, CONFIG_SYS_CACHELINE_SIZE
 	li	t8, CONFIG_SYS_CACHELINE_SIZE
 #else
 	/* Detect I-cache line size. */
@@ -128,11 +132,11 @@ NESTED(mips_cache_reset, 0, ra)
 	sllv	t8, t6, t8
 
 1:	/* Detect D-cache line size. */
-	srl	t7, t5, MIPS_CONF1_DL_SHIFT
-	andi	t7, t7, (MIPS_CONF1_DL >> MIPS_CONF1_DL_SHIFT)
-	beqz	t7, 1f
+	srl	t9, t5, MIPS_CONF1_DL_SHIFT
+	andi	t9, t9, (MIPS_CONF1_DL >> MIPS_CONF1_DL_SHIFT)
+	beqz	t9, 1f
 	li	t6, 2
-	sllv	t7, t6, t7
+	sllv	t9, t6, t9
 1:
 #endif
 
@@ -168,7 +172,7 @@ NESTED(mips_cache_reset, 0, ra)
 	addi	t6, t6, 1
 	sllv	t4, t4, t6
 1:	/* At this point t4 == I-cache sets. */
-	mul	t3, t4, t7
+	mul	t3, t4, t9
 	srl	t6, t5, MIPS_CONF1_DA_SHIFT
 	andi	t6, t6, (MIPS_CONF1_DA >> MIPS_CONF1_DA_SHIFT)
 	addi	t6, t6, 1
@@ -219,7 +223,7 @@ NESTED(mips_cache_reset, 0, ra)
 	 * then initialize D-cache.
 	 */
 	move	a1, t3
-	move	a2, t7
+	move	a2, t9
 	PTR_LA	v1, mips_init_dcache
 	jalr	v1
 
-- 
2.2.2

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

* [U-Boot] [PATCH 4/8] MIPS: refactor L1 cache config reads to a macro
  2015-01-26 15:02 [U-Boot] [PATCH 0/8] MIPS cache code cleanup Paul Burton
                   ` (2 preceding siblings ...)
  2015-01-26 15:02 ` [U-Boot] [PATCH 3/8] MIPS: unify cache initialization code Paul Burton
@ 2015-01-26 15:02 ` Paul Burton
  2015-01-26 15:02 ` [U-Boot] [PATCH 5/8] MIPS: refactor cache loops " Paul Burton
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2015-01-26 15:02 UTC (permalink / raw)
  To: u-boot

Reduce duplication between reading the configuration of the L1 dcache &
icache by performing both using a macro which calculates the appropriate
line & cache sizes from the coprocessor 0 Config1 register.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
 arch/mips/lib/cache_init.S | 97 ++++++++++++++++++++--------------------------
 1 file changed, 41 insertions(+), 56 deletions(-)

diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S
index 6c02bf9..2e036d9 100644
--- a/arch/mips/lib/cache_init.S
+++ b/arch/mips/lib/cache_init.S
@@ -97,6 +97,43 @@ LEAF(mips_init_dcache)
 9:	jr		ra
 	END(mips_init_dcache)
 
+	.macro	l1_info		sz, line_sz, off
+	.set	push
+	.set	noat
+
+	mfc0	$1, CP0_CONFIG, 1
+
+	/* detect line size */
+	srl	\line_sz, $1, \off + MIPS_CONF1_DL_SHIFT - MIPS_CONF1_DA_SHIFT
+	andi	\line_sz, \line_sz, (MIPS_CONF1_DL >> MIPS_CONF1_DL_SHIFT)
+	move	\sz, zero
+	beqz	\line_sz, 10f
+	li	\sz, 2
+	sllv	\line_sz, \sz, \line_sz
+
+	/* detect associativity */
+	srl	\sz, $1, \off + MIPS_CONF1_DA_SHIFT - MIPS_CONF1_DA_SHIFT
+	andi	\sz, \sz, (MIPS_CONF1_DA >> MIPS_CONF1_DA_SHIFT)
+	addi	\sz, \sz, 1
+
+	/* sz *= line_sz */
+	mul	\sz, \sz, \line_sz
+
+	/* detect log32(sets) */
+	srl	$1, $1, \off + MIPS_CONF1_DS_SHIFT - MIPS_CONF1_DA_SHIFT
+	andi	$1, $1, (MIPS_CONF1_DS >> MIPS_CONF1_DS_SHIFT)
+	addiu	$1, $1, 1
+	andi	$1, $1, 0x7
+
+	/* sz <<= log32(sets) */
+	sllv	\sz, \sz, $1
+
+	/* sz *= 32 */
+	li	$1, 32
+	mul	\sz, \sz, $1
+10:
+	.set	pop
+	.endm
 /*
  * mips_cache_reset - low level initialisation of the primary caches
  *
@@ -114,70 +151,18 @@ LEAF(mips_init_dcache)
 NESTED(mips_cache_reset, 0, ra)
 	move	RA, ra
 
-#if !defined(CONFIG_SYS_ICACHE_SIZE) || !defined(CONFIG_SYS_DCACHE_SIZE) || \
-    !defined(CONFIG_SYS_CACHELINE_SIZE)
-	/* read Config1 for use below */
-	mfc0	t5, CP0_CONFIG, 1
-#endif
-
-#ifdef CONFIG_SYS_CACHELINE_SIZE
-	li	t9, CONFIG_SYS_CACHELINE_SIZE
-	li	t8, CONFIG_SYS_CACHELINE_SIZE
-#else
-	/* Detect I-cache line size. */
-	srl	t8, t5, MIPS_CONF1_IL_SHIFT
-	andi	t8, t8, (MIPS_CONF1_IL >> MIPS_CONF1_IL_SHIFT)
-	beqz	t8, 1f
-	li	t6, 2
-	sllv	t8, t6, t8
-
-1:	/* Detect D-cache line size. */
-	srl	t9, t5, MIPS_CONF1_DL_SHIFT
-	andi	t9, t9, (MIPS_CONF1_DL >> MIPS_CONF1_DL_SHIFT)
-	beqz	t9, 1f
-	li	t6, 2
-	sllv	t9, t6, t9
-1:
-#endif
-
 #ifdef CONFIG_SYS_ICACHE_SIZE
 	li	t2, CONFIG_SYS_ICACHE_SIZE
+	li	t8, CONFIG_SYS_CACHELINE_SIZE
 #else
-	/* Detect I-cache size. */
-	srl	t6, t5, MIPS_CONF1_IS_SHIFT
-	andi	t6, t6, (MIPS_CONF1_IS >> MIPS_CONF1_IS_SHIFT)
-	li	t4, 32
-	xori	t2, t6, 0x7
-	beqz	t2, 1f
-	addi	t6, t6, 1
-	sllv	t4, t4, t6
-1:	/* At this point t4 == I-cache sets. */
-	mul	t2, t4, t8
-	srl	t6, t5, MIPS_CONF1_IA_SHIFT
-	andi	t6, t6, (MIPS_CONF1_IA >> MIPS_CONF1_IA_SHIFT)
-	addi	t6, t6, 1
-	/* At this point t6 == I-cache ways. */
-	mul	t2, t2, t6
+	l1_info	t2, t8, MIPS_CONF1_IA_SHIFT
 #endif
 
 #ifdef CONFIG_SYS_DCACHE_SIZE
 	li	t3, CONFIG_SYS_DCACHE_SIZE
+	li	t9, CONFIG_SYS_CACHELINE_SIZE
 #else
-	/* Detect D-cache size. */
-	srl	t6, t5, MIPS_CONF1_DS_SHIFT
-	andi	t6, t6, (MIPS_CONF1_DS >> MIPS_CONF1_DS_SHIFT)
-	li	t4, 32
-	xori	t3, t6, 0x7
-	beqz	t3, 1f
-	addi	t6, t6, 1
-	sllv	t4, t4, t6
-1:	/* At this point t4 == I-cache sets. */
-	mul	t3, t4, t9
-	srl	t6, t5, MIPS_CONF1_DA_SHIFT
-	andi	t6, t6, (MIPS_CONF1_DA >> MIPS_CONF1_DA_SHIFT)
-	addi	t6, t6, 1
-	/* At this point t6 == I-cache ways. */
-	mul	t3, t3, t6
+	l1_info	t3, t9, MIPS_CONF1_DA_SHIFT
 #endif
 
 	/* Determine the largest L1 cache size */
-- 
2.2.2

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

* [U-Boot] [PATCH 5/8] MIPS: refactor cache loops to a macro
  2015-01-26 15:02 [U-Boot] [PATCH 0/8] MIPS cache code cleanup Paul Burton
                   ` (3 preceding siblings ...)
  2015-01-26 15:02 ` [U-Boot] [PATCH 4/8] MIPS: refactor L1 cache config reads to a macro Paul Burton
@ 2015-01-26 15:02 ` Paul Burton
  2015-01-26 15:02 ` [U-Boot] [PATCH 6/8] MIPS: inline mips_init_[id]cache functions Paul Burton
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2015-01-26 15:02 UTC (permalink / raw)
  To: u-boot

Reduce duplication by performing loops through cache tags using an
assembler macro.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
 arch/mips/lib/cache_init.S | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S
index 2e036d9..dc207a6 100644
--- a/arch/mips/lib/cache_init.S
+++ b/arch/mips/lib/cache_init.S
@@ -47,28 +47,28 @@
 #endif
 	.endm
 
+	.macro cache_loop	curr, end, line_sz, op
+10:	cache		\op, 0(\curr)
+	PTR_ADDU	\curr, \curr, \line_sz
+	bne		\curr, \end, 10b
+	.endm
+
 /*
  * mips_init_icache(uint PRId, ulong icache_size, unchar icache_linesz)
  */
 LEAF(mips_init_icache)
 	blez		a1, 9f
 	mtc0		zero, CP0_TAGLO
-	/* clear tag to invalidate */
 	PTR_LI		t0, INDEX_BASE
 	PTR_ADDU	t1, t0, a1
-1:	cache		INDEX_STORE_TAG_I, 0(t0)
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 1b
+	/* clear tag to invalidate */
+	cache_loop	t0, t1, a2, INDEX_STORE_TAG_I
 	/* fill once, so data field parity is correct */
 	PTR_LI		t0, INDEX_BASE
-2:	cache		FILL, 0(t0)
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 2b
+	cache_loop	t0, t1, a2, FILL
 	/* invalidate again - prudent but not strictly neccessary */
 	PTR_LI		t0, INDEX_BASE
-1:	cache		INDEX_STORE_TAG_I, 0(t0)
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 1b
+	cache_loop	t0, t1, a2, INDEX_STORE_TAG_I
 9:	jr		ra
 	END(mips_init_icache)
 
@@ -78,12 +78,10 @@ LEAF(mips_init_icache)
 LEAF(mips_init_dcache)
 	blez		a1, 9f
 	mtc0		zero, CP0_TAGLO
-	/* clear all tags */
 	PTR_LI		t0, INDEX_BASE
 	PTR_ADDU	t1, t0, a1
-1:	cache		INDEX_STORE_TAG_D, 0(t0)
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 1b
+	/* clear all tags */
+	cache_loop	t0, t1, a2, INDEX_STORE_TAG_D
 	/* load from each line (in cached space) */
 	PTR_LI		t0, INDEX_BASE
 2:	LONG_L		zero, 0(t0)
@@ -91,9 +89,7 @@ LEAF(mips_init_dcache)
 	bne		t0, t1, 2b
 	/* clear all tags */
 	PTR_LI		t0, INDEX_BASE
-1:	cache		INDEX_STORE_TAG_D, 0(t0)
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 1b
+	cache_loop	t0, t1, a2, INDEX_STORE_TAG_D
 9:	jr		ra
 	END(mips_init_dcache)
 
-- 
2.2.2

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

* [U-Boot] [PATCH 6/8] MIPS: inline mips_init_[id]cache functions
  2015-01-26 15:02 [U-Boot] [PATCH 0/8] MIPS cache code cleanup Paul Burton
                   ` (4 preceding siblings ...)
  2015-01-26 15:02 ` [U-Boot] [PATCH 5/8] MIPS: refactor cache loops " Paul Burton
@ 2015-01-26 15:02 ` Paul Burton
  2015-01-26 15:02 ` [U-Boot] [PATCH 7/8] MIPS: allow systems to skip loads during cache init Paul Burton
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2015-01-26 15:02 UTC (permalink / raw)
  To: u-boot

The mips_init_[id]cache functions are small & only called once from a
single callsite. Inlining them allows mips_cache_reset to avoid having
to bother moving arguments around & leaves it a leaf function which is
thus able to simply keep the return address live in the ra register
throughout, simplifying the code.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
 arch/mips/lib/cache_init.S | 86 +++++++++++++++-------------------------------
 1 file changed, 28 insertions(+), 58 deletions(-)

diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S
index dc207a6..cbd04bd 100644
--- a/arch/mips/lib/cache_init.S
+++ b/arch/mips/lib/cache_init.S
@@ -18,12 +18,6 @@
 #define CONFIG_SYS_MIPS_CACHE_MODE CONF_CM_CACHABLE_NONCOHERENT
 #endif
 
-#ifdef CONFIG_64BIT
-# define RA		ta3
-#else
-# define RA		t7
-#endif
-
 #define INDEX_BASE	CKSEG0
 
 	.macro	f_fill64 dst, offset, val
@@ -53,46 +47,6 @@
 	bne		\curr, \end, 10b
 	.endm
 
-/*
- * mips_init_icache(uint PRId, ulong icache_size, unchar icache_linesz)
- */
-LEAF(mips_init_icache)
-	blez		a1, 9f
-	mtc0		zero, CP0_TAGLO
-	PTR_LI		t0, INDEX_BASE
-	PTR_ADDU	t1, t0, a1
-	/* clear tag to invalidate */
-	cache_loop	t0, t1, a2, INDEX_STORE_TAG_I
-	/* fill once, so data field parity is correct */
-	PTR_LI		t0, INDEX_BASE
-	cache_loop	t0, t1, a2, FILL
-	/* invalidate again - prudent but not strictly neccessary */
-	PTR_LI		t0, INDEX_BASE
-	cache_loop	t0, t1, a2, INDEX_STORE_TAG_I
-9:	jr		ra
-	END(mips_init_icache)
-
-/*
- * mips_init_dcache(uint PRId, ulong dcache_size, unchar dcache_linesz)
- */
-LEAF(mips_init_dcache)
-	blez		a1, 9f
-	mtc0		zero, CP0_TAGLO
-	PTR_LI		t0, INDEX_BASE
-	PTR_ADDU	t1, t0, a1
-	/* clear all tags */
-	cache_loop	t0, t1, a2, INDEX_STORE_TAG_D
-	/* load from each line (in cached space) */
-	PTR_LI		t0, INDEX_BASE
-2:	LONG_L		zero, 0(t0)
-	PTR_ADDU	t0, a2
-	bne		t0, t1, 2b
-	/* clear all tags */
-	PTR_LI		t0, INDEX_BASE
-	cache_loop	t0, t1, a2, INDEX_STORE_TAG_D
-9:	jr		ra
-	END(mips_init_dcache)
-
 	.macro	l1_info		sz, line_sz, off
 	.set	push
 	.set	noat
@@ -144,9 +98,7 @@ LEAF(mips_init_dcache)
  * RETURNS: N/A
  *
  */
-NESTED(mips_cache_reset, 0, ra)
-	move	RA, ra
-
+LEAF(mips_cache_reset)
 #ifdef CONFIG_SYS_ICACHE_SIZE
 	li	t2, CONFIG_SYS_ICACHE_SIZE
 	li	t8, CONFIG_SYS_CACHELINE_SIZE
@@ -195,20 +147,38 @@ NESTED(mips_cache_reset, 0, ra)
 	/*
 	 * Initialize the I-cache first,
 	 */
-	move	a1, t2
-	move	a2, t8
-	PTR_LA	v1, mips_init_icache
-	jalr	v1
+	blez		t2, 1f
+	mtc0		zero, CP0_TAGLO
+	PTR_LI		t0, INDEX_BASE
+	PTR_ADDU	t1, t0, t2
+	/* clear tag to invalidate */
+	cache_loop	t0, t1, t8, INDEX_STORE_TAG_I
+	/* fill once, so data field parity is correct */
+	PTR_LI		t0, INDEX_BASE
+	cache_loop	t0, t1, t8, FILL
+	/* invalidate again - prudent but not strictly neccessary */
+	PTR_LI		t0, INDEX_BASE
+	cache_loop	t0, t1, t8, INDEX_STORE_TAG_I
 
 	/*
 	 * then initialize D-cache.
 	 */
-	move	a1, t3
-	move	a2, t9
-	PTR_LA	v1, mips_init_dcache
-	jalr	v1
+1:	blez		t3, 3f
+	mtc0		zero, CP0_TAGLO
+	PTR_LI		t0, INDEX_BASE
+	PTR_ADDU	t1, t0, t3
+	/* clear all tags */
+	cache_loop	t0, t1, t9, INDEX_STORE_TAG_D
+	/* load from each line (in cached space) */
+	PTR_LI		t0, INDEX_BASE
+2:	LONG_L		zero, 0(t0)
+	PTR_ADDU	t0, t9
+	bne		t0, t1, 2b
+	/* clear all tags */
+	PTR_LI		t0, INDEX_BASE
+	cache_loop	t0, t1, t9, INDEX_STORE_TAG_D
 
-	jr	RA
+3:	jr	ra
 	END(mips_cache_reset)
 
 /*
-- 
2.2.2

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

* [U-Boot] [PATCH 7/8] MIPS: allow systems to skip loads during cache init
  2015-01-26 15:02 [U-Boot] [PATCH 0/8] MIPS cache code cleanup Paul Burton
                   ` (5 preceding siblings ...)
  2015-01-26 15:02 ` [U-Boot] [PATCH 6/8] MIPS: inline mips_init_[id]cache functions Paul Burton
@ 2015-01-26 15:02 ` Paul Burton
  2015-01-26 15:03 ` [U-Boot] [PATCH 8/8] MIPS: clear TagLo select 2 " Paul Burton
  2015-01-28 20:31 ` [U-Boot] [PATCH 0/8] MIPS cache code cleanup Daniel Schwierzeck
  8 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2015-01-26 15:02 UTC (permalink / raw)
  To: u-boot

Current MIPS systems do not require that loads be performed to force the
parity of cache lines, a simple invalidate by clearing the tag for each
line will suffice. Thus this patch makes the loads & subsequent second
invalidation conditional upon the CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD
option, and defines that for existing mips32 targets. Exceptions are
malta where this is known to be unnecessary, and qemu-mips where caches
are not implemented.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
 arch/mips/Kconfig          |  6 ++++++
 arch/mips/lib/cache_init.S | 19 +++++++++++++------
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index ef78929..bc4283d 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -36,6 +36,7 @@ config TARGET_VCT
 	select SUPPORTS_BIG_ENDIAN
 	select SUPPORTS_CPU_MIPS32_R1
 	select SUPPORTS_CPU_MIPS32_R2
+	select SYS_MIPS_CACHE_INIT_RAM_LOAD
 
 config TARGET_DBAU1X00
 	bool "Support dbau1x00"
@@ -43,12 +44,14 @@ config TARGET_DBAU1X00
 	select SUPPORTS_LITTLE_ENDIAN
 	select SUPPORTS_CPU_MIPS32_R1
 	select SUPPORTS_CPU_MIPS32_R2
+	select SYS_MIPS_CACHE_INIT_RAM_LOAD
 
 config TARGET_PB1X00
 	bool "Support pb1x00"
 	select SUPPORTS_LITTLE_ENDIAN
 	select SUPPORTS_CPU_MIPS32_R1
 	select SUPPORTS_CPU_MIPS32_R2
+	select SYS_MIPS_CACHE_INIT_RAM_LOAD
 
 
 endchoice
@@ -185,6 +188,9 @@ config 64BIT
 config SWAP_IO_SPACE
 	bool
 
+config SYS_MIPS_CACHE_INIT_RAM_LOAD
+	bool
+
 endif
 
 endmenu
diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S
index cbd04bd..04a36b2 100644
--- a/arch/mips/lib/cache_init.S
+++ b/arch/mips/lib/cache_init.S
@@ -113,6 +113,8 @@ LEAF(mips_cache_reset)
 	l1_info	t3, t9, MIPS_CONF1_DA_SHIFT
 #endif
 
+#ifdef CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD
+
 	/* Determine the largest L1 cache size */
 #if defined(CONFIG_SYS_ICACHE_SIZE) && defined(CONFIG_SYS_DCACHE_SIZE)
 #if CONFIG_SYS_ICACHE_SIZE > CONFIG_SYS_DCACHE_SIZE
@@ -134,14 +136,15 @@ LEAF(mips_cache_reset)
 	f_fill64	a0, -64, zero
 	bne		a0, a1, 2b
 
-	/*
-	 * The caches are probably in an indeterminate state,
-	 * so we force good parity into them by doing an
-	 * invalidate, load/fill, invalidate for each line.
-	 */
+#endif /* CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD */
 
 	/*
-	 * Assume bottom of RAM will generate good parity for the cache.
+	 * The caches are probably in an indeterminate state, so we force good
+	 * parity into them by doing an invalidate for each line. If
+	 * CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD is set then we'll proceed to
+	 * perform a load/fill & a further invalidate for each line, assuming
+	 * that the bottom of RAM (having just been cleared) will generate good
+	 * parity for the cache.
 	 */
 
 	/*
@@ -153,12 +156,14 @@ LEAF(mips_cache_reset)
 	PTR_ADDU	t1, t0, t2
 	/* clear tag to invalidate */
 	cache_loop	t0, t1, t8, 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
 	/* invalidate again - prudent but not strictly neccessary */
 	PTR_LI		t0, INDEX_BASE
 	cache_loop	t0, t1, t8, INDEX_STORE_TAG_I
+#endif
 
 	/*
 	 * then initialize D-cache.
@@ -169,6 +174,7 @@ LEAF(mips_cache_reset)
 	PTR_ADDU	t1, t0, t3
 	/* clear all tags */
 	cache_loop	t0, t1, t9, 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)
@@ -177,6 +183,7 @@ LEAF(mips_cache_reset)
 	/* clear all tags */
 	PTR_LI		t0, INDEX_BASE
 	cache_loop	t0, t1, t9, INDEX_STORE_TAG_D
+#endif
 
 3:	jr	ra
 	END(mips_cache_reset)
-- 
2.2.2

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

* [U-Boot] [PATCH 8/8] MIPS: clear TagLo select 2 during cache init
  2015-01-26 15:02 [U-Boot] [PATCH 0/8] MIPS cache code cleanup Paul Burton
                   ` (6 preceding siblings ...)
  2015-01-26 15:02 ` [U-Boot] [PATCH 7/8] MIPS: allow systems to skip loads during cache init Paul Burton
@ 2015-01-26 15:03 ` Paul Burton
  2015-01-28 20:57   ` Daniel Schwierzeck
  2015-01-28 20:31 ` [U-Boot] [PATCH 0/8] MIPS cache code cleanup Daniel Schwierzeck
  8 siblings, 1 reply; 17+ messages in thread
From: Paul Burton @ 2015-01-26 15:03 UTC (permalink / raw)
  To: u-boot

Current MIPS cores from Imagination Technologies use TagLo select 2 for
the data cache. The architecture requires that it is safe for software
to write to this register even if it isn't present, so take the trivial
option of clearing both selects 0 & 2.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
 arch/mips/lib/cache_init.S | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S
index 04a36b2..137d728 100644
--- a/arch/mips/lib/cache_init.S
+++ b/arch/mips/lib/cache_init.S
@@ -139,6 +139,14 @@ LEAF(mips_cache_reset)
 #endif /* CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD */
 
 	/*
+	 * 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.
+	 */
+	mtc0		zero, CP0_TAGLO
+	mtc0		zero, CP0_TAGLO, 2
+
+	/*
 	 * The caches are probably in an indeterminate state, so we force good
 	 * parity into them by doing an invalidate for each line. If
 	 * CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD is set then we'll proceed to
@@ -151,7 +159,6 @@ LEAF(mips_cache_reset)
 	 * Initialize the I-cache first,
 	 */
 	blez		t2, 1f
-	mtc0		zero, CP0_TAGLO
 	PTR_LI		t0, INDEX_BASE
 	PTR_ADDU	t1, t0, t2
 	/* clear tag to invalidate */
@@ -169,7 +176,6 @@ LEAF(mips_cache_reset)
 	 * then initialize D-cache.
 	 */
 1:	blez		t3, 3f
-	mtc0		zero, CP0_TAGLO
 	PTR_LI		t0, INDEX_BASE
 	PTR_ADDU	t1, t0, t3
 	/* clear all tags */
-- 
2.2.2

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

* [U-Boot] [PATCH 0/8] MIPS cache code cleanup
  2015-01-26 15:02 [U-Boot] [PATCH 0/8] MIPS cache code cleanup Paul Burton
                   ` (7 preceding siblings ...)
  2015-01-26 15:03 ` [U-Boot] [PATCH 8/8] MIPS: clear TagLo select 2 " Paul Burton
@ 2015-01-28 20:31 ` Daniel Schwierzeck
  2015-01-28 21:05   ` Paul Burton
  8 siblings, 1 reply; 17+ messages in thread
From: Daniel Schwierzeck @ 2015-01-28 20:31 UTC (permalink / raw)
  To: u-boot

Hi Paul,

Am 26.01.2015 um 16:02 schrieb Paul Burton:
> This series cleans up the MIPS cache code somewhat, and unifies the
> mips32 & mips64 implementations of it. This is largely in preparation
> for further patches adding L2 cache support. The final patch of this
> series fixes a bug encountered with recent cores on Malta boards.

thanks for doing this. I also have this on my to-do list.

Just a thought: if we finally have just the mips_cache_reset function in
cache_init.S, couldn't we reimplement it in C entirely? I see no reason
to implement it in assembler.

> 
> Paul Burton (8):
>   MIPS: avoid .set ISA for cache operations
>   MIPS: unify cache maintenance functions
>   MIPS: unify cache initialization code
>   MIPS: refactor L1 cache config reads to a macro
>   MIPS: refactor cache loops to a macro
>   MIPS: inline mips_init_[id]cache functions
>   MIPS: allow systems to skip loads during cache init
>   MIPS: clear TagLo select 2 during cache init
> 
>  arch/mips/Kconfig                                  |   6 +
>  arch/mips/cpu/mips32/Makefile                      |   3 +-
>  arch/mips/cpu/mips32/cpu.c                         | 119 -----------
>  arch/mips/cpu/mips64/Makefile                      |   2 +-
>  arch/mips/cpu/mips64/cache.S                       | 213 --------------------
>  arch/mips/cpu/mips64/cpu.c                         |  58 ------
>  arch/mips/include/asm/cacheops.h                   |   7 +
>  arch/mips/lib/Makefile                             |   2 +
>  arch/mips/lib/cache.c                              | 118 +++++++++++
>  arch/mips/{cpu/mips32/cache.S => lib/cache_init.S} | 222 +++++++++------------
>  10 files changed, 226 insertions(+), 524 deletions(-)
>  delete mode 100644 arch/mips/cpu/mips64/cache.S
>  create mode 100644 arch/mips/lib/cache.c
>  rename arch/mips/{cpu/mips32/cache.S => lib/cache_init.S} (59%)
> 

-- 
- Daniel

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

* [U-Boot] [PATCH 1/8] MIPS: avoid .set ISA for cache operations
  2015-01-26 15:02 ` [U-Boot] [PATCH 1/8] MIPS: avoid .set ISA for cache operations Paul Burton
@ 2015-01-28 20:43   ` Daniel Schwierzeck
  2015-01-28 21:08     ` Paul Burton
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Schwierzeck @ 2015-01-28 20:43 UTC (permalink / raw)
  To: u-boot



Am 26.01.2015 um 16:02 schrieb Paul Burton:
> As a step towards unifying the cache maintenance code for mips32 &
> mips64 CPUs, stop using ".set <ISA>" directives in the more developed
> mips32 version of the code. Instead, when present make use of the GCC
> builtin for emitting a cache instruction. When not present, simply don't
> bother with the .set directives since U-boot always builds with
> -march=mips32 or higher anyway.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> ---
>  arch/mips/cpu/mips32/cache.S     | 18 +++++-------------
>  arch/mips/cpu/mips32/cpu.c       | 40 +++++++++++++++-------------------------
>  arch/mips/include/asm/cacheops.h |  7 +++++++
>  3 files changed, 27 insertions(+), 38 deletions(-)
> 
> diff --git a/arch/mips/cpu/mips32/cache.S b/arch/mips/cpu/mips32/cache.S
> index 22bd844..fb1d84b 100644
> --- a/arch/mips/cpu/mips32/cache.S
> +++ b/arch/mips/cpu/mips32/cache.S
> @@ -22,14 +22,6 @@
>  
>  #define INDEX_BASE	CKSEG0
>  
> -	.macro	cache_op op addr
> -	.set	push
> -	.set	noreorder
> -	.set	mips3
> -	cache	\op, 0(\addr)
> -	.set	pop
> -	.endm
> -
>  	.macro	f_fill64 dst, offset, val
>  	LONG_S	\val, (\offset +  0 * LONGSIZE)(\dst)
>  	LONG_S	\val, (\offset +  1 * LONGSIZE)(\dst)
> @@ -60,17 +52,17 @@ LEAF(mips_init_icache)
>  	/* clear tag to invalidate */
>  	PTR_LI		t0, INDEX_BASE
>  	PTR_ADDU	t1, t0, a1
> -1:	cache_op	INDEX_STORE_TAG_I t0
> +1:	cache		INDEX_STORE_TAG_I, 0(t0)
>  	PTR_ADDU	t0, a2
>  	bne		t0, t1, 1b
>  	/* fill once, so data field parity is correct */
>  	PTR_LI		t0, INDEX_BASE
> -2:	cache_op	FILL t0
> +2:	cache		FILL, 0(t0)
>  	PTR_ADDU	t0, a2
>  	bne		t0, t1, 2b
>  	/* invalidate again - prudent but not strictly neccessary */
>  	PTR_LI		t0, INDEX_BASE
> -1:	cache_op	INDEX_STORE_TAG_I t0
> +1:	cache		INDEX_STORE_TAG_I, 0(t0)
>  	PTR_ADDU	t0, a2
>  	bne		t0, t1, 1b
>  9:	jr		ra
> @@ -85,7 +77,7 @@ LEAF(mips_init_dcache)
>  	/* clear all tags */
>  	PTR_LI		t0, INDEX_BASE
>  	PTR_ADDU	t1, t0, a1
> -1:	cache_op	INDEX_STORE_TAG_D t0
> +1:	cache		INDEX_STORE_TAG_D, 0(t0)
>  	PTR_ADDU	t0, a2
>  	bne		t0, t1, 1b
>  	/* load from each line (in cached space) */
> @@ -95,7 +87,7 @@ LEAF(mips_init_dcache)
>  	bne		t0, t1, 2b
>  	/* clear all tags */
>  	PTR_LI		t0, INDEX_BASE
> -1:	cache_op	INDEX_STORE_TAG_D t0
> +1:	cache		INDEX_STORE_TAG_D, 0(t0)
>  	PTR_ADDU	t0, a2
>  	bne		t0, t1, 1b
>  9:	jr		ra
> diff --git a/arch/mips/cpu/mips32/cpu.c b/arch/mips/cpu/mips32/cpu.c
> index 278865b..3f247fb 100644
> --- a/arch/mips/cpu/mips32/cpu.c
> +++ b/arch/mips/cpu/mips32/cpu.c
> @@ -12,16 +12,6 @@
>  #include <asm/cacheops.h>
>  #include <asm/reboot.h>
>  
> -#define cache_op(op,addr)						\
> -	__asm__ __volatile__(						\
> -	"	.set	push					\n"	\
> -	"	.set	noreorder				\n"	\
> -	"	.set	mips3\n\t				\n"	\
> -	"	cache	%0, %1					\n"	\
> -	"	.set	pop					\n"	\
> -	:								\
> -	: "i" (op), "R" (*(unsigned char *)(addr)))
> -
>  void __attribute__((weak)) _machine_restart(void)
>  {
>  }
> @@ -74,20 +64,20 @@ void flush_cache(ulong start_addr, ulong size)
>  {
>  	unsigned long ilsize = icache_line_size();
>  	unsigned long dlsize = dcache_line_size();
> -	unsigned long addr, aend;
> +	const volatile void *addr, *aend;

why do you need volatile?

>  
>  	/* aend will be miscalculated when size is zero, so we return here */
>  	if (size == 0)
>  		return;
>  
> -	addr = start_addr & ~(dlsize - 1);
> -	aend = (start_addr + size - 1) & ~(dlsize - 1);
> +	addr = (void *)(start_addr & ~(dlsize - 1));
> +	aend = (void *)((start_addr + size - 1) & ~(dlsize - 1));

shouldn't that be (const void *) ?

>  
>  	if (ilsize == dlsize) {
>  		/* flush I-cache & D-cache simultaneously */
>  		while (1) {
> -			cache_op(HIT_WRITEBACK_INV_D, addr);
> -			cache_op(HIT_INVALIDATE_I, addr);
> +			mips_cache(HIT_WRITEBACK_INV_D, addr);
> +			mips_cache(HIT_INVALIDATE_I, addr);
>  			if (addr == aend)
>  				break;
>  			addr += dlsize;
> @@ -97,17 +87,17 @@ void flush_cache(ulong start_addr, ulong size)
>  
>  	/* flush D-cache */
>  	while (1) {
> -		cache_op(HIT_WRITEBACK_INV_D, addr);
> +		mips_cache(HIT_WRITEBACK_INV_D, addr);
>  		if (addr == aend)
>  			break;
>  		addr += dlsize;
>  	}
>  
>  	/* flush I-cache */
> -	addr = start_addr & ~(ilsize - 1);
> -	aend = (start_addr + size - 1) & ~(ilsize - 1);
> +	addr = (void *)(start_addr & ~(ilsize - 1));
> +	aend = (void *)((start_addr + size - 1) & ~(ilsize - 1));
>  	while (1) {
> -		cache_op(HIT_INVALIDATE_I, addr);
> +		mips_cache(HIT_INVALIDATE_I, addr);
>  		if (addr == aend)
>  			break;
>  		addr += ilsize;
> @@ -117,11 +107,11 @@ 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 addr = start_addr & ~(lsize - 1);
> -	unsigned long aend = (stop - 1) & ~(lsize - 1);
> +	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
> +	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
>  
>  	while (1) {
> -		cache_op(HIT_WRITEBACK_INV_D, addr);
> +		mips_cache(HIT_WRITEBACK_INV_D, addr);
>  		if (addr == aend)
>  			break;
>  		addr += lsize;
> @@ -131,11 +121,11 @@ void flush_dcache_range(ulong start_addr, ulong stop)
>  void invalidate_dcache_range(ulong start_addr, ulong stop)
>  {
>  	unsigned long lsize = dcache_line_size();
> -	unsigned long addr = start_addr & ~(lsize - 1);
> -	unsigned long aend = (stop - 1) & ~(lsize - 1);
> +	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
> +	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
>  
>  	while (1) {
> -		cache_op(HIT_INVALIDATE_D, addr);
> +		mips_cache(HIT_INVALIDATE_D, addr);
>  		if (addr == aend)
>  			break;
>  		addr += lsize;
> diff --git a/arch/mips/include/asm/cacheops.h b/arch/mips/include/asm/cacheops.h
> index 6464250..809c966 100644
> --- a/arch/mips/include/asm/cacheops.h
> +++ b/arch/mips/include/asm/cacheops.h
> @@ -11,6 +11,13 @@
>  #ifndef	__ASM_CACHEOPS_H
>  #define	__ASM_CACHEOPS_H
>  
> +#ifdef __GCC_HAVE_BUILTIN_MIPS_CACHE
> +# define mips_cache __builtin_mips_cache
> +#else
> +# define mips_cache(op,addr) \

space after op,

> +	__asm__ __volatile__("cache	%0, %1" : : "i"(op), "R"(addr))
> +#endif
> +
>  /*
>   * Cache Operations available on all MIPS processors with R4000-style caches
>   */
> 

-- 
- Daniel

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

* [U-Boot] [PATCH 8/8] MIPS: clear TagLo select 2 during cache init
  2015-01-26 15:03 ` [U-Boot] [PATCH 8/8] MIPS: clear TagLo select 2 " Paul Burton
@ 2015-01-28 20:57   ` Daniel Schwierzeck
  2015-01-28 21:13     ` Paul Burton
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Schwierzeck @ 2015-01-28 20:57 UTC (permalink / raw)
  To: u-boot



Am 26.01.2015 um 16:03 schrieb Paul Burton:
> Current MIPS cores from Imagination Technologies use TagLo select 2 for
> the data cache. The architecture requires that it is safe for software
> to write to this register even if it isn't present, so take the trivial
> option of clearing both selects 0 & 2.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> ---
>  arch/mips/lib/cache_init.S | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S
> index 04a36b2..137d728 100644
> --- a/arch/mips/lib/cache_init.S
> +++ b/arch/mips/lib/cache_init.S
> @@ -139,6 +139,14 @@ LEAF(mips_cache_reset)
>  #endif /* CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD */
>  
>  	/*
> +	 * 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.
> +	 */
> +	mtc0		zero, CP0_TAGLO
> +	mtc0		zero, CP0_TAGLO, 2

maybe we should add new definitions to asm/mipsregs.h. In an out-of-tree
SoC port I have something like this

 #define CP0_ECC $26
 #define CP0_CACHEERR $27
 #define CP0_TAGLO $28
+#define CP0_ITAGLO $28
+#define CP0_IDATALO $28,1
+#define CP0_DTAGLO $28,2
+#define CP0_DDATALO $28,3
+#define CP0_L23TAGLO $28,4
+#define CP0_L23DATALO $28,5
 #define CP0_TAGHI $29
+#define CP0_IDATAHI $29,1
+#define CP0_DTAGHI $29,2
+#define CP0_L23TAGHI $29,4
+#define CP0_L23DATAHI $29,5
 #define CP0_ERROREPC $30
 #define CP0_DESAVE $31


> +
> +	/*
>  	 * The caches are probably in an indeterminate state, so we force good
>  	 * parity into them by doing an invalidate for each line. If
>  	 * CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD is set then we'll proceed to
> @@ -151,7 +159,6 @@ LEAF(mips_cache_reset)
>  	 * Initialize the I-cache first,
>  	 */
>  	blez		t2, 1f
> -	mtc0		zero, CP0_TAGLO
>  	PTR_LI		t0, INDEX_BASE
>  	PTR_ADDU	t1, t0, t2
>  	/* clear tag to invalidate */
> @@ -169,7 +176,6 @@ LEAF(mips_cache_reset)
>  	 * then initialize D-cache.
>  	 */
>  1:	blez		t3, 3f
> -	mtc0		zero, CP0_TAGLO
>  	PTR_LI		t0, INDEX_BASE
>  	PTR_ADDU	t1, t0, t3
>  	/* clear all tags */
> 

-- 
- Daniel

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

* [U-Boot] [PATCH 0/8] MIPS cache code cleanup
  2015-01-28 20:31 ` [U-Boot] [PATCH 0/8] MIPS cache code cleanup Daniel Schwierzeck
@ 2015-01-28 21:05   ` Paul Burton
       [not found]     ` <54C95453.8010009@gmail.com>
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Burton @ 2015-01-28 21:05 UTC (permalink / raw)
  To: u-boot

On Wed, Jan 28, 2015 at 09:31:25PM +0100, Daniel Schwierzeck wrote:
> Hi Paul,
> 
> Am 26.01.2015 um 16:02 schrieb Paul Burton:
> > This series cleans up the MIPS cache code somewhat, and unifies the
> > mips32 & mips64 implementations of it. This is largely in preparation
> > for further patches adding L2 cache support. The final patch of this
> > series fixes a bug encountered with recent cores on Malta boards.
> 
> thanks for doing this. I also have this on my to-do list.
> 
> Just a thought: if we finally have just the mips_cache_reset function in
> cache_init.S, couldn't we reimplement it in C entirely? I see no reason
> to implement it in assembler.

The only issue with that is that mips_cache_reset currently runs before
we have a stack set up, so if the C compiler were to attempt to use the
stack things would fall over. I guess we could set up the temporary
stack earlier, and it's probably unlikely the compiler would actually
make use of it so it shouldn't really slow things down. Any chance we
could squeeze this into this merge window though, and I'll move the
cache init to C next time? That will simplify my L2 code somewhat too -
thanks for the suggestion!

Paul

> > 
> > Paul Burton (8):
> >   MIPS: avoid .set ISA for cache operations
> >   MIPS: unify cache maintenance functions
> >   MIPS: unify cache initialization code
> >   MIPS: refactor L1 cache config reads to a macro
> >   MIPS: refactor cache loops to a macro
> >   MIPS: inline mips_init_[id]cache functions
> >   MIPS: allow systems to skip loads during cache init
> >   MIPS: clear TagLo select 2 during cache init
> > 
> >  arch/mips/Kconfig                                  |   6 +
> >  arch/mips/cpu/mips32/Makefile                      |   3 +-
> >  arch/mips/cpu/mips32/cpu.c                         | 119 -----------
> >  arch/mips/cpu/mips64/Makefile                      |   2 +-
> >  arch/mips/cpu/mips64/cache.S                       | 213 --------------------
> >  arch/mips/cpu/mips64/cpu.c                         |  58 ------
> >  arch/mips/include/asm/cacheops.h                   |   7 +
> >  arch/mips/lib/Makefile                             |   2 +
> >  arch/mips/lib/cache.c                              | 118 +++++++++++
> >  arch/mips/{cpu/mips32/cache.S => lib/cache_init.S} | 222 +++++++++------------
> >  10 files changed, 226 insertions(+), 524 deletions(-)
> >  delete mode 100644 arch/mips/cpu/mips64/cache.S
> >  create mode 100644 arch/mips/lib/cache.c
> >  rename arch/mips/{cpu/mips32/cache.S => lib/cache_init.S} (59%)
> > 
> 
> -- 
> - Daniel

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

* [U-Boot] [PATCH 1/8] MIPS: avoid .set ISA for cache operations
  2015-01-28 20:43   ` Daniel Schwierzeck
@ 2015-01-28 21:08     ` Paul Burton
  2015-01-28 21:18       ` Daniel Schwierzeck
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Burton @ 2015-01-28 21:08 UTC (permalink / raw)
  To: u-boot

On Wed, Jan 28, 2015 at 09:43:25PM +0100, Daniel Schwierzeck wrote:
> 
> 
> Am 26.01.2015 um 16:02 schrieb Paul Burton:
> > As a step towards unifying the cache maintenance code for mips32 &
> > mips64 CPUs, stop using ".set <ISA>" directives in the more developed
> > mips32 version of the code. Instead, when present make use of the GCC
> > builtin for emitting a cache instruction. When not present, simply don't
> > bother with the .set directives since U-boot always builds with
> > -march=mips32 or higher anyway.
> > 
> > Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> > Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> > ---
> >  arch/mips/cpu/mips32/cache.S     | 18 +++++-------------
> >  arch/mips/cpu/mips32/cpu.c       | 40 +++++++++++++++-------------------------
> >  arch/mips/include/asm/cacheops.h |  7 +++++++
> >  3 files changed, 27 insertions(+), 38 deletions(-)
> > 
> > diff --git a/arch/mips/cpu/mips32/cache.S b/arch/mips/cpu/mips32/cache.S
> > index 22bd844..fb1d84b 100644
> > --- a/arch/mips/cpu/mips32/cache.S
> > +++ b/arch/mips/cpu/mips32/cache.S
> > @@ -22,14 +22,6 @@
> >  
> >  #define INDEX_BASE	CKSEG0
> >  
> > -	.macro	cache_op op addr
> > -	.set	push
> > -	.set	noreorder
> > -	.set	mips3
> > -	cache	\op, 0(\addr)
> > -	.set	pop
> > -	.endm
> > -
> >  	.macro	f_fill64 dst, offset, val
> >  	LONG_S	\val, (\offset +  0 * LONGSIZE)(\dst)
> >  	LONG_S	\val, (\offset +  1 * LONGSIZE)(\dst)
> > @@ -60,17 +52,17 @@ LEAF(mips_init_icache)
> >  	/* clear tag to invalidate */
> >  	PTR_LI		t0, INDEX_BASE
> >  	PTR_ADDU	t1, t0, a1
> > -1:	cache_op	INDEX_STORE_TAG_I t0
> > +1:	cache		INDEX_STORE_TAG_I, 0(t0)
> >  	PTR_ADDU	t0, a2
> >  	bne		t0, t1, 1b
> >  	/* fill once, so data field parity is correct */
> >  	PTR_LI		t0, INDEX_BASE
> > -2:	cache_op	FILL t0
> > +2:	cache		FILL, 0(t0)
> >  	PTR_ADDU	t0, a2
> >  	bne		t0, t1, 2b
> >  	/* invalidate again - prudent but not strictly neccessary */
> >  	PTR_LI		t0, INDEX_BASE
> > -1:	cache_op	INDEX_STORE_TAG_I t0
> > +1:	cache		INDEX_STORE_TAG_I, 0(t0)
> >  	PTR_ADDU	t0, a2
> >  	bne		t0, t1, 1b
> >  9:	jr		ra
> > @@ -85,7 +77,7 @@ LEAF(mips_init_dcache)
> >  	/* clear all tags */
> >  	PTR_LI		t0, INDEX_BASE
> >  	PTR_ADDU	t1, t0, a1
> > -1:	cache_op	INDEX_STORE_TAG_D t0
> > +1:	cache		INDEX_STORE_TAG_D, 0(t0)
> >  	PTR_ADDU	t0, a2
> >  	bne		t0, t1, 1b
> >  	/* load from each line (in cached space) */
> > @@ -95,7 +87,7 @@ LEAF(mips_init_dcache)
> >  	bne		t0, t1, 2b
> >  	/* clear all tags */
> >  	PTR_LI		t0, INDEX_BASE
> > -1:	cache_op	INDEX_STORE_TAG_D t0
> > +1:	cache		INDEX_STORE_TAG_D, 0(t0)
> >  	PTR_ADDU	t0, a2
> >  	bne		t0, t1, 1b
> >  9:	jr		ra
> > diff --git a/arch/mips/cpu/mips32/cpu.c b/arch/mips/cpu/mips32/cpu.c
> > index 278865b..3f247fb 100644
> > --- a/arch/mips/cpu/mips32/cpu.c
> > +++ b/arch/mips/cpu/mips32/cpu.c
> > @@ -12,16 +12,6 @@
> >  #include <asm/cacheops.h>
> >  #include <asm/reboot.h>
> >  
> > -#define cache_op(op,addr)						\
> > -	__asm__ __volatile__(						\
> > -	"	.set	push					\n"	\
> > -	"	.set	noreorder				\n"	\
> > -	"	.set	mips3\n\t				\n"	\
> > -	"	cache	%0, %1					\n"	\
> > -	"	.set	pop					\n"	\
> > -	:								\
> > -	: "i" (op), "R" (*(unsigned char *)(addr)))
> > -
> >  void __attribute__((weak)) _machine_restart(void)
> >  {
> >  }
> > @@ -74,20 +64,20 @@ void flush_cache(ulong start_addr, ulong size)
> >  {
> >  	unsigned long ilsize = icache_line_size();
> >  	unsigned long dlsize = dcache_line_size();
> > -	unsigned long addr, aend;
> > +	const volatile void *addr, *aend;
> 
> why do you need volatile?
> 

I was just reflecting the type of the argument to __mips_builtin_cache:

  https://gcc.gnu.org/onlinedocs/gcc/Other-MIPS-Built-in-Functions.html

> >  
> >  	/* aend will be miscalculated when size is zero, so we return here */
> >  	if (size == 0)
> >  		return;
> >  
> > -	addr = start_addr & ~(dlsize - 1);
> > -	aend = (start_addr + size - 1) & ~(dlsize - 1);
> > +	addr = (void *)(start_addr & ~(dlsize - 1));
> > +	aend = (void *)((start_addr + size - 1) & ~(dlsize - 1));
> 
> shouldn't that be (const void *) ?
> 

I don't think it really matters since the assignment is to a more
restricted type rather than from one, but I can change it if you like.

> >  
> >  	if (ilsize == dlsize) {
> >  		/* flush I-cache & D-cache simultaneously */
> >  		while (1) {
> > -			cache_op(HIT_WRITEBACK_INV_D, addr);
> > -			cache_op(HIT_INVALIDATE_I, addr);
> > +			mips_cache(HIT_WRITEBACK_INV_D, addr);
> > +			mips_cache(HIT_INVALIDATE_I, addr);
> >  			if (addr == aend)
> >  				break;
> >  			addr += dlsize;
> > @@ -97,17 +87,17 @@ void flush_cache(ulong start_addr, ulong size)
> >  
> >  	/* flush D-cache */
> >  	while (1) {
> > -		cache_op(HIT_WRITEBACK_INV_D, addr);
> > +		mips_cache(HIT_WRITEBACK_INV_D, addr);
> >  		if (addr == aend)
> >  			break;
> >  		addr += dlsize;
> >  	}
> >  
> >  	/* flush I-cache */
> > -	addr = start_addr & ~(ilsize - 1);
> > -	aend = (start_addr + size - 1) & ~(ilsize - 1);
> > +	addr = (void *)(start_addr & ~(ilsize - 1));
> > +	aend = (void *)((start_addr + size - 1) & ~(ilsize - 1));
> >  	while (1) {
> > -		cache_op(HIT_INVALIDATE_I, addr);
> > +		mips_cache(HIT_INVALIDATE_I, addr);
> >  		if (addr == aend)
> >  			break;
> >  		addr += ilsize;
> > @@ -117,11 +107,11 @@ 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 addr = start_addr & ~(lsize - 1);
> > -	unsigned long aend = (stop - 1) & ~(lsize - 1);
> > +	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
> > +	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
> >  
> >  	while (1) {
> > -		cache_op(HIT_WRITEBACK_INV_D, addr);
> > +		mips_cache(HIT_WRITEBACK_INV_D, addr);
> >  		if (addr == aend)
> >  			break;
> >  		addr += lsize;
> > @@ -131,11 +121,11 @@ void flush_dcache_range(ulong start_addr, ulong stop)
> >  void invalidate_dcache_range(ulong start_addr, ulong stop)
> >  {
> >  	unsigned long lsize = dcache_line_size();
> > -	unsigned long addr = start_addr & ~(lsize - 1);
> > -	unsigned long aend = (stop - 1) & ~(lsize - 1);
> > +	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
> > +	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
> >  
> >  	while (1) {
> > -		cache_op(HIT_INVALIDATE_D, addr);
> > +		mips_cache(HIT_INVALIDATE_D, addr);
> >  		if (addr == aend)
> >  			break;
> >  		addr += lsize;
> > diff --git a/arch/mips/include/asm/cacheops.h b/arch/mips/include/asm/cacheops.h
> > index 6464250..809c966 100644
> > --- a/arch/mips/include/asm/cacheops.h
> > +++ b/arch/mips/include/asm/cacheops.h
> > @@ -11,6 +11,13 @@
> >  #ifndef	__ASM_CACHEOPS_H
> >  #define	__ASM_CACHEOPS_H
> >  
> > +#ifdef __GCC_HAVE_BUILTIN_MIPS_CACHE
> > +# define mips_cache __builtin_mips_cache
> > +#else
> > +# define mips_cache(op,addr) \
> 
> space after op,
> 

Right you are! In my defence it was copied from the original
implementation in cpu.c ;)

Paul

> > +	__asm__ __volatile__("cache	%0, %1" : : "i"(op), "R"(addr))
> > +#endif
> > +
> >  /*
> >   * Cache Operations available on all MIPS processors with R4000-style caches
> >   */
> > 
> 
> -- 
> - Daniel

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

* [U-Boot] [PATCH 8/8] MIPS: clear TagLo select 2 during cache init
  2015-01-28 20:57   ` Daniel Schwierzeck
@ 2015-01-28 21:13     ` Paul Burton
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2015-01-28 21:13 UTC (permalink / raw)
  To: u-boot

On Wed, Jan 28, 2015 at 09:57:30PM +0100, Daniel Schwierzeck wrote:
> Am 26.01.2015 um 16:03 schrieb Paul Burton:
> > Current MIPS cores from Imagination Technologies use TagLo select 2 for
> > the data cache. The architecture requires that it is safe for software
> > to write to this register even if it isn't present, so take the trivial
> > option of clearing both selects 0 & 2.
> > 
> > Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> > Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> > ---
> >  arch/mips/lib/cache_init.S | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S
> > index 04a36b2..137d728 100644
> > --- a/arch/mips/lib/cache_init.S
> > +++ b/arch/mips/lib/cache_init.S
> > @@ -139,6 +139,14 @@ LEAF(mips_cache_reset)
> >  #endif /* CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD */
> >  
> >  	/*
> > +	 * 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.
> > +	 */
> > +	mtc0		zero, CP0_TAGLO
> > +	mtc0		zero, CP0_TAGLO, 2
> 
> maybe we should add new definitions to asm/mipsregs.h. In an out-of-tree
> SoC port I have something like this
> 
>  #define CP0_ECC $26
>  #define CP0_CACHEERR $27
>  #define CP0_TAGLO $28
> +#define CP0_ITAGLO $28
> +#define CP0_IDATALO $28,1
> +#define CP0_DTAGLO $28,2
> +#define CP0_DDATALO $28,3
> +#define CP0_L23TAGLO $28,4
> +#define CP0_L23DATALO $28,5
>  #define CP0_TAGHI $29
> +#define CP0_IDATAHI $29,1
> +#define CP0_DTAGHI $29,2
> +#define CP0_L23TAGHI $29,4
> +#define CP0_L23DATAHI $29,5
>  #define CP0_ERROREPC $30
>  #define CP0_DESAVE $31
> 

The problem with that is that the architecture doesn't actually define
which caches the various {Tag,Data}{Lo,Hi} registers apply to. So as far
as I can tell it would be perfectly valid for some CPU to implement
$28,0 operating on the Dcache & $28,2 operating on the Icache rather
than vice-versa.

Thanks,
    Paul

> > +
> > +	/*
> >  	 * The caches are probably in an indeterminate state, so we force good
> >  	 * parity into them by doing an invalidate for each line. If
> >  	 * CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD is set then we'll proceed to
> > @@ -151,7 +159,6 @@ LEAF(mips_cache_reset)
> >  	 * Initialize the I-cache first,
> >  	 */
> >  	blez		t2, 1f
> > -	mtc0		zero, CP0_TAGLO
> >  	PTR_LI		t0, INDEX_BASE
> >  	PTR_ADDU	t1, t0, t2
> >  	/* clear tag to invalidate */
> > @@ -169,7 +176,6 @@ LEAF(mips_cache_reset)
> >  	 * then initialize D-cache.
> >  	 */
> >  1:	blez		t3, 3f
> > -	mtc0		zero, CP0_TAGLO
> >  	PTR_LI		t0, INDEX_BASE
> >  	PTR_ADDU	t1, t0, t3
> >  	/* clear all tags */
> > 
> 
> -- 
> - Daniel

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

* [U-Boot] [PATCH 1/8] MIPS: avoid .set ISA for cache operations
  2015-01-28 21:08     ` Paul Burton
@ 2015-01-28 21:18       ` Daniel Schwierzeck
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Schwierzeck @ 2015-01-28 21:18 UTC (permalink / raw)
  To: u-boot



Am 28.01.2015 um 22:08 schrieb Paul Burton:
> On Wed, Jan 28, 2015 at 09:43:25PM +0100, Daniel Schwierzeck wrote:
>>
>>
>> Am 26.01.2015 um 16:02 schrieb Paul Burton:
>>> As a step towards unifying the cache maintenance code for mips32 &
>>> mips64 CPUs, stop using ".set <ISA>" directives in the more developed
>>> mips32 version of the code. Instead, when present make use of the GCC
>>> builtin for emitting a cache instruction. When not present, simply don't
>>> bother with the .set directives since U-boot always builds with
>>> -march=mips32 or higher anyway.
>>>
>>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>>> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>> ---
>>>  arch/mips/cpu/mips32/cache.S     | 18 +++++-------------
>>>  arch/mips/cpu/mips32/cpu.c       | 40 +++++++++++++++-------------------------
>>>  arch/mips/include/asm/cacheops.h |  7 +++++++
>>>  3 files changed, 27 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/arch/mips/cpu/mips32/cache.S b/arch/mips/cpu/mips32/cache.S
>>> index 22bd844..fb1d84b 100644
>>> --- a/arch/mips/cpu/mips32/cache.S
>>> +++ b/arch/mips/cpu/mips32/cache.S
>>> @@ -22,14 +22,6 @@
>>>  
>>>  #define INDEX_BASE	CKSEG0
>>>  
>>> -	.macro	cache_op op addr
>>> -	.set	push
>>> -	.set	noreorder
>>> -	.set	mips3
>>> -	cache	\op, 0(\addr)
>>> -	.set	pop
>>> -	.endm
>>> -
>>>  	.macro	f_fill64 dst, offset, val
>>>  	LONG_S	\val, (\offset +  0 * LONGSIZE)(\dst)
>>>  	LONG_S	\val, (\offset +  1 * LONGSIZE)(\dst)
>>> @@ -60,17 +52,17 @@ LEAF(mips_init_icache)
>>>  	/* clear tag to invalidate */
>>>  	PTR_LI		t0, INDEX_BASE
>>>  	PTR_ADDU	t1, t0, a1
>>> -1:	cache_op	INDEX_STORE_TAG_I t0
>>> +1:	cache		INDEX_STORE_TAG_I, 0(t0)
>>>  	PTR_ADDU	t0, a2
>>>  	bne		t0, t1, 1b
>>>  	/* fill once, so data field parity is correct */
>>>  	PTR_LI		t0, INDEX_BASE
>>> -2:	cache_op	FILL t0
>>> +2:	cache		FILL, 0(t0)
>>>  	PTR_ADDU	t0, a2
>>>  	bne		t0, t1, 2b
>>>  	/* invalidate again - prudent but not strictly neccessary */
>>>  	PTR_LI		t0, INDEX_BASE
>>> -1:	cache_op	INDEX_STORE_TAG_I t0
>>> +1:	cache		INDEX_STORE_TAG_I, 0(t0)
>>>  	PTR_ADDU	t0, a2
>>>  	bne		t0, t1, 1b
>>>  9:	jr		ra
>>> @@ -85,7 +77,7 @@ LEAF(mips_init_dcache)
>>>  	/* clear all tags */
>>>  	PTR_LI		t0, INDEX_BASE
>>>  	PTR_ADDU	t1, t0, a1
>>> -1:	cache_op	INDEX_STORE_TAG_D t0
>>> +1:	cache		INDEX_STORE_TAG_D, 0(t0)
>>>  	PTR_ADDU	t0, a2
>>>  	bne		t0, t1, 1b
>>>  	/* load from each line (in cached space) */
>>> @@ -95,7 +87,7 @@ LEAF(mips_init_dcache)
>>>  	bne		t0, t1, 2b
>>>  	/* clear all tags */
>>>  	PTR_LI		t0, INDEX_BASE
>>> -1:	cache_op	INDEX_STORE_TAG_D t0
>>> +1:	cache		INDEX_STORE_TAG_D, 0(t0)
>>>  	PTR_ADDU	t0, a2
>>>  	bne		t0, t1, 1b
>>>  9:	jr		ra
>>> diff --git a/arch/mips/cpu/mips32/cpu.c b/arch/mips/cpu/mips32/cpu.c
>>> index 278865b..3f247fb 100644
>>> --- a/arch/mips/cpu/mips32/cpu.c
>>> +++ b/arch/mips/cpu/mips32/cpu.c
>>> @@ -12,16 +12,6 @@
>>>  #include <asm/cacheops.h>
>>>  #include <asm/reboot.h>
>>>  
>>> -#define cache_op(op,addr)						\
>>> -	__asm__ __volatile__(						\
>>> -	"	.set	push					\n"	\
>>> -	"	.set	noreorder				\n"	\
>>> -	"	.set	mips3\n\t				\n"	\
>>> -	"	cache	%0, %1					\n"	\
>>> -	"	.set	pop					\n"	\
>>> -	:								\
>>> -	: "i" (op), "R" (*(unsigned char *)(addr)))
>>> -
>>>  void __attribute__((weak)) _machine_restart(void)
>>>  {
>>>  }
>>> @@ -74,20 +64,20 @@ void flush_cache(ulong start_addr, ulong size)
>>>  {
>>>  	unsigned long ilsize = icache_line_size();
>>>  	unsigned long dlsize = dcache_line_size();
>>> -	unsigned long addr, aend;
>>> +	const volatile void *addr, *aend;
>>
>> why do you need volatile?
>>
> 
> I was just reflecting the type of the argument to __mips_builtin_cache:
> 
>   https://gcc.gnu.org/onlinedocs/gcc/Other-MIPS-Built-in-Functions.html
> 

ok, but then it's sufficient if __mips_builtin_cache adds the modifier.
No need to add it to the variables.

>>>  
>>>  	/* aend will be miscalculated when size is zero, so we return here */
>>>  	if (size == 0)
>>>  		return;
>>>  
>>> -	addr = start_addr & ~(dlsize - 1);
>>> -	aend = (start_addr + size - 1) & ~(dlsize - 1);
>>> +	addr = (void *)(start_addr & ~(dlsize - 1));
>>> +	aend = (void *)((start_addr + size - 1) & ~(dlsize - 1));
>>
>> shouldn't that be (const void *) ?
>>
> 
> I don't think it really matters since the assignment is to a more
> restricted type rather than from one, but I can change it if you like.

sure but I think it's more consistent and corresponds to coding style

> 
>>>  
>>>  	if (ilsize == dlsize) {
>>>  		/* flush I-cache & D-cache simultaneously */
>>>  		while (1) {
>>> -			cache_op(HIT_WRITEBACK_INV_D, addr);
>>> -			cache_op(HIT_INVALIDATE_I, addr);
>>> +			mips_cache(HIT_WRITEBACK_INV_D, addr);
>>> +			mips_cache(HIT_INVALIDATE_I, addr);
>>>  			if (addr == aend)
>>>  				break;
>>>  			addr += dlsize;
>>> @@ -97,17 +87,17 @@ void flush_cache(ulong start_addr, ulong size)
>>>  
>>>  	/* flush D-cache */
>>>  	while (1) {
>>> -		cache_op(HIT_WRITEBACK_INV_D, addr);
>>> +		mips_cache(HIT_WRITEBACK_INV_D, addr);
>>>  		if (addr == aend)
>>>  			break;
>>>  		addr += dlsize;
>>>  	}
>>>  
>>>  	/* flush I-cache */
>>> -	addr = start_addr & ~(ilsize - 1);
>>> -	aend = (start_addr + size - 1) & ~(ilsize - 1);
>>> +	addr = (void *)(start_addr & ~(ilsize - 1));
>>> +	aend = (void *)((start_addr + size - 1) & ~(ilsize - 1));
>>>  	while (1) {
>>> -		cache_op(HIT_INVALIDATE_I, addr);
>>> +		mips_cache(HIT_INVALIDATE_I, addr);
>>>  		if (addr == aend)
>>>  			break;
>>>  		addr += ilsize;
>>> @@ -117,11 +107,11 @@ 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 addr = start_addr & ~(lsize - 1);
>>> -	unsigned long aend = (stop - 1) & ~(lsize - 1);
>>> +	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
>>> +	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
>>>  
>>>  	while (1) {
>>> -		cache_op(HIT_WRITEBACK_INV_D, addr);
>>> +		mips_cache(HIT_WRITEBACK_INV_D, addr);
>>>  		if (addr == aend)
>>>  			break;
>>>  		addr += lsize;
>>> @@ -131,11 +121,11 @@ void flush_dcache_range(ulong start_addr, ulong stop)
>>>  void invalidate_dcache_range(ulong start_addr, ulong stop)
>>>  {
>>>  	unsigned long lsize = dcache_line_size();
>>> -	unsigned long addr = start_addr & ~(lsize - 1);
>>> -	unsigned long aend = (stop - 1) & ~(lsize - 1);
>>> +	const volatile void *addr = (void *)(start_addr & ~(lsize - 1));
>>> +	const volatile void *aend = (void *)((stop - 1) & ~(lsize - 1));
>>>  
>>>  	while (1) {
>>> -		cache_op(HIT_INVALIDATE_D, addr);
>>> +		mips_cache(HIT_INVALIDATE_D, addr);
>>>  		if (addr == aend)
>>>  			break;
>>>  		addr += lsize;
>>> diff --git a/arch/mips/include/asm/cacheops.h b/arch/mips/include/asm/cacheops.h
>>> index 6464250..809c966 100644
>>> --- a/arch/mips/include/asm/cacheops.h
>>> +++ b/arch/mips/include/asm/cacheops.h
>>> @@ -11,6 +11,13 @@
>>>  #ifndef	__ASM_CACHEOPS_H
>>>  #define	__ASM_CACHEOPS_H
>>>  
>>> +#ifdef __GCC_HAVE_BUILTIN_MIPS_CACHE
>>> +# define mips_cache __builtin_mips_cache
>>> +#else
>>> +# define mips_cache(op,addr) \
>>
>> space after op,
>>
> 
> Right you are! In my defence it was copied from the original
> implementation in cpu.c ;)

to match the __builtin_mips_cache prototype, maybe we should use

static inline void mips_cache(int op, const volatile void *addr)
{
        __asm__ __volatile__("cache %0, %1" : : "i"(op), "R"(addr));
}

> 
> Paul
> 
>>> +	__asm__ __volatile__("cache	%0, %1" : : "i"(op), "R"(addr))
>>> +#endif
>>> +
>>>  /*
>>>   * Cache Operations available on all MIPS processors with R4000-style caches
>>>   */
>>>
>>
>> -- 
>> - Daniel

-- 
- Daniel

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

* [U-Boot] [PATCH 0/8] MIPS cache code cleanup
       [not found]     ` <54C95453.8010009@gmail.com>
@ 2015-01-28 22:21       ` Paul Burton
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2015-01-28 22:21 UTC (permalink / raw)
  To: u-boot

On Wed, Jan 28, 2015 at 10:27:47PM +0100, Daniel Schwierzeck wrote:
> Am 28.01.2015 um 22:05 schrieb Paul Burton:
> > On Wed, Jan 28, 2015 at 09:31:25PM +0100, Daniel Schwierzeck wrote:
> >> Hi Paul,
> >>
> >> Am 26.01.2015 um 16:02 schrieb Paul Burton:
> >>> This series cleans up the MIPS cache code somewhat, and unifies the
> >>> mips32 & mips64 implementations of it. This is largely in preparation
> >>> for further patches adding L2 cache support. The final patch of this
> >>> series fixes a bug encountered with recent cores on Malta boards.
> >>
> >> thanks for doing this. I also have this on my to-do list.
> >>
> >> Just a thought: if we finally have just the mips_cache_reset function in
> >> cache_init.S, couldn't we reimplement it in C entirely? I see no reason
> >> to implement it in assembler.
> > 
> > The only issue with that is that mips_cache_reset currently runs before
> > we have a stack set up, so if the C compiler were to attempt to use the
> > stack things would fall over. I guess we could set up the temporary
> > stack earlier, and it's probably unlikely the compiler would actually
> > make use of it so it shouldn't really slow things down. 
> 
> I think if mips_cache_reset is a leaf function and takes no arguments or
> return values, the compiler shouldn't add any stack reservation in the
> function prologue.

My thoughts are more if the compiler tries to spill registers to the
stack. Whilst that seems unlikely to happen for such a small piece of
code, at least with optimisations turned on, I can imagine it happening
if someone adds more code (eg. L2 support ;) ) in future. At that point
we'd get crashes in the best case & weird corruption in the worst. The
compiler is also going to use the stack if optimisations are disabled
which isn't beyond the realms of possibility if debugging. So I don't
think there's any way we can use C & guarantee not to use the stack.

If we have an uncached stack then we can at least run, and assuming the
compiler doesn't make much use of the stack we could run without slowing
down too much. The only downside to this I can think of is that it
prevents us from ever moving the call to mips_cache_reset before the
call to lowlevel_init, which is something I have done for internal
builds in order to speed up execution on very slow systems (emulators
where running cached that bit earlier saves minutes in boot time).
That's something I'm somewhat hesitant to give up on... I'll keep
thinking.

> > Any chance we
> > could squeeze this into this merge window though, and I'll move the
> > cache init to C next time? That will simplify my L2 code somewhat too -
> > thanks for the suggestion!
> 
> sure

Great :) I'll address the comments you made on patch 1 & submit a v2 of
that in the morning.

Thanks,
    Paul

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

end of thread, other threads:[~2015-01-28 22:21 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-26 15:02 [U-Boot] [PATCH 0/8] MIPS cache code cleanup Paul Burton
2015-01-26 15:02 ` [U-Boot] [PATCH 1/8] MIPS: avoid .set ISA for cache operations Paul Burton
2015-01-28 20:43   ` Daniel Schwierzeck
2015-01-28 21:08     ` Paul Burton
2015-01-28 21:18       ` Daniel Schwierzeck
2015-01-26 15:02 ` [U-Boot] [PATCH 2/8] MIPS: unify cache maintenance functions Paul Burton
2015-01-26 15:02 ` [U-Boot] [PATCH 3/8] MIPS: unify cache initialization code Paul Burton
2015-01-26 15:02 ` [U-Boot] [PATCH 4/8] MIPS: refactor L1 cache config reads to a macro Paul Burton
2015-01-26 15:02 ` [U-Boot] [PATCH 5/8] MIPS: refactor cache loops " Paul Burton
2015-01-26 15:02 ` [U-Boot] [PATCH 6/8] MIPS: inline mips_init_[id]cache functions Paul Burton
2015-01-26 15:02 ` [U-Boot] [PATCH 7/8] MIPS: allow systems to skip loads during cache init Paul Burton
2015-01-26 15:03 ` [U-Boot] [PATCH 8/8] MIPS: clear TagLo select 2 " Paul Burton
2015-01-28 20:57   ` Daniel Schwierzeck
2015-01-28 21:13     ` Paul Burton
2015-01-28 20:31 ` [U-Boot] [PATCH 0/8] MIPS cache code cleanup Daniel Schwierzeck
2015-01-28 21:05   ` Paul Burton
     [not found]     ` <54C95453.8010009@gmail.com>
2015-01-28 22:21       ` 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.