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