[U-Boot] [PATCH v3 11/18] arm: Add control over cachability of memory regions

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hi Simon,

On Thu, 12 Jul 2012 08:25:11 -0700, Simon Glass <sjg@chromium.org> wrote:
> Add support for adjusting the cachability of an L1 section by updating
> the MMU. The mmu_set_region_dcache() function allows drivers to make
> these changes after the MMU is set up.
> 
> It is implemented only for ARMv7 at present.
> 
> This is needed for LCD support, where we want to make the LCD frame buffer
> write-through (or off) rather than write-back.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  arch/arm/cpu/armv7/cache_v7.c |   11 +++++++
>  arch/arm/include/asm/system.h |   30 ++++++++++++++++++++
>  arch/arm/lib/cache-cp15.c     |   62 +++++++++++++++++++++++++++++++++-------
>  3 files changed, 92 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
> index 1b4e808..5f6d039 100644
> --- a/arch/arm/cpu/armv7/cache_v7.c
> +++ b/arch/arm/cpu/armv7/cache_v7.c
> @@ -297,6 +297,12 @@ void arm_init_before_mmu(void)
>  	v7_inval_tlb();
>  }
>  
> +void mmu_page_table_flush(unsigned long start, unsigned long stop)
> +{
> +	flush_dcache_range(start, stop);
> +	v7_inval_tlb();
> +}
> +
>  /*
>   * Flush range from all levels of d-cache/unified-cache used:
>   * Affects the range [start, start + size - 1]
> @@ -329,6 +335,11 @@ void arm_init_before_mmu(void)
>  void  flush_cache(unsigned long start, unsigned long size)
>  {
>  }
> +
> +void mmu_page_table_flush(unsigned long start, unsigned long stop)
> +{
> +}
> +
>  #endif /* #ifndef CONFIG_SYS_DCACHE_OFF */
>  
>  #ifndef CONFIG_SYS_ICACHE_OFF
> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> index 2b28a26..f52a4fc 100644
> --- a/arch/arm/include/asm/system.h
> +++ b/arch/arm/include/asm/system.h
> @@ -75,6 +75,36 @@ static inline void set_cr(unsigned int val)
>  	isb();
>  }
>  
> +/* options available for data cache on each page */
> +enum dcache_option {
> +	DCACHE_OFF,
> +	DCACHE_WRITETHROUGH,
> +	DCACHE_WRITEBACK,
> +};
> +
> +/* Size of an MMU section */
> +enum {
> +	MMU_SECTION_SHIFT	= 20,
> +	MMU_SECTION_SIZE	= 1 << MMU_SECTION_SHIFT,
> +};
> +
> +/**
> + * Change the cache settings for a region.
> + *
> + * \param start		start address of memory region to change
> + * \param size		size of memory region to change
> + * \param option	dcache option to select
> + */
> +void mmu_set_region_dcache(u32 start, int size, enum dcache_option option);
> +
> +/**
> + * Register an update to the page tables, and flush the TLB
> + *
> + * \param start		start address of update in page table
> + * \param stop		stop address of update in page table
> + */
> +void mmu_page_table_flush(unsigned long start, unsigned long stop);
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #define arch_align_stack(x) (x)
> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> index 939de10..52a1fc6 100644
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -26,12 +26,6 @@
>  
>  #if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
>  
> -#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)
> -#define CACHE_SETUP	0x1a
> -#else
> -#define CACHE_SETUP	0x1e
> -#endif
> -
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  void __arm_init_before_mmu(void)
> @@ -50,9 +44,52 @@ static void cp_delay (void)
>  	asm volatile("" : : : "memory");
>  }
>  
> -static inline void dram_bank_mmu_setup(int bank)
> +void set_section_dcache(int section, enum dcache_option option)
>  {
> +	u32 value = section << MMU_SECTION_SHIFT | (3 << 10);
>  	u32 *page_table = (u32 *)gd->tlb_addr;
> +
> +	switch (option) {
> +	case DCACHE_WRITETHROUGH:
> +		value |= 0x1a;
> +		break;
> +
> +	case DCACHE_WRITEBACK:
> +		value |= 0x1e;
> +		break;
> +
> +	case DCACHE_OFF:
> +		value |= 0x12;
> +		break;
> +	}

what's the benefit of introducing an arbitrary enum rather than
defining DCACHE_WRITETHROUGH, DCACHE_WRITEBACK and DCACHE_OFF equal
respecitvely to 0x1a, 0x1e and 0x12? All it does is force this
switch case instead of a simple 'value |= option;' statement.

> +	page_table[section] = value;
> +}
> +
> +void __mmu_page_table_flush(unsigned long start, unsigned long stop)
> +{
> +	debug("%s: Warning: not implemented\n", __func__);
> +}
> +
> +void mmu_page_table_flush(unsigned long start, unsigned long stop)
> +	__attribute__((weak, alias("__mmu_page_table_flush")));
> +
> +void mmu_set_region_dcache(u32 start, int size, enum dcache_option option)
> +{
> +	u32 *page_table = (u32 *)gd->tlb_addr;
> +	u32 upto, end;
> +
> +	end = ALIGN(start + size, MMU_SECTION_SIZE) >> MMU_SECTION_SHIFT;
> +	start = start >> MMU_SECTION_SHIFT;
> +	debug("mmu_set_region_dcache start=%x, size=%x, option=%d\n",
> +	      start, size, option);
> +	for (upto = start; upto < end; upto++)
> +		set_section_dcache(upto, option);
> +	mmu_page_table_flush((u32)&page_table[start], (u32)&page_table[end]);
> +}
> +
> +static inline void dram_bank_mmu_setup(int bank)
> +{
>  	bd_t *bd = gd->bd;
>  	int	i;
>  
> @@ -60,21 +97,24 @@ static inline void dram_bank_mmu_setup(int bank)
>  	for (i = bd->bi_dram[bank].start >> 20;
>  	     i < (bd->bi_dram[bank].start + bd->bi_dram[bank].size) >> 20;
>  	     i++) {
> -		page_table[i] = i << 20 | (3 << 10) | CACHE_SETUP;
> +#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)
> +		set_section_dcache(i, DCACHE_WRITETHROUGH);
> +#else
> +		set_section_dcache(i, DCACHE_WRITEBACK);
> +#endif
>  	}
>  }
>  
>  /* to activate the MMU we need to set up virtual memory: use 1M areas */
>  static inline void mmu_setup(void)
>  {
> -	u32 *page_table = (u32 *)gd->tlb_addr;
>  	int i;
>  	u32 reg;
>  
>  	arm_init_before_mmu();
>  	/* Set up an identity-mapping for all 4GB, rw for everyone */
>  	for (i = 0; i < 4096; i++)
> -		page_table[i] = i << 20 | (3 << 10) | 0x12;
> +		set_section_dcache(i, DCACHE_OFF);
>  
>  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
>  		dram_bank_mmu_setup(i);
> @@ -82,7 +122,7 @@ static inline void mmu_setup(void)
>  
>  	/* Copy the page table address to cp15 */
>  	asm volatile("mcr p15, 0, %0, c2, c0, 0"
> -		     : : "r" (page_table) : "memory");
> +		     : : "r" (gd->tlb_addr) : "memory");
>  	/* Set the access control to all-supervisor */
>  	asm volatile("mcr p15, 0, %0, c3, c0, 0"
>  		     : : "r" (~0));


Amicalement,
-- 
Albert.