Versatile PCI DT support

From: Rob Herring <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 arch/arm/kernel/bios32.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

arch/arm/kernel/bios32.c +1 -0

index 317da88..31ec14a 100644
@@ -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>

arch/arm/kernel/bios32.c +43 -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;
+

Quoted 23 lines · Rob Herring

From: Rob Herring <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 arch/arm/kernel/bios32.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

arch/arm/kernel/bios32.c +1 -0

index 317da88..31ec14a 100644
@@ -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;
 

arch/arm/kernel/bios32.c void pcibios_fixup_bus(struct pci_bus *bus) · line 345

Quoted 5 lines

arch/arm/kernel/bios32.c +5 -0

+             /* 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?

arch/arm/kernel/bios32.c void pcibios_fixup_bus(struct pci_bus *bus) · line 348

Quoted 2 lines

arch/arm/kernel/bios32.c +2 -0

+             /* 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.

Quoted 56 lines

arch/arm/kernel/bios32.c +49 -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;
+             }
+     }

arch/arm/kernel/bios32.c void __init pci_map_io_early(unsigned long pfn) · line 746

Quoted 4 lines

arch/arm/kernel/bios32.c +4 -0

+
+     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.

From: Rob Herring <[email protected]>

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 <[email protected]>

Cc Bjorn Helgaas <[email protected]>

...

drivers/pci/host/pci-versatile.c new file +15 -0

+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);

Quoted 11 lines · Rob Herring

From: Rob Herring <[email protected]>

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.

Quoted 5 lines

Signed-off-by Rob Herring <[email protected]>

Cc Bjorn Helgaas <[email protected]>

...

+ 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?

Quoted 15 lines · Bjorn Helgaas

Quoted 14 lines · Rob Herring

From: Rob Herring <[email protected]>

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 <[email protected]>

Cc Bjorn Helgaas <[email protected]>

...

Quoted 4 lines

Quoted 1 line

+ 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.

From: Rob Herring <[email protected]>

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

Signature · 1 line

1.8.3.2

Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

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

Quoted 7 lines

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?

Quoted 19 lines · Liviu Dudau

Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

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

Quoted 7 lines

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.

Quoted 8 lines

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?

If I want to use pci_write_config_* accesses, then I need a struct pci_dev pointer for the host. I can get this since I know the slot of the host, but only after pci_bus_add_devices which is perhaps too late. Historically, the Versatile PCI driver has blocked config accesses to the host's config space. I'm just wondering if this is really correct behavior and whether the core code should do the setup.

This is setup that is done which doesn't seem like it is very specific to Versatile.

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);

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 29 lines · Rob Herring

Quoted 27 lines · Liviu Dudau

Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

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

Quoted 7 lines

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?

Quoted 6 lines

If I want to use pci_write_config_* accesses, then I need a struct pci_dev pointer for the host. I can get this since I know the slot of the host, but only after pci_bus_add_devices which is perhaps too late. Historically, the Versatile PCI driver has blocked config accesses to the host's config space. I'm just wondering if this is really correct behavior and whether the core code should do the setup.

PCI core? It knows nothing of the Versatile registers. Versatile core code? Maybe.

Quoted 8 lines

This is setup that is done which doesn't seem like it is very specific to Versatile.

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);

Why don't you do this in a versatile_fixup_bridge() call, like pci-tegra.c does?

Quoted 7 lines

/*
* 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);

My guess is that this has to be done at probe time. It just happens that for Versatile PCI they live in the host config space, but they are not really PCI config space writes.

Quoted 37 lines · Liviu Dudau

Quoted 35 lines · Rob Herring

Quoted 19 lines · Liviu Dudau

Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

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

Quoted 7 lines

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.

Quoted 8 lines

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?

If I want to use pci_write_config_* accesses, then I need a struct pci_dev pointer for the host. I can get this since I know the slot of the host, but only after pci_bus_add_devices which is perhaps too late. Historically, the Versatile PCI driver has blocked config accesses to the host's config space. I'm just wondering if this is really correct behavior and whether the core code should do the setup.

Quoted 1 line

PCI core? It knows nothing of the Versatile registers. Versatile core code? Maybe.

These are all standard config space registers based on the offsets and documentation. The only thing that I think might make this Versatile specific is a host bridge having its own config space at all is optional.

Quoted 4 lines

Quoted 4 lines

This is setup that is done which doesn't seem like it is very specific to Versatile.

Quoted 8 lines

Quoted 5 lines

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);

Why don't you do this in a versatile_fixup_bridge() call, like pci-tegra.c does?

I could, but doesn't any host need these bits set (assuming the host has a config space)?

As it stands now, the PCI core would never call a fixup because the host's config space is masked out (with pci_slot_ignore). So is that wrong?

Quoted 9 lines

Quoted 8 lines

/*
* 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);

Quoted 3 lines

My guess is that this has to be done at probe time. It just happens that for Versatile PCI they live in the host config space, but they are not really PCI config space writes.

It is not completely obvious since they use private defines, but PCI_BASE_ADDRESS_x registers are BAR registers AFAICT.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 7 lines · Rob Herring

Quoted 1 line · Liviu Dudau

PCI core? It knows nothing of the Versatile registers. Versatile core code? Maybe.

These are all standard config space registers based on the offsets and documentation. The only thing that I think might make this Versatile specific is a host bridge having its own config space at all is optional.

Sorry, got confused by your CSR_OFFSET macro into thinking it's Versatile specific (I still cannot get hold of the SP810 manual, sigh). Why don't you use PCI_COMMAND ?

For PCIe the spec is a bit more forgiving is you use ECAM, although it doesn't explicitly allow writes to the config space of the host bridge. From PCI Express Base spec, rev 3.0:

7.2.2.1. Host Bridge Requirements
For those systems that implement the ECAM, the PCI Express Host Bridge is required to translate the memory-mapped PCI Express Configuration Space accesses from the host processor to PCI Express configuration transactions. The use of Host Bridge PCI class code is Reserved for backwards compatibility; host Bridge Configuration Space is opaque to standard PCI Express software and may be implemented in an implementation specific manner that is compatible with PCI Host Bridge Type 0 Configuration Space. A PCI Express Host Bridge is not required to signal errors through a Root Complex Event Collector. This support is optional for PCI Express Host Bridges.

So it looks like you need to do those in the host bridge probe function, rather than in a generic way.

From: Rob Herring <[email protected]>

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

Signature · 1 line

1.8.3.2