diff mbox

[03/15] ARM: tegra: use fixed PCI i/o mapping

Message ID 1341600040-30993-4-git-send-email-robherring2@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rob Herring July 6, 2012, 6:40 p.m. UTC
From: Rob Herring <rob.herring@calxeda.com>

Move tegra PCI to fixed i/o mapping and remove io.h.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Colin Cross <ccross@android.com>
Cc: Olof Johansson <olof@lixom.net>
Acked-by: Stephen Warren <swarren@nvidia.com>
---
 arch/arm/Kconfig                         |    1 -
 arch/arm/mach-tegra/include/mach/io.h    |   46 ------------------------------
 arch/arm/mach-tegra/include/mach/iomap.h |    3 ++
 arch/arm/mach-tegra/io.c                 |    2 ++
 arch/arm/mach-tegra/pcie.c               |   43 ++++------------------------
 5 files changed, 10 insertions(+), 85 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/include/mach/io.h

Comments

Stephen Warren July 6, 2012, 7:44 p.m. UTC | #1
On 07/06/2012 12:40 PM, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Move tegra PCI to fixed i/o mapping and remove io.h.

Thierry, since you're the Tegra PCIe expert right now, could you please
test and/or comment on this.

I did try testing this on next-20120705 on TrimSlice (i.e. the
PCIe-based Ethernet controller), but found that PCIe has stopped working
there due to "resource collisions". I know this used to work fairly
recently, since I tested it when I added the PCIe initialization call to
board-dt-tegra20.c. The PCIe messages are:

> PCIE: port 1: link down, retrying
> PCIE: port 1: link down, retrying
> PCIE: port 1: link down, ignoring
> PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [io  0x1000-0x8fff]
> pci_bus 0000:00: root bus resource [mem 0x90000000-0x97ffffff]
> pci_bus 0000:00: root bus resource [mem 0xa0000000-0xa7ffffff pref]
> pci_bus 0000:00: No busn resource found for root bus, will use [bus 00-ff]
> pci_bus 0000:00: busn_res: [bus 00-ff] is inserted under domain [bus 00-ff]
> pci 0000:00:00.0: [10de:0bf0] type 01 class 0x060000
> pci 0000:00:00.0: PME# supported from D0 D1 D2 D3hot D3cold
> PCI: bus0: Fast back to back transfers disabled
> pci_bus 0000:01: busn_res: [bus 01-ff] is inserted under [bus 00-ff]
> pci 0000:01:00.0: [10ec:8168] type 00 class 0x020000
> pci 0000:01:00.0: reg 10: [io  0x0000-0x00ff]
> pci 0000:01:00.0: reg 18: [mem 0x00000000-0x00000fff 64bit pref]
> pci 0000:01:00.0: reg 20: [mem 0x00000000-0x00003fff 64bit pref]
> pci 0000:01:00.0: reg 30: [mem 0x00000000-0x0001ffff pref]
> pci 0000:01:00.0: supports D1 D2
> pci 0000:01:00.0: PME# supported from D0 D1 D2 D3hot D3cold
> PCI: bus1: Fast back to back transfers disabled
> pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
> pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to 01
> r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
> PCI: Device 0000:01:00.0 not available because of resource collisions
> r8169 0000:01:00.0: (unregistered net_device): enable failure
> r8169: probe of 0000:01:00.0 failed with error -22
> pci 0000:00:00.0: BAR 9: assigned [mem 0xa0000000-0xa00fffff pref]
> pci 0000:00:00.0: BAR 7: assigned [io  0x1000-0x1fff]
> pci 0000:01:00.0: BAR 6: assigned [mem 0xa0000000-0xa001ffff pref]
> pci 0000:01:00.0: BAR 4: assigned [mem 0xa0020000-0xa0023fff 64bit pref]
> pci 0000:01:00.0: BAR 2: assigned [mem 0xa0024000-0xa0024fff 64bit pref]
> pci 0000:01:00.0: BAR 0: assigned [io  0x1000-0x10ff]
> pci 0000:00:00.0: PCI bridge to [bus 01]
> pci 0000:00:00.0:   bridge window [io  0x1000-0x1fff]
> pci 0000:00:00.0:   bridge window [mem 0xa0000000-0xa00fffff pref]
> PCI: enabling device 0000:00:00.0 (0140 -> 0143)
Rob Herring July 6, 2012, 8:11 p.m. UTC | #2
On 07/06/2012 02:44 PM, Stephen Warren wrote:
> On 07/06/2012 12:40 PM, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> Move tegra PCI to fixed i/o mapping and remove io.h.
> 
> Thierry, since you're the Tegra PCIe expert right now, could you please
> test and/or comment on this.
> 
> I did try testing this on next-20120705 on TrimSlice (i.e. the
> PCIe-based Ethernet controller), but found that PCIe has stopped working
> there due to "resource collisions". I know this used to work fairly
> recently, since I tested it when I added the PCIe initialization call to
> board-dt-tegra20.c. The PCIe messages are:

This is with my change and it works currently without?

The i/o mapping of the 2nd ctrlr looked a bit screwy to me. It's not
clear to me how an i/o address of 0x11000+ gets steered to the 2nd
controller.

