| Message ID | 1391166969-25845-6-git-send-email-agraf@suse.de |
|---|---|
| State | Superseded |
| Delegated to: | York Sun |
| Headers | show |
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 PCI devices are in our physical memory layout but instead > read them dynamically from the device tree we get passed on boot. Would it be difficult to make the QEMU emulation properly implement access windows? > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > board/freescale/qemu-ppce500/qemu-ppce500.c | 193 ++++++++++++++++++++++++--- > board/freescale/qemu-ppce500/tlb.c | 13 -- > include/configs/qemu-ppce500.h | 12 -- > 3 files changed, 175 insertions(+), 43 deletions(-) > > diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c > index 6491ae9..5d4dd64 100644 > --- a/board/freescale/qemu-ppce500/qemu-ppce500.c > +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c > @@ -19,7 +19,51 @@ > #include <malloc.h> > > DECLARE_GLOBAL_DATA_PTR; > -static struct pci_controller pci1_hose; > + > +static uint64_t myfdt_readcells(const void *fdt, int node, const char *property, > + int num, int off, uint64_t defval) "my" fdt? > +{ > + int len; > + const uint32_t *prop; > + > + prop = fdt_getprop(fdt, node, property, &len); > + > + if (!prop) > + return defval; > + > + if (len < ((off + num) * sizeof(uint32_t))) > + panic("Invalid fdt"); > + > + prop += off; > + > + switch (num) { > + case 1: > + return *prop; > + case 2: > + return *(const uint64_t *)prop; > + } > + What about this function is specific to qemu-ppce500? + panic("Invalid cell size"); +} s/cell size/cell count/ > +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property, > + uint32_t defval) > +{ > + return myfdt_readcells(fdt, node, property, 1, 0, defval); > +} This looks a lot like fdt_getprop_u32_default(), except for "int node" versus path. > +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size) > +{ > + unsigned int max_cam, tsize_mask; > + int i; > + > + if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) { > + /* Convert (4^max) kB to (2^max) bytes */ > + max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10; > + tsize_mask = ~1U; > + } else { > + /* Convert (2^max) kB to (2^max) bytes */ > + max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10; > + tsize_mask = ~0U; > + } > + > + for (i = 0; size && i < 8; i++) { > + int tlb_index = find_free_tlbcam(); > + u32 camsize = __ilog2_u64(size) & tsize_mask; > + u32 align = __ilog2(virt_addr) & tsize_mask; > + unsigned int tlb_size; > + > + if (tlb_index == -1) > + break; > + > + if (align == -2) align = max_cam; -2? Besides align being unsigned, if this is meant to handle the case where virt_addr is zero, that's undefined for __ilog2() (don't rely on it being implemented with cntlzw), and you're not handling the MMUv2 case. Plus, whitespace. > + if (camsize > align) > + camsize = align; > + > + if (camsize > max_cam) > + camsize = max_cam; What about min_cam? > + } > + > +} Whitespace. > + > void pci_init_board(void) > { > - struct fsl_pci_info pci_info; > + struct pci_controller *pci_hoses; > const void *fdt = get_fdt(); > int pci_node; > + int pci_num = 0; > + int pci_count; > + const char *compat = "fsl,mpc8540-pci"; > + ulong map_addr; > > puts("\n"); > > - pci_node = fdt_path_offset(fdt, "/pci"); > - if (pci_node < 0) { > + /* Start MMIO and PIO range maps above RAM */ > + map_addr = CONFIG_MAX_MEM_MAPPED; > + > + /* Count and allocate PCI buses */ > + pci_count = myfdt_count_compatibles(fdt, compat); > + > + if (pci_count) { > + pci_hoses = malloc(sizeof(struct pci_controller) * pci_count); > + } else { > printf("PCI: disabled\n\n"); > return; > } > > - SET_STD_PCI_INFO(pci_info, 1); > - > - fsl_setup_hose(&pci1_hose, pci_info.regs); > - printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n", > - pci_info.regs); > - > - fsl_pci_init_port(&pci_info, &pci1_hose, 0); > + /* 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 = { }; > + uint64_t phys_addr; > + int pnode = fdt_parent_offset(fdt, pci_node); > + int paddress_cells; > + int address_cells; > + int size_cells; > + int off = 0; > + > + paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1); > + address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1); > + size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1); > + > + pci_info.regs = myfdt_readcells(fdt, pci_node, "reg", > + paddress_cells, 0, 0); > + > + /* MMIO range */ > + off += address_cells; > + phys_addr = myfdt_readcells(fdt, pci_node, "ranges", > + paddress_cells, off, 0); > + off += paddress_cells; > + pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges", > + size_cells, off, 0); > + off += size_cells; > + > + /* Align virtual region */ > + map_addr += pci_info.mem_size - 1; > + map_addr &= ~(pci_info.mem_size - 1); > + /* Map virtual memory for MMIO range */ > + map_tlb1_io(map_addr, phys_addr, pci_info.mem_size); > + pci_info.mem_bus = phys_addr; > + pci_info.mem_phys = phys_addr; > + map_addr += pci_info.mem_size; > + > + /* PIO range */ > + off += address_cells; > + pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges", > + paddress_cells, off, 0); > + off += paddress_cells; > + pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges", > + size_cells, off, 0); > + > + /* Align virtual region */ > + map_addr += pci_info.io_size - 1; > + map_addr &= ~(pci_info.io_size - 1); > + /* Map virtual memory for MMIO range */ > + map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size); > + pci_info.io_bus = map_addr; > + map_addr += pci_info.io_size; > + > + /* Instantiate */ > + pci_info.pci_num = pci_num + 1; > + > + fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs); > + printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n", > + pci_info.regs); > + > + fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num); > + > + /* Jump to next PCI node */ > + pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat); > + pci_num++; > + } > > puts("\n"); > } How about using fdt_translate_address() or other existing functionality? -Scott
On 04.02.2014, at 03:47, Scott Wood <scottwood@freescale.com> 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 PCI devices are in our physical memory layout but instead >> read them dynamically from the device tree we get passed on boot. > > Would it be difficult to make the QEMU emulation properly implement > access windows? What are access windows? You mean BAR region offsets? Not too hard I suppose, but it adds complexity which we were trying to avoid, no? > >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> board/freescale/qemu-ppce500/qemu-ppce500.c | 193 ++++++++++++++++++++++++--- >> board/freescale/qemu-ppce500/tlb.c | 13 -- >> include/configs/qemu-ppce500.h | 12 -- >> 3 files changed, 175 insertions(+), 43 deletions(-) >> >> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c >> index 6491ae9..5d4dd64 100644 >> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c >> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c >> @@ -19,7 +19,51 @@ >> #include <malloc.h> >> >> DECLARE_GLOBAL_DATA_PTR; >> -static struct pci_controller pci1_hose; >> + >> +static uint64_t myfdt_readcells(const void *fdt, int node, const char *property, >> + int num, int off, uint64_t defval) > > "my" fdt? Yeah, didn't want to clash with the fdt namespace. > >> +{ >> + int len; >> + const uint32_t *prop; >> + >> + prop = fdt_getprop(fdt, node, property, &len); >> + >> + if (!prop) >> + return defval; >> + >> + if (len < ((off + num) * sizeof(uint32_t))) >> + panic("Invalid fdt"); >> + >> + prop += off; >> + >> + switch (num) { >> + case 1: >> + return *prop; >> + case 2: >> + return *(const uint64_t *)prop; >> + } >> + > > What about this function is specific to qemu-ppce500? Nothing. But the less common code I touch the less I can break. There seems to be an fdt helper framework that's only targeted at a few ARM devices - not sure what to make of that. > > + panic("Invalid cell size"); > +} > > s/cell size/cell count/ > >> +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property, >> + uint32_t defval) >> +{ >> + return myfdt_readcells(fdt, node, property, 1, 0, defval); >> +} > > This looks a lot like fdt_getprop_u32_default(), except for "int node" > versus path. Well, I use it inside of a loop where I don't have the path :). > >> +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size) >> +{ >> + unsigned int max_cam, tsize_mask; >> + int i; >> + >> + if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) { >> + /* Convert (4^max) kB to (2^max) bytes */ >> + max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10; >> + tsize_mask = ~1U; >> + } else { >> + /* Convert (2^max) kB to (2^max) bytes */ >> + max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10; >> + tsize_mask = ~0U; >> + } >> + >> + for (i = 0; size && i < 8; i++) { >> + int tlb_index = find_free_tlbcam(); >> + u32 camsize = __ilog2_u64(size) & tsize_mask; >> + u32 align = __ilog2(virt_addr) & tsize_mask; >> + unsigned int tlb_size; >> + >> + if (tlb_index == -1) >> + break; >> + >> + if (align == -2) align = max_cam; > > -2? Besides align being unsigned, if this is meant to handle the case > where virt_addr is zero, that's undefined for __ilog2() (don't rely on > it being implemented with cntlzw), and you're not handling the MMUv2 > case. I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it slightly to let me choose the target virt address. Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and export that function there? > > Plus, whitespace. > >> + if (camsize > align) >> + camsize = align; >> + >> + if (camsize > max_cam) >> + camsize = max_cam; > > What about min_cam? > >> + } >> + >> +} > > Whitespace. > >> + >> void pci_init_board(void) >> { >> - struct fsl_pci_info pci_info; >> + struct pci_controller *pci_hoses; >> const void *fdt = get_fdt(); >> int pci_node; >> + int pci_num = 0; >> + int pci_count; >> + const char *compat = "fsl,mpc8540-pci"; >> + ulong map_addr; >> >> puts("\n"); >> >> - pci_node = fdt_path_offset(fdt, "/pci"); >> - if (pci_node < 0) { >> + /* Start MMIO and PIO range maps above RAM */ >> + map_addr = CONFIG_MAX_MEM_MAPPED; >> + >> + /* Count and allocate PCI buses */ >> + pci_count = myfdt_count_compatibles(fdt, compat); >> + >> + if (pci_count) { >> + pci_hoses = malloc(sizeof(struct pci_controller) * pci_count); >> + } else { >> printf("PCI: disabled\n\n"); >> return; >> } >> >> - SET_STD_PCI_INFO(pci_info, 1); >> - >> - fsl_setup_hose(&pci1_hose, pci_info.regs); >> - printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n", >> - pci_info.regs); >> - >> - fsl_pci_init_port(&pci_info, &pci1_hose, 0); >> + /* 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 = { }; >> + uint64_t phys_addr; >> + int pnode = fdt_parent_offset(fdt, pci_node); >> + int paddress_cells; >> + int address_cells; >> + int size_cells; >> + int off = 0; >> + >> + paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1); >> + address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1); >> + size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1); >> + >> + pci_info.regs = myfdt_readcells(fdt, pci_node, "reg", >> + paddress_cells, 0, 0); >> + >> + /* MMIO range */ >> + off += address_cells; >> + phys_addr = myfdt_readcells(fdt, pci_node, "ranges", >> + paddress_cells, off, 0); >> + off += paddress_cells; >> + pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges", >> + size_cells, off, 0); >> + off += size_cells; >> + >> + /* Align virtual region */ >> + map_addr += pci_info.mem_size - 1; >> + map_addr &= ~(pci_info.mem_size - 1); >> + /* Map virtual memory for MMIO range */ >> + map_tlb1_io(map_addr, phys_addr, pci_info.mem_size); >> + pci_info.mem_bus = phys_addr; >> + pci_info.mem_phys = phys_addr; >> + map_addr += pci_info.mem_size; >> + >> + /* PIO range */ >> + off += address_cells; >> + pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges", >> + paddress_cells, off, 0); >> + off += paddress_cells; >> + pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges", >> + size_cells, off, 0); >> + >> + /* Align virtual region */ >> + map_addr += pci_info.io_size - 1; >> + map_addr &= ~(pci_info.io_size - 1); >> + /* Map virtual memory for MMIO range */ >> + map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size); >> + pci_info.io_bus = map_addr; >> + map_addr += pci_info.io_size; >> + >> + /* Instantiate */ >> + pci_info.pci_num = pci_num + 1; >> + >> + fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs); >> + printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n", >> + pci_info.regs); >> + >> + fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num); >> + >> + /* Jump to next PCI node */ >> + pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat); >> + pci_num++; >> + } >> >> puts("\n"); >> } > > How about using fdt_translate_address() or other existing functionality? Mind to show exactly how? Alex
On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote: > On 04.02.2014, at 03:47, Scott Wood <scottwood@freescale.com> 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 PCI devices are in our physical memory layout but instead > >> read them dynamically from the device tree we get passed on boot. > > > > Would it be difficult to make the QEMU emulation properly implement > > access windows? > > What are access windows? You mean BAR region offsets? Not too hard I > suppose, but it adds complexity which we were trying to avoid, no? It would remove U-Boot complexity (unlike the LAW stuff where we just skip it) because we wouldn't need to care about QEMU's default settings. It should be easier to do than LAW support, and more useful (e.g. Linux currently programs this as well, we just get lucky that it misuses the device tree as configuration information). > >> +{ > >> + int len; > >> + const uint32_t *prop; > >> + > >> + prop = fdt_getprop(fdt, node, property, &len); > >> + > >> + if (!prop) > >> + return defval; > >> + > >> + if (len < ((off + num) * sizeof(uint32_t))) > >> + panic("Invalid fdt"); > >> + > >> + prop += off; > >> + > >> + switch (num) { > >> + case 1: > >> + return *prop; > >> + case 2: > >> + return *(const uint64_t *)prop; > >> + } > >> + > > > > What about this function is specific to qemu-ppce500? > > Nothing. But the less common code I touch the less I can break. The more that line of thought is applied, the uglier the codebase we'll end up with. :-) > There seems to be an fdt helper framework that's only targeted at a few ARM > devices - not sure what to make of that. Make use of whatever parts you can, and extend it with the missing bits you need. There's also common/fdt_support.c which is definitely not just used by ARM. > > + panic("Invalid cell size"); > > +} > > > > s/cell size/cell count/ > > > >> +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property, > >> + uint32_t defval) > >> +{ > >> + return myfdt_readcells(fdt, node, property, 1, 0, defval); > >> +} > > > > This looks a lot like fdt_getprop_u32_default(), except for "int node" > > versus path. > > Well, I use it inside of a loop where I don't have the path :). It still indicates where the proper place for code like this is. :-) > >> +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size) > >> +{ > >> + unsigned int max_cam, tsize_mask; > >> + int i; > >> + > >> + if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) { > >> + /* Convert (4^max) kB to (2^max) bytes */ > >> + max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10; > >> + tsize_mask = ~1U; > >> + } else { > >> + /* Convert (2^max) kB to (2^max) bytes */ > >> + max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10; > >> + tsize_mask = ~0U; > >> + } > >> + > >> + for (i = 0; size && i < 8; i++) { > >> + int tlb_index = find_free_tlbcam(); > >> + u32 camsize = __ilog2_u64(size) & tsize_mask; > >> + u32 align = __ilog2(virt_addr) & tsize_mask; > >> + unsigned int tlb_size; > >> + > >> + if (tlb_index == -1) > >> + break; > >> + > >> + if (align == -2) align = max_cam; > > > > -2? Besides align being unsigned, if this is meant to handle the case > > where virt_addr is zero, that's undefined for __ilog2() (don't rely on > > it being implemented with cntlzw), and you're not handling the MMUv2 > > case. > > I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it > slightly to let me choose the target virt address. > > Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and > export that function there? Yes. And maybe fix that align == -2 bug while you're at it. :-) > >> void pci_init_board(void) > >> { > >> - struct fsl_pci_info pci_info; > >> + struct pci_controller *pci_hoses; > >> const void *fdt = get_fdt(); > >> int pci_node; > >> + int pci_num = 0; > >> + int pci_count; > >> + const char *compat = "fsl,mpc8540-pci"; > >> + ulong map_addr; > >> > >> puts("\n"); > >> > >> - pci_node = fdt_path_offset(fdt, "/pci"); > >> - if (pci_node < 0) { > >> + /* Start MMIO and PIO range maps above RAM */ > >> + map_addr = CONFIG_MAX_MEM_MAPPED; > >> + > >> + /* Count and allocate PCI buses */ > >> + pci_count = myfdt_count_compatibles(fdt, compat); > >> + > >> + if (pci_count) { > >> + pci_hoses = malloc(sizeof(struct pci_controller) * pci_count); > >> + } else { > >> printf("PCI: disabled\n\n"); > >> return; > >> } > >> > >> - SET_STD_PCI_INFO(pci_info, 1); > >> - > >> - fsl_setup_hose(&pci1_hose, pci_info.regs); > >> - printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n", > >> - pci_info.regs); > >> - > >> - fsl_pci_init_port(&pci_info, &pci1_hose, 0); > >> + /* 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 = { }; > >> + uint64_t phys_addr; > >> + int pnode = fdt_parent_offset(fdt, pci_node); > >> + int paddress_cells; > >> + int address_cells; > >> + int size_cells; > >> + int off = 0; > >> + > >> + paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1); > >> + address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1); > >> + size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1); > >> + > >> + pci_info.regs = myfdt_readcells(fdt, pci_node, "reg", > >> + paddress_cells, 0, 0); > >> + > >> + /* MMIO range */ > >> + off += address_cells; > >> + phys_addr = myfdt_readcells(fdt, pci_node, "ranges", > >> + paddress_cells, off, 0); > >> + off += paddress_cells; > >> + pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges", > >> + size_cells, off, 0); > >> + off += size_cells; > >> + > >> + /* Align virtual region */ > >> + map_addr += pci_info.mem_size - 1; > >> + map_addr &= ~(pci_info.mem_size - 1); > >> + /* Map virtual memory for MMIO range */ > >> + map_tlb1_io(map_addr, phys_addr, pci_info.mem_size); > >> + pci_info.mem_bus = phys_addr; > >> + pci_info.mem_phys = phys_addr; > >> + map_addr += pci_info.mem_size; > >> + > >> + /* PIO range */ > >> + off += address_cells; > >> + pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges", > >> + paddress_cells, off, 0); > >> + off += paddress_cells; > >> + pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges", > >> + size_cells, off, 0); > >> + > >> + /* Align virtual region */ > >> + map_addr += pci_info.io_size - 1; > >> + map_addr &= ~(pci_info.io_size - 1); > >> + /* Map virtual memory for MMIO range */ > >> + map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size); > >> + pci_info.io_bus = map_addr; > >> + map_addr += pci_info.io_size; > >> + > >> + /* Instantiate */ > >> + pci_info.pci_num = pci_num + 1; > >> + > >> + fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs); > >> + printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n", > >> + pci_info.regs); > >> + > >> + fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num); > >> + > >> + /* Jump to next PCI node */ > >> + pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat); > >> + pci_num++; > >> + } > >> > >> puts("\n"); > >> } > > > > How about using fdt_translate_address() or other existing functionality? > > Mind to show exactly how? I guess you can't use that when you don't know the bus address, but still this is the wrong place to implement it. If getting PCI range info from the device tree is something we want to do, then it should be a new common code helper. -Scott
On 06.02.2014, at 23:52, Scott Wood <scottwood@freescale.com> wrote: > On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote: >> On 04.02.2014, at 03:47, Scott Wood <scottwood@freescale.com> 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 PCI devices are in our physical memory layout but instead >>>> read them dynamically from the device tree we get passed on boot. >>> >>> Would it be difficult to make the QEMU emulation properly implement >>> access windows? >> >> What are access windows? You mean BAR region offsets? Not too hard I >> suppose, but it adds complexity which we were trying to avoid, no? > > It would remove U-Boot complexity (unlike the LAW stuff where we just > skip it) because we wouldn't need to care about QEMU's default settings. > It should be easier to do than LAW support, and more useful (e.g. Linux > currently programs this as well, we just get lucky that it misuses the > device tree as configuration information). What complexity would it remove? We would still need to find the configuration space for the access windows, configure them and then even go as far as modifying the original device tree so we expose the new windows. > >>>> +{ >>>> + int len; >>>> + const uint32_t *prop; >>>> + >>>> + prop = fdt_getprop(fdt, node, property, &len); >>>> + >>>> + if (!prop) >>>> + return defval; >>>> + >>>> + if (len < ((off + num) * sizeof(uint32_t))) >>>> + panic("Invalid fdt"); >>>> + >>>> + prop += off; >>>> + >>>> + switch (num) { >>>> + case 1: >>>> + return *prop; >>>> + case 2: >>>> + return *(const uint64_t *)prop; >>>> + } >>>> + >>> >>> What about this function is specific to qemu-ppce500? >> >> Nothing. But the less common code I touch the less I can break. > > The more that line of thought is applied, the uglier the codebase we'll > end up with. :-) > >> There seems to be an fdt helper framework that's only targeted at a few ARM >> devices - not sure what to make of that. > > Make use of whatever parts you can, and extend it with the missing bits > you need. There's also common/fdt_support.c which is definitely not > just used by ARM. > >>> + panic("Invalid cell size"); >>> +} >>> >>> s/cell size/cell count/ >>> >>>> +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property, >>>> + uint32_t defval) >>>> +{ >>>> + return myfdt_readcells(fdt, node, property, 1, 0, defval); >>>> +} >>> >>> This looks a lot like fdt_getprop_u32_default(), except for "int node" >>> versus path. >> >> Well, I use it inside of a loop where I don't have the path :). > > It still indicates where the proper place for code like this is. :-) > >>>> +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size) >>>> +{ >>>> + unsigned int max_cam, tsize_mask; >>>> + int i; >>>> + >>>> + if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) { >>>> + /* Convert (4^max) kB to (2^max) bytes */ >>>> + max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10; >>>> + tsize_mask = ~1U; >>>> + } else { >>>> + /* Convert (2^max) kB to (2^max) bytes */ >>>> + max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10; >>>> + tsize_mask = ~0U; >>>> + } >>>> + >>>> + for (i = 0; size && i < 8; i++) { >>>> + int tlb_index = find_free_tlbcam(); >>>> + u32 camsize = __ilog2_u64(size) & tsize_mask; >>>> + u32 align = __ilog2(virt_addr) & tsize_mask; >>>> + unsigned int tlb_size; >>>> + >>>> + if (tlb_index == -1) >>>> + break; >>>> + >>>> + if (align == -2) align = max_cam; >>> >>> -2? Besides align being unsigned, if this is meant to handle the case >>> where virt_addr is zero, that's undefined for __ilog2() (don't rely on >>> it being implemented with cntlzw), and you're not handling the MMUv2 >>> case. >> >> I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it >> slightly to let me choose the target virt address. >> >> Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and >> export that function there? > > Yes. > > And maybe fix that align == -2 bug while you're at it. :-) I'll leave that to you :P Alex
On 06.02.2014, at 23:52, Scott Wood <scottwood@freescale.com> wrote: > On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote: >> On 04.02.2014, at 03:47, Scott Wood <scottwood@freescale.com> 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 PCI devices are in our physical memory layout but instead >>>> read them dynamically from the device tree we get passed on boot. >>> >>> Would it be difficult to make the QEMU emulation properly implement >>> access windows? >> >> What are access windows? You mean BAR region offsets? Not too hard I >> suppose, but it adds complexity which we were trying to avoid, no? > > It would remove U-Boot complexity (unlike the LAW stuff where we just > skip it) because we wouldn't need to care about QEMU's default settings. > It should be easier to do than LAW support, and more useful (e.g. Linux > currently programs this as well, we just get lucky that it misuses the > device tree as configuration information). > >>>> +{ >>>> + int len; >>>> + const uint32_t *prop; >>>> + >>>> + prop = fdt_getprop(fdt, node, property, &len); >>>> + >>>> + if (!prop) >>>> + return defval; >>>> + >>>> + if (len < ((off + num) * sizeof(uint32_t))) >>>> + panic("Invalid fdt"); >>>> + >>>> + prop += off; >>>> + >>>> + switch (num) { >>>> + case 1: >>>> + return *prop; >>>> + case 2: >>>> + return *(const uint64_t *)prop; >>>> + } >>>> + >>> >>> What about this function is specific to qemu-ppce500? >> >> Nothing. But the less common code I touch the less I can break. > > The more that line of thought is applied, the uglier the codebase we'll > end up with. :-) > >> There seems to be an fdt helper framework that's only targeted at a few ARM >> devices - not sure what to make of that. > > Make use of whatever parts you can, and extend it with the missing bits > you need. There's also common/fdt_support.c which is definitely not > just used by ARM. > >>> + panic("Invalid cell size"); >>> +} >>> >>> s/cell size/cell count/ >>> >>>> +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property, >>>> + uint32_t defval) >>>> +{ >>>> + return myfdt_readcells(fdt, node, property, 1, 0, defval); >>>> +} >>> >>> This looks a lot like fdt_getprop_u32_default(), except for "int node" >>> versus path. >> >> Well, I use it inside of a loop where I don't have the path :). > > It still indicates where the proper place for code like this is. :-) > >>>> +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size) >>>> +{ >>>> + unsigned int max_cam, tsize_mask; >>>> + int i; >>>> + >>>> + if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) { >>>> + /* Convert (4^max) kB to (2^max) bytes */ >>>> + max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10; >>>> + tsize_mask = ~1U; >>>> + } else { >>>> + /* Convert (2^max) kB to (2^max) bytes */ >>>> + max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10; >>>> + tsize_mask = ~0U; >>>> + } >>>> + >>>> + for (i = 0; size && i < 8; i++) { >>>> + int tlb_index = find_free_tlbcam(); >>>> + u32 camsize = __ilog2_u64(size) & tsize_mask; >>>> + u32 align = __ilog2(virt_addr) & tsize_mask; >>>> + unsigned int tlb_size; >>>> + >>>> + if (tlb_index == -1) >>>> + break; >>>> + >>>> + if (align == -2) align = max_cam; >>> >>> -2? Besides align being unsigned, if this is meant to handle the case >>> where virt_addr is zero, that's undefined for __ilog2() (don't rely on >>> it being implemented with cntlzw), and you're not handling the MMUv2 >>> case. >> >> I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it >> slightly to let me choose the target virt address. >> >> Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and >> export that function there? > > Yes. > > And maybe fix that align == -2 bug while you're at it. :-) > >>>> void pci_init_board(void) >>>> { >>>> - struct fsl_pci_info pci_info; >>>> + struct pci_controller *pci_hoses; >>>> const void *fdt = get_fdt(); >>>> int pci_node; >>>> + int pci_num = 0; >>>> + int pci_count; >>>> + const char *compat = "fsl,mpc8540-pci"; >>>> + ulong map_addr; >>>> >>>> puts("\n"); >>>> >>>> - pci_node = fdt_path_offset(fdt, "/pci"); >>>> - if (pci_node < 0) { >>>> + /* Start MMIO and PIO range maps above RAM */ >>>> + map_addr = CONFIG_MAX_MEM_MAPPED; >>>> + >>>> + /* Count and allocate PCI buses */ >>>> + pci_count = myfdt_count_compatibles(fdt, compat); >>>> + >>>> + if (pci_count) { >>>> + pci_hoses = malloc(sizeof(struct pci_controller) * pci_count); >>>> + } else { >>>> printf("PCI: disabled\n\n"); >>>> return; >>>> } >>>> >>>> - SET_STD_PCI_INFO(pci_info, 1); >>>> - >>>> - fsl_setup_hose(&pci1_hose, pci_info.regs); >>>> - printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n", >>>> - pci_info.regs); >>>> - >>>> - fsl_pci_init_port(&pci_info, &pci1_hose, 0); >>>> + /* 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 = { }; >>>> + uint64_t phys_addr; >>>> + int pnode = fdt_parent_offset(fdt, pci_node); >>>> + int paddress_cells; >>>> + int address_cells; >>>> + int size_cells; >>>> + int off = 0; >>>> + >>>> + paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1); >>>> + address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1); >>>> + size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1); >>>> + >>>> + pci_info.regs = myfdt_readcells(fdt, pci_node, "reg", >>>> + paddress_cells, 0, 0); >>>> + >>>> + /* MMIO range */ >>>> + off += address_cells; >>>> + phys_addr = myfdt_readcells(fdt, pci_node, "ranges", >>>> + paddress_cells, off, 0); >>>> + off += paddress_cells; >>>> + pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges", >>>> + size_cells, off, 0); >>>> + off += size_cells; >>>> + >>>> + /* Align virtual region */ >>>> + map_addr += pci_info.mem_size - 1; >>>> + map_addr &= ~(pci_info.mem_size - 1); >>>> + /* Map virtual memory for MMIO range */ >>>> + map_tlb1_io(map_addr, phys_addr, pci_info.mem_size); >>>> + pci_info.mem_bus = phys_addr; >>>> + pci_info.mem_phys = phys_addr; >>>> + map_addr += pci_info.mem_size; >>>> + >>>> + /* PIO range */ >>>> + off += address_cells; >>>> + pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges", >>>> + paddress_cells, off, 0); >>>> + off += paddress_cells; >>>> + pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges", >>>> + size_cells, off, 0); >>>> + >>>> + /* Align virtual region */ >>>> + map_addr += pci_info.io_size - 1; >>>> + map_addr &= ~(pci_info.io_size - 1); >>>> + /* Map virtual memory for MMIO range */ >>>> + map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size); >>>> + pci_info.io_bus = map_addr; >>>> + map_addr += pci_info.io_size; >>>> + >>>> + /* Instantiate */ >>>> + pci_info.pci_num = pci_num + 1; >>>> + >>>> + fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs); >>>> + printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n", >>>> + pci_info.regs); >>>> + >>>> + fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num); >>>> + >>>> + /* Jump to next PCI node */ >>>> + pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat); >>>> + pci_num++; >>>> + } >>>> >>>> puts("\n"); >>>> } >>> >>> How about using fdt_translate_address() or other existing functionality? >> >> Mind to show exactly how? > > I guess you can't use that when you don't know the bus address, but > still this is the wrong place to implement it. If getting PCI range > info from the device tree is something we want to do, then it should be > a new common code helper. The more I think about this the less of an idea I have how to do any generic API for this at all. And I'm not convinced anything generic is going to help anyone. Do a git grep on fdt_translate_address over the code base and you will find a _single_ caller. So even if we come up with something now, nobody's going to use it. Alex
On 07.02.2014, at 15:54, Alexander Graf <agraf@suse.de> wrote: > > On 06.02.2014, at 23:52, Scott Wood <scottwood@freescale.com> wrote: > >> On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote: >>>> >>>> How about using fdt_translate_address() or other existing functionality? >>> >>> Mind to show exactly how? >> >> I guess you can't use that when you don't know the bus address, but >> still this is the wrong place to implement it. If getting PCI range >> info from the device tree is something we want to do, then it should be >> a new common code helper. > > The more I think about this the less of an idea I have how to do any generic API for this at all. And I'm not convinced anything generic is going to help anyone. Do a git grep on fdt_translate_address over the code base and you will find a _single_ caller. > > So even if we come up with something now, nobody's going to use it. Hrm. Maybe fdt_get_base_address() is the answer? Alex
On Fri, 2014-02-07 at 13:25 +0100, Alexander Graf wrote: > On 06.02.2014, at 23:52, Scott Wood <scottwood@freescale.com> wrote: > > > On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote: > >> On 04.02.2014, at 03:47, Scott Wood <scottwood@freescale.com> 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 PCI devices are in our physical memory layout but instead > >>>> read them dynamically from the device tree we get passed on boot. > >>> > >>> Would it be difficult to make the QEMU emulation properly implement > >>> access windows? > >> > >> What are access windows? You mean BAR region offsets? Not too hard I > >> suppose, but it adds complexity which we were trying to avoid, no? > > > > It would remove U-Boot complexity (unlike the LAW stuff where we just > > skip it) because we wouldn't need to care about QEMU's default settings. > > It should be easier to do than LAW support, and more useful (e.g. Linux > > currently programs this as well, we just get lucky that it misuses the > > device tree as configuration information). > > What complexity would it remove? Getting the PCI translation window addresses from the device tree. > We would still need to find the configuration space for the access > windows, That's easier -- just a standard reg lookup. > configure them That code's already there. > and then even go as far as modifying the original device tree so we > expose the new windows. It would be nice if we did this regardless of QEMU. As is, IIRC we have some device trees that don't match what U-Boot does, which works (at least in Linux) because Linux reprograms it to match the device tree. -Scott
On Fri, 2014-02-07 at 16:00 +0100, Alexander Graf wrote: > On 07.02.2014, at 15:54, Alexander Graf <agraf@suse.de> wrote: > > > > > On 06.02.2014, at 23:52, Scott Wood <scottwood@freescale.com> wrote: > > > >> On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote: > >>>> > >>>> How about using fdt_translate_address() or other existing functionality? > >>> > >>> Mind to show exactly how? > >> > >> I guess you can't use that when you don't know the bus address, but > >> still this is the wrong place to implement it. If getting PCI range > >> info from the device tree is something we want to do, then it should be > >> a new common code helper. > > > > The more I think about this the less of an idea I have how to do any generic API for this at all. And I'm not convinced anything generic is going to help anyone. Do a git grep on fdt_translate_address over the code base and you will find a _single_ caller. > > > > So even if we come up with something now, nobody's going to use it. > > Hrm. Maybe fdt_get_base_address() is the answer? Yes, that looks close to what you want. It'd have to be extended to look for the various types of PCI ranges and return a (cpu address, bus address, size) tuple rather than just a base address. It's moot though if we can reprogram the PCI windows. -Scott
diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c index 6491ae9..5d4dd64 100644 --- a/board/freescale/qemu-ppce500/qemu-ppce500.c +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c @@ -19,7 +19,51 @@ #include <malloc.h> DECLARE_GLOBAL_DATA_PTR; -static struct pci_controller pci1_hose; + +static uint64_t myfdt_readcells(const void *fdt, int node, const char *property, + int num, int off, uint64_t defval) +{ + int len; + const uint32_t *prop; + + prop = fdt_getprop(fdt, node, property, &len); + + if (!prop) + return defval; + + if (len < ((off + num) * sizeof(uint32_t))) + panic("Invalid fdt"); + + prop += off; + + switch (num) { + case 1: + return *prop; + case 2: + return *(const uint64_t *)prop; + } + + panic("Invalid cell size"); +} + +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property, + uint32_t defval) +{ + return myfdt_readcells(fdt, node, property, 1, 0, defval); +} + +static int myfdt_count_compatibles(const void *fdt, const char *compat) +{ + int node, num = 0; + + node = fdt_node_offset_by_compatible(fdt, -1, compat); + while (node != -FDT_ERR_NOTFOUND) { + node = fdt_node_offset_by_compatible(fdt, node, compat); + num++; + } + + return num; +} static const void *get_fdt(void) { @@ -39,13 +83,9 @@ uint64_t get_phys_ccsrbar_addr(void) 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]; + root_address_cells = myfdt_one_cell(fdt, root_node, "#address-cells", 1); + address_cells = myfdt_one_cell(fdt, soc_node, "#address-cells", + root_address_cells); /* Read CCSRBAR address from device tree */ prop = fdt_getprop(fdt, soc_node, "ranges", &len); @@ -114,27 +154,144 @@ int checkboard(void) return 0; } +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size) +{ + unsigned int max_cam, tsize_mask; + int i; + + if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) { + /* Convert (4^max) kB to (2^max) bytes */ + max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10; + tsize_mask = ~1U; + } else { + /* Convert (2^max) kB to (2^max) bytes */ + max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10; + tsize_mask = ~0U; + } + + for (i = 0; size && i < 8; i++) { + int tlb_index = find_free_tlbcam(); + u32 camsize = __ilog2_u64(size) & tsize_mask; + u32 align = __ilog2(virt_addr) & tsize_mask; + unsigned int tlb_size; + + if (tlb_index == -1) + break; + + if (align == -2) align = max_cam; + if (camsize > align) + camsize = align; + + if (camsize > max_cam) + camsize = max_cam; + + tlb_size = camsize - 10; + + set_tlb(1, virt_addr, phys_addr, + MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, + 0, tlb_index, tlb_size, 1); + + /* Remember the mapping in our address map */ + addrmap_set_entry(virt_addr, phys_addr, 1ULL << camsize, + tlb_index); + + size -= 1ULL << camsize; + virt_addr += 1UL << camsize; + phys_addr += 1UL << camsize; + } + +} + void pci_init_board(void) { - struct fsl_pci_info pci_info; + struct pci_controller *pci_hoses; const void *fdt = get_fdt(); int pci_node; + int pci_num = 0; + int pci_count; + const char *compat = "fsl,mpc8540-pci"; + ulong map_addr; puts("\n"); - pci_node = fdt_path_offset(fdt, "/pci"); - if (pci_node < 0) { + /* Start MMIO and PIO range maps above RAM */ + map_addr = CONFIG_MAX_MEM_MAPPED; + + /* Count and allocate PCI buses */ + pci_count = myfdt_count_compatibles(fdt, compat); + + if (pci_count) { + pci_hoses = malloc(sizeof(struct pci_controller) * pci_count); + } else { printf("PCI: disabled\n\n"); return; } - SET_STD_PCI_INFO(pci_info, 1); - - fsl_setup_hose(&pci1_hose, pci_info.regs); - printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n", - pci_info.regs); - - fsl_pci_init_port(&pci_info, &pci1_hose, 0); + /* 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 = { }; + uint64_t phys_addr; + int pnode = fdt_parent_offset(fdt, pci_node); + int paddress_cells; + int address_cells; + int size_cells; + int off = 0; + + paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1); + address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1); + size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1); + + pci_info.regs = myfdt_readcells(fdt, pci_node, "reg", + paddress_cells, 0, 0); + + /* MMIO range */ + off += address_cells; + phys_addr = myfdt_readcells(fdt, pci_node, "ranges", + paddress_cells, off, 0); + off += paddress_cells; + pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges", + size_cells, off, 0); + off += size_cells; + + /* Align virtual region */ + map_addr += pci_info.mem_size - 1; + map_addr &= ~(pci_info.mem_size - 1); + /* Map virtual memory for MMIO range */ + map_tlb1_io(map_addr, phys_addr, pci_info.mem_size); + pci_info.mem_bus = phys_addr; + pci_info.mem_phys = phys_addr; + map_addr += pci_info.mem_size; + + /* PIO range */ + off += address_cells; + pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges", + paddress_cells, off, 0); + off += paddress_cells; + pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges", + size_cells, off, 0); + + /* Align virtual region */ + map_addr += pci_info.io_size - 1; + map_addr &= ~(pci_info.io_size - 1); + /* Map virtual memory for MMIO range */ + map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size); + pci_info.io_bus = map_addr; + map_addr += pci_info.io_size; + + /* Instantiate */ + pci_info.pci_num = pci_num + 1; + + fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs); + printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n", + pci_info.regs); + + fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num); + + /* Jump to next PCI node */ + pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat); + pci_num++; + } puts("\n"); } diff --git a/board/freescale/qemu-ppce500/tlb.c b/board/freescale/qemu-ppce500/tlb.c index a600296..cf51d0e 100644 --- a/board/freescale/qemu-ppce500/tlb.c +++ b/board/freescale/qemu-ppce500/tlb.c @@ -11,19 +11,6 @@ #include <asm/mmu.h> struct fsl_e_tlb_entry tlb_table[] = { - /* - * TLB 1: 256M Non-cacheable, guarded - */ - SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT, CONFIG_SYS_PCI_PHYS, - MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, - 0, 1, BOOKE_PAGESZ_256M, 1), - - /* - * TLB 2: 256M Non-cacheable, guarded - */ - SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT + 0x10000000, CONFIG_SYS_PCI_PHYS + 0x10000000, - MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, - 0, 2, BOOKE_PAGESZ_256M, 1), }; int num_tlb_entries = ARRAY_SIZE(tlb_table); diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h index 7ef7235..f7e59bc 100644 --- a/include/configs/qemu-ppce500.h +++ b/include/configs/qemu-ppce500.h @@ -127,18 +127,6 @@ extern unsigned long long get_phys_ccsrbar_addr_early(void); * Memory space is mapped 1-1, but I/O space must start from 0. */ -#define CONFIG_SYS_PCI_VIRT 0xc0000000 /* 512M PCI TLB */ -#define CONFIG_SYS_PCI_PHYS 0xc0000000 /* 512M PCI TLB */ - -#define CONFIG_SYS_PCI1_MEM_VIRT 0xc0000000 -#define CONFIG_SYS_PCI1_MEM_BUS 0xc0000000 -#define CONFIG_SYS_PCI1_MEM_PHYS 0xc0000000 -#define CONFIG_SYS_PCI1_MEM_SIZE 0x20000000 /* 512M */ -#define CONFIG_SYS_PCI1_IO_VIRT 0xe1000000 -#define CONFIG_SYS_PCI1_IO_BUS 0x00000000 -#define CONFIG_SYS_PCI1_IO_PHYS 0xe1000000 -#define CONFIG_SYS_PCI1_IO_SIZE 0x00010000 /* 64k */ - #ifdef CONFIG_PCI #define CONFIG_PCI_INDIRECT_BRIDGE #define CONFIG_NET_MULTI
The definition of our ppce500 PV machine is that every address is dynamically determined through device tree bindings. So don't hardcode where PCI devices are in our physical memory layout but instead read them dynamically from the device tree we get passed on boot. Signed-off-by: Alexander Graf <agraf@suse.de> --- board/freescale/qemu-ppce500/qemu-ppce500.c | 193 ++++++++++++++++++++++++--- board/freescale/qemu-ppce500/tlb.c | 13 -- include/configs/qemu-ppce500.h | 12 -- 3 files changed, 175 insertions(+), 43 deletions(-)