* [PATCH v2] PCI: tegra: Use physical range for I/O mapping
@ 2014-11-27 8:54 ` Thierry Reding
0 siblings, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2014-11-27 8:54 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Arnd Bergmann, Liviu Dudau, Marc Zyngier,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Commit 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO
resources") changed how I/O resources are parsed from DT. Rather than
containing the physical address of the I/O region, the addresses will
now be in I/O address space.
On Tegra the union of all ranges is used to expose a top-level memory-
mapped resource for the PCI host bridge. This helps to make /proc/iomem
more readable.
Combining both of the above, the union would now include the I/O space
region. This causes a regression on Tegra20, where the physical base
address of the PCIe controller (and therefore of the union) is located
at physical address 0x80000000. Since I/O space starts at 0, the union
will now include all of system RAM which starts at 0x00000000.
This commit fixes this by keeping two copies of the I/O range: one that
represents the range in the CPU's physical address space, the other for
the range in the I/O address space. This allows the translation setup
within the driver to reuse the physical addresses. The code registering
the I/O region with the PCI core uses both ranges to establish the
mapping.
Fixes: 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO resources")
Reported-by: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
Suggested-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Marc, any chance of getting a Tested-by from you on this? I think you
said you were seeing this on Harmony and I'm pretty sure this should fix
the issue for you as well.
Bjorn, this fixes a regression on Tegra20 (it's hidden on later Tegra
generations because the PCIe controller has moved to the first 1 GiB of
physical address space) that was introduced in 3.18-rc1, so it would be
good to get this in before the final 3.18 release.
Changes in v2:
- remove cleanup not essential to fix the regression (will be part of a
follow-up patch)
drivers/pci/host/pci-tegra.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 3d43874319be..19bb19c7db4a 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,6 @@ 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->mem);
if (err < 0)
@@ -668,14 +668,12 @@ 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->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 +784,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 +796,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 +1687,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:
--
2.1.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] PCI: tegra: Use physical range for I/O mapping
@ 2014-11-27 9:12 ` Arnd Bergmann
0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2014-11-27 9:12 UTC (permalink / raw)
To: Thierry Reding
Cc: Bjorn Helgaas, Liviu Dudau, Marc Zyngier, linux-pci, linux-tegra
On Thursday 27 November 2014 09:54:09 Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Commit 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO
> resources") changed how I/O resources are parsed from DT. Rather than
> containing the physical address of the I/O region, the addresses will
> now be in I/O address space.
>
> On Tegra the union of all ranges is used to expose a top-level memory-
> mapped resource for the PCI host bridge. This helps to make /proc/iomem
> more readable.
>
> Combining both of the above, the union would now include the I/O space
> region. This causes a regression on Tegra20, where the physical base
> address of the PCIe controller (and therefore of the union) is located
> at physical address 0x80000000. Since I/O space starts at 0, the union
> will now include all of system RAM which starts at 0x00000000.
>
> This commit fixes this by keeping two copies of the I/O range: one that
> represents the range in the CPU's physical address space, the other for
> the range in the I/O address space. This allows the translation setup
> within the driver to reuse the physical addresses. The code registering
> the I/O region with the PCI core uses both ranges to establish the
> mapping.
>
> Fixes: 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO resources")
> Reported-by: Marc Zyngier <marc.zyngier@arm.com>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] PCI: tegra: Use physical range for I/O mapping
@ 2014-11-27 9:12 ` Arnd Bergmann
0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2014-11-27 9:12 UTC (permalink / raw)
To: Thierry Reding
Cc: Bjorn Helgaas, Liviu Dudau, Marc Zyngier,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
On Thursday 27 November 2014 09:54:09 Thierry Reding wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> Commit 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO
> resources") changed how I/O resources are parsed from DT. Rather than
> containing the physical address of the I/O region, the addresses will
> now be in I/O address space.
>
> On Tegra the union of all ranges is used to expose a top-level memory-
> mapped resource for the PCI host bridge. This helps to make /proc/iomem
> more readable.
>
> Combining both of the above, the union would now include the I/O space
> region. This causes a regression on Tegra20, where the physical base
> address of the PCIe controller (and therefore of the union) is located
> at physical address 0x80000000. Since I/O space starts at 0, the union
> will now include all of system RAM which starts at 0x00000000.
>
> This commit fixes this by keeping two copies of the I/O range: one that
> represents the range in the CPU's physical address space, the other for
> the range in the I/O address space. This allows the translation setup
> within the driver to reuse the physical addresses. The code registering
> the I/O region with the PCI core uses both ranges to establish the
> mapping.
>
> Fixes: 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO resources")
> Reported-by: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
> Suggested-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: tegra: Use physical range for I/O mapping
2014-11-27 8:54 ` Thierry Reding
(?)
(?)
@ 2014-11-27 9:20 ` Marc Zyngier
-1 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2014-11-27 9:20 UTC (permalink / raw)
To: Thierry Reding, Bjorn Helgaas
Cc: Arnd Bergmann, Liviu Dudau, linux-pci@vger.kernel.org,
linux-tegra@vger.kernel.org
On 27/11/14 08:54, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Commit 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO
> resources") changed how I/O resources are parsed from DT. Rather than
> containing the physical address of the I/O region, the addresses will
> now be in I/O address space.
>
> On Tegra the union of all ranges is used to expose a top-level memory-
> mapped resource for the PCI host bridge. This helps to make /proc/iomem
> more readable.
>
> Combining both of the above, the union would now include the I/O space
> region. This causes a regression on Tegra20, where the physical base
> address of the PCIe controller (and therefore of the union) is located
> at physical address 0x80000000. Since I/O space starts at 0, the union
> will now include all of system RAM which starts at 0x00000000.
>
> This commit fixes this by keeping two copies of the I/O range: one that
> represents the range in the CPU's physical address space, the other for
> the range in the I/O address space. This allows the translation setup
> within the driver to reuse the physical addresses. The code registering
> the I/O region with the PCI core uses both ranges to establish the
> mapping.
>
> Fixes: 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO resources")
> Reported-by: Marc Zyngier <marc.zyngier@arm.com>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Just gave it a go, and it works just fine here. Thanks!
Tested-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] PCI: tegra: Use physical range for I/O mapping
2014-11-27 8:54 ` Thierry Reding
` (2 preceding siblings ...)
(?)
@ 2014-12-02 0:01 ` Bjorn Helgaas
-1 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2014-12-02 0:01 UTC (permalink / raw)
To: Thierry Reding
Cc: Arnd Bergmann, Liviu Dudau, Marc Zyngier,
linux-pci@vger.kernel.org, linux-tegra@vger.kernel.org
On Thu, Nov 27, 2014 at 1:54 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Commit 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO
> resources") changed how I/O resources are parsed from DT. Rather than
> containing the physical address of the I/O region, the addresses will
> now be in I/O address space.
>
> On Tegra the union of all ranges is used to expose a top-level memory-
> mapped resource for the PCI host bridge. This helps to make /proc/iomem
> more readable.
>
> Combining both of the above, the union would now include the I/O space
> region. This causes a regression on Tegra20, where the physical base
> address of the PCIe controller (and therefore of the union) is located
> at physical address 0x80000000. Since I/O space starts at 0, the union
> will now include all of system RAM which starts at 0x00000000.
>
> This commit fixes this by keeping two copies of the I/O range: one that
> represents the range in the CPU's physical address space, the other for
> the range in the I/O address space. This allows the translation setup
> within the driver to reuse the physical addresses. The code registering
> the I/O region with the PCI core uses both ranges to establish the
> mapping.
>
> Fixes: 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO resources")
> Reported-by: Marc Zyngier <marc.zyngier@arm.com>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Applied to for-linus for v3.18 with:
Tested-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Thanks!
> ---
> Marc, any chance of getting a Tested-by from you on this? I think you
> said you were seeing this on Harmony and I'm pretty sure this should fix
> the issue for you as well.
>
> Bjorn, this fixes a regression on Tegra20 (it's hidden on later Tegra
> generations because the PCIe controller has moved to the first 1 GiB of
> physical address space) that was introduced in 3.18-rc1, so it would be
> good to get this in before the final 3.18 release.
>
> Changes in v2:
> - remove cleanup not essential to fix the regression (will be part of a
> follow-up patch)
>
> drivers/pci/host/pci-tegra.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index 3d43874319be..19bb19c7db4a 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,6 @@ 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->mem);
> if (err < 0)
> @@ -668,14 +668,12 @@ 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->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 +784,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 +796,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 +1687,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:
> --
> 2.1.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread