On Tue, 2014-02-11 at 01:10 +0100, Alexander Graf wrote:
> diff --git a/arch/powerpc/cpu/mpc85xx/start.S
> b/arch/powerpc/cpu/mpc85xx/start.S
> index bb0025c..8982c78 100644
> --- a/arch/powerpc/cpu/mpc85xx/start.S
> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> @@ -80,6 +80,11 @@ _start_e500:
> li r1,MSR_DE
> mtmsr r1
>
> +#ifdef CONFIG_QEMU_E500
> + /* Save our ePAPR device tree pointer before we clobber it */
> + mr r24, r3
> +#endif
FWIW, it should be harmless to do this unconditionally (though in that
case I'd insert "(if we have one)" in the comment.
> #ifdef CONFIG_SYS_FSL_ERRATUM_A004510
> mfspr r3,SPRN_SVR
> rlwinm r3,r3,0,0xff
> @@ -514,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)
This shouldn't be necessary, if you have
CONFIG_SYS_CCSR_DO_NOT_RELOCATE.
> diff --git a/arch/powerpc/cpu/mpc85xx/tlb.c b/arch/powerpc/cpu/mpc85xx/tlb.c
> index 2011fb8..0e0b483 100644
> --- a/arch/powerpc/cpu/mpc85xx/tlb.c
> +++ b/arch/powerpc/cpu/mpc85xx/tlb.c
> @@ -36,6 +36,10 @@ void init_tlbs(void)
> tlb_table[i].mas7);
> }
>
> +#ifdef CONFIG_SYS_USE_DYNAMIC_TLBS
> + init_tlbs_dynamic();
> +#endif
You could avoid the ifdef by moving a stub implementation into the
header -- or possibly better, avoid the config symbol entirely by using
a weak symbol.
Better still, make init_tlbs() weak. Then you don't need a config
symbol, a new function name, or a dummy tlb_table[] -- you just redefine
init_tlbs() in board code.
> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c
> b/board/freescale/qemu-ppce500/qemu-ppce500.c
> new file mode 100644
> index 0000000..fc546b9
> --- /dev/null
> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
> @@ -0,0 +1,334 @@
> +/*
> + * Copyright 2007,2009-2014 Freescale Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <pci.h>
> +#include <asm/processor.h>
> +#include <asm/mmu.h>
> +#include <asm/fsl_pci.h>
> +#include <asm/io.h>
> +#include <libfdt.h>
> +#include <fdt_support.h>
> +#include <netdev.h>
> +#include <fdtdec.h>
> +#include <errno.h>
> +#include <malloc.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static void *get_fdt(void)
> +{
> + return (void *)gd->fdt_blob;
> +}
Does this return virtual or physical?
> +
> +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);
Don't use the physical address as a virtual address. Use a known-good
virtual address. Using unknown physical addresses as virtual addresses
is generally a bad idea (it's different for stuff whose physical address
is hardcoded in U-Boot), but it's particularly bad here because you'll
create an overlapping TLB entry if the fdt happens to live within your
initial memory mapping.
OK, I see you use CONFIG_SYS_TMPVIRT for this elsewhere, but I guess
forgot to use it here (why is fdt mapping code duplicated?).
> +void pci_init_board(void)
> +{
> + struct pci_controller *pci_hoses;
> + void *fdt = get_fdt();
> + int pci_node;
> + int pci_num = 0;
> + int pci_count = 0;
> + const char *compat = "fsl,mpc8540-pci";
> + ulong map_addr;
May want to look for arbitrary PCI host controllers (device_type would
be simplest), in case the QEMU machine ever gets an upgrade to PCIe.
> +
> + puts("\n");
> +
> + /* Start MMIO and PIO range maps above RAM */
> + map_addr = CONFIG_MAX_MEM_MAPPED;
It'd be better to hardcode virtual addresses for this (as other boards
do), and limit the size you map to the smaller of the hardcoded size or
the device tree size.
> + /* Count and allocate PCI buses */
> + pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
> + while (pci_node != -FDT_ERR_NOTFOUND) {
> + pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat);
> + pci_count++;
> + }
> +
> + if (pci_count) {
> + pci_hoses = malloc(sizeof(struct pci_controller) * pci_count);
> + } else {
> + printf("PCI: disabled\n\n");
> + return;
> + }
> +
> + /* Spawn PCI buses based on device tree */
> + pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
> + while (pci_node != -FDT_ERR_NOTFOUND) {
> + struct fsl_pci_info pci_info = { };
> + const fdt32_t *reg;
> + int r;
> +
> + reg = fdt_getprop(fdt, pci_node, "reg", NULL);
> + pci_info.regs = fdt_translate_address((void *)fdt, pci_node,
> reg);
Unnecessary cast.
> +void init_tlbs_dynamic(void)
> +{
> + unsigned long fdt_phys = (unsigned long)get_fdt();
> + unsigned long fdt_phys_tlb = fdt_phys & ~0xffffful;
> + unsigned long fdt_virt_tlb = CONFIG_SYS_TMPVIRT;
> + unsigned long fdt_virt = fdt_virt_tlb + (fdt_phys & 0xffffful);
> + u32 mas0, mas1, mas2, mas3, mas7;
> + phys_size_t ram_size;
> +
> + /*
> + * Create a temporary AS=1 map for the fdt
> + *
> + * We use ESEL=0 here to overwrite the previous AS=0 map for ourselves
> + * which was only 4k big. This way we don't have to clear any other
> maps.
> + */
I don't think it's generally safe to assume that this entry is ESEL 0 --
though I'm wondering if the current TLB code is assuming that (or at
least, assuming that whatever entry is used gets overwritten by the TLB
table).
> + mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(0);
> + mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
> + mas2 = FSL_BOOKE_MAS2(fdt_virt_tlb, 0);
> + mas3 = FSL_BOOKE_MAS3(fdt_phys_tlb, 0, MAS3_SW|MAS3_SR);
> + mas7 = FSL_BOOKE_MAS7(fdt_phys_tlb);
What if the fdt straddles a 1M boundary?
> + write_tlb(mas0, mas1, mas2, mas3, mas7);
> + gd->fdt_blob = (void *)fdt_virt;
> +
> + /* Fetch RAM size from the fdt */
> + ram_size = fixed_sdram();
Why not just call get_linear_ram_size() directly?
> + /* And remove our fdt map again */
> + gd->fdt_blob = (void *)fdt_phys;
> + disable_tlb(0);
> +
> + /* Create a dynamic AS=0 CCSRBAR mapping */
> + mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
> + mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_1M);
> + mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
> + mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS, 0, MAS3_SW|MAS3_SR);
> + mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_CCSRBAR_PHYS);
> +
> + write_tlb(mas0, mas1, mas2, mas3, mas7);
Why is this not done via tlb_map_range (after calling
init_used_tlb_cams, of course)?
> +
> + /* Create a RAM map that spans all accessible RAM */
> + init_used_tlb_cams();
> + setup_ddr_tlbs(ram_size >> 20);
> +}
> +
> +void init_laws(void)
> +{
> + /* We don't emulate LAWs yet */
> +}
> +
> +static uint32_t get_cpu_freq(void)
> +{
> + void *fdt = get_fdt();
> + int cpus_node = fdt_path_offset(fdt, "/cpus");
> + int cpu_node = fdt_first_subnode(fdt, cpus_node);
> + const char *prop = "clock-frequency";
> + return fdt_getprop_u32_default_node(fdt, cpu_node, 0, prop, 0);
> +}
> +
> +void get_sys_info(sys_info_t *sys_info)
> +{
> + int freq = get_cpu_freq();
> +
> + memset(sys_info, 0, sizeof(sys_info_t));
> + sys_info->freq_systembus = freq;
> + sys_info->freq_ddrbus = freq;
> + sys_info->freq_processor[0] = freq;
> +}
> +
Again, if you're doing this you really should override get_tbclk()
instead of letting it use the much faster cpufreq/8.
> +int get_clocks (void)
> +{
> + sys_info_t sys_info;
> +
> + get_sys_info(&sys_info);
> +
> + gd->cpu_clk = sys_info.freq_processor[0];
> + gd->bus_clk = sys_info.freq_systembus;
> + gd->mem_clk = sys_info.freq_ddrbus;
> + gd->arch.lbc_clk = sys_info.freq_ddrbus;
I wonder if with higher frequencies we'll trigger overflows in some
drivers. :-)
> +/* Physical address should be a function call */
> +#ifndef __ASSEMBLY__
> +extern unsigned long long get_phys_ccsrbar_addr_early(void);
> +#endif
Where does this get called?
-Scott
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
