| Message ID | 1417018897-3965-1-git-send-email-thierry.reding@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On Wednesday 26 November 2014 17:21:37 Thierry Reding wrote: > - > + pci_add_resource_offset(&sys->resources, &pcie->pio, sys->io_offset); > pci_add_resource_offset(&sys->resources, &pcie->mem, sys->mem_offset); > pci_add_resource_offset(&sys->resources, &pcie->prefetch, > sys->mem_offset); > You don't set sys->io_offset anywhere, which is a bug if you have multiple instances of the PCI host in one system. In my draft patch, I was setting both io_offset and mem_offset for consistency, and while mem_offset would in practice be always zero (as discussed on IRC), the io_offset in fact has a realistic chance of being nonzero and you should definitely set it. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 26, 2014 at 05:55:05PM +0100, Arnd Bergmann wrote: > On Wednesday 26 November 2014 17:21:37 Thierry Reding wrote: > > - > > + pci_add_resource_offset(&sys->resources, &pcie->pio, sys->io_offset); > > pci_add_resource_offset(&sys->resources, &pcie->mem, sys->mem_offset); > > pci_add_resource_offset(&sys->resources, &pcie->prefetch, > > sys->mem_offset); > > > > You don't set sys->io_offset anywhere, which is a bug if you have multiple > instances of the PCI host in one system. In my draft patch, I was setting > both io_offset and mem_offset for consistency, and while mem_offset would > in practice be always zero (as discussed on IRC), the io_offset in fact has > a realistic chance of being nonzero and you should definitely set it. I was going to do that in a follow-up patch since it isn't needed to fix this particular regression. Thierry
On Wednesday 26 November 2014 21:18:03 Thierry Reding wrote: > On Wed, Nov 26, 2014 at 05:55:05PM +0100, Arnd Bergmann wrote: > > On Wednesday 26 November 2014 17:21:37 Thierry Reding wrote: > > > - > > > + pci_add_resource_offset(&sys->resources, &pcie->pio, sys->io_offset); > > > pci_add_resource_offset(&sys->resources, &pcie->mem, sys->mem_offset); > > > pci_add_resource_offset(&sys->resources, &pcie->prefetch, > > > sys->mem_offset); > > > > > > > You don't set sys->io_offset anywhere, which is a bug if you have multiple > > instances of the PCI host in one system. In my draft patch, I was setting > > both io_offset and mem_offset for consistency, and while mem_offset would > > in practice be always zero (as discussed on IRC), the io_offset in fact has > > a realistic chance of being nonzero and you should definitely set it. > > I was going to do that in a follow-up patch since it isn't needed to fix > this particular regression. Makes sense, but then I think you can do the request_resource() and pci_add_resource_offset() additions in the same follow-up patch as well, and only change the computation in the regression fix. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c index 3d43874319be..9c75bd998a38 100644 --- a/drivers/pci/host/pci-tegra.c +++ b/drivers/pci/host/pci-tegra.c @@ -276,6 +276,7 @@ struct tegra_pcie { struct resource all; struct resource io; + struct resource pio; struct resource mem; struct resource prefetch; struct resource busn; @@ -658,7 +659,14 @@ static int tegra_pcie_setup(int nr, struct pci_sys_data *sys) { struct tegra_pcie *pcie = sys_to_pcie(sys); int err; - phys_addr_t io_start; + + err = devm_request_resource(pcie->dev, &pcie->all, &pcie->io); + if (err < 0) + return err; + + err = devm_request_resource(pcie->dev, &ioport_resource, &pcie->pio); + if (err < 0) + return err; err = devm_request_resource(pcie->dev, &pcie->all, &pcie->mem); if (err < 0) @@ -668,14 +676,13 @@ static int tegra_pcie_setup(int nr, struct pci_sys_data *sys) if (err) return err; - io_start = pci_pio_to_address(pcie->io.start); - + pci_add_resource_offset(&sys->resources, &pcie->pio, sys->io_offset); pci_add_resource_offset(&sys->resources, &pcie->mem, sys->mem_offset); pci_add_resource_offset(&sys->resources, &pcie->prefetch, sys->mem_offset); pci_add_resource(&sys->resources, &pcie->busn); - pci_ioremap_io(nr * SZ_64K, io_start); + pci_ioremap_io(pcie->pio.start, pcie->io.start); return 1; } @@ -786,7 +793,6 @@ static irqreturn_t tegra_pcie_isr(int irq, void *arg) static void tegra_pcie_setup_translations(struct tegra_pcie *pcie) { u32 fpci_bar, size, axi_address; - phys_addr_t io_start = pci_pio_to_address(pcie->io.start); /* Bar 0: type 1 extended configuration space */ fpci_bar = 0xfe100000; @@ -799,7 +805,7 @@ static void tegra_pcie_setup_translations(struct tegra_pcie *pcie) /* Bar 1: downstream IO bar */ fpci_bar = 0xfdfc0000; size = resource_size(&pcie->io); - axi_address = io_start; + axi_address = pcie->io.start; afi_writel(pcie, axi_address, AFI_AXI_BAR1_START); afi_writel(pcie, size >> 12, AFI_AXI_BAR1_SZ); afi_writel(pcie, fpci_bar, AFI_FPCI_BAR1); @@ -1690,8 +1696,23 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) switch (res.flags & IORESOURCE_TYPE_BITS) { case IORESOURCE_IO: - memcpy(&pcie->io, &res, sizeof(res)); - pcie->io.name = np->full_name; + memcpy(&pcie->pio, &res, sizeof(res)); + pcie->pio.name = np->full_name; + + /* + * The Tegra PCIe host bridge uses this to program the + * mapping of the I/O space to the physical address, + * so we override the .start and .end fields here that + * of_pci_range_to_resource() converted to I/O space. + * We also set the IORESOURCE_MEM type to clarify that + * the resource is in the physical memory space. + */ + pcie->io.start = range.cpu_addr; + pcie->io.end = range.cpu_addr + range.size - 1; + pcie->io.flags = IORESOURCE_MEM; + pcie->io.name = "I/O"; + + memcpy(&res, &pcie->io, sizeof(res)); break; case IORESOURCE_MEM: