[PATCH 0/3] Versatile PCI DT support

[PATCH 0/3] Versatile PCI DT support

[Rob Herring]

From: Rob Herring <robh@kernel.org>

This series adds a platform driver for Versatile PB's PCI host using
Liviu's recent patch series[1] for DT parsing and setup.

The first patch is a hack to get Liviu's current patches to work on ARM.
It at least shows we are not that far off from being able to use the
series on ARM.

I've tested PCI under QEMU, but need someone with actual h/w to test.
A branch with this series and which includes full conversion of
Versatile to DT is available here[2].

Rob

[1] https://lkml.org/lkml/2014/3/14/279
[2] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git versatile

Rob Herring (3):
  ARM: hackup pcibios support for commmon bridge code
  dt/bindings: add versatile PCI binding
  pci: add DT based ARM Versatile PCI host driver

 .../devicetree/bindings/pci/versatile.txt          |  59 +++++
 arch/arm/kernel/bios32.c                           |  63 +++++
 drivers/pci/host/Kconfig                           |   4 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-versatile.c                   | 275 +++++++++++++++++++++
 5 files changed, 402 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/versatile.txt
 create mode 100644 drivers/pci/host/pci-versatile.c

-- 
1.8.3.2

[PATCH 1/3] ARM: hackup pcibios support for commmon bridge code

[Rob Herring]

From: Rob Herring <robh@kernel.org>

Signed-off-by: Rob Herring <robh@kernel.org>
---
 arch/arm/kernel/bios32.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 317da88..31ec14a 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/of_pci.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/map.h>
@@ -337,6 +338,15 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		u16 cmd;
 
+		/* Ignore fully discovered devices */
+		if (dev->is_added)
+			continue;
+
+		set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
+
+		/* Read default IRQs and fixup if necessary */
+		dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
+
 		pci_read_config_word(dev, PCI_COMMAND, &cmd);
 		cmd |= features;
 		pci_write_config_word(dev, PCI_COMMAND, cmd);
@@ -681,3 +691,56 @@ void __init pci_map_io_early(unsigned long pfn)
 	pci_io_desc.pfn = pfn;
 	iotable_init(&pci_io_desc, 1);
 }
+
+struct ioresource {
+	struct list_head list;
+	phys_addr_t start;
+	resource_size_t size;
+};
+
+static LIST_HEAD(io_list);
+
+int pci_register_io_range(phys_addr_t address, resource_size_t size)
+{
+	struct ioresource *res;
+	resource_size_t allocated_size = 0;
+
+	/* find if the range has not been already allocated */
+	list_for_each_entry(res, &io_list, list) {
+		if (address >= res->start &&
+			address + size <= res->start + size)
+			return 0;
+		allocated_size += res->size;
+	}
+
+	/* range not already registered, check for space */
+	if (allocated_size + size > IO_SPACE_LIMIT)
+		return -E2BIG;
+
+	/* add the range in the list */
+	res = kzalloc(sizeof(*res), GFP_KERNEL);
+	if (!res)
+		return -ENOMEM;
+	res->start = address;
+	res->size = size;
+
+	list_add_tail(&res->list, &io_list);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_register_io_range);
+
+unsigned long pci_address_to_pio(phys_addr_t address)
+{
+	struct ioresource *res;
+
+	list_for_each_entry(res, &io_list, list) {
+		if (address >= res->start &&
+			address < res->start + res->size) {
+			return res->start - address;
+		}
+	}
+
+	return (unsigned long)-1;
+}
+EXPORT_SYMBOL_GPL(pci_address_to_pio);
-- 
1.8.3.2

[PATCH 2/3] dt/bindings: add versatile PCI binding

[Rob Herring]

From: Rob Herring <robh@kernel.org>

Add binding documentation for the PCI controller found on Versatile PB
boards.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/pci/versatile.txt          | 59 ++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/versatile.txt

