Re: [Qemu-ppc] [PATCH v2 4/6] PPC 85xx: Find CCSRBAR on ppce500 from device tree

[Alexander Graf]

On 04.02.2014, at 03:24, Scott Wood <address@hidden> wrote:

> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>> The definition of our ppce500 PV machine is that every address is dynamically
>> determined through device tree bindings.
>> 
>> So don't hardcode where CCSR is in our physical memory layout but instead
>> read it dynamically from the device tree we get passed on boot.
>> 
>> Signed-off-by: Alexander Graf <address@hidden>
>> ---
>> arch/powerpc/cpu/mpc85xx/start.S            |    2 +
>> board/freescale/qemu-ppce500/qemu-ppce500.c |   83 
>> +++++++++++++++++++++++++++
>> include/configs/qemu-ppce500.h              |   12 +++-
>> 3 files changed, 96 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/cpu/mpc85xx/start.S 
>> b/arch/powerpc/cpu/mpc85xx/start.S
>> index 0e593d2..2672c20 100644
>> --- a/arch/powerpc/cpu/mpc85xx/start.S
>> +++ b/arch/powerpc/cpu/mpc85xx/start.S
>> @@ -519,6 +519,7 @@ nexti:   mflr    r1              /* R1 = our PC */
>>  * As a general rule, TLB0 is used for short-term TLBs, and TLB1 is used for
>>  * long-term TLBs, so we use TLB0 here.
>>  */
>> +#if !defined(CONFIG_DYNAMIC_CCSRBAR)
>> #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS)
>> 
>> #if !defined(CONFIG_SYS_CCSRBAR_PHYS_HIGH) || 
>> !defined(CONFIG_SYS_CCSRBAR_PHYS_LOW)
>> @@ -703,6 +704,7 @@ delete_temp_tlbs:
>>      delete_tlb0_entry 1, CONFIG_SYS_CCSRBAR + 0x1000, MAS2_I|MAS2_G, r3
>> 
>> #endif /* #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS) */
>> +#endif /* #if !defined(CONFIG_DYNAMIC_CCSRBAR) */
>> 
>> #if defined(CONFIG_SYS_FSL_QORIQ_CHASSIS2) && defined(CONFIG_E6500)
>> create_ccsr_l2_tlb:
>> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c 
>> b/board/freescale/qemu-ppce500/qemu-ppce500.c
>> index b33b6b1..6491ae9 100644
>> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
>> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
>> @@ -26,6 +26,89 @@ static const void *get_fdt(void)
>>      return gd->fdt_blob;
>> }
>> 
>> +uint64_t get_phys_ccsrbar_addr(void)
>> +{
>> +    const void *fdt = get_fdt();
>> +    int root_node = fdt_path_offset(fdt, "/");
>> +    int soc_node = fdt_path_offset(fdt, "/soc");
>> +    const uint32_t *prop;
>> +    const uint64_t *prop64;
>> +    int len;
>> +    int root_address_cells = 1;
>> +    int address_cells = 1;
>> +    uint64_t r = 0;
>> +
>> +    /* Read CCSRBAR address length and size from device tree */
>> +    prop = fdt_getprop(fdt, root_node, "#address-cells", &len);
>> +    if (prop && (len >= 4))
>> +            root_address_cells = prop[0];
>> +
>> +    prop = fdt_getprop(fdt, soc_node, "#address-cells", &len);
>> +    if (prop && (len >= 4))
>> +            address_cells = prop[0];
>> +
>> +    /* Read CCSRBAR address from device tree */
>> +    prop = fdt_getprop(fdt, soc_node, "ranges", &len);
>> +    if (prop && (len >= ((address_cells + root_address_cells) * 4))) {
>> +            /* The physical address starts after the child's offset */
>> +            prop += address_cells;
>> +            prop64 = (const uint64_t *)prop;
>> +
>> +            if (root_address_cells == 1)
>> +                    r = prop[0];
>> +            else if (root_address_cells == 2)
>> +                    r = prop64[0];
>> +    }
>> +
>> +    if (!r)
>> +            panic("Couldn't find CCSR in FDT");
>> +
>> +    return r;
>> +}
>> +
>> +uint64_t get_phys_ccsrbar_addr_early(void)
>> +{
>> +    u32 mas0, mas1, mas2, mas3, mas7;
>> +    ulong fdt = (ulong)get_fdt();
>> +    uint64_t r;
>> +
>> +    /*
>> +     * To be able to read the FDT we need to create a temporary TLB
>> +     * map for it.
>> +     */
>> +
>> +    mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(10);
>> +    mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
>> +    mas2 = FSL_BOOKE_MAS2(fdt, 0);
>> +    mas3 = FSL_BOOKE_MAS3(fdt, 0, MAS3_SW|MAS3_SR);
>> +    mas7 = FSL_BOOKE_MAS7(fdt);
>> +
>> +    write_tlb(mas0, mas1, mas2, mas3, mas7);
>> +
>> +    r = get_phys_ccsrbar_addr();
>> +
>> +    disable_tlb(10);
>> +
>> +    return r;
>> +}
>> +
>> +int board_early_init_f(void)
>> +{
>> +    u32 mas0, mas1, mas2, mas3, mas7;
>> +    uint64_t phys_ccsr_address = get_phys_ccsrbar_addr();
>> +
>> +    /* Extend the CCSR map from cpu_init_early_f() */
>> +    mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
>> +    mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_64M);
>> +    mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
>> +    mas3 = FSL_BOOKE_MAS3(phys_ccsr_address, 0, MAS3_SW|MAS3_SR);
>> +    mas7 = FSL_BOOKE_MAS7(phys_ccsr_address);
>> +
>> +    write_tlb(mas0, mas1, mas2, mas3, mas7);
>> +
>> +    return 0;
>> +}
>> +
>> int checkboard(void)
>> {
>>      return 0;
> 
> You're adding new code to map CCSR and FDT, but not removing the code to
> do the same thing you added in patch 3/6.

Good point, This chunk can go now.

> Why 64M?

Because that was the TLB entry it replaced:

+       /*
+        * TLB 3:       64M     Non-cacheable, guarded
+        * 0xe000_0000  1M      CCSRBAR
+        * 0xe100_0000  255M    PCI IO range
+        */
+       SET_TLB_ENTRY(1, CONFIG_SYS_CCSRBAR, CONFIG_SYS_CCSRBAR_PHYS,
+                     MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+                     0, 3, BOOKE_PAGESZ_64M, 1),

which I just copied from

  board/freescale/mpc8544ds/tlb.c

but yeah, it's not really necessary to be that big.

> 
>> diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h
>> index 1ebaf51..7ef7235 100644
>> --- a/include/configs/qemu-ppce500.h
>> +++ b/include/configs/qemu-ppce500.h
>> @@ -44,8 +44,18 @@
>> #define CONFIG_SYS_ALT_MEMTEST
>> #define CONFIG_PANIC_HANG    /* do not reset board on panic */
>> 
>> +/* Needed to fill the ccsrbar pointer */
>> +#define CONFIG_BOARD_EARLY_INIT_F
>> +
>> +/* Virtual address to CCSRBAR */
>> #define CONFIG_SYS_CCSRBAR           0xe0000000
>> -#define CONFIG_SYS_CCSRBAR_PHYS_LOW CONFIG_SYS_CCSRBAR
>> +/* Physical address should be a function call */
>> +#ifndef __ASSEMBLY__
>> +extern unsigned long long get_phys_ccsrbar_addr_early(void);
>> +#endif
>> +#define CONFIG_SYS_CCSRBAR_PHYS_LOW 
>> ((uint32_t)get_phys_ccsrbar_addr_early())
>> +#define CONFIG_SYS_CCSRBAR_PHYS_HIGH        (get_phys_ccsrbar_addr_early() 
>> >> 32)
>> +#define CONFIG_DYNAMIC_CCSRBAR
>> 
> 
> What cares about CONFIG_SYS_CCSRBAR_PHYS* other than the code you
> skipped with an ifndef in start.S, and related header ifdeffery (which
> could be skipped by the existing CONFIG_SYS_CCSR_DO_NOT_RELOCATE
> instead)?

Nothing. I'll replace it.


Alex