[U-Boot] [PATCH 0/3] MIPS cache cleanups

This short series cleans up the MIPS cache code in preparation for introducing support for L2 cache support. It's hopefully a useful standalone cleanup as-is, so I'll submit it now.
Paul Burton (3): MIPS: Move cache sizes to Kconfig MIPS: Split I & D cache line size config MIPS: Abstract cache op loops with a macro
arch/mips/Kconfig | 16 ++++++++ arch/mips/lib/cache.c | 79 ++++++++++++---------------------------- arch/mips/lib/cache_init.S | 10 ++--- board/dbau1x00/Kconfig | 12 ++++++ board/micronas/vct/Kconfig | 12 ++++++ board/pb1x00/Kconfig | 12 ++++++ board/qca/ap121/Kconfig | 12 ++++++ board/qca/ap143/Kconfig | 12 ++++++ board/qemu-mips/Kconfig | 12 ++++++ board/tplink/wdr4300/Kconfig | 12 ++++++ include/configs/ap121.h | 5 --- include/configs/ap143.h | 5 --- include/configs/dbau1x00.h | 7 ---- include/configs/pb1x00.h | 6 --- include/configs/qemu-mips.h | 7 ---- include/configs/qemu-mips64.h | 7 ---- include/configs/tplink_wdr4300.h | 5 --- include/configs/vct.h | 7 ---- 18 files changed, 129 insertions(+), 109 deletions(-)