diff --git a/Documentation/devicetree/bindings/pci/versatile.txt b/Documentation/devicetree/bindings/pci/versatile.txt
new file mode 100644
index 0000000..2cc6071
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/versatile.txt
@@ -0,0 +1,59 @@
+* ARM Versatile Platform Baseboard PCI interface
+
+PCI host controller found on the ARM Versatile PB board's FPGA.
+
+Required properties:
+- compatible: should contain "arm,versatile-pci" to identify the Versatile PCI
+  controller.
+- reg: base addresses and lengths of the pci controller. There must be 3
+  entries:
+	- Versatile specific registers
+	- Self Config space
+	- Config space
+- #address-cells: set to <3>
+- #size-cells: set to <2>
+- device_type: set to "pci"
+- bus-range: set to <0 0xff>
+- ranges: ranges for the PCI memory and I/O regions
+- #interrupt-cells: set to <1>
+- interrupt-map-mask and interrupt-map: standard PCI properties	to define
+	the mapping of the PCI interface to interrupt numbers.
+
+Example:
+
+pci-controller at 10001000 {
+	compatible = "arm,versatile-pci";
+	device_type = "pci";
+	reg = <0x10001000 0x1000
+	       0x41000000 0x10000
+	       0x42000000 0x100000>;
+	bus-range = <0 0xff>;
+	#address-cells = <3>;
+	#size-cells = <2>;
+	#interrupt-cells = <1>;
+
+	ranges = <0x01000000 0 0x00000000 0x43000000 0 0x00010000   /* downstream I/O */
+		  0x02000000 0 0x50000000 0x50000000 0 0x10000000   /* non-prefetchable memory */
+		  0x42000000 0 0x60000000 0x60000000 0 0x10000000>; /* prefetchable memory */
+
+	interrupt-map-mask = <0x1800 0 0 7>;
+	interrupt-map = <0x1800 0 0 1 &sic 28
+			 0x1800 0 0 2 &sic 29
+			 0x1800 0 0 3 &sic 30
+			 0x1800 0 0 4 &sic 27
+
+			 0x1000 0 0 1 &sic 27
+			 0x1000 0 0 2 &sic 28
+			 0x1000 0 0 3 &sic 29
+			 0x1000 0 0 4 &sic 30
+
+			 0x0800 0 0 1 &sic 30
+			 0x0800 0 0 2 &sic 27
+			 0x0800 0 0 3 &sic 28
+			 0x0800 0 0 4 &sic 29
+
+			 0x0000 0 0 1 &sic 29
+			 0x0000 0 0 2 &sic 30
+			 0x0000 0 0 3 &sic 27
+			 0x0000 0 0 4 &sic 28>;
+};
-- 
1.8.3.2

[PATCH 3/3] pci: add DT based ARM Versatile PCI host driver

[Rob Herring]

From: Rob Herring <robh@kernel.org>

This converts the Versatile PCI host code to a platform driver using
of_create_pci_host_bridge for parsing DT and setup.

I think more of this setup could be done by the core code. There are
accesses to the host's config space (accesses using local_pci_cfg_base)
which seem like they could be done by the core code or using standard
config space accessors. The problem is bridge->bus->self is needed, but
it does not get setup. I'm not exactly sure how that should work.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/Kconfig         |   4 +
 drivers/pci/host/Makefile        |   1 +
 drivers/pci/host/pci-versatile.c | 275 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 280 insertions(+)
 create mode 100644 drivers/pci/host/pci-versatile.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 47d46c6..b4dd911 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -33,4 +33,8 @@ config PCI_RCAR_GEN2
 	  There are 3 internal PCI controllers available with a single
 	  built-in EHCI/OHCI host controller present on each one.
 
+config PCI_VERSATILE
+	bool "ARM Versatile PB PCI controller"
+	depends on ARCH_VERSATILE
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 13fb333..fe67ab3 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
 obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
 obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
 obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
+obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
diff --git a/drivers/pci/host/pci-versatile.c b/drivers/pci/host/pci-versatile.c
new file mode 100644
index 0000000..98abd1f
--- /dev/null
+++ b/drivers/pci/host/pci-versatile.c
@@ -0,0 +1,275 @@
+/*
+ * Copyright 2004 Koninklijke Philips Electronics NV
+ *
+ * Conversion to platform driver and DT:
+ * Copyright 2014 Linaro Ltd.
+ *
+ * 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.
+ *
+ * 14/04/2005 Initial version, colin.king at philips.com
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+static void __iomem *versatile_pci_base;
+static void __iomem *versatile_cfg_base[2];
+
+#define PCI_IMAP(m)		(versatile_pci_base + ((m) * 4))
+#define PCI_SMAP(m)		(versatile_pci_base + 0x14 + ((m) * 4))
+#define PCI_SELFID		(versatile_pci_base + 0xc)
+
+#define DEVICE_ID_OFFSET		0x00
+#define CSR_OFFSET			0x04
+#define CLASS_ID_OFFSET			0x08
+
+#define VP_PCI_DEVICE_ID		0x030010ee
+#define VP_PCI_CLASS_ID			0x0b400000
+
+static unsigned long pci_slot_ignore;
+
+static int __init versatile_pci_slot_ignore(char *str)
+{
+	int retval;
+	int slot;
+
+	while ((retval = get_option(&str, &slot))) {
+		if ((slot < 0) || (slot > 31))
+			pr_err("Illegal slot value: %d\n", slot);
+		else
+			pci_slot_ignore |= (1 << slot);
+	}
+	return 1;
+}
+
+__setup("pci_slot_ignore=", versatile_pci_slot_ignore);
+
+
+static void __iomem *__pci_addr(struct pci_bus *bus,
+				unsigned int devfn, int offset)
+{
+	unsigned int busnr = bus->number;
+
+	/*
+	 * Trap out illegal values
+	 */
+	BUG_ON(offset > 255);
+	BUG_ON(busnr > 255);
+	BUG_ON(devfn > 255);
+
+	return versatile_cfg_base[1] + ((busnr << 16) |
+		(PCI_SLOT(devfn) << 11) | (PCI_FUNC(devfn) << 8) | offset);
+}
+
+static int versatile_read_config(struct pci_bus *bus, unsigned int devfn,
+				 int where, int size, u32 *val)
+{
+	void __iomem *addr = __pci_addr(bus, devfn, where & ~3);
+	u32 v;
+	int slot = PCI_SLOT(devfn);
+
+	if (pci_slot_ignore & (1 << slot)) {
+		/* Ignore this slot */
+		v = (1 << (8 * size)) - 1;
+	} else {
+		switch (size) {
+		case 1:
+			v = readl(addr);
+			if (where & 2)
+				v >>= 16;
+			if (where & 1)
+				v >>= 8;
+			v &= 0xff;
+			break;
+
+		case 2:
+			v = readl(addr);
+			if (where & 2)
+				v >>= 16;
+			v &= 0xffff;
+			break;
+
+		default:
+			v = readl(addr);
+			break;
+		}
+	}
+
+	*val = v;
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int versatile_write_config(struct pci_bus *bus, unsigned int devfn,
+				  int where, int size, u32 val)
+{
+	void __iomem *addr = __pci_addr(bus, devfn, where);
+	int slot = PCI_SLOT(devfn);
+
+	if (pci_slot_ignore & (1 << slot))
+		return PCIBIOS_SUCCESSFUL;
+
+	switch (size) {
+	case 1:
+		writeb((u8)val, addr);
+		break;
+
+	case 2:
+		writew((u16)val, addr);
+		break;
+
+	case 4:
+		writel(val, addr);
+		break;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops pci_versatile_ops = {
+	.read	= versatile_read_config,
+	.write	= versatile_write_config,
+};
+
+static const struct of_device_id versatile_pci_of_match[] = {
+	{ .compatible = "arm,versatile-pci", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, versatile_pci_of_match);
+
+/* Unused, temporary to satisfy ARM arch code */
+struct pci_sys_data sys;
+
+static int versatile_pci_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	int ret, i, mem = 1, myslot = -1;
+	unsigned int lastbus;
+	u32 val;
+	struct pci_host_bridge *bridge;
+	void __iomem *local_pci_cfg_base;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+	versatile_pci_base = devm_ioremap_resource(&pdev->dev, res);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res)
+		return -ENODEV;
+	versatile_cfg_base[0] = devm_ioremap_resource(&pdev->dev, res);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (!res)
+		return -ENODEV;
+	versatile_cfg_base[1] = devm_ioremap_resource(&pdev->dev, res);
+
+	bridge = of_create_pci_host_bridge(&pdev->dev, &pci_versatile_ops, &sys);
+	if (!bridge)
+		return -ENODEV;
+
+	/*
+	 *  We need to discover the PCI core first to configure itself
+	 *  before the main PCI probing is performed
+	 */
+	for (i = 0; i < 32; i++) {
+		if ((readl(versatile_cfg_base[0] + (i << 11) + DEVICE_ID_OFFSET) == VP_PCI_DEVICE_ID) &&
+		    (readl(versatile_cfg_base[0] + (i << 11) + CLASS_ID_OFFSET) == VP_PCI_CLASS_ID)) {
+			myslot = i;
+			break;
+		}
+	}
+	if (myslot == -1) {
+		dev_err(&pdev->dev, "Cannot find PCI core!\n");
+		ret = -EIO;
+		goto out;
+	}
+	/*
+	 * Do not to map Versatile FPGA PCI device into memory space
+	 */
+	pci_slot_ignore |= (1 << myslot);
+
+	dev_info(&pdev->dev, "PCI core found (slot %d)\n", myslot);
+
+	writel(myslot, PCI_SELFID);
+	local_pci_cfg_base = versatile_cfg_base[1] + (myslot << 11);
+
+	val = readl(local_pci_cfg_base + CSR_OFFSET);
+	val |= PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_INVALIDATE;
+	writel(val, local_pci_cfg_base + CSR_OFFSET);
+
+	/*
+	 * Configure the PCI inbound memory windows to be 1:1 mapped to SDRAM
+	 */
+	writel(PHYS_OFFSET, local_pci_cfg_base + PCI_BASE_ADDRESS_0);
+	writel(PHYS_OFFSET, local_pci_cfg_base + PCI_BASE_ADDRESS_1);
+	writel(PHYS_OFFSET, local_pci_cfg_base + PCI_BASE_ADDRESS_2);
+
+	/*
+	 * For many years the kernel and QEMU were symbiotically buggy
+	 * in that they both assumed the same broken IRQ mapping.
+	 * QEMU therefore attempts to auto-detect old broken kernels
+	 * so that they still work on newer QEMU as they did on old
+	 * QEMU. Since we now use the correct (ie matching-hardware)
+	 * IRQ mapping we write a definitely different value to a
+	 * PCI_INTERRUPT_LINE register to tell QEMU that we expect
+	 * real hardware behaviour and it need not be backwards
+	 * compatible for us. This write is harmless on real hardware.
+	 */
+	writel(0, versatile_cfg_base[0] + PCI_INTERRUPT_LINE);
+
+	pci_bus_for_each_resource(bridge->bus, res, i) {
+		if (!res || (resource_type(res) != IORESOURCE_MEM))
+			continue;
+
+		writel(res->start >> 28, PCI_IMAP(mem));
+		writel(PHYS_OFFSET >> 28, PCI_SMAP(mem));
+
+		mem++;
+	}
+
+	pci_ioremap_io(0, bridge->io_base);
+
+	/* We always enable PCI domains and we keep domain 0 backward
+	 * compatible in /proc for video cards
+	 */
+	pci_add_flags(PCI_ENABLE_PROC_DOMAINS);
+	pci_add_flags(PCI_REASSIGN_ALL_BUS | PCI_REASSIGN_ALL_RSRC);
+
+	lastbus = pci_scan_child_bus(bridge->bus);
+	pci_bus_update_busn_res_end(bridge->bus, lastbus);
+
+	pci_assign_unassigned_bus_resources(bridge->bus);
+
+	pci_bus_add_devices(bridge->bus);
+
+	return 0;
+
+ out:
+	return ret;
+}
+
+static struct platform_driver versatile_pci_driver = {
+	.driver = {
+		.name = "versatile-pci",
+		.owner = THIS_MODULE,
+		.of_match_table = versatile_pci_of_match,
+		.suppress_bind_attrs = true,
+	},
+	.probe = versatile_pci_probe,
+};
+module_platform_driver(versatile_pci_driver);
+
+MODULE_DESCRIPTION("Versatile PCI driver");
+MODULE_LICENSE("GPLv2");
-- 
1.8.3.2

[PATCH 0/3] Versatile PCI DT support

[Liviu Dudau]

On Thu, Mar 27, 2014 at 10:46:35PM +0000, Rob Herring wrote:
> From: Rob Herring <robh@kernel.org>
> 
> This series adds a platform driver for Versatile PB's PCI host using
> Liviu's recent patch series[1] for DT parsing and setup.
> 
> The first patch is a hack to get Liviu's current patches to work on ARM.
> It at least shows we are not that far off from being able to use the
> series on ARM.
> 
> I've tested PCI under QEMU, but need someone with actual h/w to test.
> A branch with this series and which includes full conversion of
> Versatile to DT is available here[2].

Hi Rob,

Thanks for doing this. Hopefully others will take inspiration and start to
convert their host bridge controllers as well.

I will post an updated series soon. (Un)fortunately it will look slightly
different than the last one I've posted as I am trying to get more code
that I've currently placed in arch/arm64/kernel/pci.c into drivers/pci,
so you will need to revisit your series.

Regarding your comment about bridge->bus->self being needed: root busses
are supposed to have bus->self == NULL, and the bridge->bus *is* the root
bus. Where would you need to use bridge->bus->self?

Best regards,
Liviu

> 
> Rob
> 
> [1] https://lkml.org/lkml/2014/3/14/279
> [2] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git versatile
> 
> Rob Herring (3):
>   ARM: hackup pcibios support for commmon bridge code
>   dt/bindings: add versatile PCI binding
>   pci: add DT based ARM Versatile PCI host driver
> 
>  .../devicetree/bindings/pci/versatile.txt          |  59 +++++
>  arch/arm/kernel/bios32.c                           |  63 +++++
>  drivers/pci/host/Kconfig                           |   4 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pci-versatile.c                   | 275 +++++++++++++++++++++
>  5 files changed, 402 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/versatile.txt
>  create mode 100644 drivers/pci/host/pci-versatile.c
> 
> -- 
> 1.8.3.2
> 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

[PATCH 0/3] Versatile PCI DT support

[Bjorn Helgaas]

On Thu, Mar 27, 2014 at 05:46:35PM -0500, Rob Herring wrote:
> From: Rob Herring <robh@kernel.org>
> 
> This series adds a platform driver for Versatile PB's PCI host using
> Liviu's recent patch series[1] for DT parsing and setup.
> 
> The first patch is a hack to get Liviu's current patches to work on ARM.
> It at least shows we are not that far off from being able to use the
> series on ARM.

Since this depends on Liviu's work that is not yet upstream, I guess I
can't do anything with this yet, right?  Poke me again when I can.

I assume you would want a MAINTAINERS update, too (at least, *I* do :))

> I've tested PCI under QEMU, but need someone with actual h/w to test.
> A branch with this series and which includes full conversion of
> Versatile to DT is available here[2].
> 
> Rob
> 
> [1] https://lkml.org/lkml/2014/3/14/279
> [2] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git versatile
> 
> Rob Herring (3):
>   ARM: hackup pcibios support for commmon bridge code
>   dt/bindings: add versatile PCI binding
>   pci: add DT based ARM Versatile PCI host driver
> 
>  .../devicetree/bindings/pci/versatile.txt          |  59 +++++
>  arch/arm/kernel/bios32.c                           |  63 +++++
>  drivers/pci/host/Kconfig                           |   4 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pci-versatile.c                   | 275 +++++++++++++++++++++
>  5 files changed, 402 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/versatile.txt
>  create mode 100644 drivers/pci/host/pci-versatile.c
> 
> -- 
> 1.8.3.2
> 

[PATCH 1/3] ARM: hackup pcibios support for commmon bridge code

[Bjorn Helgaas]

On Thu, Mar 27, 2014 at 05:46:36PM -0500, Rob Herring wrote:
> From: Rob Herring <robh@kernel.org>
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  arch/arm/kernel/bios32.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index 317da88..31ec14a 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -11,6 +11,7 @@
>  #include <linux/slab.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
> +#include <linux/of_pci.h>
>  
>  #include <asm/mach-types.h>
>  #include <asm/mach/map.h>
> @@ -337,6 +338,15 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
>  		u16 cmd;
>  
> +		/* Ignore fully discovered devices */
> +		if (dev->is_added)
> +			continue;
> +
> +		set_dev_node(&dev->dev, pcibus_to_node(dev->bus));

We already do this in pci_device_add(); why do we need it here, too?

> +		/* Read default IRQs and fixup if necessary */
> +		dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);

