[PATCH v2 0/3] common/board_f: Make reserve_mmu generic
Changes in v2: - split the original patch in 3 parts - add function comments - drop #ifdefs around asm/cache.h includes - drop reserve_mmu function and leave only arch_reserve_mmu
For the following suggestion: "Can you please try to use if() instead of #if as much as possible?"
I tried replacing in reserve_mmu #if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) with if (!(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF))) { ... } but it caused compile issues on different arm platforms such as: ... arch/arm/lib/cache.c: In function 'arm_reserve_mmu': arch/arm/lib/cache.c:134:11: error: 'volatile struct arch_global_data' has no member named 'tlb_size' gd->arch.tlb_size = PGTABLE_SIZE; ^ arch/arm/lib/cache.c:135:28: error: 'volatile struct arch_global_data' has no member named 'tlb_size' gd->relocaddr -= gd->arch.tlb_size; ^ arch/arm/lib/cache.c:140:11: error: 'volatile struct arch_global_data' has no member named 'tlb_addr' gd->arch.tlb_addr = gd->relocaddr; In file included from include/common.h:34:0, from arch/arm/lib/cache.c:9: arch/arm/lib/cache.c:141:52: error: 'volatile struct arch_global_data' has no member named 'tlb_addr' ...
Ovidiu Panait (3): common/board_f: Move arm-specific reserve_mmu to arch/arm/lib/cache.c arm: asm/cache.c: Introduce arm_reserve_mmu common/board_f: Make reserve_mmu generic
arch/arm/include/asm/cache.h | 11 +++++++++++ arch/arm/lib/cache.c | 33 +++++++++++++++++++++++++++++++++ arch/arm/mach-versal/cpu.c | 3 ++- arch/arm/mach-zynqmp/cpu.c | 3 ++- common/board_f.c | 29 ++--------------------------- include/init.h | 13 ++++++++++++- 6 files changed, 62 insertions(+), 30 deletions(-)
Move the ARM-specific reserve_mmu definition from common/board_f.c to arch/arm/lib/cache.c.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com --- arch/arm/lib/cache.c | 28 ++++++++++++++++++++++++++++ common/board_f.c | 28 ---------------------------- 2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index 007d4ebc49..b8e1e340a1 100644 --- a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -10,6 +10,8 @@ #include <cpu_func.h> #include <malloc.h>
+DECLARE_GLOBAL_DATA_PTR; + /* * Flush range from all levels of d-cache/unified-cache. * Affects the range [start, start + size - 1]. @@ -118,3 +120,29 @@ void invalidate_l2_cache(void) isb(); } #endif + +__weak int reserve_mmu(void) +{ +#if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) + /* reserve TLB table */ + gd->arch.tlb_size = PGTABLE_SIZE; + gd->relocaddr -= gd->arch.tlb_size; + + /* round down to next 64 kB limit */ + gd->relocaddr &= ~(0x10000 - 1); + + gd->arch.tlb_addr = gd->relocaddr; + debug("TLB table from %08lx to %08lx\n", gd->arch.tlb_addr, + gd->arch.tlb_addr + gd->arch.tlb_size); + +#ifdef CONFIG_SYS_MEM_RESERVE_SECURE + /* + * Record allocated tlb_addr in case gd->tlb_addr to be overwritten + * with location within secure ram. + */ + gd->arch.tlb_allocated = gd->arch.tlb_addr; +#endif +#endif + + return 0; +} diff --git a/common/board_f.c b/common/board_f.c index 82a164752a..a88bd64630 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -385,34 +385,6 @@ static int reserve_round_4k(void) return 0; }
-#ifdef CONFIG_ARM -__weak int reserve_mmu(void) -{ -#if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) - /* reserve TLB table */ - gd->arch.tlb_size = PGTABLE_SIZE; - gd->relocaddr -= gd->arch.tlb_size; - - /* round down to next 64 kB limit */ - gd->relocaddr &= ~(0x10000 - 1); - - gd->arch.tlb_addr = gd->relocaddr; - debug("TLB table from %08lx to %08lx\n", gd->arch.tlb_addr, - gd->arch.tlb_addr + gd->arch.tlb_size); - -#ifdef CONFIG_SYS_MEM_RESERVE_SECURE - /* - * Record allocated tlb_addr in case gd->tlb_addr to be overwritten - * with location within secure ram. - */ - gd->arch.tlb_allocated = gd->arch.tlb_addr; -#endif -#endif - - return 0; -} -#endif - static int reserve_video(void) { #ifdef CONFIG_DM_VIDEO
On Sun, 29 Mar 2020 at 11:59, Ovidiu Panait ovpanait@gmail.com wrote:
Move the ARM-specific reserve_mmu definition from common/board_f.c to arch/arm/lib/cache.c.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com
arch/arm/lib/cache.c | 28 ++++++++++++++++++++++++++++ common/board_f.c | 28 ---------------------------- 2 files changed, 28 insertions(+), 28 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Sun, Mar 29, 2020 at 08:57:39PM +0300, Ovidiu Panait wrote:
Move the ARM-specific reserve_mmu definition from common/board_f.c to arch/arm/lib/cache.c.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
As a preparation for turning reserve_mmu into an arch-specific variant, introduce arm_reserve_mmu on ARM. It implements the default routine for reserving memory for MMU TLB and needs to be weakly defined in order to allow for machines to override it.
Without this decoupling, after introducing arch_reserve_mmu, there would be two weak definitions for it, one in common/board_f.c and one in arch/arm/lib/cache.c.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com --- arch/arm/include/asm/cache.h | 11 +++++++++++ arch/arm/lib/cache.c | 5 +++++ arch/arm/mach-versal/cpu.c | 3 ++- arch/arm/mach-zynqmp/cpu.c | 3 ++- 4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h index 950ec1e793..c20e05ec7f 100644 --- a/arch/arm/include/asm/cache.h +++ b/arch/arm/include/asm/cache.h @@ -49,4 +49,15 @@ void dram_bank_mmu_setup(int bank); */ #define ARCH_DMA_MINALIGN CONFIG_SYS_CACHELINE_SIZE
+/* + * arm_reserve_mmu() - Reserve memory for MMU TLB table + * + * Default implementation for reserving memory for MMU TLB table. It is used + * during generic board init sequence in common/board_f.c. Weakly defined, so + * that machines can override it if needed. + * + * Return: 0 if OK + */ +int arm_reserve_mmu(void); + #endif /* _ASM_CACHE_H */ diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index b8e1e340a1..3cbed602eb 100644 --- a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -122,6 +122,11 @@ void invalidate_l2_cache(void) #endif
__weak int reserve_mmu(void) +{ + return arm_reserve_mmu(); +} + +__weak int arm_reserve_mmu(void) { #if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) /* reserve TLB table */ diff --git a/arch/arm/mach-versal/cpu.c b/arch/arm/mach-versal/cpu.c index 6ee6cd43ec..c14c5bb39c 100644 --- a/arch/arm/mach-versal/cpu.c +++ b/arch/arm/mach-versal/cpu.c @@ -9,6 +9,7 @@ #include <asm/io.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h> +#include <asm/cache.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -98,7 +99,7 @@ u64 get_page_table_size(void) }
#if defined(CONFIG_SYS_MEM_RSVD_FOR_MMU) -int reserve_mmu(void) +int arm_reserve_mmu(void) { tcm_init(TCM_LOCK); gd->arch.tlb_size = PGTABLE_SIZE; diff --git a/arch/arm/mach-zynqmp/cpu.c b/arch/arm/mach-zynqmp/cpu.c index 442427bc11..811684a9f8 100644 --- a/arch/arm/mach-zynqmp/cpu.c +++ b/arch/arm/mach-zynqmp/cpu.c @@ -11,6 +11,7 @@ #include <asm/armv8/mmu.h> #include <asm/io.h> #include <zynqmp_firmware.h> +#include <asm/cache.h>
#define ZYNQ_SILICON_VER_MASK 0xF000 #define ZYNQ_SILICON_VER_SHIFT 12 @@ -116,7 +117,7 @@ void tcm_init(u8 mode) #endif
#ifdef CONFIG_SYS_MEM_RSVD_FOR_MMU -int reserve_mmu(void) +int arm_reserve_mmu(void) { tcm_init(TCM_LOCK); gd->arch.tlb_size = PGTABLE_SIZE;
Hi Ovidiu,
On Sun, 29 Mar 2020 at 11:59, Ovidiu Panait ovpanait@gmail.com wrote:
As a preparation for turning reserve_mmu into an arch-specific variant, introduce arm_reserve_mmu on ARM. It implements the default routine for reserving memory for MMU TLB and needs to be weakly defined in order to allow for machines to override it.
Without this decoupling, after introducing arch_reserve_mmu, there would be two weak definitions for it, one in common/board_f.c and one in arch/arm/lib/cache.c.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com
arch/arm/include/asm/cache.h | 11 +++++++++++ arch/arm/lib/cache.c | 5 +++++ arch/arm/mach-versal/cpu.c | 3 ++- arch/arm/mach-zynqmp/cpu.c | 3 ++- 4 files changed, 20 insertions(+), 2 deletions(-)
I'm not a fan of weak functions and here we have two layers of them. But I'm not sure what else to suggest, other than a Kconfig to enable/disable the generic arm_reserve_mmu().
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h index 950ec1e793..c20e05ec7f 100644 --- a/arch/arm/include/asm/cache.h +++ b/arch/arm/include/asm/cache.h @@ -49,4 +49,15 @@ void dram_bank_mmu_setup(int bank); */ #define ARCH_DMA_MINALIGN CONFIG_SYS_CACHELINE_SIZE
+/*
- arm_reserve_mmu() - Reserve memory for MMU TLB table
- Default implementation for reserving memory for MMU TLB table. It is used
- during generic board init sequence in common/board_f.c. Weakly defined, so
- that machines can override it if needed.
- Return: 0 if OK
- */
+int arm_reserve_mmu(void);
#endif /* _ASM_CACHE_H */ diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index b8e1e340a1..3cbed602eb 100644 --- a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -122,6 +122,11 @@ void invalidate_l2_cache(void) #endif
__weak int reserve_mmu(void) +{
return arm_reserve_mmu();+}
+__weak int arm_reserve_mmu(void) { #if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) /* reserve TLB table */ diff --git a/arch/arm/mach-versal/cpu.c b/arch/arm/mach-versal/cpu.c index 6ee6cd43ec..c14c5bb39c 100644 --- a/arch/arm/mach-versal/cpu.c +++ b/arch/arm/mach-versal/cpu.c @@ -9,6 +9,7 @@ #include <asm/io.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h> +#include <asm/cache.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -98,7 +99,7 @@ u64 get_page_table_size(void) }
#if defined(CONFIG_SYS_MEM_RSVD_FOR_MMU) -int reserve_mmu(void) +int arm_reserve_mmu(void) { tcm_init(TCM_LOCK); gd->arch.tlb_size = PGTABLE_SIZE; diff --git a/arch/arm/mach-zynqmp/cpu.c b/arch/arm/mach-zynqmp/cpu.c index 442427bc11..811684a9f8 100644 --- a/arch/arm/mach-zynqmp/cpu.c +++ b/arch/arm/mach-zynqmp/cpu.c @@ -11,6 +11,7 @@ #include <asm/armv8/mmu.h> #include <asm/io.h> #include <zynqmp_firmware.h> +#include <asm/cache.h>
#define ZYNQ_SILICON_VER_MASK 0xF000 #define ZYNQ_SILICON_VER_SHIFT 12 @@ -116,7 +117,7 @@ void tcm_init(u8 mode) #endif
#ifdef CONFIG_SYS_MEM_RSVD_FOR_MMU -int reserve_mmu(void) +int arm_reserve_mmu(void) { tcm_init(TCM_LOCK); gd->arch.tlb_size = PGTABLE_SIZE; -- 2.17.1
On Sun, Mar 29, 2020 at 08:57:40PM +0300, Ovidiu Panait wrote:
As a preparation for turning reserve_mmu into an arch-specific variant, introduce arm_reserve_mmu on ARM. It implements the default routine for reserving memory for MMU TLB and needs to be weakly defined in order to allow for machines to override it.
Without this decoupling, after introducing arch_reserve_mmu, there would be two weak definitions for it, one in common/board_f.c and one in arch/arm/lib/cache.c.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Introduce arch_reserve_mmu to allow for architecture-specific reserve_mmu routines. Also, define a weak nop stub for it.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com --- arch/arm/lib/cache.c | 2 +- common/board_f.c | 9 ++++++--- include/init.h | 13 ++++++++++++- 3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index 3cbed602eb..44dde26065 100644 --- a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -121,7 +121,7 @@ void invalidate_l2_cache(void) } #endif
-__weak int reserve_mmu(void) +int arch_reserve_mmu(void) { return arm_reserve_mmu(); } diff --git a/common/board_f.c b/common/board_f.c index a88bd64630..52750dd307 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -385,6 +385,11 @@ static int reserve_round_4k(void) return 0; }
+__weak int arch_reserve_mmu(void) +{ + return 0; +} + static int reserve_video(void) { #ifdef CONFIG_DM_VIDEO @@ -942,9 +947,7 @@ static const init_fnc_t init_sequence_f[] = { reserve_pram, #endif reserve_round_4k, -#ifdef CONFIG_ARM - reserve_mmu, -#endif + arch_reserve_mmu, reserve_video, reserve_trace, reserve_uboot, diff --git a/include/init.h b/include/init.h index 2a33a3fd1e..9ef88c966b 100644 --- a/include/init.h +++ b/include/init.h @@ -129,6 +129,18 @@ int testdram(void); */ int arch_reserve_stacks(void);
+/** + * arch_reserve_mmu() - Reserve memory for MMU TLB table + * + * Architecture-specific routine for reserving memory for the MMU TLB table. + * This is used in generic board init sequence in common/board_f.c. + * + * If an implementation is not provided, it will just be a nop stub. + * + * Return: 0 if OK + */ +int arch_reserve_mmu(void); + /** * init_cache_f_r() - Turn on the cache in preparation for relocation * @@ -145,7 +157,6 @@ int init_cache_f_r(void); int print_cpuinfo(void); #endif int timer_init(void); -int reserve_mmu(void); int misc_init_f(void);
#if defined(CONFIG_DTB_RESELECT)
On Sun, 29 Mar 2020 at 11:59, Ovidiu Panait ovpanait@gmail.com wrote:
Introduce arch_reserve_mmu to allow for architecture-specific reserve_mmu routines. Also, define a weak nop stub for it.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com
arch/arm/lib/cache.c | 2 +- common/board_f.c | 9 ++++++--- include/init.h | 13 ++++++++++++- 3 files changed, 19 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
This series is very well put together, thank you.
On Sun, Mar 29, 2020 at 08:57:41PM +0300, Ovidiu Panait wrote:
Introduce arch_reserve_mmu to allow for architecture-specific reserve_mmu routines. Also, define a weak nop stub for it.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (3)
-
Ovidiu Panait -
Simon Glass -
Tom Rini