Move details of the L1 cache line sizes & total sizes into Kconfig, defaulting to 0 & using 0 to indicate that the value should be autodetected.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
arch/mips/Kconfig | 12 ++++++++++++ arch/mips/lib/cache.c | 2 +- arch/mips/lib/cache_init.S | 6 +++--- board/dbau1x00/Kconfig | 9 +++++++++ board/micronas/vct/Kconfig | 9 +++++++++ board/pb1x00/Kconfig | 9 +++++++++ board/qca/ap121/Kconfig | 9 +++++++++ board/qca/ap143/Kconfig | 9 +++++++++ board/qemu-mips/Kconfig | 9 +++++++++ board/tplink/wdr4300/Kconfig | 9 +++++++++ include/configs/ap121.h | 5 ----- include/configs/ap143.h | 5 ----- include/configs/dbau1x00.h | 7 ------- include/configs/pb1x00.h | 6 ------ include/configs/qemu-mips.h | 7 ------- include/configs/qemu-mips64.h | 7 ------- include/configs/tplink_wdr4300.h | 5 ----- include/configs/vct.h | 7 ------- 18 files changed, 79 insertions(+), 53 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index abaeaf0..13f1164 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -243,6 +243,18 @@ config SWAP_IO_SPACE config SYS_MIPS_CACHE_INIT_RAM_LOAD bool
+config SYS_DCACHE_SIZE + hex + default 0 + +config SYS_ICACHE_SIZE + hex + default 0 + +config SYS_CACHELINE_SIZE + hex + default 0 + config MIPS_L1_CACHE_SHIFT_4 bool
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 7482005..7695325 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -9,7 +9,7 @@ #include <asm/cacheops.h> #include <asm/mipsregs.h>
-#ifdef CONFIG_SYS_CACHELINE_SIZE +#if CONFIG_SYS_CACHELINE_SIZE != 0
static inline unsigned long icache_line_size(void) { diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S index 08b7c3a..e4a44ff 100644 --- a/arch/mips/lib/cache_init.S +++ b/arch/mips/lib/cache_init.S @@ -99,14 +99,14 @@ * */ LEAF(mips_cache_reset) -#ifdef CONFIG_SYS_ICACHE_SIZE +#if CONFIG_SYS_ICACHE_SIZE != 0 li t2, CONFIG_SYS_ICACHE_SIZE li t8, CONFIG_SYS_CACHELINE_SIZE #else l1_info t2, t8, MIPS_CONF1_IA_SHF #endif
-#ifdef CONFIG_SYS_DCACHE_SIZE +#if CONFIG_SYS_DCACHE_SIZE != 0 li t3, CONFIG_SYS_DCACHE_SIZE li t9, CONFIG_SYS_CACHELINE_SIZE #else @@ -116,7 +116,7 @@ LEAF(mips_cache_reset) #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 != 0) && (CONFIG_SYS_DCACHE_SIZE != 0) #if CONFIG_SYS_ICACHE_SIZE > CONFIG_SYS_DCACHE_SIZE li v0, CONFIG_SYS_ICACHE_SIZE #else diff --git a/board/dbau1x00/Kconfig b/board/dbau1x00/Kconfig index 342ec59..1715a28 100644 --- a/board/dbau1x00/Kconfig +++ b/board/dbau1x00/Kconfig @@ -12,6 +12,15 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0xbfc00000
+config SYS_DCACHE_SIZE + default 16384 + +config SYS_ICACHE_SIZE + default 16384 + +config SYS_CACHELINE_SIZE + default 32 + menu "dbau1x00 board options"
choice diff --git a/board/micronas/vct/Kconfig b/board/micronas/vct/Kconfig index 535a77b..5bb6f03 100644 --- a/board/micronas/vct/Kconfig +++ b/board/micronas/vct/Kconfig @@ -12,6 +12,15 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0x87000000
+config SYS_DCACHE_SIZE + default 16384 + +config SYS_ICACHE_SIZE + default 16384 + +config SYS_CACHELINE_SIZE + default 32 + menu "vct board options"
choice diff --git a/board/pb1x00/Kconfig b/board/pb1x00/Kconfig index 236a410..27b2ef0 100644 --- a/board/pb1x00/Kconfig +++ b/board/pb1x00/Kconfig @@ -12,4 +12,13 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0x83800000
+config SYS_DCACHE_SIZE + default 16384 + +config SYS_ICACHE_SIZE + default 16384 + +config SYS_CACHELINE_SIZE + default 32 + endif diff --git a/board/qca/ap121/Kconfig b/board/qca/ap121/Kconfig index c3ecc8f..1ace0e5 100644 --- a/board/qca/ap121/Kconfig +++ b/board/qca/ap121/Kconfig @@ -12,4 +12,13 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0x9f000000
+config SYS_DCACHE_SIZE + default 0x8000 + +config SYS_ICACHE_SIZE + default 0x10000 + +config SYS_CACHELINE_SIZE + default 32 + endif diff --git a/board/qca/ap143/Kconfig b/board/qca/ap143/Kconfig index 5ea5d6f..ac73782 100644 --- a/board/qca/ap143/Kconfig +++ b/board/qca/ap143/Kconfig @@ -12,4 +12,13 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0x9f000000
+config SYS_DCACHE_SIZE + default 0x8000 + +config SYS_ICACHE_SIZE + default 0x10000 + +config SYS_CACHELINE_SIZE + default 32 + endif diff --git a/board/qemu-mips/Kconfig b/board/qemu-mips/Kconfig index 3de1f44..66957e7 100644 --- a/board/qemu-mips/Kconfig +++ b/board/qemu-mips/Kconfig @@ -11,4 +11,13 @@ config SYS_TEXT_BASE default 0xbfc00000 if 32BIT default 0xffffffffbfc00000 if 64BIT
+config SYS_DCACHE_SIZE + default 16384 + +config SYS_ICACHE_SIZE + default 16384 + +config SYS_CACHELINE_SIZE + default 32 + endif diff --git a/board/tplink/wdr4300/Kconfig b/board/tplink/wdr4300/Kconfig index 65785bd..bbec2e5 100644 --- a/board/tplink/wdr4300/Kconfig +++ b/board/tplink/wdr4300/Kconfig @@ -15,4 +15,13 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0xa1000000
+config SYS_DCACHE_SIZE + default 0x8000 + +config SYS_ICACHE_SIZE + default 0x10000 + +config SYS_CACHELINE_SIZE + default 32 + endif diff --git a/include/configs/ap121.h b/include/configs/ap121.h index 6f69f31..f069d50 100644 --- a/include/configs/ap121.h +++ b/include/configs/ap121.h @@ -15,11 +15,6 @@ #define CONFIG_SYS_MHZ 200 #define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 1000000)
-/* Cache Configuration */ -#define CONFIG_SYS_DCACHE_SIZE 0x8000 -#define CONFIG_SYS_ICACHE_SIZE 0x10000 -#define CONFIG_SYS_CACHELINE_SIZE 32 - #define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_MALLOC_LEN 0x40000 diff --git a/include/configs/ap143.h b/include/configs/ap143.h index f907c02..e45f743 100644 --- a/include/configs/ap143.h +++ b/include/configs/ap143.h @@ -15,11 +15,6 @@ #define CONFIG_SYS_MHZ 325 #define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 1000000)
-/* Cache Configuration */ -#define CONFIG_SYS_DCACHE_SIZE 0x8000 -#define CONFIG_SYS_ICACHE_SIZE 0x10000 -#define CONFIG_SYS_CACHELINE_SIZE 32 - #define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_MALLOC_LEN 0x40000 diff --git a/include/configs/dbau1x00.h b/include/configs/dbau1x00.h index 68d9e36..68ff025 100644 --- a/include/configs/dbau1x00.h +++ b/include/configs/dbau1x00.h @@ -202,11 +202,4 @@ #define CONFIG_SYS_ATA_ALT_OFFSET 0x0100 #endif /* CONFIG_DBAU1550 */
-/*----------------------------------------------------------------------- - * Cache Configuration - */ -#define CONFIG_SYS_DCACHE_SIZE 16384 -#define CONFIG_SYS_ICACHE_SIZE 16384 -#define CONFIG_SYS_CACHELINE_SIZE 32 - #endif /* __CONFIG_H */ diff --git a/include/configs/pb1x00.h b/include/configs/pb1x00.h index 869768a..b907419 100644 --- a/include/configs/pb1x00.h +++ b/include/configs/pb1x00.h @@ -144,12 +144,6 @@ #define CONFIG_SYS_ATA_ALT_OFFSET 0x0100
#endif -/*----------------------------------------------------------------------- - * Cache Configuration - */ -#define CONFIG_SYS_DCACHE_SIZE 16384 -#define CONFIG_SYS_ICACHE_SIZE 16384 -#define CONFIG_SYS_CACHELINE_SIZE 32
/* * BOOTP options diff --git a/include/configs/qemu-mips.h b/include/configs/qemu-mips.h index 246ee01..f58fc4c 100644 --- a/include/configs/qemu-mips.h +++ b/include/configs/qemu-mips.h @@ -132,11 +132,4 @@
#define CONFIG_LZMA
-/*----------------------------------------------------------------------- - * Cache Configuration - */ -#define CONFIG_SYS_DCACHE_SIZE 16384 -#define CONFIG_SYS_ICACHE_SIZE 16384 -#define CONFIG_SYS_CACHELINE_SIZE 32 - #endif /* __CONFIG_H */ diff --git a/include/configs/qemu-mips64.h b/include/configs/qemu-mips64.h index 60a3a71..2190d16 100644 --- a/include/configs/qemu-mips64.h +++ b/include/configs/qemu-mips64.h @@ -132,11 +132,4 @@
#define CONFIG_LZMA
-/*----------------------------------------------------------------------- - * Cache Configuration - */ -#define CONFIG_SYS_DCACHE_SIZE 16384 -#define CONFIG_SYS_ICACHE_SIZE 16384 -#define CONFIG_SYS_CACHELINE_SIZE 32 - #endif /* __CONFIG_H */ diff --git a/include/configs/tplink_wdr4300.h b/include/configs/tplink_wdr4300.h index 09a69fe..6273711 100644 --- a/include/configs/tplink_wdr4300.h +++ b/include/configs/tplink_wdr4300.h @@ -15,11 +15,6 @@ #define CONFIG_SYS_MHZ 280 #define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 1000000)
-/* Cache Configuration */ -#define CONFIG_SYS_DCACHE_SIZE 0x8000 -#define CONFIG_SYS_ICACHE_SIZE 0x10000 -#define CONFIG_SYS_CACHELINE_SIZE 32 - #define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_MALLOC_LEN 0x40000 diff --git a/include/configs/vct.h b/include/configs/vct.h index 68eb089..cc5e354 100644 --- a/include/configs/vct.h +++ b/include/configs/vct.h @@ -204,13 +204,6 @@ #endif /* CONFIG_VCT_ONENAND */
/* - * Cache Configuration - */ -#define CONFIG_SYS_DCACHE_SIZE 16384 -#define CONFIG_SYS_ICACHE_SIZE 16384 -#define CONFIG_SYS_CACHELINE_SIZE 32 - -/* * I2C/EEPROM */ #define CONFIG_SYS_I2C

On 05/26/2016 05:58 PM, Paul Burton wrote:
Move details of the L1 cache line sizes & total sizes into Kconfig, defaulting to 0 & using 0 to indicate that the value should be autodetected.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/Kconfig | 12 ++++++++++++ arch/mips/lib/cache.c | 2 +- arch/mips/lib/cache_init.S | 6 +++--- board/dbau1x00/Kconfig | 9 +++++++++ board/micronas/vct/Kconfig | 9 +++++++++ board/pb1x00/Kconfig | 9 +++++++++ board/qca/ap121/Kconfig | 9 +++++++++ board/qca/ap143/Kconfig | 9 +++++++++ board/qemu-mips/Kconfig | 9 +++++++++ board/tplink/wdr4300/Kconfig | 9 +++++++++ include/configs/ap121.h | 5 ----- include/configs/ap143.h | 5 ----- include/configs/dbau1x00.h | 7 ------- include/configs/pb1x00.h | 6 ------ include/configs/qemu-mips.h | 7 ------- include/configs/qemu-mips64.h | 7 ------- include/configs/tplink_wdr4300.h | 5 ----- include/configs/vct.h | 7 ------- 18 files changed, 79 insertions(+), 53 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index abaeaf0..13f1164 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -243,6 +243,18 @@ config SWAP_IO_SPACE config SYS_MIPS_CACHE_INIT_RAM_LOAD bool
+config SYS_DCACHE_SIZE
- hex
- default 0
+config SYS_ICACHE_SIZE
- hex
- default 0
+config SYS_CACHELINE_SIZE
- hex
- default 0
Please add some short help text to those options.
config MIPS_L1_CACHE_SHIFT_4 bool
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 7482005..7695325 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -9,7 +9,7 @@ #include <asm/cacheops.h> #include <asm/mipsregs.h>
-#ifdef CONFIG_SYS_CACHELINE_SIZE +#if CONFIG_SYS_CACHELINE_SIZE != 0
Wouldn't it make more sense to introduce something like CONFIG_HAVE_CACHE_SUPPORT instead , so you don't need this #if CONFIG_FOO != 0 construct all over the place ?
Cool stuff otherwise, thanks!
static inline unsigned long icache_line_size(void) { diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S index 08b7c3a..e4a44ff 100644 --- a/arch/mips/lib/cache_init.S +++ b/arch/mips/lib/cache_init.S @@ -99,14 +99,14 @@
*/ LEAF(mips_cache_reset) -#ifdef CONFIG_SYS_ICACHE_SIZE +#if CONFIG_SYS_ICACHE_SIZE != 0 li t2, CONFIG_SYS_ICACHE_SIZE li t8, CONFIG_SYS_CACHELINE_SIZE #else l1_info t2, t8, MIPS_CONF1_IA_SHF #endif
-#ifdef CONFIG_SYS_DCACHE_SIZE +#if CONFIG_SYS_DCACHE_SIZE != 0 li t3, CONFIG_SYS_DCACHE_SIZE li t9, CONFIG_SYS_CACHELINE_SIZE #else @@ -116,7 +116,7 @@ LEAF(mips_cache_reset) #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 != 0) && (CONFIG_SYS_DCACHE_SIZE != 0) #if CONFIG_SYS_ICACHE_SIZE > CONFIG_SYS_DCACHE_SIZE li v0, CONFIG_SYS_ICACHE_SIZE #else diff --git a/board/dbau1x00/Kconfig b/board/dbau1x00/Kconfig index 342ec59..1715a28 100644 --- a/board/dbau1x00/Kconfig +++ b/board/dbau1x00/Kconfig @@ -12,6 +12,15 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0xbfc00000
+config SYS_DCACHE_SIZE
- default 16384
+config SYS_ICACHE_SIZE
- default 16384
+config SYS_CACHELINE_SIZE
- default 32
menu "dbau1x00 board options"
choice diff --git a/board/micronas/vct/Kconfig b/board/micronas/vct/Kconfig index 535a77b..5bb6f03 100644 --- a/board/micronas/vct/Kconfig +++ b/board/micronas/vct/Kconfig @@ -12,6 +12,15 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0x87000000
+config SYS_DCACHE_SIZE
- default 16384
+config SYS_ICACHE_SIZE
- default 16384
+config SYS_CACHELINE_SIZE
- default 32
menu "vct board options"
choice diff --git a/board/pb1x00/Kconfig b/board/pb1x00/Kconfig index 236a410..27b2ef0 100644 --- a/board/pb1x00/Kconfig +++ b/board/pb1x00/Kconfig @@ -12,4 +12,13 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0x83800000
+config SYS_DCACHE_SIZE
- default 16384
+config SYS_ICACHE_SIZE
- default 16384
+config SYS_CACHELINE_SIZE
- default 32
endif diff --git a/board/qca/ap121/Kconfig b/board/qca/ap121/Kconfig index c3ecc8f..1ace0e5 100644 --- a/board/qca/ap121/Kconfig +++ b/board/qca/ap121/Kconfig @@ -12,4 +12,13 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0x9f000000
+config SYS_DCACHE_SIZE
- default 0x8000
+config SYS_ICACHE_SIZE
- default 0x10000
+config SYS_CACHELINE_SIZE
- default 32
endif diff --git a/board/qca/ap143/Kconfig b/board/qca/ap143/Kconfig index 5ea5d6f..ac73782 100644 --- a/board/qca/ap143/Kconfig +++ b/board/qca/ap143/Kconfig @@ -12,4 +12,13 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0x9f000000
+config SYS_DCACHE_SIZE
- default 0x8000
+config SYS_ICACHE_SIZE
- default 0x10000
+config SYS_CACHELINE_SIZE
- default 32
endif diff --git a/board/qemu-mips/Kconfig b/board/qemu-mips/Kconfig index 3de1f44..66957e7 100644 --- a/board/qemu-mips/Kconfig +++ b/board/qemu-mips/Kconfig @@ -11,4 +11,13 @@ config SYS_TEXT_BASE default 0xbfc00000 if 32BIT default 0xffffffffbfc00000 if 64BIT
+config SYS_DCACHE_SIZE
- default 16384
+config SYS_ICACHE_SIZE
- default 16384
+config SYS_CACHELINE_SIZE
- default 32
endif diff --git a/board/tplink/wdr4300/Kconfig b/board/tplink/wdr4300/Kconfig index 65785bd..bbec2e5 100644 --- a/board/tplink/wdr4300/Kconfig +++ b/board/tplink/wdr4300/Kconfig @@ -15,4 +15,13 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0xa1000000
+config SYS_DCACHE_SIZE
- default 0x8000
+config SYS_ICACHE_SIZE
- default 0x10000
+config SYS_CACHELINE_SIZE
- default 32
endif diff --git a/include/configs/ap121.h b/include/configs/ap121.h index 6f69f31..f069d50 100644 --- a/include/configs/ap121.h +++ b/include/configs/ap121.h @@ -15,11 +15,6 @@ #define CONFIG_SYS_MHZ 200 #define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 1000000)
-/* Cache Configuration */ -#define CONFIG_SYS_DCACHE_SIZE 0x8000 -#define CONFIG_SYS_ICACHE_SIZE 0x10000 -#define CONFIG_SYS_CACHELINE_SIZE 32
#define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_MALLOC_LEN 0x40000 diff --git a/include/configs/ap143.h b/include/configs/ap143.h index f907c02..e45f743 100644 --- a/include/configs/ap143.h +++ b/include/configs/ap143.h @@ -15,11 +15,6 @@ #define CONFIG_SYS_MHZ 325 #define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 1000000)
-/* Cache Configuration */ -#define CONFIG_SYS_DCACHE_SIZE 0x8000 -#define CONFIG_SYS_ICACHE_SIZE 0x10000 -#define CONFIG_SYS_CACHELINE_SIZE 32
#define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_MALLOC_LEN 0x40000 diff --git a/include/configs/dbau1x00.h b/include/configs/dbau1x00.h index 68d9e36..68ff025 100644 --- a/include/configs/dbau1x00.h +++ b/include/configs/dbau1x00.h @@ -202,11 +202,4 @@ #define CONFIG_SYS_ATA_ALT_OFFSET 0x0100 #endif /* CONFIG_DBAU1550 */
-/*-----------------------------------------------------------------------
- Cache Configuration
- */
-#define CONFIG_SYS_DCACHE_SIZE 16384 -#define CONFIG_SYS_ICACHE_SIZE 16384 -#define CONFIG_SYS_CACHELINE_SIZE 32
#endif /* __CONFIG_H */ diff --git a/include/configs/pb1x00.h b/include/configs/pb1x00.h index 869768a..b907419 100644 --- a/include/configs/pb1x00.h +++ b/include/configs/pb1x00.h @@ -144,12 +144,6 @@ #define CONFIG_SYS_ATA_ALT_OFFSET 0x0100
#endif -/*-----------------------------------------------------------------------
- Cache Configuration
- */
-#define CONFIG_SYS_DCACHE_SIZE 16384 -#define CONFIG_SYS_ICACHE_SIZE 16384 -#define CONFIG_SYS_CACHELINE_SIZE 32
/*
- BOOTP options
diff --git a/include/configs/qemu-mips.h b/include/configs/qemu-mips.h index 246ee01..f58fc4c 100644 --- a/include/configs/qemu-mips.h +++ b/include/configs/qemu-mips.h @@ -132,11 +132,4 @@
#define CONFIG_LZMA
-/*-----------------------------------------------------------------------
- Cache Configuration
- */
-#define CONFIG_SYS_DCACHE_SIZE 16384 -#define CONFIG_SYS_ICACHE_SIZE 16384 -#define CONFIG_SYS_CACHELINE_SIZE 32
#endif /* __CONFIG_H */ diff --git a/include/configs/qemu-mips64.h b/include/configs/qemu-mips64.h index 60a3a71..2190d16 100644 --- a/include/configs/qemu-mips64.h +++ b/include/configs/qemu-mips64.h @@ -132,11 +132,4 @@
#define CONFIG_LZMA
-/*-----------------------------------------------------------------------
- Cache Configuration
- */
-#define CONFIG_SYS_DCACHE_SIZE 16384 -#define CONFIG_SYS_ICACHE_SIZE 16384 -#define CONFIG_SYS_CACHELINE_SIZE 32
#endif /* __CONFIG_H */ diff --git a/include/configs/tplink_wdr4300.h b/include/configs/tplink_wdr4300.h index 09a69fe..6273711 100644 --- a/include/configs/tplink_wdr4300.h +++ b/include/configs/tplink_wdr4300.h @@ -15,11 +15,6 @@ #define CONFIG_SYS_MHZ 280 #define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 1000000)
-/* Cache Configuration */ -#define CONFIG_SYS_DCACHE_SIZE 0x8000 -#define CONFIG_SYS_ICACHE_SIZE 0x10000 -#define CONFIG_SYS_CACHELINE_SIZE 32
#define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_MALLOC_LEN 0x40000 diff --git a/include/configs/vct.h b/include/configs/vct.h index 68eb089..cc5e354 100644 --- a/include/configs/vct.h +++ b/include/configs/vct.h @@ -204,13 +204,6 @@ #endif /* CONFIG_VCT_ONENAND */
/*
- Cache Configuration
- */
-#define CONFIG_SYS_DCACHE_SIZE 16384 -#define CONFIG_SYS_ICACHE_SIZE 16384 -#define CONFIG_SYS_CACHELINE_SIZE 32
-/*
- I2C/EEPROM
*/ #define CONFIG_SYS_I2C

On Thu, May 26, 2016 at 06:10:38PM +0200, Marek Vasut wrote:
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 7482005..7695325 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -9,7 +9,7 @@ #include <asm/cacheops.h> #include <asm/mipsregs.h>
-#ifdef CONFIG_SYS_CACHELINE_SIZE +#if CONFIG_SYS_CACHELINE_SIZE != 0
Wouldn't it make more sense to introduce something like CONFIG_HAVE_CACHE_SUPPORT instead , so you don't need this #if CONFIG_FOO != 0 construct all over the place ?
Hi Marek,
It's not about whether cache support is present (we always build cache support), it's about whether we are hardcoding the sizes of the caches or detecting them at runtime. The latter is especially useful on FPGA-based platforms like Malta where the CPU (& caches) can change. I suppose I could add something like CONFIG_SYS_CACHE_SIZE_AUTO, but I still think it would be cleanest to have that default to a value based upon whether a board has set a non-zero size for any of the caches.
Cool stuff otherwise, thanks!
Thanks, Paul

On 05/27/2016 12:36 PM, Paul Burton wrote:
On Thu, May 26, 2016 at 06:10:38PM +0200, Marek Vasut wrote:
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 7482005..7695325 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -9,7 +9,7 @@ #include <asm/cacheops.h> #include <asm/mipsregs.h>
-#ifdef CONFIG_SYS_CACHELINE_SIZE +#if CONFIG_SYS_CACHELINE_SIZE != 0
Wouldn't it make more sense to introduce something like CONFIG_HAVE_CACHE_SUPPORT instead , so you don't need this #if CONFIG_FOO != 0 construct all over the place ?
Hi Marek,
It's not about whether cache support is present (we always build cache support), it's about whether we are hardcoding the sizes of the caches or detecting them at runtime. The latter is especially useful on FPGA-based platforms like Malta where the CPU (& caches) can change. I suppose I could add something like CONFIG_SYS_CACHE_SIZE_AUTO, but I still think it would be cleanest to have that default to a value based upon whether a board has set a non-zero size for any of the caches.
Auto-detecting cacheline size is cool, but that'd need much more work. Just grep through drivers/ for CONFIG_SYS_CACHELINE_SIZE and you'll see a lot of code depends on non-zero value in it. Setting it to zero will just bite you at some point in a nasty way.
Cool stuff otherwise, thanks!
Thanks, Paul

On Fri, May 27, 2016 at 04:32:26PM +0200, Marek Vasut wrote:
On 05/27/2016 12:36 PM, Paul Burton wrote:
On Thu, May 26, 2016 at 06:10:38PM +0200, Marek Vasut wrote:
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 7482005..7695325 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -9,7 +9,7 @@ #include <asm/cacheops.h> #include <asm/mipsregs.h>
-#ifdef CONFIG_SYS_CACHELINE_SIZE +#if CONFIG_SYS_CACHELINE_SIZE != 0
Wouldn't it make more sense to introduce something like CONFIG_HAVE_CACHE_SUPPORT instead , so you don't need this #if CONFIG_FOO != 0 construct all over the place ?
Hi Marek,
It's not about whether cache support is present (we always build cache support), it's about whether we are hardcoding the sizes of the caches or detecting them at runtime. The latter is especially useful on FPGA-based platforms like Malta where the CPU (& caches) can change. I suppose I could add something like CONFIG_SYS_CACHE_SIZE_AUTO, but I still think it would be cleanest to have that default to a value based upon whether a board has set a non-zero size for any of the caches.
Auto-detecting cacheline size is cool, but that'd need much more work. Just grep through drivers/ for CONFIG_SYS_CACHELINE_SIZE and you'll see a lot of code depends on non-zero value in it. Setting it to zero will just bite you at some point in a nasty way.
Hi Marek,
We're already using the cache size auto-detection on Malta, and on 2 other FPGA-based boards internally. I've submitted v2 which preserves CONFIG_SYS_CACHELINE_SIZE as a synonym of ARCH_DMA_MINALIGN for the drivers that are still using it.
Thanks, Paul

On 05/27/2016 04:34 PM, Paul Burton wrote:
On Fri, May 27, 2016 at 04:32:26PM +0200, Marek Vasut wrote:
On 05/27/2016 12:36 PM, Paul Burton wrote:
On Thu, May 26, 2016 at 06:10:38PM +0200, Marek Vasut wrote:
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 7482005..7695325 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -9,7 +9,7 @@ #include <asm/cacheops.h> #include <asm/mipsregs.h>
-#ifdef CONFIG_SYS_CACHELINE_SIZE +#if CONFIG_SYS_CACHELINE_SIZE != 0
Wouldn't it make more sense to introduce something like CONFIG_HAVE_CACHE_SUPPORT instead , so you don't need this #if CONFIG_FOO != 0 construct all over the place ?
Hi Marek,
It's not about whether cache support is present (we always build cache support), it's about whether we are hardcoding the sizes of the caches or detecting them at runtime. The latter is especially useful on FPGA-based platforms like Malta where the CPU (& caches) can change. I suppose I could add something like CONFIG_SYS_CACHE_SIZE_AUTO, but I still think it would be cleanest to have that default to a value based upon whether a board has set a non-zero size for any of the caches.
Auto-detecting cacheline size is cool, but that'd need much more work. Just grep through drivers/ for CONFIG_SYS_CACHELINE_SIZE and you'll see a lot of code depends on non-zero value in it. Setting it to zero will just bite you at some point in a nasty way.
Hi Marek,
Hi!
We're already using the cache size auto-detection on Malta, and on 2 other FPGA-based boards internally. I've submitted v2 which preserves CONFIG_SYS_CACHELINE_SIZE as a synonym of ARCH_DMA_MINALIGN for the drivers that are still using it.
That's a good workaround for now. Would you be interested in fixing this runtime cache configuration properly ?
Off-topic: Is malta that mipsfpga or is that something else ? Can I synthesise that mipsfpga into some altera FPGA ? If so, which one is a good pick ?
Thanks, Paul

On Fri, May 27, 2016 at 04:40:07PM +0200, Marek Vasut wrote:
Hi Marek,
Hi!
Hi again Marek :)
We're already using the cache size auto-detection on Malta, and on 2 other FPGA-based boards internally. I've submitted v2 which preserves CONFIG_SYS_CACHELINE_SIZE as a synonym of ARCH_DMA_MINALIGN for the drivers that are still using it.
That's a good workaround for now. Would you be interested in fixing this runtime cache configuration properly ?
We use the runtime cache size detection on Malta, on SEAD3 & on Boston. The latter 2 aren't upstream yet of course, but all 3 are FPGA-based platforms used to develop, test & showcase new CPUs. As such the CPU (& by association the cache) can change just by flashing a new bitfile to the board, and it's very convenient for a single build of U-Boot to work regardless of the bitfile in use.
It all works fine so long as drivers are well behaved, and nothing on any of those platforms uses CONFIG_SYS_CACHELINE_SIZE. I'm not sure there's anything to fix really apart from drivers should possibly move to use ARCH_DMA_MINALIGN, but I wouldn't feel comfortable doing a global replacement as I don't have a way to test many boards for other architectures.
Off-topic: Is malta that mipsfpga or is that something else ? Can I synthesise that mipsfpga into some altera FPGA ? If so, which one is a good pick ?
I don't know much about the MIPSfpga project to be honest, but I can forward your question to someone who does.
Thanks, Paul

On 05/27/2016 04:54 PM, Paul Burton wrote:
On Fri, May 27, 2016 at 04:40:07PM +0200, Marek Vasut wrote:
Hi Marek,
Hi!
Hi again Marek :)
Hi!
We're already using the cache size auto-detection on Malta, and on 2 other FPGA-based boards internally. I've submitted v2 which preserves CONFIG_SYS_CACHELINE_SIZE as a synonym of ARCH_DMA_MINALIGN for the drivers that are still using it.
That's a good workaround for now. Would you be interested in fixing this runtime cache configuration properly ?
We use the runtime cache size detection on Malta, on SEAD3 & on Boston. The latter 2 aren't upstream yet of course, but all 3 are FPGA-based platforms used to develop, test & showcase new CPUs. As such the CPU (& by association the cache) can change just by flashing a new bitfile to the board, and it's very convenient for a single build of U-Boot to work regardless of the bitfile in use.
Right, this much I understand.
It all works fine so long as drivers are well behaved, and nothing on any of those platforms uses CONFIG_SYS_CACHELINE_SIZE. I'm not sure there's anything to fix really apart from drivers should possibly move to use ARCH_DMA_MINALIGN, but I wouldn't feel comfortable doing a global replacement as I don't have a way to test many boards for other architectures.
I went through the u-boot sources again and it's mostly USB to blame, but I don't think USB should use ARCH_DMA_MINALIGN. USB controllers usually have their own DMA engine which might have different alignment requirements from the one in the CPU. This would imply that the entire concept of ARCH_DMA_MINALIGN is becoming increasingly inapplicable to modern CPUs and we might need some sort of DMA API to deal with that.
But ok, that would be a lot of work far beyond the scope of this patchset. I suspect setting CONFIG_SYS_CACHELINE_SIZE to sensible default so it wouldn't break USB would be a good start. 128 might be a good pick, since I don't think there are systems with cache line size longer than that and so flushing/invalidating that much would assure it works on whole cacheline at least.
Off-topic: Is malta that mipsfpga or is that something else ? Can I synthesise that mipsfpga into some altera FPGA ? If so, which one is a good pick ?
I don't know much about the MIPSfpga project to be honest, but I can forward your question to someone who does.
That's OK, I will poke myself into it before bothering you with it, thanks!
Thanks, Paul

Am 27.05.2016 um 16:40 schrieb Marek Vasut:
On 05/27/2016 04:34 PM, Paul Burton wrote:
On Fri, May 27, 2016 at 04:32:26PM +0200, Marek Vasut wrote:
On 05/27/2016 12:36 PM, Paul Burton wrote:
On Thu, May 26, 2016 at 06:10:38PM +0200, Marek Vasut wrote:
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 7482005..7695325 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -9,7 +9,7 @@ #include <asm/cacheops.h> #include <asm/mipsregs.h>
-#ifdef CONFIG_SYS_CACHELINE_SIZE +#if CONFIG_SYS_CACHELINE_SIZE != 0
Wouldn't it make more sense to introduce something like CONFIG_HAVE_CACHE_SUPPORT instead , so you don't need this #if CONFIG_FOO != 0 construct all over the place ?
Hi Marek,
It's not about whether cache support is present (we always build cache support), it's about whether we are hardcoding the sizes of the caches or detecting them at runtime. The latter is especially useful on FPGA-based platforms like Malta where the CPU (& caches) can change. I suppose I could add something like CONFIG_SYS_CACHE_SIZE_AUTO, but I still think it would be cleanest to have that default to a value based upon whether a board has set a non-zero size for any of the caches.
Auto-detecting cacheline size is cool, but that'd need much more work. Just grep through drivers/ for CONFIG_SYS_CACHELINE_SIZE and you'll see a lot of code depends on non-zero value in it. Setting it to zero will just bite you at some point in a nasty way.
Hi Marek,
Hi!
We're already using the cache size auto-detection on Malta, and on 2 other FPGA-based boards internally. I've submitted v2 which preserves CONFIG_SYS_CACHELINE_SIZE as a synonym of ARCH_DMA_MINALIGN for the drivers that are still using it.
That's a good workaround for now. Would you be interested in fixing this runtime cache configuration properly ?
there is no runtime configuration for ARCH_DMA_MINALIGN. This value is defined at compile-time by Kconfig option MIPS_L1_CACHE_SHIFT. This is similar to what Linux is doing.