Rob
> 
>> PCIE: port 1: link down, retrying
>> PCIE: port 1: link down, retrying
>> PCIE: port 1: link down, ignoring
>> PCI host bridge to bus 0000:00
>> pci_bus 0000:00: root bus resource [io  0x1000-0x8fff]
>> pci_bus 0000:00: root bus resource [mem 0x90000000-0x97ffffff]
>> pci_bus 0000:00: root bus resource [mem 0xa0000000-0xa7ffffff pref]
>> pci_bus 0000:00: No busn resource found for root bus, will use [bus 00-ff]
>> pci_bus 0000:00: busn_res: [bus 00-ff] is inserted under domain [bus 00-ff]
>> pci 0000:00:00.0: [10de:0bf0] type 01 class 0x060000
>> pci 0000:00:00.0: PME# supported from D0 D1 D2 D3hot D3cold
>> PCI: bus0: Fast back to back transfers disabled
>> pci_bus 0000:01: busn_res: [bus 01-ff] is inserted under [bus 00-ff]
>> pci 0000:01:00.0: [10ec:8168] type 00 class 0x020000
>> pci 0000:01:00.0: reg 10: [io  0x0000-0x00ff]
>> pci 0000:01:00.0: reg 18: [mem 0x00000000-0x00000fff 64bit pref]
>> pci 0000:01:00.0: reg 20: [mem 0x00000000-0x00003fff 64bit pref]
>> pci 0000:01:00.0: reg 30: [mem 0x00000000-0x0001ffff pref]
>> pci 0000:01:00.0: supports D1 D2
>> pci 0000:01:00.0: PME# supported from D0 D1 D2 D3hot D3cold
>> PCI: bus1: Fast back to back transfers disabled
>> pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
>> pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to 01
>> r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>> PCI: Device 0000:01:00.0 not available because of resource collisions
>> r8169 0000:01:00.0: (unregistered net_device): enable failure
>> r8169: probe of 0000:01:00.0 failed with error -22
>> pci 0000:00:00.0: BAR 9: assigned [mem 0xa0000000-0xa00fffff pref]
>> pci 0000:00:00.0: BAR 7: assigned [io  0x1000-0x1fff]
>> pci 0000:01:00.0: BAR 6: assigned [mem 0xa0000000-0xa001ffff pref]
>> pci 0000:01:00.0: BAR 4: assigned [mem 0xa0020000-0xa0023fff 64bit pref]
>> pci 0000:01:00.0: BAR 2: assigned [mem 0xa0024000-0xa0024fff 64bit pref]
>> pci 0000:01:00.0: BAR 0: assigned [io  0x1000-0x10ff]
>> pci 0000:00:00.0: PCI bridge to [bus 01]
>> pci 0000:00:00.0:   bridge window [io  0x1000-0x1fff]
>> pci 0000:00:00.0:   bridge window [mem 0xa0000000-0xa00fffff pref]
>> PCI: enabling device 0000:00:00.0 (0140 -> 0143)
Stephen Warren July 6, 2012, 8:16 p.m. UTC | #3
On 07/06/2012 02:11 PM, Rob Herring wrote:
> On 07/06/2012 02:44 PM, Stephen Warren wrote:
>> On 07/06/2012 12:40 PM, Rob Herring wrote:
>>> From: Rob Herring <rob.herring@calxeda.com>
>>>
>>> Move tegra PCI to fixed i/o mapping and remove io.h.
>>
>> Thierry, since you're the Tegra PCIe expert right now, could you please
>> test and/or comment on this.
>>
>> I did try testing this on next-20120705 on TrimSlice (i.e. the
>> PCIe-based Ethernet controller), but found that PCIe has stopped working
>> there due to "resource collisions". I know this used to work fairly
>> recently, since I tested it when I added the PCIe initialization call to
>> board-dt-tegra20.c. The PCIe messages are:
> 
> This is with my change and it works currently without?

Sorry, no, it's broken even without your change. Hence, I can't test the
impact of your change. Well, I saw the same failure with your patches
too, but that isn't really conclusive testing of your change:-)
Stephen Warren July 6, 2012, 8:36 p.m. UTC | #4
On 07/06/2012 02:16 PM, Stephen Warren wrote:
> On 07/06/2012 02:11 PM, Rob Herring wrote:
>> On 07/06/2012 02:44 PM, Stephen Warren wrote:
>>> On 07/06/2012 12:40 PM, Rob Herring wrote:
>>>> From: Rob Herring <rob.herring@calxeda.com>
>>>>
>>>> Move tegra PCI to fixed i/o mapping and remove io.h.
>>>
>>> Thierry, since you're the Tegra PCIe expert right now, could you please
>>> test and/or comment on this.
>>>
>>> I did try testing this on next-20120705 on TrimSlice (i.e. the
>>> PCIe-based Ethernet controller), but found that PCIe has stopped working
>>> there due to "resource collisions". I know this used to work fairly
>>> recently, since I tested it when I added the PCIe initialization call to
>>> board-dt-tegra20.c. The PCIe messages are:
>>
>> This is with my change and it works currently without?
> 
> Sorry, no, it's broken even without your change. Hence, I can't test the
> impact of your change. Well, I saw the same failure with your patches
> too, but that isn't really conclusive testing of your change:-)

