Hi, On Sat, 28 Mar 2020 at 12:25, Ovidiu Panait <ovpan...@gmail.com> wrote: > > Introduce arch_reserve_mmu to allow for architecture-specific reserve_mmu > routines. > > For ARM, move the reserve_mmu definition from common/board_f.c to > arch/arm/lib/cache.c.
Can you please do that bit in an initial patch before this one? > Define arm_reserve_mmu and make it a weak define to allow > machines to override it (in mach-versal/cpu.c and mach-zynqmp/cpu.c). > > Signed-off-by: Ovidiu Panait <ovpan...@gmail.com> > --- > arch/arm/include/asm/cache.h | 2 ++ > arch/arm/lib/cache.c | 33 +++++++++++++++++++++++++++++++++ > arch/arm/mach-versal/cpu.c | 6 +++++- > arch/arm/mach-zynqmp/cpu.c | 6 +++++- > common/board_f.c | 32 ++++++-------------------------- > include/init.h | 2 +- > 6 files changed, 52 insertions(+), 29 deletions(-) Can you please try to use if() instead of #if as much as possible? > > diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h > index 950ec1e793..dbb9c554ae 100644 > --- a/arch/arm/include/asm/cache.h > +++ b/arch/arm/include/asm/cache.h > @@ -49,4 +49,6 @@ void dram_bank_mmu_setup(int bank); > */ > #define ARCH_DMA_MINALIGN CONFIG_SYS_CACHELINE_SIZE > > +int arm_reserve_mmu(void); Function comment > + > #endif /* _ASM_CACHE_H */ > diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c > index 007d4ebc49..76c10a577b 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,34 @@ void invalidate_l2_cache(void) > isb(); > } > #endif > + > +int arch_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 */ > + 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/arch/arm/mach-versal/cpu.c b/arch/arm/mach-versal/cpu.c > index 6ee6cd43ec..6c5da8b29e 100644 > --- a/arch/arm/mach-versal/cpu.c > +++ b/arch/arm/mach-versal/cpu.c > @@ -10,6 +10,10 @@ > #include <asm/arch/hardware.h> > #include <asm/arch/sys_proto.h> > > +#if defined(CONFIG_SYS_MEM_RSVD_FOR_MMU) Can you drop that #if? > +#include <asm/cache.h> > +#endif > + > DECLARE_GLOBAL_DATA_PTR; > > #define VERSAL_MEM_MAP_USED 5 > @@ -98,7 +102,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..363a20b621 100644 > --- a/arch/arm/mach-zynqmp/cpu.c > +++ b/arch/arm/mach-zynqmp/cpu.c > @@ -12,6 +12,10 @@ > #include <asm/io.h> > #include <zynqmp_firmware.h> > > +#ifdef CONFIG_SYS_MEM_RSVD_FOR_MMU Can you drop this #ifdef? > +#include <asm/cache.h> > +#endif > + > #define ZYNQ_SILICON_VER_MASK 0xF000 > #define ZYNQ_SILICON_VER_SHIFT 12 > > @@ -116,7 +120,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; > diff --git a/common/board_f.c b/common/board_f.c > index 82a164752a..2ab23cf239 100644 > --- a/common/board_f.c > +++ b/common/board_f.c > @@ -385,33 +385,15 @@ static int reserve_round_4k(void) > return 0; > } > > -#ifdef CONFIG_ARM > -__weak int reserve_mmu(void) > +__weak int arch_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_mmu(void) > +{ > + return arch_reserve_mmu(); > +} Then can we just put arch_reserve_mmu() in the thing below and drop this fnuction? > > static int reserve_video(void) > { > @@ -970,9 +952,7 @@ static const init_fnc_t init_sequence_f[] = { > reserve_pram, > #endif > reserve_round_4k, > -#ifdef CONFIG_ARM > reserve_mmu, > -#endif > reserve_video, > reserve_trace, > reserve_uboot, > diff --git a/include/init.h b/include/init.h > index 2a33a3fd1e..5700dc7ecb 100644 > --- a/include/init.h > +++ b/include/init.h > @@ -145,7 +145,7 @@ int init_cache_f_r(void); > int print_cpuinfo(void); > #endif > int timer_init(void); > -int reserve_mmu(void); > +int arch_reserve_mmu(void); function comment > int misc_init_f(void); > > #if defined(CONFIG_DTB_RESELECT) > -- > 2.17.1 > Regards, Simon