On 05/27/2016 05:43 PM, Daniel Schwierzeck wrote:
Am 27.05.2016 um 16:40 schrieb Marek Vasut:
On 05/27/2016 04:34 PM, Paul Burton wrote:
On Fri, May 27, 2016 at 04:32:26PM +0200, Marek Vasut wrote:
On 05/27/2016 12:36 PM, Paul Burton wrote:
On Thu, May 26, 2016 at 06:10:38PM +0200, Marek Vasut wrote:
> diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c > index 7482005..7695325 100644 > --- a/arch/mips/lib/cache.c > +++ b/arch/mips/lib/cache.c > @@ -9,7 +9,7 @@ > #include <asm/cacheops.h> > #include <asm/mipsregs.h> > > -#ifdef CONFIG_SYS_CACHELINE_SIZE > +#if CONFIG_SYS_CACHELINE_SIZE != 0
Wouldn't it make more sense to introduce something like CONFIG_HAVE_CACHE_SUPPORT instead , so you don't need this #if CONFIG_FOO != 0 construct all over the place ?
Hi Marek,
It's not about whether cache support is present (we always build cache support), it's about whether we are hardcoding the sizes of the caches or detecting them at runtime. The latter is especially useful on FPGA-based platforms like Malta where the CPU (& caches) can change. I suppose I could add something like CONFIG_SYS_CACHE_SIZE_AUTO, but I still think it would be cleanest to have that default to a value based upon whether a board has set a non-zero size for any of the caches.
Auto-detecting cacheline size is cool, but that'd need much more work. Just grep through drivers/ for CONFIG_SYS_CACHELINE_SIZE and you'll see a lot of code depends on non-zero value in it. Setting it to zero will just bite you at some point in a nasty way.
Hi Marek,
Hi!
We're already using the cache size auto-detection on Malta, and on 2 other FPGA-based boards internally. I've submitted v2 which preserves CONFIG_SYS_CACHELINE_SIZE as a synonym of ARCH_DMA_MINALIGN for the drivers that are still using it.
That's a good workaround for now. Would you be interested in fixing this runtime cache configuration properly ?
there is no runtime configuration for ARCH_DMA_MINALIGN. This value is defined at compile-time by Kconfig option MIPS_L1_CACHE_SHIFT. This is similar to what Linux is doing.
So the DMA_MINALIGN is always a multiple of cacheline size on MIPS ?

