[U-Boot] [PATCH 01/11] powerpc/mpc85xx: Enable L2 at the beginning of U-boot for E6500

[Scott Wood]

On 08/24/2012 06:02 PM, York Sun wrote:
> On 08/24/2012 03:53 PM, Scott Wood wrote:
>> On 08/17/2012 01:27 PM, York Sun wrote:
>>> Using E6500 L1 cache as initram requires L2 cache enabled.
>>> Add l2-cache cluster enabling.
>>>
>>> Signed-off-by: York Sun <yorksun at freescale.com>
>>> Signed-off-by: Kumar Gala <galak at kernel.crashing.org>
>>> ---
>>>   arch/powerpc/cpu/mpc85xx/cpu_init.c   |   39 ++++++++++++++++++++++-
>>>   arch/powerpc/cpu/mpc85xx/start.S      |   57
>>> +++++++++++++++++++++++++++++++++
>>>   arch/powerpc/include/asm/immap_85xx.h |   43 +++++++++++++++++++++++++
>>>   3 files changed, 138 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c
>>> b/arch/powerpc/cpu/mpc85xx/cpu_init.c
>>> index 2c78905..1a2858a 100644
>>> --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
>>> +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
>>> @@ -309,6 +309,34 @@ static void __fsl_serdes__init(void)
>>>   }
>>>   __attribute__((weak, alias("__fsl_serdes__init"))) void
>>> fsl_serdes_init(void);
>>>
>>> +#ifdef CONFIG_E6500
>>> +int enable_cluster_l2(void)
>>
>> If enabling L2 is required for the stack, how are we enabling it in C
>> code?  Is this just for non-boot clusters?
> 
> Yes, this is for non-boot clusters. The boot cluster is enabled in start.S.

OK, maybe put a comment on that first EOC check (which I missed).
Anything we can do to help people navigate what's going on during 85xx
boot would be welcome.

>>> +        out_be32(&l2cache->l2csr0, L2CSR0_L2FI|L2CSR0_L2LFC);
>>> +        while ((in_be32(&l2cache->l2csr0) &
>>> (L2CSR0_L2FI|L2CSR0_L2LFC)) != 0)
>>> +            ;
>>
>> Timeout?
> 
> I don't have a spec telling me how long.

You can estimate an upper bound, so we print a message if we get stuck.
 It shouldn't take longer than a second, right?

>>> @@ -322,6 +350,11 @@ int cpu_init_r(void)
>>>   #ifdef CONFIG_SYS_LBC_LCRR
>>>       volatile fsl_lbc_t *lbc = LBC_BASE_ADDR;
>>>   #endif
>>> +#ifdef CONFIG_L2_CACHE
>>> +    volatile ccsr_l2cache_t *l2cache = (void
>>> *)CONFIG_SYS_MPC85xx_L2_ADDR;
>>> +#elif defined(CONFIG_E6500)
>>> +    struct ccsr_cluster_l2 * l2cache = (void
>>> *)CONFIG_SYS_FSL_CLUSTER_1_L2;
>>> +#endif
>>
>> If CONFIG_L2_CACHE doesn't apply to e6500, then CONFIG_L2_CACHE is
>> misnamed.
> 
> Maybe it's time to introduce a new name?

Sure.  What is it supposed to refer to?

>>> diff --git a/arch/powerpc/cpu/mpc85xx/start.S
>>> b/arch/powerpc/cpu/mpc85xx/start.S
>>> index 2e1d265..739127f 100644
>>> --- a/arch/powerpc/cpu/mpc85xx/start.S
>>> +++ b/arch/powerpc/cpu/mpc85xx/start.S
>>> @@ -762,6 +762,63 @@ delete_temp_tlbs:
>>>       tlbwe
>>>   #endif /* #if (CONFIG_SYS_CCSRBAR_DEFAULT !=
>>> CONFIG_SYS_CCSRBAR_PHYS) */
>>>
>>> +#ifdef CONFIG_E6500
>>> +create_ccsr_l2_tlb:
>>> +    /*
>>> +     * Create a TLB for the MMR location of CCSR
>>> +     * to access L2CSR0 register
>>> +     */
>>> +    lis     r0, FSL_BOOKE_MAS0(0, 0, 0)@h
>>> +    ori     r0, r0, FSL_BOOKE_MAS0(0, 0, 0)@l
>>> +
>>> +    lis     r1, FSL_BOOKE_MAS1(1, 0, 0, 0, BOOKE_PAGESZ_4K)@h
>>> +    ori     r1, r1, FSL_BOOKE_MAS1(1, 0, 0, 0, BOOKE_PAGESZ_4K)@l
>>> +    lis     r2, FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR + 0xC20000,
>>> (MAS2_I|MAS2_G))@h
>>> +    ori     r2, r2, FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR + 0xC20000,
>>> (MAS2_I|MAS2_G))@l
>>> +    lis     r3, FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS_LOW +
>>> 0xC20000, 0, (MAS3_SW|MAS3_SR))@h
>>> +    ori     r3, r3, FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS_LOW +
>>> 0xC20000, 0, (MAS3_SW|MAS3_SR))@l
>>> +    lis    r7, CONFIG_SYS_CCSRBAR_PHYS_HIGH at h
>>> +    ori    r7, r7, CONFIG_SYS_CCSRBAR_PHYS_HIGH at l
>>> +    mtspr   MAS0, r0
>>> +    mtspr   MAS1, r1
>>> +    mtspr   MAS2, r2
>>> +    mtspr   MAS3, r3
>>> +    mtspr   MAS7, r7
>>> +    isync
>>> +    msync
>>> +    tlbwe
>>
>> Let's make a macro (asm, not cpp) out of this instead of copy and
>> pasting all over the place.  And stop misusing r1/r2.
>>
> 
> That's a good idea. I am also tired of this long copy-n-paste. Using a
> macro increase the chance of overwriting registers, doesn't it? Any good
> idea to avoid?

If you can get away with one or two temporary registers, you could pass
the temporaries in as parameters.  Otherwise, document what you clobber,
and stick to registers that are normally caller-saved.

-Scott