Could this be done in pcibios_enable_device() or some other per-device
interface()?  I'd like to get rid of pcibios_fixup_bus() eventually.

>  		pci_read_config_word(dev, PCI_COMMAND, &cmd);
>  		cmd |= features;
>  		pci_write_config_word(dev, PCI_COMMAND, cmd);
> @@ -681,3 +691,56 @@ void __init pci_map_io_early(unsigned long pfn)
>  	pci_io_desc.pfn = pfn;
>  	iotable_init(&pci_io_desc, 1);
>  }
> +
> +struct ioresource {
> +	struct list_head list;
> +	phys_addr_t start;
> +	resource_size_t size;
> +};
> +
> +static LIST_HEAD(io_list);
> +
> +int pci_register_io_range(phys_addr_t address, resource_size_t size)
> +{
> +	struct ioresource *res;
> +	resource_size_t allocated_size = 0;
> +
> +	/* find if the range has not been already allocated */
> +	list_for_each_entry(res, &io_list, list) {
> +		if (address >= res->start &&
> +			address + size <= res->start + size)
> +			return 0;
> +		allocated_size += res->size;
> +	}
> +
> +	/* range not already registered, check for space */
> +	if (allocated_size + size > IO_SPACE_LIMIT)
> +		return -E2BIG;
> +
> +	/* add the range in the list */
> +	res = kzalloc(sizeof(*res), GFP_KERNEL);
> +	if (!res)
> +		return -ENOMEM;
> +	res->start = address;
> +	res->size = size;
> +
> +	list_add_tail(&res->list, &io_list);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_register_io_range);
> +
> +unsigned long pci_address_to_pio(phys_addr_t address)
> +{
> +	struct ioresource *res;
> +
> +	list_for_each_entry(res, &io_list, list) {
> +		if (address >= res->start &&
> +			address < res->start + res->size) {
> +			return res->start - address;
> +		}
> +	}
> +
> +	return (unsigned long)-1;
> +}
> +EXPORT_SYMBOL_GPL(pci_address_to_pio);