Marek Vasut <marex <at> denx.de> writes:
...
Off-topic: Is malta that mipsfpga or is that something else ? Can I synthesise that mipsfpga into some altera FPGA ? If so, which one is a good pick ?
Hi Marek
Malta is a generic MIPS evaluation platform. https://www.linux-mips.org/wiki/MIPS_Malta
MIPSfpga is currently for universities/academics.
A microAptiv core (including verilog source code) is given to academics to play with. The whole pack includes teaching resources, exercises, guides etc.
If you do come under the definition of a University/Academic, do check it out https://community.imgtec.com/university/
Reference designs to run bare metal code runs on both Xilinx/Altera FPGAs.
However, reference designs to run the Linux Kernel are Xilinx specific.
Some more detail here.
https://kernel.org/doc/Documentation/devicetree/bindings/mips/img/xilfpga.tx...
Hope that helps.
Thanks ZubairLK

Am 26.05.2016 um 17:58 schrieb Paul Burton:
Move details of the L1 cache line sizes & total sizes into Kconfig, defaulting to 0 & using 0 to indicate that the value should be autodetected.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/Kconfig | 12 ++++++++++++ arch/mips/lib/cache.c | 2 +- arch/mips/lib/cache_init.S | 6 +++--- board/dbau1x00/Kconfig | 9 +++++++++ board/micronas/vct/Kconfig | 9 +++++++++ board/pb1x00/Kconfig | 9 +++++++++ board/qca/ap121/Kconfig | 9 +++++++++ board/qca/ap143/Kconfig | 9 +++++++++ board/qemu-mips/Kconfig | 9 +++++++++ board/tplink/wdr4300/Kconfig | 9 +++++++++ include/configs/ap121.h | 5 ----- include/configs/ap143.h | 5 ----- include/configs/dbau1x00.h | 7 ------- include/configs/pb1x00.h | 6 ------ include/configs/qemu-mips.h | 7 ------- include/configs/qemu-mips64.h | 7 ------- include/configs/tplink_wdr4300.h | 5 ----- include/configs/vct.h | 7 ------- 18 files changed, 79 insertions(+), 53 deletions(-)
applied to u-boot-mips, thanks!

Allow L1 Icache & L1 Dcache line size to be specified separately, since there's no architectural mandate that they be the same. The [id]cache_line_size functions are tidied up to take advantage of the fact that the Kconfig entries are always present to simply check them for zero rather than needing to #ifdef on their presence.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
arch/mips/Kconfig | 6 +++++- arch/mips/lib/cache.c | 22 +++++++--------------- arch/mips/lib/cache_init.S | 4 ++-- board/dbau1x00/Kconfig | 5 ++++- board/micronas/vct/Kconfig | 5 ++++- board/pb1x00/Kconfig | 5 ++++- board/qca/ap121/Kconfig | 5 ++++- board/qca/ap143/Kconfig | 5 ++++- board/qemu-mips/Kconfig | 5 ++++- board/tplink/wdr4300/Kconfig | 5 ++++- 10 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 13f1164..8af8799 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -247,11 +247,15 @@ config SYS_DCACHE_SIZE hex default 0
+config SYS_DCACHE_LINE_SIZE + hex + default 0 + config SYS_ICACHE_SIZE hex default 0
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE hex default 0
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 7695325..2bb91c6 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -9,23 +9,13 @@ #include <asm/cacheops.h> #include <asm/mipsregs.h>
-#if CONFIG_SYS_CACHELINE_SIZE != 0 - 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; -} + unsigned long conf1, il;
-#else /* !CONFIG_SYS_CACHELINE_SIZE */ + if (CONFIG_SYS_ICACHE_LINE_SIZE != 0) + return CONFIG_SYS_ICACHE_LINE_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_SHF; if (!il) @@ -36,6 +26,10 @@ static inline unsigned long icache_line_size(void) static inline unsigned long dcache_line_size(void) { unsigned long conf1, dl; + + if (CONFIG_SYS_DCACHE_LINE_SIZE != 0) + return CONFIG_SYS_DCACHE_LINE_SIZE; + conf1 = read_c0_config1(); dl = (conf1 & MIPS_CONF1_DL) >> MIPS_CONF1_DL_SHF; if (!dl) @@ -43,8 +37,6 @@ static inline unsigned long dcache_line_size(void) return 2 << dl; }
-#endif /* !CONFIG_SYS_CACHELINE_SIZE */ - void flush_cache(ulong start_addr, ulong size) { unsigned long ilsize = icache_line_size(); diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S index e4a44ff..41cb3a6 100644 --- a/arch/mips/lib/cache_init.S +++ b/arch/mips/lib/cache_init.S @@ -101,14 +101,14 @@ LEAF(mips_cache_reset) #if CONFIG_SYS_ICACHE_SIZE != 0 li t2, CONFIG_SYS_ICACHE_SIZE - li t8, CONFIG_SYS_CACHELINE_SIZE + li t8, CONFIG_SYS_ICACHE_LINE_SIZE #else l1_info t2, t8, MIPS_CONF1_IA_SHF #endif
#if CONFIG_SYS_DCACHE_SIZE != 0 li t3, CONFIG_SYS_DCACHE_SIZE - li t9, CONFIG_SYS_CACHELINE_SIZE + li t9, CONFIG_SYS_DCACHE_LINE_SIZE #else l1_info t3, t9, MIPS_CONF1_DA_SHF #endif diff --git a/board/dbau1x00/Kconfig b/board/dbau1x00/Kconfig index 1715a28..448176d 100644 --- a/board/dbau1x00/Kconfig +++ b/board/dbau1x00/Kconfig @@ -15,10 +15,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 16384
+config SYS_DCACHE_LINE_SIZE + default 32 + config SYS_ICACHE_SIZE default 16384
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
menu "dbau1x00 board options" diff --git a/board/micronas/vct/Kconfig b/board/micronas/vct/Kconfig index 5bb6f03..df7c029 100644 --- a/board/micronas/vct/Kconfig +++ b/board/micronas/vct/Kconfig @@ -15,10 +15,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 16384
+config SYS_DCACHE_LINE_SIZE + default 32 + config SYS_ICACHE_SIZE default 16384
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
menu "vct board options" diff --git a/board/pb1x00/Kconfig b/board/pb1x00/Kconfig index 27b2ef0..ef8905d 100644 --- a/board/pb1x00/Kconfig +++ b/board/pb1x00/Kconfig @@ -15,10 +15,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 16384
+config SYS_DCACHE_LINE_SIZE + default 32 + config SYS_ICACHE_SIZE default 16384
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
endif diff --git a/board/qca/ap121/Kconfig b/board/qca/ap121/Kconfig index 1ace0e5..09a06d8 100644 --- a/board/qca/ap121/Kconfig +++ b/board/qca/ap121/Kconfig @@ -15,10 +15,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 0x8000
+config SYS_DCACHE_LINE_SIZE + default 32 + config SYS_ICACHE_SIZE default 0x10000
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
endif diff --git a/board/qca/ap143/Kconfig b/board/qca/ap143/Kconfig index ac73782..f3d658d 100644 --- a/board/qca/ap143/Kconfig +++ b/board/qca/ap143/Kconfig @@ -15,10 +15,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 0x8000
+config SYS_DCACHE_LINE_SIZE + default 32 + config SYS_ICACHE_SIZE default 0x10000
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
endif diff --git a/board/qemu-mips/Kconfig b/board/qemu-mips/Kconfig index 66957e7..e696a12 100644 --- a/board/qemu-mips/Kconfig +++ b/board/qemu-mips/Kconfig @@ -14,10 +14,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 16384
+config SYS_DCACHE_LINE_SIZE + default 32 + config SYS_ICACHE_SIZE default 16384
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
endif diff --git a/board/tplink/wdr4300/Kconfig b/board/tplink/wdr4300/Kconfig index bbec2e5..86afd76 100644 --- a/board/tplink/wdr4300/Kconfig +++ b/board/tplink/wdr4300/Kconfig @@ -18,10 +18,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 0x8000
+config SYS_DCACHE_LINE_SIZE + default 32 + config SYS_ICACHE_SIZE default 0x10000
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
endif

