Re: [PATCH v2 01/15] powerpc/85xx: cache operations for Freescale SoCs based on BOOK3E

[Scott Wood <scottwood@freescale.com>]

On 04/19/2013 05:47:34 AM, Zhao Chenhui wrote:
> These cache operations support Freescale SoCs based on BOOK3E.
> Move L1 cache operations to fsl_booke_cache.S in order to maintain
> easily. And, add cache operations for backside L2 cache and platform  
> cache.
> 
> The backside L2 cache appears on e500mc and e5500 core. The platform  
> cache
> supported by this patch is L2 Look-Aside Cache, which appears on SoCs
> with e500v1/e500v2 core, such as MPC8572, P1020, etc.
> 
> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>  arch/powerpc/include/asm/cacheflush.h |    8 ++
>  arch/powerpc/kernel/Makefile          |    1 +
>  arch/powerpc/kernel/fsl_booke_cache.S |  210  
> +++++++++++++++++++++++++++++++++
>  arch/powerpc/kernel/head_fsl_booke.S  |   74 ------------
>  4 files changed, 219 insertions(+), 74 deletions(-)
>  create mode 100644 arch/powerpc/kernel/fsl_booke_cache.S
> 
> diff --git a/arch/powerpc/include/asm/cacheflush.h  
> b/arch/powerpc/include/asm/cacheflush.h
> index b843e35..bc3f937 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -32,6 +32,14 @@ extern void flush_dcache_page(struct page *page);
> 
>  extern void __flush_disable_L1(void);
> 
> +#ifdef CONFIG_FSL_SOC_BOOKE
> +void flush_dcache_L1(void);
> +void flush_backside_L2_cache(void);
> +void disable_backside_L2_cache(void);
> +void flush_disable_L2(void);
> +void invalidate_enable_L2(void);
> +#endif

Don't ifdef prototypes unless there's a good reason, such as providing  
an inline alternative.

Why do you have "flush_backside_L2_cache" and  
"disable_backside_L2_cache" as something different from  
"flush_disable_L2"?  The latter should flush whatever L2 is present.   
Don't treat pre-corenet as the default.

Why do we even need to distinguish L1 from L2 at all?  Shouldn't the  
function that gets exposed just be "flush and disable data caches that  
are specific to this cpu"?  What should happen on e6500?

-Scott