ISTR some discussion about this, so maybe this has already been addressed,
but it doesn't seem right to export generically-named symbols like these
from the ARM arch code.  And they don't look very ARM-specific.

Bjorn

[PATCH 3/3] pci: add DT based ARM Versatile PCI host driver

[Bjorn Helgaas]

On Thu, Mar 27, 2014 at 05:46:38PM -0500, Rob Herring wrote:
> From: Rob Herring <robh@kernel.org>
> 
> This converts the Versatile PCI host code to a platform driver using
> of_create_pci_host_bridge for parsing DT and setup.
> 
> I think more of this setup could be done by the core code. There are
> accesses to the host's config space (accesses using local_pci_cfg_base)
> which seem like they could be done by the core code or using standard
> config space accessors. The problem is bridge->bus->self is needed, but
> it does not get setup. I'm not exactly sure how that should work.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ...

> +static int versatile_pci_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	int ret, i, mem = 1, myslot = -1;
> +	unsigned int lastbus;
> +	u32 val;
> +	struct pci_host_bridge *bridge;
> +	void __iomem *local_pci_cfg_base;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +	versatile_pci_base = devm_ioremap_resource(&pdev->dev, res);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (!res)
> +		return -ENODEV;
> +	versatile_cfg_base[0] = devm_ioremap_resource(&pdev->dev, res);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +	if (!res)
> +		return -ENODEV;
> +	versatile_cfg_base[1] = devm_ioremap_resource(&pdev->dev, res);
> +
> +	bridge = of_create_pci_host_bridge(&pdev->dev, &pci_versatile_ops, &sys);

Are we still relying on the PCI core to default to a bus number range of
00-ff?  Can we not do that?

Bjorn

[PATCH 3/3] pci: add DT based ARM Versatile PCI host driver

[Rob Herring]

On Thu, Apr 24, 2014 at 6:24 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Mar 27, 2014 at 05:46:38PM -0500, Rob Herring wrote:
>> From: Rob Herring <robh@kernel.org>
>>
>> This converts the Versatile PCI host code to a platform driver using
>> of_create_pci_host_bridge for parsing DT and setup.
>>
>> I think more of this setup could be done by the core code. There are
>> accesses to the host's config space (accesses using local_pci_cfg_base)
>> which seem like they could be done by the core code or using standard
>> config space accessors. The problem is bridge->bus->self is needed, but
>> it does not get setup. I'm not exactly sure how that should work.
>>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> ...

>> +     bridge = of_create_pci_host_bridge(&pdev->dev, &pci_versatile_ops, &sys);
>
> Are we still relying on the PCI core to default to a bus number range of
> 00-ff?  Can we not do that?

No, that comes from DT.

Rob

[PATCH 1/3] ARM: hackup pcibios support for commmon bridge code

[Rob Herring]

On Thu, Apr 24, 2014 at 6:20 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Mar 27, 2014 at 05:46:36PM -0500, Rob Herring wrote:
>> From: Rob Herring <robh@kernel.org>
>>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  arch/arm/kernel/bios32.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 63 insertions(+)
>>
>> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
>> index 317da88..31ec14a 100644
>> --- a/arch/arm/kernel/bios32.c
>> +++ b/arch/arm/kernel/bios32.c
>> @@ -11,6 +11,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/init.h>
>>  #include <linux/io.h>
>> +#include <linux/of_pci.h>
>>
>>  #include <asm/mach-types.h>
>>  #include <asm/mach/map.h>
>> @@ -337,6 +338,15 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>>       list_for_each_entry(dev, &bus->devices, bus_list) {
>>               u16 cmd;
>>
>> +             /* Ignore fully discovered devices */
>> +             if (dev->is_added)
>> +                     continue;
>> +
>> +             set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
>
> We already do this in pci_device_add(); why do we need it here, too?

Because it was in the arm64 code. I'm not really sure.

>> +             /* Read default IRQs and fixup if necessary */
>> +             dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
>
> Could this be done in pcibios_enable_device() or some other per-device
> interface()?  I'd like to get rid of pcibios_fixup_bus() eventually.

Probably. This should return NO_IRQ in the !OF case. We'd need to make
sure that works.

I had a loop at pcibios_fixup_bus across architectures and could make
no sense of why things were done in here. It all seemed a bit random
and left me with questions like shouldn't something always be needed.
Probably a lot of pieces are redundant now that didn't use to be. It
needs someone that knows more about PCI than me to look at.

>>               pci_read_config_word(dev, PCI_COMMAND, &cmd);
>>               cmd |= features;
>>               pci_write_config_word(dev, PCI_COMMAND, cmd);
>> @@ -681,3 +691,56 @@ void __init pci_map_io_early(unsigned long pfn)
>>       pci_io_desc.pfn = pfn;
>>       iotable_init(&pci_io_desc, 1);
>>  }
>> +
>> +struct ioresource {
>> +     struct list_head list;
>> +     phys_addr_t start;
>> +     resource_size_t size;
>> +};
>> +
>> +static LIST_HEAD(io_list);
>> +
>> +int pci_register_io_range(phys_addr_t address, resource_size_t size)
>> +{
>> +     struct ioresource *res;
>> +     resource_size_t allocated_size = 0;
>> +
>> +     /* find if the range has not been already allocated */
>> +     list_for_each_entry(res, &io_list, list) {
>> +             if (address >= res->start &&
>> +                     address + size <= res->start + size)
>> +                     return 0;
>> +             allocated_size += res->size;
>> +     }
>> +
>> +     /* range not already registered, check for space */
>> +     if (allocated_size + size > IO_SPACE_LIMIT)
>> +             return -E2BIG;
>> +
>> +     /* add the range in the list */
>> +     res = kzalloc(sizeof(*res), GFP_KERNEL);
>> +     if (!res)
>> +             return -ENOMEM;
>> +     res->start = address;
>> +     res->size = size;
>> +
>> +     list_add_tail(&res->list, &io_list);
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_register_io_range);
>> +
>> +unsigned long pci_address_to_pio(phys_addr_t address)
>> +{
>> +     struct ioresource *res;
>> +
>> +     list_for_each_entry(res, &io_list, list) {
>> +             if (address >= res->start &&
>> +                     address < res->start + res->size) {
>> +                     return res->start - address;
>> +             }
>> +     }
>> +
>> +     return (unsigned long)-1;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_address_to_pio);
>
> ISTR some discussion about this, so maybe this has already been addressed,
> but it doesn't seem right to export generically-named symbols like these
> from the ARM arch code.  And they don't look very ARM-specific.

Right, I expect these to go away with Liviu's the next version.

Rob

[PATCH 3/3] pci: add DT based ARM Versatile PCI host driver

[Bjorn Helgaas]

On Thu, Apr 24, 2014 at 5:37 PM, Rob Herring <robherring2@gmail.com> wrote:
> On Thu, Apr 24, 2014 at 6:24 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Thu, Mar 27, 2014 at 05:46:38PM -0500, Rob Herring wrote:
>>> From: Rob Herring <robh@kernel.org>
>>>
>>> This converts the Versatile PCI host code to a platform driver using
>>> of_create_pci_host_bridge for parsing DT and setup.
>>>
>>> I think more of this setup could be done by the core code. There are
>>> accesses to the host's config space (accesses using local_pci_cfg_base)
>>> which seem like they could be done by the core code or using standard
>>> config space accessors. The problem is bridge->bus->self is needed, but
>>> it does not get setup. I'm not exactly sure how that should work.
>>>
>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>> ...
>
>>> +     bridge = of_create_pci_host_bridge(&pdev->dev, &pci_versatile_ops, &sys);
>>
>> Are we still relying on the PCI core to default to a bus number range of
>> 00-ff?  Can we not do that?
>
> No, that comes from DT.

Oh, good.  That's done inside of_create_pci_host_bridge(), then?  (I
haven't looked at that code.)

Bjorn