On 05/26/2016 05:58 PM, Paul Burton wrote:
Allow L1 Icache & L1 Dcache line size to be specified separately, since there's no architectural mandate that they be the same. The [id]cache_line_size functions are tidied up to take advantage of the fact that the Kconfig entries are always present to simply check them for zero rather than needing to #ifdef on their presence.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/Kconfig | 6 +++++- arch/mips/lib/cache.c | 22 +++++++--------------- arch/mips/lib/cache_init.S | 4 ++-- board/dbau1x00/Kconfig | 5 ++++- board/micronas/vct/Kconfig | 5 ++++- board/pb1x00/Kconfig | 5 ++++- board/qca/ap121/Kconfig | 5 ++++- board/qca/ap143/Kconfig | 5 ++++- board/qemu-mips/Kconfig | 5 ++++- board/tplink/wdr4300/Kconfig | 5 ++++- 10 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 13f1164..8af8799 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -247,11 +247,15 @@ config SYS_DCACHE_SIZE hex default 0
+config SYS_DCACHE_LINE_SIZE
- hex
- default 0
config SYS_ICACHE_SIZE hex default 0
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE
Be careful here, CONFIG_SYS_CACHELINE_SIZE is established all over the u-boot, so this might cause build breakage. You should keep some compatibility CONFIG_SYS_CACHELINE_SIZE entry and set it to default to SYS_DCACHE_LINE_SIZE , since it's mostly used for DMA anyway.
hex default 0
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 7695325..2bb91c6 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -9,23 +9,13 @@ #include <asm/cacheops.h> #include <asm/mipsregs.h>
-#if CONFIG_SYS_CACHELINE_SIZE != 0
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;
-}
- unsigned long conf1, il;
-#else /* !CONFIG_SYS_CACHELINE_SIZE */
- if (CONFIG_SYS_ICACHE_LINE_SIZE != 0)
return CONFIG_SYS_ICACHE_LINE_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_SHF; if (!il)
@@ -36,6 +26,10 @@ static inline unsigned long icache_line_size(void) static inline unsigned long dcache_line_size(void) { unsigned long conf1, dl;
- if (CONFIG_SYS_DCACHE_LINE_SIZE != 0)
return CONFIG_SYS_DCACHE_LINE_SIZE;
- conf1 = read_c0_config1(); dl = (conf1 & MIPS_CONF1_DL) >> MIPS_CONF1_DL_SHF; if (!dl)
@@ -43,8 +37,6 @@ static inline unsigned long dcache_line_size(void) return 2 << dl; }
-#endif /* !CONFIG_SYS_CACHELINE_SIZE */
void flush_cache(ulong start_addr, ulong size) { unsigned long ilsize = icache_line_size(); diff --git a/arch/mips/lib/cache_init.S b/arch/mips/lib/cache_init.S index e4a44ff..41cb3a6 100644 --- a/arch/mips/lib/cache_init.S +++ b/arch/mips/lib/cache_init.S @@ -101,14 +101,14 @@ LEAF(mips_cache_reset) #if CONFIG_SYS_ICACHE_SIZE != 0 li t2, CONFIG_SYS_ICACHE_SIZE
- li t8, CONFIG_SYS_CACHELINE_SIZE
- li t8, CONFIG_SYS_ICACHE_LINE_SIZE
#else l1_info t2, t8, MIPS_CONF1_IA_SHF #endif
#if CONFIG_SYS_DCACHE_SIZE != 0 li t3, CONFIG_SYS_DCACHE_SIZE
- li t9, CONFIG_SYS_CACHELINE_SIZE
- li t9, CONFIG_SYS_DCACHE_LINE_SIZE
#else l1_info t3, t9, MIPS_CONF1_DA_SHF #endif diff --git a/board/dbau1x00/Kconfig b/board/dbau1x00/Kconfig index 1715a28..448176d 100644 --- a/board/dbau1x00/Kconfig +++ b/board/dbau1x00/Kconfig @@ -15,10 +15,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 16384
+config SYS_DCACHE_LINE_SIZE
- default 32
config SYS_ICACHE_SIZE default 16384
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
menu "dbau1x00 board options" diff --git a/board/micronas/vct/Kconfig b/board/micronas/vct/Kconfig index 5bb6f03..df7c029 100644 --- a/board/micronas/vct/Kconfig +++ b/board/micronas/vct/Kconfig @@ -15,10 +15,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 16384
+config SYS_DCACHE_LINE_SIZE
- default 32
config SYS_ICACHE_SIZE default 16384
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
menu "vct board options" diff --git a/board/pb1x00/Kconfig b/board/pb1x00/Kconfig index 27b2ef0..ef8905d 100644 --- a/board/pb1x00/Kconfig +++ b/board/pb1x00/Kconfig @@ -15,10 +15,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 16384
+config SYS_DCACHE_LINE_SIZE
- default 32
config SYS_ICACHE_SIZE default 16384
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
endif diff --git a/board/qca/ap121/Kconfig b/board/qca/ap121/Kconfig index 1ace0e5..09a06d8 100644 --- a/board/qca/ap121/Kconfig +++ b/board/qca/ap121/Kconfig @@ -15,10 +15,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 0x8000
+config SYS_DCACHE_LINE_SIZE
- default 32
config SYS_ICACHE_SIZE default 0x10000
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
endif diff --git a/board/qca/ap143/Kconfig b/board/qca/ap143/Kconfig index ac73782..f3d658d 100644 --- a/board/qca/ap143/Kconfig +++ b/board/qca/ap143/Kconfig @@ -15,10 +15,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 0x8000
+config SYS_DCACHE_LINE_SIZE
- default 32
config SYS_ICACHE_SIZE default 0x10000
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
endif diff --git a/board/qemu-mips/Kconfig b/board/qemu-mips/Kconfig index 66957e7..e696a12 100644 --- a/board/qemu-mips/Kconfig +++ b/board/qemu-mips/Kconfig @@ -14,10 +14,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 16384
+config SYS_DCACHE_LINE_SIZE
- default 32
config SYS_ICACHE_SIZE default 16384
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
endif diff --git a/board/tplink/wdr4300/Kconfig b/board/tplink/wdr4300/Kconfig index bbec2e5..86afd76 100644 --- a/board/tplink/wdr4300/Kconfig +++ b/board/tplink/wdr4300/Kconfig @@ -18,10 +18,13 @@ config SYS_TEXT_BASE config SYS_DCACHE_SIZE default 0x8000
+config SYS_DCACHE_LINE_SIZE
- default 32
config SYS_ICACHE_SIZE default 0x10000
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE default 32
endif

Am 26.05.2016 um 18:12 schrieb Marek Vasut:
On 05/26/2016 05:58 PM, Paul Burton wrote:
Allow L1 Icache & L1 Dcache line size to be specified separately, since there's no architectural mandate that they be the same. The [id]cache_line_size functions are tidied up to take advantage of the fact that the Kconfig entries are always present to simply check them for zero rather than needing to #ifdef on their presence.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/Kconfig | 6 +++++- arch/mips/lib/cache.c | 22 +++++++--------------- arch/mips/lib/cache_init.S | 4 ++-- board/dbau1x00/Kconfig | 5 ++++- board/micronas/vct/Kconfig | 5 ++++- board/pb1x00/Kconfig | 5 ++++- board/qca/ap121/Kconfig | 5 ++++- board/qca/ap143/Kconfig | 5 ++++- board/qemu-mips/Kconfig | 5 ++++- board/tplink/wdr4300/Kconfig | 5 ++++- 10 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 13f1164..8af8799 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -247,11 +247,15 @@ config SYS_DCACHE_SIZE hex default 0
+config SYS_DCACHE_LINE_SIZE
- hex
- default 0
config SYS_ICACHE_SIZE hex default 0
-config SYS_CACHELINE_SIZE +config SYS_ICACHE_LINE_SIZE
Be careful here, CONFIG_SYS_CACHELINE_SIZE is established all over the u-boot, so this might cause build breakage. You should keep some compatibility CONFIG_SYS_CACHELINE_SIZE entry and set it to default to SYS_DCACHE_LINE_SIZE , since it's mostly used for DMA anyway.
Marek is right. Some USB drivers still use CONFIG_SYS_CACHELINE_SIZE instead of ARCH_DMA_MINALIGN so CONFIG_SYS_CACHELINE_SIZE needs to be kept as Kconfig symbol.

Am 26.05.2016 um 17:58 schrieb Paul Burton:
Allow L1 Icache & L1 Dcache line size to be specified separately, since there's no architectural mandate that they be the same. The [id]cache_line_size functions are tidied up to take advantage of the fact that the Kconfig entries are always present to simply check them for zero rather than needing to #ifdef on their presence.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/Kconfig | 6 +++++- arch/mips/lib/cache.c | 22 +++++++--------------- arch/mips/lib/cache_init.S | 4 ++-- board/dbau1x00/Kconfig | 5 ++++- board/micronas/vct/Kconfig | 5 ++++- board/pb1x00/Kconfig | 5 ++++- board/qca/ap121/Kconfig | 5 ++++- board/qca/ap143/Kconfig | 5 ++++- board/qemu-mips/Kconfig | 5 ++++- board/tplink/wdr4300/Kconfig | 5 ++++- 10 files changed, 42 insertions(+), 25 deletions(-)
applied to u-boot-mips, thanks!

The various cache maintenance routines perform a number of loops over cache lines. Rather than duplicate the code for performing such loops, abstract it out into a new cache_loop macro which performs an arbitrary number of cache ops on a range of addresses. This reduces duplication in the existing L1 cache maintenance code & will allow for not adding further duplication when introducing L2 cache support.
Signed-off-by: Paul Burton paul.burton@imgtec.com
---
arch/mips/lib/cache.c | 59 ++++++++++++++++----------------------------------- 1 file changed, 18 insertions(+), 41 deletions(-)
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 2bb91c6..320335c 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -37,82 +37,59 @@ static inline unsigned long dcache_line_size(void) return 2 << dl; }
+#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)); \ + const unsigned int cache_ops[] = { ops }; \ + unsigned int i; \ + \ + for (; addr <= aend; addr += lsize) { \ + for (i = 0; i < ARRAY_SIZE(cache_ops); i++) \ + mips_cache(cache_ops[i], addr); \ + } \ +} while (0) + void flush_cache(ulong start_addr, ulong size) { unsigned long ilsize = icache_line_size(); unsigned long dlsize = dcache_line_size(); - const void *addr, *aend;
/* aend will be miscalculated when size is zero, so we return here */ if (size == 0) return;
- addr = (const void *)(start_addr & ~(dlsize - 1)); - aend = (const 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; - } + cache_loop(start_addr, start_addr + size, ilsize, + HIT_WRITEBACK_INV_D, HIT_INVALIDATE_I); return; }
/* flush D-cache */ - while (1) { - mips_cache(HIT_WRITEBACK_INV_D, addr); - if (addr == aend) - break; - addr += dlsize; - } + cache_loop(start_addr, start_addr + size, dlsize, HIT_WRITEBACK_INV_D);
/* flush I-cache */ - addr = (const void *)(start_addr & ~(ilsize - 1)); - aend = (const void *)((start_addr + size - 1) & ~(ilsize - 1)); - while (1) { - mips_cache(HIT_INVALIDATE_I, addr); - if (addr == aend) - break; - addr += ilsize; - } + cache_loop(start_addr, start_addr + size, ilsize, HIT_INVALIDATE_I); }
void flush_dcache_range(ulong start_addr, ulong stop) { unsigned long lsize = dcache_line_size(); - const void *addr = (const void *)(start_addr & ~(lsize - 1)); - const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
/* aend will be miscalculated when size is zero, so we return here */ if (start_addr == stop) return;
- while (1) { - mips_cache(HIT_WRITEBACK_INV_D, addr); - if (addr == aend) - break; - addr += lsize; - } + cache_loop(start_addr, stop, lsize, HIT_WRITEBACK_INV_D); }
void invalidate_dcache_range(ulong start_addr, ulong stop) { unsigned long lsize = dcache_line_size(); - const void *addr = (const void *)(start_addr & ~(lsize - 1)); - const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
/* aend will be miscalculated when size is zero, so we return here */ if (start_addr == stop) return;
- while (1) { - mips_cache(HIT_INVALIDATE_D, addr); - if (addr == aend) - break; - addr += lsize; - } + cache_loop(start_addr, stop, lsize, HIT_INVALIDATE_I); }

On 05/26/2016 05:58 PM, Paul Burton wrote:
The various cache maintenance routines perform a number of loops over cache lines. Rather than duplicate the code for performing such loops, abstract it out into a new cache_loop macro which performs an arbitrary number of cache ops on a range of addresses. This reduces duplication in the existing L1 cache maintenance code & will allow for not adding further duplication when introducing L2 cache support.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/lib/cache.c | 59 ++++++++++++++++----------------------------------- 1 file changed, 18 insertions(+), 41 deletions(-)
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 2bb91c6..320335c 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -37,82 +37,59 @@ static inline unsigned long dcache_line_size(void) return 2 << dl; }
+#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)); \
- const unsigned int cache_ops[] = { ops }; \
Can't you turn this into a function instead and pass some flags to select the ops instead ?
- unsigned int i; \
\
- for (; addr <= aend; addr += lsize) { \
for (i = 0; i < ARRAY_SIZE(cache_ops); i++) \
mips_cache(cache_ops[i], addr); \
- } \
+} while (0)
void flush_cache(ulong start_addr, ulong size) { unsigned long ilsize = icache_line_size(); unsigned long dlsize = dcache_line_size();
const void *addr, *aend;
/* aend will be miscalculated when size is zero, so we return here */ if (size == 0) return;
addr = (const void *)(start_addr & ~(dlsize - 1));
aend = (const 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;
}
cache_loop(start_addr, start_addr + size, ilsize,
HIT_WRITEBACK_INV_D, HIT_INVALIDATE_I);
return; }
/* flush D-cache */
- while (1) {
mips_cache(HIT_WRITEBACK_INV_D, addr);
if (addr == aend)
break;
addr += dlsize;
- }
cache_loop(start_addr, start_addr + size, dlsize, HIT_WRITEBACK_INV_D);
/* flush I-cache */
- addr = (const void *)(start_addr & ~(ilsize - 1));
- aend = (const void *)((start_addr + size - 1) & ~(ilsize - 1));
- while (1) {
mips_cache(HIT_INVALIDATE_I, addr);
if (addr == aend)
break;
addr += ilsize;
- }
- cache_loop(start_addr, start_addr + size, ilsize, HIT_INVALIDATE_I);
}
void flush_dcache_range(ulong start_addr, ulong stop) { unsigned long lsize = dcache_line_size();
const void *addr = (const void *)(start_addr & ~(lsize - 1));
const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
/* aend will be miscalculated when size is zero, so we return here */ if (start_addr == stop) return;
while (1) {
mips_cache(HIT_WRITEBACK_INV_D, addr);
if (addr == aend)
break;
addr += lsize;
}
- cache_loop(start_addr, stop, lsize, HIT_WRITEBACK_INV_D);
}
void invalidate_dcache_range(ulong start_addr, ulong stop) { unsigned long lsize = dcache_line_size();
const void *addr = (const void *)(start_addr & ~(lsize - 1));
const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
/* aend will be miscalculated when size is zero, so we return here */ if (start_addr == stop) return;
while (1) {
mips_cache(HIT_INVALIDATE_D, addr);
if (addr == aend)
break;
addr += lsize;
}
- cache_loop(start_addr, stop, lsize, HIT_INVALIDATE_I);
}

On Thu, May 26, 2016 at 06:13:17PM +0200, Marek Vasut wrote:
On 05/26/2016 05:58 PM, Paul Burton wrote:
The various cache maintenance routines perform a number of loops over cache lines. Rather than duplicate the code for performing such loops, abstract it out into a new cache_loop macro which performs an arbitrary number of cache ops on a range of addresses. This reduces duplication in the existing L1 cache maintenance code & will allow for not adding further duplication when introducing L2 cache support.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/lib/cache.c | 59 ++++++++++++++++----------------------------------- 1 file changed, 18 insertions(+), 41 deletions(-)
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 2bb91c6..320335c 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -37,82 +37,59 @@ static inline unsigned long dcache_line_size(void) return 2 << dl; }
+#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)); \
- const unsigned int cache_ops[] = { ops }; \
Can't you turn this into a function instead and pass some flags to select the ops instead ?
Hi Marek,
The problem is that then both that function & its callers would need to know about the types of cache ops, and there'd need to be some mapping from flags to the actual cache op values (of which there'll be a couple more once L2 support is added). I think it's much simpler & cleaner to just go with the macro in this case - it keeps knowledge of which cache ops to use with the callers, and it ends up generating the same code as before (ie. the inner loop gets unrolled to just the relevant cache instructions).
Thanks, Paul
- unsigned int i; \
\
- for (; addr <= aend; addr += lsize) { \
for (i = 0; i < ARRAY_SIZE(cache_ops); i++) \
mips_cache(cache_ops[i], addr); \
- } \
+} while (0)
void flush_cache(ulong start_addr, ulong size) { unsigned long ilsize = icache_line_size(); unsigned long dlsize = dcache_line_size();
const void *addr, *aend;
/* aend will be miscalculated when size is zero, so we return here */ if (size == 0) return;
addr = (const void *)(start_addr & ~(dlsize - 1));
aend = (const 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;
}
cache_loop(start_addr, start_addr + size, ilsize,
HIT_WRITEBACK_INV_D, HIT_INVALIDATE_I);
return; }
/* flush D-cache */
- while (1) {
mips_cache(HIT_WRITEBACK_INV_D, addr);
if (addr == aend)
break;
addr += dlsize;
- }
cache_loop(start_addr, start_addr + size, dlsize, HIT_WRITEBACK_INV_D);
/* flush I-cache */
- addr = (const void *)(start_addr & ~(ilsize - 1));
- aend = (const void *)((start_addr + size - 1) & ~(ilsize - 1));
- while (1) {
mips_cache(HIT_INVALIDATE_I, addr);
if (addr == aend)
break;
addr += ilsize;
- }
- cache_loop(start_addr, start_addr + size, ilsize, HIT_INVALIDATE_I);
}
void flush_dcache_range(ulong start_addr, ulong stop) { unsigned long lsize = dcache_line_size();
const void *addr = (const void *)(start_addr & ~(lsize - 1));
const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
/* aend will be miscalculated when size is zero, so we return here */ if (start_addr == stop) return;
while (1) {
mips_cache(HIT_WRITEBACK_INV_D, addr);
if (addr == aend)
break;
addr += lsize;
}
- cache_loop(start_addr, stop, lsize, HIT_WRITEBACK_INV_D);
}
void invalidate_dcache_range(ulong start_addr, ulong stop) { unsigned long lsize = dcache_line_size();
const void *addr = (const void *)(start_addr & ~(lsize - 1));
const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
/* aend will be miscalculated when size is zero, so we return here */ if (start_addr == stop) return;
while (1) {
mips_cache(HIT_INVALIDATE_D, addr);
if (addr == aend)
break;
addr += lsize;
}
- cache_loop(start_addr, stop, lsize, HIT_INVALIDATE_I);
}
-- Best regards, Marek Vasut

On 05/27/2016 12:30 PM, Paul Burton wrote:
On Thu, May 26, 2016 at 06:13:17PM +0200, Marek Vasut wrote:
On 05/26/2016 05:58 PM, Paul Burton wrote:
The various cache maintenance routines perform a number of loops over cache lines. Rather than duplicate the code for performing such loops, abstract it out into a new cache_loop macro which performs an arbitrary number of cache ops on a range of addresses. This reduces duplication in the existing L1 cache maintenance code & will allow for not adding further duplication when introducing L2 cache support.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/lib/cache.c | 59 ++++++++++++++++----------------------------------- 1 file changed, 18 insertions(+), 41 deletions(-)
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index 2bb91c6..320335c 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -37,82 +37,59 @@ static inline unsigned long dcache_line_size(void) return 2 << dl; }
+#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)); \
- const unsigned int cache_ops[] = { ops }; \
Can't you turn this into a function instead and pass some flags to select the ops instead ?
Hi Marek,
Hi!
The problem is that then both that function & its callers would need to know about the types of cache ops, and there'd need to be some mapping from flags to the actual cache op values (of which there'll be a couple more once L2 support is added).
But the callers already know which ops they must invoke, right ? You can just have enum {} for the bit definitions. It's all local to this file, so I don't see a problem.
I think it's much simpler & cleaner to just go with the macro in this case - it keeps knowledge of which cache ops to use with the callers, and it ends up generating the same code as before (ie. the inner loop gets unrolled to just the relevant cache instructions).
I would expect GCC to take care of inlining the code, but you can always use the always_inline directive if that's your concern. Even cleaner way might be to use CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED and compile the cache file with -O2 .
Thanks, Paul
- unsigned int i; \
\
- for (; addr <= aend; addr += lsize) { \
for (i = 0; i < ARRAY_SIZE(cache_ops); i++) \
mips_cache(cache_ops[i], addr); \
- } \
+} while (0)
void flush_cache(ulong start_addr, ulong size) { unsigned long ilsize = icache_line_size(); unsigned long dlsize = dcache_line_size();
const void *addr, *aend;
/* aend will be miscalculated when size is zero, so we return here */ if (size == 0) return;
addr = (const void *)(start_addr & ~(dlsize - 1));
aend = (const 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;
}
cache_loop(start_addr, start_addr + size, ilsize,
HIT_WRITEBACK_INV_D, HIT_INVALIDATE_I);
return; }
/* flush D-cache */
- while (1) {
mips_cache(HIT_WRITEBACK_INV_D, addr);
if (addr == aend)
break;
addr += dlsize;
- }
cache_loop(start_addr, start_addr + size, dlsize, HIT_WRITEBACK_INV_D);
/* flush I-cache */
- addr = (const void *)(start_addr & ~(ilsize - 1));
- aend = (const void *)((start_addr + size - 1) & ~(ilsize - 1));
- while (1) {
mips_cache(HIT_INVALIDATE_I, addr);
if (addr == aend)
break;
addr += ilsize;
- }
- cache_loop(start_addr, start_addr + size, ilsize, HIT_INVALIDATE_I);
}
void flush_dcache_range(ulong start_addr, ulong stop) { unsigned long lsize = dcache_line_size();
const void *addr = (const void *)(start_addr & ~(lsize - 1));
const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
/* aend will be miscalculated when size is zero, so we return here */ if (start_addr == stop) return;
while (1) {
mips_cache(HIT_WRITEBACK_INV_D, addr);
if (addr == aend)
break;
addr += lsize;
}
- cache_loop(start_addr, stop, lsize, HIT_WRITEBACK_INV_D);
}
void invalidate_dcache_range(ulong start_addr, ulong stop) { unsigned long lsize = dcache_line_size();
const void *addr = (const void *)(start_addr & ~(lsize - 1));
const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
/* aend will be miscalculated when size is zero, so we return here */ if (start_addr == stop) return;
while (1) {
mips_cache(HIT_INVALIDATE_D, addr);
if (addr == aend)
break;
addr += lsize;
}
- cache_loop(start_addr, stop, lsize, HIT_INVALIDATE_I);
}
-- Best regards, Marek Vasut

On Fri, May 27, 2016 at 04:36:21PM +0200, Marek Vasut wrote:
The problem is that then both that function & its callers would need to know about the types of cache ops, and there'd need to be some mapping from flags to the actual cache op values (of which there'll be a couple more once L2 support is added).
But the callers already know which ops they must invoke, right ? You can just have enum {} for the bit definitions. It's all local to this file, so I don't see a problem.
It's all local to the file, but it's still an extra level of abstraction that I don't see the value of. It just means that both callers & callee have to know about various cache ops, and I don't see the point in duplicating that knowledge. I think it's much cleaner to keep that knowledge in one place - the caller.
I think it's much simpler & cleaner to just go with the macro in this case - it keeps knowledge of which cache ops to use with the callers, and it ends up generating the same code as before (ie. the inner loop gets unrolled to just the relevant cache instructions).
I would expect GCC to take care of inlining the code, but you can always use the always_inline directive if that's your concern. Even cleaner way might be to use CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED and compile the cache file with -O2 .
It may well inline it, remove the flags indirection & generate the same code - I haven't tried it to find out. Making the code less clear with that abstraction, even if the compiler will come along & clean it back up, doesn't make much sense to me as someone who has to read the code.
I understand the preference for functions over macros, but I think this is a case where the macro has strengths that a function cannot provide (ignoring perhaps varargs which would be even more messy).
Thanks, Paul

On 05/27/2016 04:48 PM, Paul Burton wrote:
On Fri, May 27, 2016 at 04:36:21PM +0200, Marek Vasut wrote:
The problem is that then both that function & its callers would need to know about the types of cache ops, and there'd need to be some mapping from flags to the actual cache op values (of which there'll be a couple more once L2 support is added).
But the callers already know which ops they must invoke, right ? You can just have enum {} for the bit definitions. It's all local to this file, so I don't see a problem.
It's all local to the file, but it's still an extra level of abstraction that I don't see the value of. It just means that both callers & callee have to know about various cache ops, and I don't see the point in duplicating that knowledge. I think it's much cleaner to keep that knowledge in one place - the caller.
OK, I see what you mean.
I think it's much simpler & cleaner to just go with the macro in this case - it keeps knowledge of which cache ops to use with the callers, and it ends up generating the same code as before (ie. the inner loop gets unrolled to just the relevant cache instructions).
I would expect GCC to take care of inlining the code, but you can always use the always_inline directive if that's your concern. Even cleaner way might be to use CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED and compile the cache file with -O2 .
It may well inline it, remove the flags indirection & generate the same code - I haven't tried it to find out. Making the code less clear with that abstraction, even if the compiler will come along & clean it back up, doesn't make much sense to me as someone who has to read the code.
It makes sense when you debug the code with GDB though. It's much easier to locate lines if you don't use huge variadic macros.
I understand the preference for functions over macros, but I think this is a case where the macro has strengths that a function cannot provide (ignoring perhaps varargs which would be even more messy).
What would you say to turning this into a variadic function then ? It would avoid having the same knowledge in both caller and callee and it would not be a macro, so it's a win-win. What do you think ?
Thanks, Paul
participants (4)
-
Daniel Schwierzeck
-
Marek Vasut
-
Paul Burton
-
Zubair Lutfullah Kakakhel