Aha. The PCIe problem only shows up when booting TrimSlice using DT (in
next-20120705 or Tegra's for-next branch).

When booting using board files, PCIe works, both without and with your
patch, on top of next-20120705.

So, your patches are tested on Tegra and still working.
Thierry Reding July 8, 2012, 6:09 a.m. UTC | #5
On Fri, Jul 06, 2012 at 01:40:28PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Move tegra PCI to fixed i/o mapping and remove io.h.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Colin Cross <ccross@android.com>
> Cc: Olof Johansson <olof@lixom.net>
> Acked-by: Stephen Warren <swarren@nvidia.com>
> ---
>  arch/arm/Kconfig                         |    1 -
>  arch/arm/mach-tegra/include/mach/io.h    |   46 ------------------------------
>  arch/arm/mach-tegra/include/mach/iomap.h |    3 ++
>  arch/arm/mach-tegra/io.c                 |    2 ++
>  arch/arm/mach-tegra/pcie.c               |   43 ++++------------------------
>  5 files changed, 10 insertions(+), 85 deletions(-)
>  delete mode 100644 arch/arm/mach-tegra/include/mach/io.h

Hi Rob,

generally this looks good. However I've been working on a rewrite of the
Tegra PCIe support to make it work as a driver and add DT support. This
entails that PCIe support isn't initialized until very late in the
process because it makes use of deferred probe if some regulators aren't
available. One problem caused by this is that it suddenly requires a lot
of code marked as __init to be available after the init phase and I see
that you've introduced pci_map_io_single_pfn() that is __init annotated,
so it will cause problems when used with my patches.

I'm not very familiar with the inner workings of the iotable and the
mappings initialized by it, but I wonder if this can be done dynamically
at a later stage. The way this is currently done in this patch, the I/O
region is statically mapped from a fixed offset within the PCIe address
range. Part of the patches to add DT support is to allow this region to
be defined by the DT, so that will obviously also create problems.

If both of those issues can be easily addressed, then this certainly
looks very nice.

Thierry

> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 8fb7e4a..ac446e3 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -644,7 +644,6 @@ config ARCH_TEGRA
>  	select HAVE_CLK
>  	select HAVE_SMP
>  	select MIGHT_HAVE_CACHE_L2X0
> -	select NEED_MACH_IO_H if PCI
>  	select ARCH_HAS_CPUFREQ
>  	help
>  	  This enables support for NVIDIA Tegra based systems (Tegra APX,
> diff --git a/arch/arm/mach-tegra/include/mach/io.h b/arch/arm/mach-tegra/include/mach/io.h
> deleted file mode 100644
> index fe700f9..0000000
> --- a/arch/arm/mach-tegra/include/mach/io.h
> +++ /dev/null
> @@ -1,46 +0,0 @@
> -/*
> - * arch/arm/mach-tegra/include/mach/io.h
> - *
> - * Copyright (C) 2010 Google, Inc.
> - *
> - * Author:
> - *	Colin Cross <ccross@google.com>
> - *	Erik Gilling <konkers@google.com>
> - *
> - * This software is licensed under the terms of the GNU General Public
> - * License version 2, as published by the Free Software Foundation, and
> - * may be copied, distributed, and modified under those terms.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - *
> - */
> -
> -#ifndef __MACH_TEGRA_IO_H
> -#define __MACH_TEGRA_IO_H
> -
> -#define IO_SPACE_LIMIT 0xffff
> -
> -#ifndef __ASSEMBLER__
> -
> -#ifdef CONFIG_TEGRA_PCI
> -extern void __iomem *tegra_pcie_io_base;
> -
> -static inline void __iomem *__io(unsigned long addr)
> -{
> -	return tegra_pcie_io_base + (addr & IO_SPACE_LIMIT);
> -}
> -#else
> -static inline void __iomem *__io(unsigned long addr)
> -{
> -	return (void __iomem *)addr;
> -}
> -#endif
> -
> -#define __io(a)         __io(a)
> -
> -#endif
> -
> -#endif
> diff --git a/arch/arm/mach-tegra/include/mach/iomap.h b/arch/arm/mach-tegra/include/mach/iomap.h
> index 7e76da7..fee3a94 100644
> --- a/arch/arm/mach-tegra/include/mach/iomap.h
> +++ b/arch/arm/mach-tegra/include/mach/iomap.h
> @@ -303,6 +303,9 @@
>  #define IO_APB_VIRT	IOMEM(0xFE300000)
>  #define IO_APB_SIZE	SZ_1M
>  
> +#define TEGRA_PCIE_BASE		0x80000000
> +#define TEGRA_PCIE_IO_BASE	(TEGRA_PCIE_BASE + SZ_4M)
> +
>  #define IO_TO_VIRT_BETWEEN(p, st, sz)	((p) >= (st) && (p) < ((st) + (sz)))
>  #define IO_TO_VIRT_XLATE(p, pst, vst)	(((p) - (pst) + (vst)))
>  
> diff --git a/arch/arm/mach-tegra/io.c b/arch/arm/mach-tegra/io.c
> index 58b4baf..7a29e10 100644
> --- a/arch/arm/mach-tegra/io.c
> +++ b/arch/arm/mach-tegra/io.c
> @@ -26,6 +26,7 @@
>  
>  #include <asm/page.h>
>  #include <asm/mach/map.h>
> +#include <asm/mach/pci.h>
>  #include <mach/iomap.h>
>  
>  #include "board.h"
> @@ -59,5 +60,6 @@ static struct map_desc tegra_io_desc[] __initdata = {
>  
>  void __init tegra_map_common_io(void)
>  {
> +	pci_map_io_single(TEGRA_PCIE_IO_BASE);
>  	iotable_init(tegra_io_desc, ARRAY_SIZE(tegra_io_desc));
>  }
> diff --git a/arch/arm/mach-tegra/pcie.c b/arch/arm/mach-tegra/pcie.c
> index 0e09137..c989a8e 100644
> --- a/arch/arm/mach-tegra/pcie.c
> +++ b/arch/arm/mach-tegra/pcie.c
> @@ -171,8 +171,6 @@ static void __iomem *reg_pmc_base = IO_ADDRESS(TEGRA_PMC_BASE);
>   * 0x90000000 - 0x9fffffff - non-prefetchable memory
>   * 0xa0000000 - 0xbfffffff - prefetchable memory
>   */
> -#define TEGRA_PCIE_BASE		0x80000000
> -
>  #define PCIE_REGS_SZ		SZ_16K
>  #define PCIE_CFG_OFF		PCIE_REGS_SZ
>  #define PCIE_CFG_SZ		SZ_1M
> @@ -180,8 +178,6 @@ static void __iomem *reg_pmc_base = IO_ADDRESS(TEGRA_PMC_BASE);
>  #define PCIE_EXT_CFG_SZ		SZ_1M
>  #define PCIE_IOMAP_SZ		(PCIE_REGS_SZ + PCIE_CFG_SZ + PCIE_EXT_CFG_SZ)
>  
> -#define MMIO_BASE		(TEGRA_PCIE_BASE + SZ_4M)
> -#define MMIO_SIZE		SZ_64K
>  #define MEM_BASE_0		(TEGRA_PCIE_BASE + SZ_256M)
>  #define MEM_SIZE_0		SZ_128M
>  #define MEM_BASE_1		(MEM_BASE_0 + MEM_SIZE_0)
> @@ -223,17 +219,7 @@ struct tegra_pcie_info {
>  	struct clk		*pll_e;
>  };
>  
> -static struct tegra_pcie_info tegra_pcie = {
> -	.res_mmio = {
> -		.name = "PCI IO",
> -		.start = MMIO_BASE,
> -		.end = MMIO_BASE + MMIO_SIZE - 1,
> -		.flags = IORESOURCE_MEM,
> -	},
> -};
> -
> -void __iomem *tegra_pcie_io_base;
> -EXPORT_SYMBOL(tegra_pcie_io_base);
> +static struct tegra_pcie_info tegra_pcie;
>  
>  static inline void afi_writel(u32 value, unsigned long offset)
>  {
> @@ -403,7 +389,7 @@ static int tegra_pcie_setup(int nr, struct pci_sys_data *sys)
>  		pp->res[0].end = pp->res[0].start + SZ_32K - 1;
>  	} else {
>  		pp->res[0].start = PCIBIOS_MIN_IO + SZ_32K;
> -		pp->res[0].end = IO_SPACE_LIMIT;
> +		pp->res[0].end = SZ_64K - 1;
>  	}
>  	pp->res[0].flags = IORESOURCE_IO;
>  	if (request_resource(&ioport_resource, &pp->res[0]))
> @@ -541,8 +527,8 @@ static void tegra_pcie_setup_translations(void)
>  
>  	/* Bar 2: downstream IO bar */
>  	fpci_bar = ((__u32)0xfdfc << 16);
> -	size = MMIO_SIZE;
> -	axi_address = MMIO_BASE;
> +	size = SZ_64K;
> +	axi_address = TEGRA_PCIE_IO_BASE;
>  	afi_writel(axi_address, AFI_AXI_BAR2_START);
>  	afi_writel(size >> 12, AFI_AXI_BAR2_SZ);
>  	afi_writel(fpci_bar, AFI_FPCI_BAR2);
> @@ -776,7 +762,6 @@ static void tegra_pcie_clocks_put(void)
>  
>  static int __init tegra_pcie_get_resources(void)
>  {
> -	struct resource *res_mmio = &tegra_pcie.res_mmio;
>  	int err;
>  
>  	err = tegra_pcie_clocks_get();
> @@ -798,34 +783,16 @@ static int __init tegra_pcie_get_resources(void)
>  		goto err_map_reg;
>  	}
>  
> -	err = request_resource(&iomem_resource, res_mmio);
> -	if (err) {
> -		pr_err("PCIE: Failed to request resources: %d\n", err);
> -		goto err_req_io;
> -	}
> -
> -	tegra_pcie_io_base = ioremap_nocache(res_mmio->start,
> -					     resource_size(res_mmio));
> -	if (tegra_pcie_io_base == NULL) {
> -		pr_err("PCIE: Failed to map IO\n");
> -		err = -ENOMEM;
> -		goto err_map_io;
> -	}
> -
>  	err = request_irq(INT_PCIE_INTR, tegra_pcie_isr,
>  			  IRQF_SHARED, "PCIE", &tegra_pcie);
>  	if (err) {
>  		pr_err("PCIE: Failed to register IRQ: %d\n", err);
> -		goto err_irq;
> +		goto err_req_io;
>  	}
>  	set_irq_flags(INT_PCIE_INTR, IRQF_VALID);
>  
>  	return 0;
>  
> -err_irq:
> -	iounmap(tegra_pcie_io_base);
> -err_map_io:
> -	release_resource(&tegra_pcie.res_mmio);
>  err_req_io:
>  	iounmap(tegra_pcie.regs);
>  err_map_reg:
> -- 
> 1.7.9.5
> 
> 
>
Rob Herring July 8, 2012, 2:17 p.m. UTC | #6
On 07/08/2012 01:09 AM, Thierry Reding wrote:
> On Fri, Jul 06, 2012 at 01:40:28PM -0500, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> Move tegra PCI to fixed i/o mapping and remove io.h.
>>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>> Cc: Colin Cross <ccross@android.com>
>> Cc: Olof Johansson <olof@lixom.net>
>> Acked-by: Stephen Warren <swarren@nvidia.com>
>> ---
>>  arch/arm/Kconfig                         |    1 -
>>  arch/arm/mach-tegra/include/mach/io.h    |   46 ------------------------------
>>  arch/arm/mach-tegra/include/mach/iomap.h |    3 ++
>>  arch/arm/mach-tegra/io.c                 |    2 ++
>>  arch/arm/mach-tegra/pcie.c               |   43 ++++------------------------
>>  5 files changed, 10 insertions(+), 85 deletions(-)
>>  delete mode 100644 arch/arm/mach-tegra/include/mach/io.h
> 
> Hi Rob,
> 
> generally this looks good. However I've been working on a rewrite of the
> Tegra PCIe support to make it work as a driver and add DT support. This
> entails that PCIe support isn't initialized until very late in the
> process because it makes use of deferred probe if some regulators aren't
> available. One problem caused by this is that it suddenly requires a lot
> of code marked as __init to be available after the init phase and I see
> that you've introduced pci_map_io_single_pfn() that is __init annotated,
> so it will cause problems when used with my patches.

This function can only be called during .map_io anyway, so the __init is
irrelevant.

> I'm not very familiar with the inner workings of the iotable and the
> mappings initialized by it, but I wonder if this can be done dynamically
> at a later stage. The way this is currently done in this patch, the I/O
> region is statically mapped from a fixed offset within the PCIe address
> range. Part of the patches to add DT support is to allow this region to
> be defined by the DT, so that will obviously also create problems.

Is the i/o address something you could extract from DT earlier? This can
be done separately if it doesn't require information from the driver.

I'm sure exactly how to do a fixed virtual mapping other than the
io_table mappings. There was some discussion of use fixmap region
previously, but doing so will be a bit more complex. I'll look into this
some.

Rob

> If both of those issues can be easily addressed, then this certainly
> looks very nice.
> 
> Thierry
> 
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 8fb7e4a..ac446e3 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -644,7 +644,6 @@ config ARCH_TEGRA
>>  	select HAVE_CLK
>>  	select HAVE_SMP
>>  	select MIGHT_HAVE_CACHE_L2X0
>> -	select NEED_MACH_IO_H if PCI
>>  	select ARCH_HAS_CPUFREQ
>>  	help
>>  	  This enables support for NVIDIA Tegra based systems (Tegra APX,
>> diff --git a/arch/arm/mach-tegra/include/mach/io.h b/arch/arm/mach-tegra/include/mach/io.h
>> deleted file mode 100644
>> index fe700f9..0000000
>> --- a/arch/arm/mach-tegra/include/mach/io.h
>> +++ /dev/null
>> @@ -1,46 +0,0 @@
>> -/*
>> - * arch/arm/mach-tegra/include/mach/io.h
>> - *
>> - * Copyright (C) 2010 Google, Inc.
>> - *
>> - * Author:
>> - *	Colin Cross <ccross@google.com>
>> - *	Erik Gilling <konkers@google.com>
>> - *
>> - * This software is licensed under the terms of the GNU General Public
>> - * License version 2, as published by the Free Software Foundation, and
>> - * may be copied, distributed, and modified under those terms.
>> - *
>> - * This program is distributed in the hope that it will be useful,
>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> - * GNU General Public License for more details.
>> - *
>> - */
>> -
>> -#ifndef __MACH_TEGRA_IO_H
>> -#define __MACH_TEGRA_IO_H
>> -
>> -#define IO_SPACE_LIMIT 0xffff
>> -
>> -#ifndef __ASSEMBLER__
>> -
>> -#ifdef CONFIG_TEGRA_PCI
>> -extern void __iomem *tegra_pcie_io_base;
>> -
>> -static inline void __iomem *__io(unsigned long addr)
>> -{
>> -	return tegra_pcie_io_base + (addr & IO_SPACE_LIMIT);
>> -}
>> -#else
>> -static inline void __iomem *__io(unsigned long addr)
>> -{
>> -	return (void __iomem *)addr;
>> -}
>> -#endif
>> -
>> -#define __io(a)         __io(a)
>> -
>> -#endif
>> -
>> -#endif
>> diff --git a/arch/arm/mach-tegra/include/mach/iomap.h b/arch/arm/mach-tegra/include/mach/iomap.h
>> index 7e76da7..fee3a94 100644
>> --- a/arch/arm/mach-tegra/include/mach/iomap.h
>> +++ b/arch/arm/mach-tegra/include/mach/iomap.h
>> @@ -303,6 +303,9 @@
>>  #define IO_APB_VIRT	IOMEM(0xFE300000)
>>  #define IO_APB_SIZE	SZ_1M
>>  
>> +#define TEGRA_PCIE_BASE		0x80000000
>> +#define TEGRA_PCIE_IO_BASE	(TEGRA_PCIE_BASE + SZ_4M)
>> +
>>  #define IO_TO_VIRT_BETWEEN(p, st, sz)	((p) >= (st) && (p) < ((st) + (sz)))
>>  #define IO_TO_VIRT_XLATE(p, pst, vst)	(((p) - (pst) + (vst)))
>>  
>> diff --git a/arch/arm/mach-tegra/io.c b/arch/arm/mach-tegra/io.c
>> index 58b4baf..7a29e10 100644
>> --- a/arch/arm/mach-tegra/io.c
>> +++ b/arch/arm/mach-tegra/io.c
>> @@ -26,6 +26,7 @@
>>  
>>  #include <asm/page.h>
>>  #include <asm/mach/map.h>
>> +#include <asm/mach/pci.h>
>>  #include <mach/iomap.h>
>>  
>>  #include "board.h"
>> @@ -59,5 +60,6 @@ static struct map_desc tegra_io_desc[] __initdata = {
>>  
>>  void __init tegra_map_common_io(void)
>>  {
>> +	pci_map_io_single(TEGRA_PCIE_IO_BASE);
>>  	iotable_init(tegra_io_desc, ARRAY_SIZE(tegra_io_desc));
>>  }
>> diff --git a/arch/arm/mach-tegra/pcie.c b/arch/arm/mach-tegra/pcie.c
>> index 0e09137..c989a8e 100644
>> --- a/arch/arm/mach-tegra/pcie.c
>> +++ b/arch/arm/mach-tegra/pcie.c
>> @@ -171,8 +171,6 @@ static void __iomem *reg_pmc_base = IO_ADDRESS(TEGRA_PMC_BASE);
>>   * 0x90000000 - 0x9fffffff - non-prefetchable memory
>>   * 0xa0000000 - 0xbfffffff - prefetchable memory
>>   */
>> -#define TEGRA_PCIE_BASE		0x80000000
>> -
>>  #define PCIE_REGS_SZ		SZ_16K
>>  #define PCIE_CFG_OFF		PCIE_REGS_SZ
>>  #define PCIE_CFG_SZ		SZ_1M
>> @@ -180,8 +178,6 @@ static void __iomem *reg_pmc_base = IO_ADDRESS(TEGRA_PMC_BASE);
>>  #define PCIE_EXT_CFG_SZ		SZ_1M
>>  #define PCIE_IOMAP_SZ		(PCIE_REGS_SZ + PCIE_CFG_SZ + PCIE_EXT_CFG_SZ)
>>  
>> -#define MMIO_BASE		(TEGRA_PCIE_BASE + SZ_4M)
>> -#define MMIO_SIZE		SZ_64K
>>  #define MEM_BASE_0		(TEGRA_PCIE_BASE + SZ_256M)
>>  #define MEM_SIZE_0		SZ_128M
>>  #define MEM_BASE_1		(MEM_BASE_0 + MEM_SIZE_0)
>> @@ -223,17 +219,7 @@ struct tegra_pcie_info {
>>  	struct clk		*pll_e;
>>  };
>>  
>> -static struct tegra_pcie_info tegra_pcie = {
>> -	.res_mmio = {
>> -		.name = "PCI IO",
>> -		.start = MMIO_BASE,
>> -		.end = MMIO_BASE + MMIO_SIZE - 1,
>> -		.flags = IORESOURCE_MEM,
>> -	},
>> -};
>> -
>> -void __iomem *tegra_pcie_io_base;
>> -EXPORT_SYMBOL(tegra_pcie_io_base);
>> +static struct tegra_pcie_info tegra_pcie;
>>  
>>  static inline void afi_writel(u32 value, unsigned long offset)
>>  {
>> @@ -403,7 +389,7 @@ static int tegra_pcie_setup(int nr, struct pci_sys_data *sys)
>>  		pp->res[0].end = pp->res[0].start + SZ_32K - 1;
>>  	} else {
>>  		pp->res[0].start = PCIBIOS_MIN_IO + SZ_32K;
>> -		pp->res[0].end = IO_SPACE_LIMIT;
>> +		pp->res[0].end = SZ_64K - 1;
>>  	}
>>  	pp->res[0].flags = IORESOURCE_IO;
>>  	if (request_resource(&ioport_resource, &pp->res[0]))
>> @@ -541,8 +527,8 @@ static void tegra_pcie_setup_translations(void)
>>  
>>  	/* Bar 2: downstream IO bar */
>>  	fpci_bar = ((__u32)0xfdfc << 16);
>> -	size = MMIO_SIZE;
>> -	axi_address = MMIO_BASE;
>> +	size = SZ_64K;
>> +	axi_address = TEGRA_PCIE_IO_BASE;
>>  	afi_writel(axi_address, AFI_AXI_BAR2_START);
>>  	afi_writel(size >> 12, AFI_AXI_BAR2_SZ);
>>  	afi_writel(fpci_bar, AFI_FPCI_BAR2);
>> @@ -776,7 +762,6 @@ static void tegra_pcie_clocks_put(void)
>>  
>>  static int __init tegra_pcie_get_resources(void)
>>  {
>> -	struct resource *res_mmio = &tegra_pcie.res_mmio;
>>  	int err;
>>  
>>  	err = tegra_pcie_clocks_get();
>> @@ -798,34 +783,16 @@ static int __init tegra_pcie_get_resources(void)
>>  		goto err_map_reg;
>>  	}
>>  
>> -	err = request_resource(&iomem_resource, res_mmio);
>> -	if (err) {
>> -		pr_err("PCIE: Failed to request resources: %d\n", err);
>> -		goto err_req_io;
>> -	}
>> -
>> -	tegra_pcie_io_base = ioremap_nocache(res_mmio->start,
>> -					     resource_size(res_mmio));
>> -	if (tegra_pcie_io_base == NULL) {
>> -		pr_err("PCIE: Failed to map IO\n");
>> -		err = -ENOMEM;
>> -		goto err_map_io;
>> -	}
>> -
>>  	err = request_irq(INT_PCIE_INTR, tegra_pcie_isr,
>>  			  IRQF_SHARED, "PCIE", &tegra_pcie);
>>  	if (err) {
>>  		pr_err("PCIE: Failed to register IRQ: %d\n", err);
>> -		goto err_irq;
>> +		goto err_req_io;
>>  	}
>>  	set_irq_flags(INT_PCIE_INTR, IRQF_VALID);
>>  
>>  	return 0;
>>  
>> -err_irq:
>> -	iounmap(tegra_pcie_io_base);
>> -err_map_io:
>> -	release_resource(&tegra_pcie.res_mmio);
>>  err_req_io:
>>  	iounmap(tegra_pcie.regs);
>>  err_map_reg:
>> -- 
>> 1.7.9.5
>>
>>
>>
Arnd Bergmann July 8, 2012, 4:35 p.m. UTC | #7
On Sunday 08 July 2012, Rob Herring wrote:
> > I'm not very familiar with the inner workings of the iotable and the
> > mappings initialized by it, but I wonder if this can be done dynamically
> > at a later stage. The way this is currently done in this patch, the I/O
> > region is statically mapped from a fixed offset within the PCIe address
> > range. Part of the patches to add DT support is to allow this region to
> > be defined by the DT, so that will obviously also create problems.
> 
> Is the i/o address something you could extract from DT earlier? This can
> be done separately if it doesn't require information from the driver.
> 
> I'm sure exactly how to do a fixed virtual mapping other than the
> io_table mappings. There was some discussion of use fixmap region
> previously, but doing so will be a bit more complex. I'll look into this
> some.

I think you can call ioremap_page_range() to do this.

	Arnd
Rob Herring July 8, 2012, 8:33 p.m. UTC | #8
On 07/08/2012 11:35 AM, Arnd Bergmann wrote:
> On Sunday 08 July 2012, Rob Herring wrote:
>>> I'm not very familiar with the inner workings of the iotable and the
>>> mappings initialized by it, but I wonder if this can be done dynamically
>>> at a later stage. The way this is currently done in this patch, the I/O
>>> region is statically mapped from a fixed offset within the PCIe address
>>> range. Part of the patches to add DT support is to allow this region to
>>> be defined by the DT, so that will obviously also create problems.
>>
>> Is the i/o address something you could extract from DT earlier? This can
>> be done separately if it doesn't require information from the driver.
>>
>> I'm sure exactly how to do a fixed virtual mapping other than the
>> io_table mappings. There was some discussion of use fixmap region
>> previously, but doing so will be a bit more complex. I'll look into this
>> some.
> 
> I think you can call ioremap_page_range() to do this.

Thanks for the pointer. But this has to be in 2 steps. First reserve the
virtual space and then map it. For the first part I think something like
this function will work:

void __init pci_reserve_io(void)
{
	struct vm_struct *vm;

	vm = early_alloc_aligned(sizeof(*vm), __alignof__(*vm));

	vm->addr = (void *)PCI_IO_VIRT_BASE;
	vm->size = SZ_1M;
	vm->phys_addr = 0;
	vm->flags = VM_IOREMAP | VM_ARM_STATIC_MAPPING;
	vm->flags |= VM_ARM_MTYPE(MT_DEVICE);
	vm->caller = pci_reserve_io;
	vm_area_add_early(vm++);
}

There's a big fat warning on vm_area_add_early from Nico to not use
unless you know what you're doing. I'll pretend I do...

Rob
Nicolas Pitre July 9, 2012, 2:53 a.m. UTC | #9
On Sun, 8 Jul 2012, Rob Herring wrote:

> On 07/08/2012 11:35 AM, Arnd Bergmann wrote:
> > On Sunday 08 July 2012, Rob Herring wrote:
> >>> I'm not very familiar with the inner workings of the iotable and the
> >>> mappings initialized by it, but I wonder if this can be done dynamically
> >>> at a later stage. The way this is currently done in this patch, the I/O
> >>> region is statically mapped from a fixed offset within the PCIe address
> >>> range. Part of the patches to add DT support is to allow this region to
> >>> be defined by the DT, so that will obviously also create problems.
> >>
> >> Is the i/o address something you could extract from DT earlier? This can
> >> be done separately if it doesn't require information from the driver.
> >>
> >> I'm sure exactly how to do a fixed virtual mapping other than the
> >> io_table mappings. There was some discussion of use fixmap region
> >> previously, but doing so will be a bit more complex. I'll look into this
> >> some.
> > 
> > I think you can call ioremap_page_range() to do this.
> 
> Thanks for the pointer. But this has to be in 2 steps. First reserve the
> virtual space and then map it. For the first part I think something like
> this function will work:
> 
> void __init pci_reserve_io(void)
> {
> 	struct vm_struct *vm;
> 
> 	vm = early_alloc_aligned(sizeof(*vm), __alignof__(*vm));
> 
> 	vm->addr = (void *)PCI_IO_VIRT_BASE;
> 	vm->size = SZ_1M;
> 	vm->phys_addr = 0;
> 	vm->flags = VM_IOREMAP | VM_ARM_STATIC_MAPPING;
> 	vm->flags |= VM_ARM_MTYPE(MT_DEVICE);
> 	vm->caller = pci_reserve_io;
> 	vm_area_add_early(vm++);
> }
> 
> There's a big fat warning on vm_area_add_early from Nico to not use
> unless you know what you're doing. I'll pretend I do...

:-)

This warning was inherited from vm_area_register_early() which used to 
make the core of what vm_area_add_early() is today.

Your usage of vm_area_add_early() should be fine as long as it is done 
before vmalloc_init(() is called.


Nicolas
diff mbox

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 8fb7e4a..ac446e3 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -644,7 +644,6 @@  config ARCH_TEGRA
 	select HAVE_CLK
 	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
-	select NEED_MACH_IO_H if PCI
 	select ARCH_HAS_CPUFREQ
 	help
 	  This enables support for NVIDIA Tegra based systems (Tegra APX,
diff --git a/arch/arm/mach-tegra/include/mach/io.h b/arch/arm/mach-tegra/include/mach/io.h
deleted file mode 100644
index fe700f9..0000000
--- a/arch/arm/mach-tegra/include/mach/io.h
+++ /dev/null
@@ -1,46 +0,0 @@ 
-/*
- * arch/arm/mach-tegra/include/mach/io.h
- *
- * Copyright (C) 2010 Google, Inc.
- *
- * Author:
- *	Colin Cross <ccross@google.com>
- *	Erik Gilling <konkers@google.com>
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-#ifndef __MACH_TEGRA_IO_H
-#define __MACH_TEGRA_IO_H
-
-#define IO_SPACE_LIMIT 0xffff
-
-#ifndef __ASSEMBLER__
-
-#ifdef CONFIG_TEGRA_PCI
-extern void __iomem *tegra_pcie_io_base;
-
-static inline void __iomem *__io(unsigned long addr)
-{
-	return tegra_pcie_io_base + (addr & IO_SPACE_LIMIT);
-}
-#else
-static inline void __iomem *__io(unsigned long addr)
-{
-	return (void __iomem *)addr;
-}
-#endif
-
-#define __io(a)         __io(a)
-
-#endif
-
-#endif
diff --git a/arch/arm/mach-tegra/include/mach/iomap.h b/arch/arm/mach-tegra/include/mach/iomap.h
index 7e76da7..fee3a94 100644
--- a/arch/arm/mach-tegra/include/mach/iomap.h
+++ b/arch/arm/mach-tegra/include/mach/iomap.h
@@ -303,6 +303,9 @@ 
 #define IO_APB_VIRT	IOMEM(0xFE300000)
 #define IO_APB_SIZE	SZ_1M
 
+#define TEGRA_PCIE_BASE		0x80000000
+#define TEGRA_PCIE_IO_BASE	(TEGRA_PCIE_BASE + SZ_4M)
+
 #define IO_TO_VIRT_BETWEEN(p, st, sz)	((p) >= (st) && (p) < ((st) + (sz)))
 #define IO_TO_VIRT_XLATE(p, pst, vst)	(((p) - (pst) + (vst)))
 
diff --git a/arch/arm/mach-tegra/io.c b/arch/arm/mach-tegra/io.c
index 58b4baf..7a29e10 100644
--- a/arch/arm/mach-tegra/io.c
+++ b/arch/arm/mach-tegra/io.c
@@ -26,6 +26,7 @@ 
 
 #include <asm/page.h>
 #include <asm/mach/map.h>
+#include <asm/mach/pci.h>
 #include <mach/iomap.h>
 
 #include "board.h"
@@ -59,5 +60,6 @@  static struct map_desc tegra_io_desc[] __initdata = {
 
 void __init tegra_map_common_io(void)
 {
+	pci_map_io_single(TEGRA_PCIE_IO_BASE);
 	iotable_init(tegra_io_desc, ARRAY_SIZE(tegra_io_desc));
 }
diff --git a/arch/arm/mach-tegra/pcie.c b/arch/arm/mach-tegra/pcie.c
index 0e09137..c989a8e 100644
--- a/arch/arm/mach-tegra/pcie.c
+++ b/arch/arm/mach-tegra/pcie.c
@@ -171,8 +171,6 @@  static void __iomem *reg_pmc_base = IO_ADDRESS(TEGRA_PMC_BASE);
  * 0x90000000 - 0x9fffffff - non-prefetchable memory
  * 0xa0000000 - 0xbfffffff - prefetchable memory
  */
-#define TEGRA_PCIE_BASE		0x80000000
-
 #define PCIE_REGS_SZ		SZ_16K
 #define PCIE_CFG_OFF		PCIE_REGS_SZ
 #define PCIE_CFG_SZ		SZ_1M
@@ -180,8 +178,6 @@  static void __iomem *reg_pmc_base = IO_ADDRESS(TEGRA_PMC_BASE);
 #define PCIE_EXT_CFG_SZ		SZ_1M
 #define PCIE_IOMAP_SZ		(PCIE_REGS_SZ + PCIE_CFG_SZ + PCIE_EXT_CFG_SZ)
 
-#define MMIO_BASE		(TEGRA_PCIE_BASE + SZ_4M)
-#define MMIO_SIZE		SZ_64K
 #define MEM_BASE_0		(TEGRA_PCIE_BASE + SZ_256M)
 #define MEM_SIZE_0		SZ_128M
 #define MEM_BASE_1		(MEM_BASE_0 + MEM_SIZE_0)
@@ -223,17 +219,7 @@  struct tegra_pcie_info {
 	struct clk		*pll_e;
 };
 
-static struct tegra_pcie_info tegra_pcie = {
-	.res_mmio = {
-		.name = "PCI IO",
-		.start = MMIO_BASE,
-		.end = MMIO_BASE + MMIO_SIZE - 1,
-		.flags = IORESOURCE_MEM,
-	},
-};
-
-void __iomem *tegra_pcie_io_base;
-EXPORT_SYMBOL(tegra_pcie_io_base);
+static struct tegra_pcie_info tegra_pcie;
 
 static inline void afi_writel(u32 value, unsigned long offset)
 {
@@ -403,7 +389,7 @@  static int tegra_pcie_setup(int nr, struct pci_sys_data *sys)
 		pp->res[0].end = pp->res[0].start + SZ_32K - 1;
 	} else {
 		pp->res[0].start = PCIBIOS_MIN_IO + SZ_32K;
-		pp->res[0].end = IO_SPACE_LIMIT;
+		pp->res[0].end = SZ_64K - 1;
 	}
 	pp->res[0].flags = IORESOURCE_IO;
 	if (request_resource(&ioport_resource, &pp->res[0]))
@@ -541,8 +527,8 @@  static void tegra_pcie_setup_translations(void)
 
 	/* Bar 2: downstream IO bar */
 	fpci_bar = ((__u32)0xfdfc << 16);
-	size = MMIO_SIZE;
-	axi_address = MMIO_BASE;
+	size = SZ_64K;
+	axi_address = TEGRA_PCIE_IO_BASE;
 	afi_writel(axi_address, AFI_AXI_BAR2_START);
 	afi_writel(size >> 12, AFI_AXI_BAR2_SZ);
 	afi_writel(fpci_bar, AFI_FPCI_BAR2);
@@ -776,7 +762,6 @@  static void tegra_pcie_clocks_put(void)
 
 static int __init tegra_pcie_get_resources(void)
 {
-	struct resource *res_mmio = &tegra_pcie.res_mmio;
 	int err;
 
 	err = tegra_pcie_clocks_get();
@@ -798,34 +783,16 @@  static int __init tegra_pcie_get_resources(void)
 		goto err_map_reg;
 	}
 
-	err = request_resource(&iomem_resource, res_mmio);
-	if (err) {
-		pr_err("PCIE: Failed to request resources: %d\n", err);
-		goto err_req_io;
-	}
-
-	tegra_pcie_io_base = ioremap_nocache(res_mmio->start,
-					     resource_size(res_mmio));
-	if (tegra_pcie_io_base == NULL) {
-		pr_err("PCIE: Failed to map IO\n");
-		err = -ENOMEM;
-		goto err_map_io;
-	}
-
 	err = request_irq(INT_PCIE_INTR, tegra_pcie_isr,
 			  IRQF_SHARED, "PCIE", &tegra_pcie);
 	if (err) {
 		pr_err("PCIE: Failed to register IRQ: %d\n", err);
-		goto err_irq;
+		goto err_req_io;
 	}
 	set_irq_flags(INT_PCIE_INTR, IRQF_VALID);
 
 	return 0;
 
-err_irq:
-	iounmap(tegra_pcie_io_base);
-err_map_io:
-	release_resource(&tegra_pcie.res_mmio);
 err_req_io:
 	iounmap(tegra_pcie.regs);
 err_map_reg: