[U-Boot] [PATCH 8/9] MIPS: L2 cache support

[Paul Burton]

On 08/09/16 11:40, Marek Vasut wrote:
> On 09/08/2016 12:31 PM, Paul Burton wrote:
>> On 08/09/16 11:04, Marek Vasut wrote:
>>> On 09/07/2016 07:48 PM, Paul Burton wrote:
>>>> This patch adds support for initialising & maintaining L2 caches on MIPS
>>>> systems. The L2 cache configuration may be advertised through either
>>>> coprocessor 0 or the MIPS Coherence Manager depending upon the system,
>>>> and support for both is included.
>>>>
>>>> If the L2 can be bypassed then we bypass it early in boot & initialise
>>>> the L1 caches first, such that we can start making use of the L1
>>>> instruction cache as early as possible. Otherwise we initialise the L2
>>>> first such that the L1s have no opportunity to generate access to the
>>>> uninitialised L2.
>>>>
>>>> Signed-off-by: Paul Burton <paul.burton at imgtec.com>
>>>> ---
>>>>
>>>>  arch/mips/Kconfig                   |   6 ++
>>>>  arch/mips/include/asm/cm.h          |  38 ++++++++
>>>>  arch/mips/include/asm/global_data.h |   3 +
>>>>  arch/mips/include/asm/mipsregs.h    |   5 +
>>>>  arch/mips/lib/cache.c               |  62 ++++++++++++-
>>>>  arch/mips/lib/cache_init.S          | 180 +++++++++++++++++++++++++++++++++++-
>>>>  6 files changed, 288 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>>>> index 732d129..8f97727 100644
>>>> --- a/arch/mips/Kconfig
>>>> +++ b/arch/mips/Kconfig
>>>> @@ -301,6 +301,12 @@ config MIPS_L1_CACHE_SHIFT
>>>>  	default "4" if MIPS_L1_CACHE_SHIFT_4
>>>>  	default "5"
>>>>  
>>>> +config MIPS_L2_CACHE
>>>> +	bool
>>>> +	help
>>>> +	  Select this if your system includes an L2 cache and you want U-Boot
>>>> +	  to initialise & maintain it.
>>>> +
>>>>  config DYNAMIC_IO_PORT_BASE
>>>>  	bool
>>>>  
>>>> diff --git a/arch/mips/include/asm/cm.h b/arch/mips/include/asm/cm.h
>>>> index 0261733..62ecef2 100644
>>>> --- a/arch/mips/include/asm/cm.h
>>>> +++ b/arch/mips/include/asm/cm.h
>>>> @@ -12,8 +12,46 @@
>>>>  #define GCR_BASE			0x0008
>>>>  #define GCR_BASE_UPPER			0x000c
>>>>  #define GCR_REV				0x0030
>>>> +#define GCR_L2_CONFIG			0x0130
>>>> +#define GCR_L2_TAG_ADDR			0x0600
>>>> +#define GCR_L2_TAG_ADDR_UPPER		0x0604
>>>> +#define GCR_L2_TAG_STATE		0x0608
>>>> +#define GCR_L2_TAG_STATE_UPPER		0x060c
>>>> +#define GCR_L2_DATA			0x0610
>>>> +#define GCR_L2_DATA_UPPER		0x0614
>>>>  
>>>>  /* GCR_REV CM versions */
>>>>  #define GCR_REV_CM3			0x0800
>>>>  
>>>> +/* GCR_L2_CONFIG fields */
>>>> +#define GCR_L2_CONFIG_ASSOC_SHIFT	0
>>>> +#define GCR_L2_CONFIG_ASSOC_BITS	8
>>>> +#define GCR_L2_CONFIG_LINESZ_SHIFT	8
>>>> +#define GCR_L2_CONFIG_LINESZ_BITS	4
>>>> +#define GCR_L2_CONFIG_SETSZ_SHIFT	12
>>>> +#define GCR_L2_CONFIG_SETSZ_BITS	4
>>>> +#define GCR_L2_CONFIG_BYPASS		(1 << 20)
>>>> +
>>>> +#ifndef __ASSEMBLY__
>>>> +
>>>> +#include <asm/io.h>
>>>> +
>>>> +static inline void *mips_cm_base(void)
>>>> +{
>>>> +	return (void *)CKSEG1ADDR(CONFIG_MIPS_CM_BASE);
>>>> +}
>>>> +
>>>> +static inline unsigned long mips_cm_l2_line_size(void)
>>>> +{
>>>> +	unsigned long l2conf, line_sz;
>>>> +
>>>> +	l2conf = __raw_readl(mips_cm_base() + GCR_L2_CONFIG);
>>>> +
>>>> +	line_sz = l2conf >> GCR_L2_CONFIG_LINESZ_SHIFT;
>>>> +	line_sz &= GENMASK(GCR_L2_CONFIG_LINESZ_BITS - 1, 0);
>>>> +	return line_sz ? (2 << line_sz) : 0;
>>>> +}
>>>
>>> Drop the inline stuff, the compiler can decide that itself, especially
>>> on static functions. The inline is only a hint anyway.
>>>
>>> [...]
>>
>> Hi Marek,
>>
>> Nope - inline does have an impact for functions in headers. If it's not
>> there & a file includes a header but doesn't use *all* of the functions
>> in it then the compiler will warn about the functions being unused. With
>> inline that isn't the case. Thus "static inline" is used quite
>> extensively in many of the headers under include/.
> 
> Uh, I didn't notice these functions were placed in a header file. Is
> that really necessary at all or can you move them to a C file ?

I could move them, but then the compiler wouldn't get an opportunity to
inline these short functions. I don't see any downside to keeping these
in the same place as the registers they access are defined.

Thanks,
    Paul

> 
>> Thanks,
>>     Paul
>>
> 
>