linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] [RFC] Support for creating generic host_bridge from device tree
@ 2014-02-28 13:08 Liviu Dudau
  2014-02-28 13:08 ` [PATCH v3 1/5] pci: Introduce pci_register_io_range() helper function Liviu Dudau
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Liviu Dudau @ 2014-02-28 13:08 UTC (permalink / raw)
  To: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, linaro-kernel
  Cc: Benjamin Herrenschmidt, LKML, devicetree, LAKML

This is v3 of my attempt to add support for a generic pci_host_bridge controller created
from a description passed in the device tree.

Changes from v2:
  - Use range->cpu_addr when calling pci_address_to_pio()
  - Introduce pci_register_io_range() helper function in order to register
    io ranges ahead of their conversion to PIO values. This is needed as no
    information is being stored yet regarding the range mapping, making
    pci_address_to_pio() fail. Default weak implementation does nothing,
    to cover the default weak implementation of pci_address_to_pio() that
    expects direct mapping of physical addresses into PIO values (x86 view).

Changes from v1:
  - Add patch to fix conversion of IO ranges into IO resources.
  - Added a domain_nr member to pci_host_bridge structure, and a new function
    to create a root bus in a given domain number. In order to facilitate that
    I propose changing the order of initialisation between pci_host_bridge and
    it's related bus in pci_create_root_bus() as sort of a rever of 7b5436635800.
    This is done in patch 1/4 and 2/4.
  - Added a simple allocator of domain numbers in drivers/pci/host-bridge.c. The
    code will first try to get a domain id from of_alias_get_id(..., "pci-domain")
    and if that fails assign the next unallocated domain id.
  - Changed the name of the function that creates the generic host bridge from
    pci_host_bridge_of_init to of_create_pci_host_bridge and exported as GPL symbol.

v1 thread here: https://lkml.org/lkml/2014/2/3/380

The following is an edit of the original blurb:

Following the discussion started here [1], I now have a proposal for tackling
generic support for host bridges described via device tree. It is an initial
stab at it, to try to get feedback and suggestions, but it is functional enough
that I have PCI Express for arm64 working on an FPGA using the patch that I am
also publishing that adds support for PCI for that platform.

Looking at the existing architectures that fit the requirements (use of device
tree and PCI) yields the powerpc and microblaze as generic enough to make them
candidates for conversion. I have a tentative patch for microblaze that I can
only compile test it, unfortunately using qemu-microblaze leads to an early
crash in the kernel.

As Bjorn has mentioned in the previous discussion, the idea is to add to
struct pci_host_bridge enough data to be able to reduce the size or remove the
architecture specific pci_controller structure. arm64 support actually manages
to get rid of all the architecture static data and has no pci_controller structure
defined. For host bridge drivers that means a change of API unless architectures
decide to provide a compatibility layer (comments here please).

In order to initialise a host bridge with the new API, the following example
code is sufficient for a _probe() function:

static int myhostbridge_probe(struct platform_device *pdev)
{
	int err;
	struct device_node *dev;
	struct pci_host_bridge *bridge;
	struct myhostbridge_port *pp;
	resource_size_t lastbus;

	dev = pdev->dev.of_node;

	if (!of_device_is_available(dev)) {
		pr_warn("%s: disabled\n", dev->full_name);
		return -ENODEV;
	}

	pp = kzalloc(sizeof(struct myhostbridge_port), GFP_KERNEL);
	if (!pp)
		return -ENOMEM;

	bridge = of_create_pci_host_bridge(&pdev->dev, &myhostbridge_ops, pp);
	if (!bridge) {
		err = -EINVAL;
		goto bridge_init_fail;
	}

	err = myhostbridge_setup(bridge->bus);
	if (err)
		goto bridge_init_fail;

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

bridge_init_fail:
	kfree(pp);
	return err;
}

[1] http://thread.gmane.org/gmane.linux.kernel.pci/25946

Best regards,
Liviu


Liviu Dudau (5):
  pci: Introduce pci_register_io_range() helper function.
  pci: OF: Fix the conversion of IO ranges into IO resources.
  pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.
  pci: Introduce a domain number for pci_host_bridge.
  pci: Add support for creating a generic host_bridge from device tree

 drivers/of/address.c       |  39 +++++++++++++
 drivers/pci/host-bridge.c  | 134 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c        |  66 ++++++++++++++--------
 include/linux/of_address.h |  14 +----
 include/linux/pci.h        |  17 ++++++
 5 files changed, 236 insertions(+), 34 deletions(-)

-- 
1.9.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/5] pci: Introduce pci_register_io_range() helper function.
  2014-02-28 13:08 [PATCH v3 0/5] [RFC] Support for creating generic host_bridge from device tree Liviu Dudau
@ 2014-02-28 13:08 ` Liviu Dudau
  2014-02-28 13:08 ` [PATCH v3 2/5] pci: OF: Fix the conversion of IO ranges into IO resources Liviu Dudau
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Liviu Dudau @ 2014-02-28 13:08 UTC (permalink / raw)
  To: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, linaro-kernel
  Cc: Benjamin Herrenschmidt, LKML, devicetree, LAKML

Some architectures do not share x86 simple view of the I/O space and
instead use a range of addresses that map to external devices. For PCI,
these ranges can be expressed by OF bindings in a device tree file.

Introduce a pci_register_io_range() helper function that can be used
by the architecture code to keep track of the io ranges described by the
PCI bindings.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 1a54f1f..d1bb30f 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -619,6 +619,11 @@ const __be32 *of_get_address(struct device_node *dev, int index, u64 *size,
 }
 EXPORT_SYMBOL(of_get_address);
 
+int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+{
+	return 0;
+}
+
 unsigned long __weak pci_address_to_pio(phys_addr_t address)
 {
 	if (address > IO_SPACE_LIMIT)
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 5f6ed6b..40c418d 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device, int index);
 extern const __be32 *of_get_address(struct device_node *dev, int index,
 			   u64 *size, unsigned int *flags);
 
+extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
 extern unsigned long pci_address_to_pio(phys_addr_t addr);
 
 extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/5] pci: OF: Fix the conversion of IO ranges into IO resources.
  2014-02-28 13:08 [PATCH v3 0/5] [RFC] Support for creating generic host_bridge from device tree Liviu Dudau
  2014-02-28 13:08 ` [PATCH v3 1/5] pci: Introduce pci_register_io_range() helper function Liviu Dudau
@ 2014-02-28 13:08 ` Liviu Dudau
  2014-02-28 13:08 ` [PATCH v3 3/5] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus Liviu Dudau
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Liviu Dudau @ 2014-02-28 13:08 UTC (permalink / raw)
  To: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, linaro-kernel
  Cc: Benjamin Herrenschmidt, LKML, devicetree, LAKML

The ranges property for a host bridge controller in DT describes
the mapping between the PCI bus address and the CPU physical address.
The resources framework however expects that the IO resources start
at a pseudo "port" address 0 (zero) and have a maximum size of IO_SPACE_LIMIT.
The conversion from pci ranges to resources failed to take that into account.

In the process move the function into drivers/of/address.c as it
now depends on pci_address_to_pio() code.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>

diff --git a/drivers/of/address.c b/drivers/of/address.c
index d1bb30f..d595d98 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -724,3 +724,37 @@ void __iomem *of_iomap(struct device_node *np, int index)
 	return ioremap(res.start, resource_size(&res));
 }
 EXPORT_SYMBOL(of_iomap);
+
+/**
+ * of_pci_range_to_resource - Create a resource from an of_pci_range
+ * @range:	the PCI range that describes the resource
+ * @np:		device node where the range belongs to
+ * @res:	pointer to a valid resource that will be updated to
+ *              reflect the values contained in the range.
+ * Note that if the range is an IO range, the resource will be converted
+ * using pci_address_to_pio() which can fail if it is called too early or
+ * if the range cannot be matched to any host bridge IO space (our case here).
+ * To guard against that we try to register the IO range first.
+ * If that fails we know that pci_address_to_pio() will do too.
+ */
+void of_pci_range_to_resource(struct of_pci_range *range,
+	struct device_node *np, struct resource *res)
+{
+	res->flags = range->flags;
+	if (res->flags & IORESOURCE_IO) {
+		unsigned long port = -1;
+		if (!pci_register_io_range(range->cpu_addr, range->size))
+			port = pci_address_to_pio(range->cpu_addr);
+		if (port == (unsigned long)-1) {
+			res->start = (resource_size_t)OF_BAD_ADDR;
+			res->end = (resource_size_t)OF_BAD_ADDR;
+			return;
+		}
+		res->start = port;
+	} else {
+		res->start = range->cpu_addr;
+	}
+	res->end = res->start + range->size - 1;
+	res->parent = res->child = res->sibling = NULL;
+	res->name = np->full_name;
+}
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 40c418d..3fe500a 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -23,17 +23,8 @@ struct of_pci_range {
 #define for_each_of_pci_range(parser, range) \
 	for (; of_pci_range_parser_one(parser, range);)
 
-static inline void of_pci_range_to_resource(struct of_pci_range *range,
-					    struct device_node *np,
-					    struct resource *res)
-{
-	res->flags = range->flags;
-	res->start = range->cpu_addr;
-	res->end = range->cpu_addr + range->size - 1;
-	res->parent = res->child = res->sibling = NULL;
-	res->name = np->full_name;
-}
-
+extern void of_pci_range_to_resource(struct of_pci_range *range,
+		struct device_node *np, struct resource *res);
 /* Translate a DMA address from device space to CPU space */
 extern u64 of_translate_dma_address(struct device_node *dev,
 				    const __be32 *in_addr);
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 3/5] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.
  2014-02-28 13:08 [PATCH v3 0/5] [RFC] Support for creating generic host_bridge from device tree Liviu Dudau
  2014-02-28 13:08 ` [PATCH v3 1/5] pci: Introduce pci_register_io_range() helper function Liviu Dudau
  2014-02-28 13:08 ` [PATCH v3 2/5] pci: OF: Fix the conversion of IO ranges into IO resources Liviu Dudau
@ 2014-02-28 13:08 ` Liviu Dudau
  2014-02-28 13:08 ` [PATCH v3 4/5] pci: Introduce a domain number for pci_host_bridge Liviu Dudau
  2014-02-28 13:08 ` [PATCH v3 5/5] pci: Add support for creating a generic host_bridge from device tree Liviu Dudau
  4 siblings, 0 replies; 9+ messages in thread
From: Liviu Dudau @ 2014-02-28 13:08 UTC (permalink / raw)
  To: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, linaro-kernel
  Cc: Benjamin Herrenschmidt, LKML, devicetree, LAKML

Before commit 7b5436635800 the pci_host_bridge was created before the root bus.
As that commit has added a needless dependency on the bus for pci_alloc_host_bridge()
the creation order has been changed for no good reason. Revert the order of
creation as we are going to depend on the pci_host_bridge structure to retrieve the
domain number of the root bus.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6e34498..78ccba0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -505,7 +505,7 @@ static void pci_release_host_bridge_dev(struct device *dev)
 	kfree(bridge);
 }
 
-static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b)
+static struct pci_host_bridge *pci_alloc_host_bridge(void)
 {
 	struct pci_host_bridge *bridge;
 
@@ -514,7 +514,6 @@ static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b)
 		return NULL;
 
 	INIT_LIST_HEAD(&bridge->windows);
-	bridge->bus = b;
 	return bridge;
 }
 
@@ -1727,9 +1726,21 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	char bus_addr[64];
 	char *fmt;
 
+	bridge = pci_alloc_host_bridge();
+	if (!bridge)
+		return NULL;
+
+	bridge->dev.parent = parent;
+	bridge->dev.release = pci_release_host_bridge_dev;
+	error = pcibios_root_bridge_prepare(bridge);
+	if (error) {
+		kfree(bridge);
+		return NULL;
+	}
+
 	b = pci_alloc_bus();
 	if (!b)
-		return NULL;
+		goto err_out;
 
 	b->sysdata = sysdata;
 	b->ops = ops;
@@ -1738,26 +1749,15 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	if (b2) {
 		/* If we already got to this bus through a different bridge, ignore it */
 		dev_dbg(&b2->dev, "bus already known\n");
-		goto err_out;
+		goto err_bus_out;
 	}
 
-	bridge = pci_alloc_host_bridge(b);
-	if (!bridge)
-		goto err_out;
-
-	bridge->dev.parent = parent;
-	bridge->dev.release = pci_release_host_bridge_dev;
+	bridge->bus = b;
 	dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus);
-	error = pcibios_root_bridge_prepare(bridge);
-	if (error) {
-		kfree(bridge);
-		goto err_out;
-	}
-
 	error = device_register(&bridge->dev);
 	if (error) {
 		put_device(&bridge->dev);
-		goto err_out;
+		goto err_bus_out;
 	}
 	b->bridge = get_device(&bridge->dev);
 	device_enable_async_suspend(b->bridge);
@@ -1814,8 +1814,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 class_dev_reg_err:
 	put_device(&bridge->dev);
 	device_unregister(&bridge->dev);
-err_out:
+err_bus_out:
 	kfree(b);
+err_out:
+	kfree(bridge);
 	return NULL;
 }
 
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 4/5] pci: Introduce a domain number for pci_host_bridge.
  2014-02-28 13:08 [PATCH v3 0/5] [RFC] Support for creating generic host_bridge from device tree Liviu Dudau
                   ` (2 preceding siblings ...)
  2014-02-28 13:08 ` [PATCH v3 3/5] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus Liviu Dudau
@ 2014-02-28 13:08 ` Liviu Dudau
  2014-02-28 13:08 ` [PATCH v3 5/5] pci: Add support for creating a generic host_bridge from device tree Liviu Dudau
  4 siblings, 0 replies; 9+ messages in thread
From: Liviu Dudau @ 2014-02-28 13:08 UTC (permalink / raw)
  To: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, linaro-kernel
  Cc: Benjamin Herrenschmidt, LKML, devicetree, LAKML

Make it easier to discover the domain number of a bus by storing
the number in pci_host_bridge for the root bus. Several architectures
have their own way of storing this information, so it makes sense
to try to unify the code. While at this, add a new function that
creates a root bus in a given domain and make pci_create_root_bus()
a wrapper around this function.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 78ccba0..1b2f45c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1714,8 +1714,9 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
 {
 }
 
-struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
-		struct pci_ops *ops, void *sysdata, struct list_head *resources)
+struct pci_bus *pci_create_root_bus_in_domain(struct device *parent,
+		int domain, int bus, struct pci_ops *ops, void *sysdata,
+		struct list_head *resources)
 {
 	int error;
 	struct pci_host_bridge *bridge;
@@ -1732,6 +1733,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 
 	bridge->dev.parent = parent;
 	bridge->dev.release = pci_release_host_bridge_dev;
+	bridge->domain_nr = domain;
 	error = pcibios_root_bridge_prepare(bridge);
 	if (error) {
 		kfree(bridge);
@@ -1745,7 +1747,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	b->sysdata = sysdata;
 	b->ops = ops;
 	b->number = b->busn_res.start = bus;
-	b2 = pci_find_bus(pci_domain_nr(b), bus);
+	b2 = pci_find_bus(bridge->domain_nr, bus);
 	if (b2) {
 		/* If we already got to this bus through a different bridge, ignore it */
 		dev_dbg(&b2->dev, "bus already known\n");
@@ -1753,7 +1755,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	}
 
 	bridge->bus = b;
-	dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus);
+	dev_set_name(&bridge->dev, "pci%04x:%02x", bridge->domain_nr, bus);
 	error = device_register(&bridge->dev);
 	if (error) {
 		put_device(&bridge->dev);
@@ -1768,7 +1770,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 
 	b->dev.class = &pcibus_class;
 	b->dev.parent = b->bridge;
-	dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
+	dev_set_name(&b->dev, "%04x:%02x", bridge->domain_nr, bus);
 	error = device_register(&b->dev);
 	if (error)
 		goto class_dev_reg_err;
@@ -1821,6 +1823,22 @@ err_out:
 	return NULL;
 }
 
+struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
+		struct pci_ops *ops, void *sysdata, struct list_head *resources)
+{
+	int domain_nr;
+	struct pci_bus *b = pci_alloc_bus();
+	if (!b)
+		return NULL;
+
+	b->sysdata = sysdata;
+	domain_nr = pci_domain_nr(b);
+	kfree(b);
+
+	return pci_create_root_bus_in_domain(parent, domain_nr, bus,
+				ops, sysdata, resources);
+}
+
 int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
 {
 	struct resource *res = &b->busn_res;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 33aa2ca..1eed009 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -394,6 +394,7 @@ struct pci_host_bridge_window {
 struct pci_host_bridge {
 	struct device dev;
 	struct pci_bus *bus;		/* root bus */
+	int domain_nr;
 	struct list_head windows;	/* pci_host_bridge_windows */
 	void (*release_fn)(struct pci_host_bridge *);
 	void *release_data;
@@ -747,6 +748,9 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 				    struct pci_ops *ops, void *sysdata,
 				    struct list_head *resources);
+struct pci_bus *pci_create_root_bus_in_domain(struct device *parent,
+			int domain, int bus, struct pci_ops *ops,
+			void *sysdata, struct list_head *resources);
 int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
 int pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
 void pci_bus_release_busn_res(struct pci_bus *b);
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 5/5] pci: Add support for creating a generic host_bridge from device tree
  2014-02-28 13:08 [PATCH v3 0/5] [RFC] Support for creating generic host_bridge from device tree Liviu Dudau
                   ` (3 preceding siblings ...)
  2014-02-28 13:08 ` [PATCH v3 4/5] pci: Introduce a domain number for pci_host_bridge Liviu Dudau
@ 2014-02-28 13:08 ` Liviu Dudau
       [not found]   ` <CACoXjck59jA1Ub8mHB0zn16krCxagUgH8BmJVnLQ8KGpFg7WpA@mail.gmail.com>
  4 siblings, 1 reply; 9+ messages in thread
From: Liviu Dudau @ 2014-02-28 13:08 UTC (permalink / raw)
  To: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, linaro-kernel
  Cc: Benjamin Herrenschmidt, LKML, devicetree, LAKML

Several platforms use a rather generic version of parsing
the device tree to find the host bridge ranges. Move the common code
into the generic PCI code and use it to create a pci_host_bridge
structure that can be used by arch code.

Based on early attempts by Andrew Murray to unify the code.
Used powerpc and microblaze PCI code as starting point.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>

diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index 06ace62..feb8436 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -6,9 +6,13 @@
 #include <linux/init.h>
 #include <linux/pci.h>
 #include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
 
 #include "pci.h"
 
+static int domain_nr;
+
 static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
 {
 	while (bus->parent)
@@ -91,3 +95,133 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
 	res->end = region->end + offset;
 }
 EXPORT_SYMBOL(pcibios_bus_to_resource);
+
+/**
+ * pci_host_bridge_of_get_ranges - Parse PCI host bridge resources from DT
+ * @dev: device node of the host bridge having the range property
+ * @resources: list where the range of resources will be added after DT parsing
+ * @io_base: pointer to a variable that will contain the physical address for
+ * the start of the I/O range.
+ *
+ * If this function returns an error then the @resources list will be freed.
+ *
+ * This function will parse the "ranges" property of a PCI host bridge device
+ * node and setup the resource mapping based on its content. It is expected
+ * that the property conforms with the Power ePAPR document.
+ *
+ * Each architecture is then offered the chance of applying their own
+ * filtering of pci_host_bridge_windows based on their own restrictions by
+ * calling pcibios_fixup_bridge_ranges(). The filtered list of windows
+ * can then be used when creating a pci_host_bridge structure.
+ */
+static int pci_host_bridge_of_get_ranges(struct device_node *dev,
+		struct list_head *resources, resource_size_t *io_base)
+{
+	struct resource *res;
+	struct of_pci_range range;
+	struct of_pci_range_parser parser;
+	int err;
+
+	pr_info("PCI host bridge %s ranges:\n", dev->full_name);
+
+	/* Check for ranges property */
+	err = of_pci_range_parser_init(&parser, dev);
+	if (err)
+		return err;
+
+	pr_debug("Parsing ranges property...\n");
+	for_each_of_pci_range(&parser, &range) {
+		/* Read next ranges element */
+		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
+				range.pci_space, range.pci_addr);
+		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
+					range.cpu_addr, range.size);
+
+		/*
+		 * If we failed translation or got a zero-sized region
+		 * then skip this range
+		 */
+		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
+			continue;
+
+		res = kzalloc(sizeof(struct resource), GFP_KERNEL);
+		if (!res) {
+			err = -ENOMEM;
+			goto bridge_ranges_nomem;
+		}
+
+		of_pci_range_to_resource(&range, dev, res);
+
+		if (resource_type(res) == IORESOURCE_IO)
+			*io_base = range.cpu_addr;
+
+		pci_add_resource_offset(resources, res,
+				res->start - range.pci_addr);
+	}
+
+	/* Apply architecture specific fixups for the ranges */
+	pcibios_fixup_bridge_ranges(resources);
+
+	return 0;
+
+bridge_ranges_nomem:
+	pci_free_resource_list(resources);
+	return err;
+}
+
+/**
+ * of_create_pci_host_bridge - Create a PCI host bridge structure using
+ * information passed in the DT.
+ * @parent: device owning this host bridge
+ * @ops: pci_ops associated with the host controller
+ * @host_data: opaque data structure used by the host controller.
+ *
+ * returns a pointer to the newly created pci_host_bridge structure, or
+ * NULL if the call failed.
+ *
+ * This function will try to obtain the host bridge domain number by
+ * using of_alias_get_id() call with "pci-domain" as a stem. If that
+ * fails, a local allocator will be used that will put each host bridge
+ * in a new domain.
+ */
+struct pci_host_bridge *
+of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops, void *host_data)
+{
+	int err, domain, busno;
+	struct resource bus_range;
+	struct pci_bus *root_bus;
+	struct pci_host_bridge *bridge;
+	resource_size_t io_base;
+	LIST_HEAD(res);
+
+	domain = of_alias_get_id(parent->of_node, "pci-domain");
+	if (domain == -ENODEV)
+		domain = domain_nr++;
+
+	err = of_pci_parse_bus_range(parent->of_node, &bus_range);
+	if (err) {
+		dev_info(parent, "No bus range for %s, using default [0-255]\n",
+			parent->of_node->full_name);
+		bus_range.start = 0;
+		bus_range.end = 255;
+		bus_range.flags = IORESOURCE_BUS;
+	}
+	busno = bus_range.start;
+	pci_add_resource(&res, &bus_range);
+
+	/* now parse the rest of host bridge bus ranges */
+	if (pci_host_bridge_of_get_ranges(parent->of_node, &res, &io_base))
+		return NULL;
+
+	/* then create the root bus */
+	root_bus = pci_create_root_bus_in_domain(parent, domain, busno,
+						ops, host_data, &res);
+	if (!root_bus)
+		return NULL;
+
+	bridge = to_pci_host_bridge(root_bus->bridge);
+	bridge->io_base = io_base;
+
+	return bridge;
+}
+EXPORT_SYMBOL_GPL(of_create_pci_host_bridge);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1eed009..0c5e269 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -395,6 +395,7 @@ struct pci_host_bridge {
 	struct device dev;
 	struct pci_bus *bus;		/* root bus */
 	int domain_nr;
+	resource_size_t io_base;	/* physical address for the start of I/O area */
 	struct list_head windows;	/* pci_host_bridge_windows */
 	void (*release_fn)(struct pci_host_bridge *);
 	void *release_data;
@@ -1786,11 +1787,23 @@ static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
 	return bus ? bus->dev.of_node : NULL;
 }
 
+struct pci_host_bridge *
+of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops,
+			void *host_data);
+
+void pcibios_fixup_bridge_ranges(struct list_head *resources);
 #else /* CONFIG_OF */
 static inline void pci_set_of_node(struct pci_dev *dev) { }
 static inline void pci_release_of_node(struct pci_dev *dev) { }
 static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
 static inline void pci_release_bus_of_node(struct pci_bus *bus) { }
+
+static inline struct pci_host_bridge *
+pci_host_bridge_of_init(struct device *parent, struct pci_ops *ops,
+			void *host_data)
+{
+	return NULL;
+}
 #endif  /* CONFIG_OF */
 
 #ifdef CONFIG_EEH
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 5/5] pci: Add support for creating a generic host_bridge from device tree
       [not found]   ` <CACoXjck59jA1Ub8mHB0zn16krCxagUgH8BmJVnLQ8KGpFg7WpA@mail.gmail.com>
@ 2014-03-01  2:07     ` Tanmay Inamdar
  2014-03-01  2:39       ` Liviu Dudau
  0 siblings, 1 reply; 9+ messages in thread
From: Tanmay Inamdar @ 2014-03-01  2:07 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon,
	linaro-kernel, Benjamin Herrenschmidt, LKML, LAKML, devicetree,
	patches

Earlier email did not deliver to mailing lists because of plain text
setting problem on my side. Apologies for spamming. Sending it again.

Hello Liviu,

While porting X-Gene PCIe driver to v2 series, following problems were observed.

1. In 'of_create_pci_host_bridge' function, bus_range is defined
locally. So, while walking over list of resources in bridge->windows
later, during X-Gene controller related setup, garbage values are
found in the resource. Please allocate it dynamically.

2. 'domain_nr' problem is partially solved. There are still some
places where functions are getting invalid domain_nr.  For example,
'pci_alloc_child_bus' tries to get domain_nr when bridge is not
assigned to bus. You may want to look for all the places where
pci_domain_nr is used. Please see below dump -->

pci 0001:00:00.0: scanning [bus 00-00] behind bridge, pass 1
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at
/home/tinamdar/work/open-source/linux/fs/sysfs/dir.c:52
sysfs_warn_dup+0x80/0xc0()
sysfs: cannot create duplicate filename '/class/pci_bus/0000:01'
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.0-rc4+ #37
Call trace:
[<ffffffc000088140>] dump_backtrace+0x0/0x140
[<ffffffc000088294>] show_stack+0x14/0x20
[<ffffffc0004f64b0>] dump_stack+0x78/0xc4
[<ffffffc000096c88>] warn_slowpath_common+0x88/0xc0
[<ffffffc000096d10>] warn_slowpath_fmt+0x50/0x60
[<ffffffc0001b8260>] sysfs_warn_dup+0x80/0xc0
[<ffffffc0001b8718>] sysfs_do_create_link_sd.isra.2+0xf8/0x100
[<ffffffc0001b8740>] sysfs_create_link+0x20/0x40
[<ffffffc000322f5c>] device_add+0x41c/0x520
[<ffffffc00032307c>] device_register+0x1c/0x40
[<ffffffc0004f1924>] pci_add_new_bus+0x284/0x380
[<ffffffc0002c51a0>] pci_scan_bridge+0x4e0/0x540
[<ffffffc0002c52b4>] pci_scan_child_bus+0xb4/0x140
[<ffffffc0004f1a34>] pci_rescan_bus+0x14/0x40
[<ffffffc0006a12d4>] xgene_pcie_probe_bridge+0x688/0x750
[<ffffffc000327c64>] platform_drv_probe+0x24/0x60
[<ffffffc000325d74>] really_probe+0xf4/0x220
[<ffffffc000325fc4>] __driver_attach+0xa4/0xc0
[<ffffffc000323e18>] bus_for_each_dev+0x58/0xa0
[<ffffffc000325800>] driver_attach+0x20/0x40
[<ffffffc0003253d0>] bus_add_driver+0x150/0x220
[<ffffffc000326780>] driver_register+0x60/0x120
[<ffffffc000327c20>] __platform_driver_register+0x60/0x80
[<ffffffc0006a0c44>] xgene_pcie_driver_init+0x18/0x20
[<ffffffc0000814c4>] do_one_initcall+0xe4/0x160
[<ffffffc00068c938>] kernel_init_freeable+0x138/0x1d8
[<ffffffc0004f06b0>] kernel_init+0x10/0xe0
---[ end trace 53db1c3a7fbdeb88 ]---
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at
/home/tinamdar/work/open-source/linux/drivers/pci/probe.c:711
pci_add_new_bus+0x36c/0x380()

Thanks,
Tanmay

On Fri, Feb 28, 2014 at 6:01 PM, Tanmay Inamdar <tinamdar@apm.com> wrote:
> Hello Liviu,
>
> While porting X-Gene PCIe driver to v2 series, following problems were
> observed.
>
> 1. In 'of_create_pci_host_bridge' function, bus_range is defined locally.
> So, while walking over list of resources in bridge->windows later, during
> X-Gene controller related setup, garbage values are found in the resource.
> Please allocate it dynamically.
>
> 2. 'domain_nr' problem is partially solved. There are still some places
> where functions are getting invalid domain_nr.  For example,
> 'pci_alloc_child_bus' tries to get domain_nr when bridge is not assigned to
> bus. You may want to look for all the places where pci_domain_nr is used.
> Please see below dump -->
>
> pci 0001:00:00.0: scanning [bus 00-00] behind bridge, pass 1
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at
> /home/tinamdar/work/open-source/linux/fs/sysfs/dir.c:52
> sysfs_warn_dup+0x80/0xc0()
> sysfs: cannot create duplicate filename '/class/pci_bus/0000:01'
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.0-rc4+ #37
> Call trace:
> [<ffffffc000088140>] dump_backtrace+0x0/0x140
> [<ffffffc000088294>] show_stack+0x14/0x20
> [<ffffffc0004f64b0>] dump_stack+0x78/0xc4
> [<ffffffc000096c88>] warn_slowpath_common+0x88/0xc0
> [<ffffffc000096d10>] warn_slowpath_fmt+0x50/0x60
> [<ffffffc0001b8260>] sysfs_warn_dup+0x80/0xc0
> [<ffffffc0001b8718>] sysfs_do_create_link_sd.isra.2+0xf8/0x100
> [<ffffffc0001b8740>] sysfs_create_link+0x20/0x40
> [<ffffffc000322f5c>] device_add+0x41c/0x520
> [<ffffffc00032307c>] device_register+0x1c/0x40
> [<ffffffc0004f1924>] pci_add_new_bus+0x284/0x380
> [<ffffffc0002c51a0>] pci_scan_bridge+0x4e0/0x540
> [<ffffffc0002c52b4>] pci_scan_child_bus+0xb4/0x140
> [<ffffffc0004f1a34>] pci_rescan_bus+0x14/0x40
> [<ffffffc0006a12d4>] xgene_pcie_probe_bridge+0x688/0x750
> [<ffffffc000327c64>] platform_drv_probe+0x24/0x60
> [<ffffffc000325d74>] really_probe+0xf4/0x220
> [<ffffffc000325fc4>] __driver_attach+0xa4/0xc0
> [<ffffffc000323e18>] bus_for_each_dev+0x58/0xa0
> [<ffffffc000325800>] driver_attach+0x20/0x40
> [<ffffffc0003253d0>] bus_add_driver+0x150/0x220
> [<ffffffc000326780>] driver_register+0x60/0x120
> [<ffffffc000327c20>] __platform_driver_register+0x60/0x80
> [<ffffffc0006a0c44>] xgene_pcie_driver_init+0x18/0x20
> [<ffffffc0000814c4>] do_one_initcall+0xe4/0x160
> [<ffffffc00068c938>] kernel_init_freeable+0x138/0x1d8
> [<ffffffc0004f06b0>] kernel_init+0x10/0xe0
> ---[ end trace 53db1c3a7fbdeb88 ]---
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at
> /home/tinamdar/work/open-source/linux/drivers/pci/probe.c:711
> pci_add_new_bus+0x36c/0x380()
>
> Thanks,
> Tanmay
>
>
>
> On Fri, Feb 28, 2014 at 5:08 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>>
>> Several platforms use a rather generic version of parsing
>> the device tree to find the host bridge ranges. Move the common code
>> into the generic PCI code and use it to create a pci_host_bridge
>> structure that can be used by arch code.
>>
>> Based on early attempts by Andrew Murray to unify the code.
>> Used powerpc and microblaze PCI code as starting point.
>>
>> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>>
>> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>> index 06ace62..feb8436 100644
>> --- a/drivers/pci/host-bridge.c
>> +++ b/drivers/pci/host-bridge.c
>> @@ -6,9 +6,13 @@
>>  #include <linux/init.h>
>>  #include <linux/pci.h>
>>  #include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_pci.h>
>>
>>  #include "pci.h"
>>
>> +static int domain_nr;
>> +
>>  static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
>>  {
>>         while (bus->parent)
>> @@ -91,3 +95,133 @@ void pcibios_bus_to_resource(struct pci_bus *bus,
>> struct resource *res,
>>         res->end = region->end + offset;
>>  }
>>  EXPORT_SYMBOL(pcibios_bus_to_resource);
>> +
>> +/**
>> + * pci_host_bridge_of_get_ranges - Parse PCI host bridge resources from
>> DT
>> + * @dev: device node of the host bridge having the range property
>> + * @resources: list where the range of resources will be added after DT
>> parsing
>> + * @io_base: pointer to a variable that will contain the physical address
>> for
>> + * the start of the I/O range.
>> + *
>> + * If this function returns an error then the @resources list will be
>> freed.
>> + *
>> + * This function will parse the "ranges" property of a PCI host bridge
>> device
>> + * node and setup the resource mapping based on its content. It is
>> expected
>> + * that the property conforms with the Power ePAPR document.
>> + *
>> + * Each architecture is then offered the chance of applying their own
>> + * filtering of pci_host_bridge_windows based on their own restrictions
>> by
>> + * calling pcibios_fixup_bridge_ranges(). The filtered list of windows
>> + * can then be used when creating a pci_host_bridge structure.
>> + */
>> +static int pci_host_bridge_of_get_ranges(struct device_node *dev,
>> +               struct list_head *resources, resource_size_t *io_base)
>> +{
>> +       struct resource *res;
>> +       struct of_pci_range range;
>> +       struct of_pci_range_parser parser;
>> +       int err;
>> +
>> +       pr_info("PCI host bridge %s ranges:\n", dev->full_name);
>> +
>> +       /* Check for ranges property */
>> +       err = of_pci_range_parser_init(&parser, dev);
>> +       if (err)
>> +               return err;
>> +
>> +       pr_debug("Parsing ranges property...\n");
>> +       for_each_of_pci_range(&parser, &range) {
>> +               /* Read next ranges element */
>> +               pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
>> +                               range.pci_space, range.pci_addr);
>> +               pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
>> +                                       range.cpu_addr, range.size);
>> +
>> +               /*
>> +                * If we failed translation or got a zero-sized region
>> +                * then skip this range
>> +                */
>> +               if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
>> +                       continue;
>> +
>> +               res = kzalloc(sizeof(struct resource), GFP_KERNEL);
>> +               if (!res) {
>> +                       err = -ENOMEM;
>> +                       goto bridge_ranges_nomem;
>> +               }
>> +
>> +               of_pci_range_to_resource(&range, dev, res);
>> +
>> +               if (resource_type(res) == IORESOURCE_IO)
>> +                       *io_base = range.cpu_addr;
>> +
>> +               pci_add_resource_offset(resources, res,
>> +                               res->start - range.pci_addr);
>> +       }
>> +
>> +       /* Apply architecture specific fixups for the ranges */
>> +       pcibios_fixup_bridge_ranges(resources);
>> +
>> +       return 0;
>> +
>> +bridge_ranges_nomem:
>> +       pci_free_resource_list(resources);
>> +       return err;
>> +}
>> +
>> +/**
>> + * of_create_pci_host_bridge - Create a PCI host bridge structure using
>> + * information passed in the DT.
>> + * @parent: device owning this host bridge
>> + * @ops: pci_ops associated with the host controller
>> + * @host_data: opaque data structure used by the host controller.
>> + *
>> + * returns a pointer to the newly created pci_host_bridge structure, or
>> + * NULL if the call failed.
>> + *
>> + * This function will try to obtain the host bridge domain number by
>> + * using of_alias_get_id() call with "pci-domain" as a stem. If that
>> + * fails, a local allocator will be used that will put each host bridge
>> + * in a new domain.
>> + */
>> +struct pci_host_bridge *
>> +of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops,
>> void *host_data)
>> +{
>> +       int err, domain, busno;
>> +       struct resource bus_range;
>> +       struct pci_bus *root_bus;
>> +       struct pci_host_bridge *bridge;
>> +       resource_size_t io_base;
>> +       LIST_HEAD(res);
>> +
>> +       domain = of_alias_get_id(parent->of_node, "pci-domain");
>> +       if (domain == -ENODEV)
>> +               domain = domain_nr++;
>> +
>> +       err = of_pci_parse_bus_range(parent->of_node, &bus_range);
>> +       if (err) {
>> +               dev_info(parent, "No bus range for %s, using default
>> [0-255]\n",
>> +                       parent->of_node->full_name);
>> +               bus_range.start = 0;
>> +               bus_range.end = 255;
>> +               bus_range.flags = IORESOURCE_BUS;
>> +       }
>> +       busno = bus_range.start;
>> +       pci_add_resource(&res, &bus_range);
>> +
>> +       /* now parse the rest of host bridge bus ranges */
>> +       if (pci_host_bridge_of_get_ranges(parent->of_node, &res,
>> &io_base))
>> +               return NULL;
>> +
>> +       /* then create the root bus */
>> +       root_bus = pci_create_root_bus_in_domain(parent, domain, busno,
>> +                                               ops, host_data, &res);
>> +       if (!root_bus)
>> +               return NULL;
>> +
>> +       bridge = to_pci_host_bridge(root_bus->bridge);
>> +       bridge->io_base = io_base;
>> +
>> +       return bridge;
>> +}
>> +EXPORT_SYMBOL_GPL(of_create_pci_host_bridge);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 1eed009..0c5e269 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -395,6 +395,7 @@ struct pci_host_bridge {
>>         struct device dev;
>>         struct pci_bus *bus;            /* root bus */
>>         int domain_nr;
>> +       resource_size_t io_base;        /* physical address for the start
>> of I/O area */
>>         struct list_head windows;       /* pci_host_bridge_windows */
>>         void (*release_fn)(struct pci_host_bridge *);
>>         void *release_data;
>> @@ -1786,11 +1787,23 @@ static inline struct device_node
>> *pci_bus_to_OF_node(struct pci_bus *bus)
>>         return bus ? bus->dev.of_node : NULL;
>>  }
>>
>> +struct pci_host_bridge *
>> +of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops,
>> +                       void *host_data);
>> +
>> +void pcibios_fixup_bridge_ranges(struct list_head *resources);
>>  #else /* CONFIG_OF */
>>  static inline void pci_set_of_node(struct pci_dev *dev) { }
>>  static inline void pci_release_of_node(struct pci_dev *dev) { }
>>  static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
>>  static inline void pci_release_bus_of_node(struct pci_bus *bus) { }
>> +
>> +static inline struct pci_host_bridge *
>> +pci_host_bridge_of_init(struct device *parent, struct pci_ops *ops,
>> +                       void *host_data)
>> +{
>> +       return NULL;
>> +}
>>  #endif  /* CONFIG_OF */
>>
>>  #ifdef CONFIG_EEH
>> --
>> 1.9.0
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 5/5] pci: Add support for creating a generic host_bridge from device tree
  2014-03-01  2:07     ` Tanmay Inamdar
@ 2014-03-01  2:39       ` Liviu Dudau
  2014-03-01  8:30         ` Tanmay Inamdar
  0 siblings, 1 reply; 9+ messages in thread
From: Liviu Dudau @ 2014-03-01  2:39 UTC (permalink / raw)
  To: Tanmay Inamdar
  Cc: Liviu Dudau, linux-pci, Bjorn Helgaas, Catalin Marinas,
	Will Deacon, linaro-kernel, Benjamin Herrenschmidt, LKML, LAKML,
	devicetree, patches

On Fri, Feb 28, 2014 at 06:07:05PM -0800, Tanmay Inamdar wrote:
> Earlier email did not deliver to mailing lists because of plain text
> setting problem on my side. Apologies for spamming. Sending it again.
> 
> Hello Liviu,
> 

Hello Tanmay,

> While porting X-Gene PCIe driver to v2 series, following problems were observed.

Thanks for trying it out.

> 
> 1. In 'of_create_pci_host_bridge' function, bus_range is defined
> locally. So, while walking over list of resources in bridge->windows
> later, during X-Gene controller related setup, garbage values are
> found in the resource. Please allocate it dynamically.

Bah, sorry for that. Will fix.

> 
> 2. 'domain_nr' problem is partially solved. There are still some
> places where functions are getting invalid domain_nr.  For example,
> 'pci_alloc_child_bus' tries to get domain_nr when bridge is not
> assigned to bus. You may want to look for all the places where
> pci_domain_nr is used. Please see below dump -->
> 
> pci 0001:00:00.0: scanning [bus 00-00] behind bridge, pass 1
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at
> /home/tinamdar/work/open-source/linux/fs/sysfs/dir.c:52
> sysfs_warn_dup+0x80/0xc0()
> sysfs: cannot create duplicate filename '/class/pci_bus/0000:01'
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.0-rc4+ #37
> Call trace:
> [<ffffffc000088140>] dump_backtrace+0x0/0x140
> [<ffffffc000088294>] show_stack+0x14/0x20
> [<ffffffc0004f64b0>] dump_stack+0x78/0xc4
> [<ffffffc000096c88>] warn_slowpath_common+0x88/0xc0
> [<ffffffc000096d10>] warn_slowpath_fmt+0x50/0x60
> [<ffffffc0001b8260>] sysfs_warn_dup+0x80/0xc0
> [<ffffffc0001b8718>] sysfs_do_create_link_sd.isra.2+0xf8/0x100
> [<ffffffc0001b8740>] sysfs_create_link+0x20/0x40
> [<ffffffc000322f5c>] device_add+0x41c/0x520
> [<ffffffc00032307c>] device_register+0x1c/0x40
> [<ffffffc0004f1924>] pci_add_new_bus+0x284/0x380
> [<ffffffc0002c51a0>] pci_scan_bridge+0x4e0/0x540
> [<ffffffc0002c52b4>] pci_scan_child_bus+0xb4/0x140
> [<ffffffc0004f1a34>] pci_rescan_bus+0x14/0x40
> [<ffffffc0006a12d4>] xgene_pcie_probe_bridge+0x688/0x750
> [<ffffffc000327c64>] platform_drv_probe+0x24/0x60
> [<ffffffc000325d74>] really_probe+0xf4/0x220
> [<ffffffc000325fc4>] __driver_attach+0xa4/0xc0
> [<ffffffc000323e18>] bus_for_each_dev+0x58/0xa0
> [<ffffffc000325800>] driver_attach+0x20/0x40
> [<ffffffc0003253d0>] bus_add_driver+0x150/0x220
> [<ffffffc000326780>] driver_register+0x60/0x120
> [<ffffffc000327c20>] __platform_driver_register+0x60/0x80
> [<ffffffc0006a0c44>] xgene_pcie_driver_init+0x18/0x20
> [<ffffffc0000814c4>] do_one_initcall+0xe4/0x160

do you have your xgene_pcie_driver_init being called out of some subsys_initcall?
If so, remove it and let the generic DT parsing code match your driver. The
bridge should've been associated with the root bus by the time the child busses
are scanned and allocated, unless I'm missing something obvious.

Also, can you share your version of your driver with me?

Best regards,
Liviu

> [<ffffffc00068c938>] kernel_init_freeable+0x138/0x1d8
> [<ffffffc0004f06b0>] kernel_init+0x10/0xe0
> ---[ end trace 53db1c3a7fbdeb88 ]---
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at
> /home/tinamdar/work/open-source/linux/drivers/pci/probe.c:711
> pci_add_new_bus+0x36c/0x380()
> 
> Thanks,
> Tanmay
> 
> On Fri, Feb 28, 2014 at 6:01 PM, Tanmay Inamdar <tinamdar@apm.com> wrote:
> > Hello Liviu,
> >
> > While porting X-Gene PCIe driver to v2 series, following problems were
> > observed.
> >
> > 1. In 'of_create_pci_host_bridge' function, bus_range is defined locally.
> > So, while walking over list of resources in bridge->windows later, during
> > X-Gene controller related setup, garbage values are found in the resource.
> > Please allocate it dynamically.
> >
> > 2. 'domain_nr' problem is partially solved. There are still some places
> > where functions are getting invalid domain_nr.  For example,
> > 'pci_alloc_child_bus' tries to get domain_nr when bridge is not assigned to
> > bus. You may want to look for all the places where pci_domain_nr is used.
> > Please see below dump -->
> >
> > pci 0001:00:00.0: scanning [bus 00-00] behind bridge, pass 1
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 1 at
> > /home/tinamdar/work/open-source/linux/fs/sysfs/dir.c:52
> > sysfs_warn_dup+0x80/0xc0()
> > sysfs: cannot create duplicate filename '/class/pci_bus/0000:01'
> > Modules linked in:
> > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.0-rc4+ #37
> > Call trace:
> > [<ffffffc000088140>] dump_backtrace+0x0/0x140
> > [<ffffffc000088294>] show_stack+0x14/0x20
> > [<ffffffc0004f64b0>] dump_stack+0x78/0xc4
> > [<ffffffc000096c88>] warn_slowpath_common+0x88/0xc0
> > [<ffffffc000096d10>] warn_slowpath_fmt+0x50/0x60
> > [<ffffffc0001b8260>] sysfs_warn_dup+0x80/0xc0
> > [<ffffffc0001b8718>] sysfs_do_create_link_sd.isra.2+0xf8/0x100
> > [<ffffffc0001b8740>] sysfs_create_link+0x20/0x40
> > [<ffffffc000322f5c>] device_add+0x41c/0x520
> > [<ffffffc00032307c>] device_register+0x1c/0x40
> > [<ffffffc0004f1924>] pci_add_new_bus+0x284/0x380
> > [<ffffffc0002c51a0>] pci_scan_bridge+0x4e0/0x540
> > [<ffffffc0002c52b4>] pci_scan_child_bus+0xb4/0x140
> > [<ffffffc0004f1a34>] pci_rescan_bus+0x14/0x40
> > [<ffffffc0006a12d4>] xgene_pcie_probe_bridge+0x688/0x750
> > [<ffffffc000327c64>] platform_drv_probe+0x24/0x60
> > [<ffffffc000325d74>] really_probe+0xf4/0x220
> > [<ffffffc000325fc4>] __driver_attach+0xa4/0xc0
> > [<ffffffc000323e18>] bus_for_each_dev+0x58/0xa0
> > [<ffffffc000325800>] driver_attach+0x20/0x40
> > [<ffffffc0003253d0>] bus_add_driver+0x150/0x220
> > [<ffffffc000326780>] driver_register+0x60/0x120
> > [<ffffffc000327c20>] __platform_driver_register+0x60/0x80
> > [<ffffffc0006a0c44>] xgene_pcie_driver_init+0x18/0x20
> > [<ffffffc0000814c4>] do_one_initcall+0xe4/0x160
> > [<ffffffc00068c938>] kernel_init_freeable+0x138/0x1d8
> > [<ffffffc0004f06b0>] kernel_init+0x10/0xe0
> > ---[ end trace 53db1c3a7fbdeb88 ]---
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 1 at
> > /home/tinamdar/work/open-source/linux/drivers/pci/probe.c:711
> > pci_add_new_bus+0x36c/0x380()
> >
> > Thanks,
> > Tanmay
> >
> >
> >
> > On Fri, Feb 28, 2014 at 5:08 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> >>
> >> Several platforms use a rather generic version of parsing
> >> the device tree to find the host bridge ranges. Move the common code
> >> into the generic PCI code and use it to create a pci_host_bridge
> >> structure that can be used by arch code.
> >>
> >> Based on early attempts by Andrew Murray to unify the code.
> >> Used powerpc and microblaze PCI code as starting point.
> >>
> >> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> >>
> >> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> >> index 06ace62..feb8436 100644
> >> --- a/drivers/pci/host-bridge.c
> >> +++ b/drivers/pci/host-bridge.c
> >> @@ -6,9 +6,13 @@
> >>  #include <linux/init.h>
> >>  #include <linux/pci.h>
> >>  #include <linux/module.h>
> >> +#include <linux/of_address.h>
> >> +#include <linux/of_pci.h>
> >>
> >>  #include "pci.h"
> >>
> >> +static int domain_nr;
> >> +
> >>  static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
> >>  {
> >>         while (bus->parent)
> >> @@ -91,3 +95,133 @@ void pcibios_bus_to_resource(struct pci_bus *bus,
> >> struct resource *res,
> >>         res->end = region->end + offset;
> >>  }
> >>  EXPORT_SYMBOL(pcibios_bus_to_resource);
> >> +
> >> +/**
> >> + * pci_host_bridge_of_get_ranges - Parse PCI host bridge resources from
> >> DT
> >> + * @dev: device node of the host bridge having the range property
> >> + * @resources: list where the range of resources will be added after DT
> >> parsing
> >> + * @io_base: pointer to a variable that will contain the physical address
> >> for
> >> + * the start of the I/O range.
> >> + *
> >> + * If this function returns an error then the @resources list will be
> >> freed.
> >> + *
> >> + * This function will parse the "ranges" property of a PCI host bridge
> >> device
> >> + * node and setup the resource mapping based on its content. It is
> >> expected
> >> + * that the property conforms with the Power ePAPR document.
> >> + *
> >> + * Each architecture is then offered the chance of applying their own
> >> + * filtering of pci_host_bridge_windows based on their own restrictions
> >> by
> >> + * calling pcibios_fixup_bridge_ranges(). The filtered list of windows
> >> + * can then be used when creating a pci_host_bridge structure.
> >> + */
> >> +static int pci_host_bridge_of_get_ranges(struct device_node *dev,
> >> +               struct list_head *resources, resource_size_t *io_base)
> >> +{
> >> +       struct resource *res;
> >> +       struct of_pci_range range;
> >> +       struct of_pci_range_parser parser;
> >> +       int err;
> >> +
> >> +       pr_info("PCI host bridge %s ranges:\n", dev->full_name);
> >> +
> >> +       /* Check for ranges property */
> >> +       err = of_pci_range_parser_init(&parser, dev);
> >> +       if (err)
> >> +               return err;
> >> +
> >> +       pr_debug("Parsing ranges property...\n");
> >> +       for_each_of_pci_range(&parser, &range) {
> >> +               /* Read next ranges element */
> >> +               pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
> >> +                               range.pci_space, range.pci_addr);
> >> +               pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
> >> +                                       range.cpu_addr, range.size);
> >> +
> >> +               /*
> >> +                * If we failed translation or got a zero-sized region
> >> +                * then skip this range
> >> +                */
> >> +               if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
> >> +                       continue;
> >> +
> >> +               res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> >> +               if (!res) {
> >> +                       err = -ENOMEM;
> >> +                       goto bridge_ranges_nomem;
> >> +               }
> >> +
> >> +               of_pci_range_to_resource(&range, dev, res);
> >> +
> >> +               if (resource_type(res) == IORESOURCE_IO)
> >> +                       *io_base = range.cpu_addr;
> >> +
> >> +               pci_add_resource_offset(resources, res,
> >> +                               res->start - range.pci_addr);
> >> +       }
> >> +
> >> +       /* Apply architecture specific fixups for the ranges */
> >> +       pcibios_fixup_bridge_ranges(resources);
> >> +
> >> +       return 0;
> >> +
> >> +bridge_ranges_nomem:
> >> +       pci_free_resource_list(resources);
> >> +       return err;
> >> +}
> >> +
> >> +/**
> >> + * of_create_pci_host_bridge - Create a PCI host bridge structure using
> >> + * information passed in the DT.
> >> + * @parent: device owning this host bridge
> >> + * @ops: pci_ops associated with the host controller
> >> + * @host_data: opaque data structure used by the host controller.
> >> + *
> >> + * returns a pointer to the newly created pci_host_bridge structure, or
> >> + * NULL if the call failed.
> >> + *
> >> + * This function will try to obtain the host bridge domain number by
> >> + * using of_alias_get_id() call with "pci-domain" as a stem. If that
> >> + * fails, a local allocator will be used that will put each host bridge
> >> + * in a new domain.
> >> + */
> >> +struct pci_host_bridge *
> >> +of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops,
> >> void *host_data)
> >> +{
> >> +       int err, domain, busno;
> >> +       struct resource bus_range;
> >> +       struct pci_bus *root_bus;
> >> +       struct pci_host_bridge *bridge;
> >> +       resource_size_t io_base;
> >> +       LIST_HEAD(res);
> >> +
> >> +       domain = of_alias_get_id(parent->of_node, "pci-domain");
> >> +       if (domain == -ENODEV)
> >> +               domain = domain_nr++;
> >> +
> >> +       err = of_pci_parse_bus_range(parent->of_node, &bus_range);
> >> +       if (err) {
> >> +               dev_info(parent, "No bus range for %s, using default
> >> [0-255]\n",
> >> +                       parent->of_node->full_name);
> >> +               bus_range.start = 0;
> >> +               bus_range.end = 255;
> >> +               bus_range.flags = IORESOURCE_BUS;
> >> +       }
> >> +       busno = bus_range.start;
> >> +       pci_add_resource(&res, &bus_range);
> >> +
> >> +       /* now parse the rest of host bridge bus ranges */
> >> +       if (pci_host_bridge_of_get_ranges(parent->of_node, &res,
> >> &io_base))
> >> +               return NULL;
> >> +
> >> +       /* then create the root bus */
> >> +       root_bus = pci_create_root_bus_in_domain(parent, domain, busno,
> >> +                                               ops, host_data, &res);
> >> +       if (!root_bus)
> >> +               return NULL;
> >> +
> >> +       bridge = to_pci_host_bridge(root_bus->bridge);
> >> +       bridge->io_base = io_base;
> >> +
> >> +       return bridge;
> >> +}
> >> +EXPORT_SYMBOL_GPL(of_create_pci_host_bridge);
> >> diff --git a/include/linux/pci.h b/include/linux/pci.h
> >> index 1eed009..0c5e269 100644
> >> --- a/include/linux/pci.h
> >> +++ b/include/linux/pci.h
> >> @@ -395,6 +395,7 @@ struct pci_host_bridge {
> >>         struct device dev;
> >>         struct pci_bus *bus;            /* root bus */
> >>         int domain_nr;
> >> +       resource_size_t io_base;        /* physical address for the start
> >> of I/O area */
> >>         struct list_head windows;       /* pci_host_bridge_windows */
> >>         void (*release_fn)(struct pci_host_bridge *);
> >>         void *release_data;
> >> @@ -1786,11 +1787,23 @@ static inline struct device_node
> >> *pci_bus_to_OF_node(struct pci_bus *bus)
> >>         return bus ? bus->dev.of_node : NULL;
> >>  }
> >>
> >> +struct pci_host_bridge *
> >> +of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops,
> >> +                       void *host_data);
> >> +
> >> +void pcibios_fixup_bridge_ranges(struct list_head *resources);
> >>  #else /* CONFIG_OF */
> >>  static inline void pci_set_of_node(struct pci_dev *dev) { }
> >>  static inline void pci_release_of_node(struct pci_dev *dev) { }
> >>  static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
> >>  static inline void pci_release_bus_of_node(struct pci_bus *bus) { }
> >> +
> >> +static inline struct pci_host_bridge *
> >> +pci_host_bridge_of_init(struct device *parent, struct pci_ops *ops,
> >> +                       void *host_data)
> >> +{
> >> +       return NULL;
> >> +}
> >>  #endif  /* CONFIG_OF */
> >>
> >>  #ifdef CONFIG_EEH
> >> --
> >> 1.9.0
> >>
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 5/5] pci: Add support for creating a generic host_bridge from device tree
  2014-03-01  2:39       ` Liviu Dudau
@ 2014-03-01  8:30         ` Tanmay Inamdar
  0 siblings, 0 replies; 9+ messages in thread
From: Tanmay Inamdar @ 2014-03-01  8:30 UTC (permalink / raw)
  To: Tanmay Inamdar, Liviu Dudau, linux-pci, Bjorn Helgaas,
	Catalin Marinas, Will Deacon, linaro-kernel,
	Benjamin Herrenschmidt, LKML, LAKML, devicetree, patches

Hello,

On Fri, Feb 28, 2014 at 6:39 PM, Liviu Dudau <liviu@dudau.co.uk> wrote:
> On Fri, Feb 28, 2014 at 06:07:05PM -0800, Tanmay Inamdar wrote:
>> Earlier email did not deliver to mailing lists because of plain text
>> setting problem on my side. Apologies for spamming. Sending it again.
>>
>> Hello Liviu,
>>
>
> Hello Tanmay,
>
>> While porting X-Gene PCIe driver to v2 series, following problems were observed.
>
> Thanks for trying it out.
>
>>
>> 1. In 'of_create_pci_host_bridge' function, bus_range is defined
>> locally. So, while walking over list of resources in bridge->windows
>> later, during X-Gene controller related setup, garbage values are
>> found in the resource. Please allocate it dynamically.
>
> Bah, sorry for that. Will fix.
>
>>
>> 2. 'domain_nr' problem is partially solved. There are still some
>> places where functions are getting invalid domain_nr.  For example,
>> 'pci_alloc_child_bus' tries to get domain_nr when bridge is not
>> assigned to bus. You may want to look for all the places where
>> pci_domain_nr is used. Please see below dump -->
>>
>> pci 0001:00:00.0: scanning [bus 00-00] behind bridge, pass 1
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 1 at
>> /home/tinamdar/work/open-source/linux/fs/sysfs/dir.c:52
>> sysfs_warn_dup+0x80/0xc0()
>> sysfs: cannot create duplicate filename '/class/pci_bus/0000:01'
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.0-rc4+ #37
>> Call trace:
>> [<ffffffc000088140>] dump_backtrace+0x0/0x140
>> [<ffffffc000088294>] show_stack+0x14/0x20
>> [<ffffffc0004f64b0>] dump_stack+0x78/0xc4
>> [<ffffffc000096c88>] warn_slowpath_common+0x88/0xc0
>> [<ffffffc000096d10>] warn_slowpath_fmt+0x50/0x60
>> [<ffffffc0001b8260>] sysfs_warn_dup+0x80/0xc0
>> [<ffffffc0001b8718>] sysfs_do_create_link_sd.isra.2+0xf8/0x100
>> [<ffffffc0001b8740>] sysfs_create_link+0x20/0x40
>> [<ffffffc000322f5c>] device_add+0x41c/0x520
>> [<ffffffc00032307c>] device_register+0x1c/0x40
>> [<ffffffc0004f1924>] pci_add_new_bus+0x284/0x380
>> [<ffffffc0002c51a0>] pci_scan_bridge+0x4e0/0x540
>> [<ffffffc0002c52b4>] pci_scan_child_bus+0xb4/0x140
>> [<ffffffc0004f1a34>] pci_rescan_bus+0x14/0x40
>> [<ffffffc0006a12d4>] xgene_pcie_probe_bridge+0x688/0x750
>> [<ffffffc000327c64>] platform_drv_probe+0x24/0x60
>> [<ffffffc000325d74>] really_probe+0xf4/0x220
>> [<ffffffc000325fc4>] __driver_attach+0xa4/0xc0
>> [<ffffffc000323e18>] bus_for_each_dev+0x58/0xa0
>> [<ffffffc000325800>] driver_attach+0x20/0x40
>> [<ffffffc0003253d0>] bus_add_driver+0x150/0x220
>> [<ffffffc000326780>] driver_register+0x60/0x120
>> [<ffffffc000327c20>] __platform_driver_register+0x60/0x80
>> [<ffffffc0006a0c44>] xgene_pcie_driver_init+0x18/0x20
>> [<ffffffc0000814c4>] do_one_initcall+0xe4/0x160
>
> do you have your xgene_pcie_driver_init being called out of some subsys_initcall?
> If so, remove it and let the generic DT parsing code match your driver. The

I am using 'module_platform_driver' which is nothing but module_init.

> bridge should've been associated with the root bus by the time the child busses
> are scanned and allocated, unless I'm missing something obvious.
>

If you look at the implementation of 'pci_alloc_child_bus', you will
find that 'dev_set_name(&child->dev, "%04x:%02x",
pci_domain_nr(child), busnr);' is done before 'child->bridge =
get_device(&bridge->dev);' Hence pcie_domain_nr finds NULL bridge and
returns 0. That might be the problem. Please correct me if I am wrong.

> Also, can you share your version of your driver with me?

Sure. I am in the process of sending out RFC version. I will post it
in a day or two.

>
> Best regards,
> Liviu
>
>> [<ffffffc00068c938>] kernel_init_freeable+0x138/0x1d8
>> [<ffffffc0004f06b0>] kernel_init+0x10/0xe0
>> ---[ end trace 53db1c3a7fbdeb88 ]---
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 1 at
>> /home/tinamdar/work/open-source/linux/drivers/pci/probe.c:711
>> pci_add_new_bus+0x36c/0x380()
>>
>> Thanks,
>> Tanmay
>>
>> On Fri, Feb 28, 2014 at 6:01 PM, Tanmay Inamdar <tinamdar@apm.com> wrote:
>> > Hello Liviu,
>> >
>> > While porting X-Gene PCIe driver to v2 series, following problems were
>> > observed.
>> >
>> > 1. In 'of_create_pci_host_bridge' function, bus_range is defined locally.
>> > So, while walking over list of resources in bridge->windows later, during
>> > X-Gene controller related setup, garbage values are found in the resource.
>> > Please allocate it dynamically.
>> >
>> > 2. 'domain_nr' problem is partially solved. There are still some places
>> > where functions are getting invalid domain_nr.  For example,
>> > 'pci_alloc_child_bus' tries to get domain_nr when bridge is not assigned to
>> > bus. You may want to look for all the places where pci_domain_nr is used.
>> > Please see below dump -->
>> >
>> > pci 0001:00:00.0: scanning [bus 00-00] behind bridge, pass 1
>> > ------------[ cut here ]------------
>> > WARNING: CPU: 0 PID: 1 at
>> > /home/tinamdar/work/open-source/linux/fs/sysfs/dir.c:52
>> > sysfs_warn_dup+0x80/0xc0()
>> > sysfs: cannot create duplicate filename '/class/pci_bus/0000:01'
>> > Modules linked in:
>> > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.0-rc4+ #37
>> > Call trace:
>> > [<ffffffc000088140>] dump_backtrace+0x0/0x140
>> > [<ffffffc000088294>] show_stack+0x14/0x20
>> > [<ffffffc0004f64b0>] dump_stack+0x78/0xc4
>> > [<ffffffc000096c88>] warn_slowpath_common+0x88/0xc0
>> > [<ffffffc000096d10>] warn_slowpath_fmt+0x50/0x60
>> > [<ffffffc0001b8260>] sysfs_warn_dup+0x80/0xc0
>> > [<ffffffc0001b8718>] sysfs_do_create_link_sd.isra.2+0xf8/0x100
>> > [<ffffffc0001b8740>] sysfs_create_link+0x20/0x40
>> > [<ffffffc000322f5c>] device_add+0x41c/0x520
>> > [<ffffffc00032307c>] device_register+0x1c/0x40
>> > [<ffffffc0004f1924>] pci_add_new_bus+0x284/0x380
>> > [<ffffffc0002c51a0>] pci_scan_bridge+0x4e0/0x540
>> > [<ffffffc0002c52b4>] pci_scan_child_bus+0xb4/0x140
>> > [<ffffffc0004f1a34>] pci_rescan_bus+0x14/0x40
>> > [<ffffffc0006a12d4>] xgene_pcie_probe_bridge+0x688/0x750
>> > [<ffffffc000327c64>] platform_drv_probe+0x24/0x60
>> > [<ffffffc000325d74>] really_probe+0xf4/0x220
>> > [<ffffffc000325fc4>] __driver_attach+0xa4/0xc0
>> > [<ffffffc000323e18>] bus_for_each_dev+0x58/0xa0
>> > [<ffffffc000325800>] driver_attach+0x20/0x40
>> > [<ffffffc0003253d0>] bus_add_driver+0x150/0x220
>> > [<ffffffc000326780>] driver_register+0x60/0x120
>> > [<ffffffc000327c20>] __platform_driver_register+0x60/0x80
>> > [<ffffffc0006a0c44>] xgene_pcie_driver_init+0x18/0x20
>> > [<ffffffc0000814c4>] do_one_initcall+0xe4/0x160
>> > [<ffffffc00068c938>] kernel_init_freeable+0x138/0x1d8
>> > [<ffffffc0004f06b0>] kernel_init+0x10/0xe0
>> > ---[ end trace 53db1c3a7fbdeb88 ]---
>> > ------------[ cut here ]------------
>> > WARNING: CPU: 0 PID: 1 at
>> > /home/tinamdar/work/open-source/linux/drivers/pci/probe.c:711
>> > pci_add_new_bus+0x36c/0x380()
>> >
>> > Thanks,
>> > Tanmay
>> >
>> >
>> >
>> > On Fri, Feb 28, 2014 at 5:08 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> >>
>> >> Several platforms use a rather generic version of parsing
>> >> the device tree to find the host bridge ranges. Move the common code
>> >> into the generic PCI code and use it to create a pci_host_bridge
>> >> structure that can be used by arch code.
>> >>
>> >> Based on early attempts by Andrew Murray to unify the code.
>> >> Used powerpc and microblaze PCI code as starting point.
>> >>
>> >> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>> >>
>> >> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>> >> index 06ace62..feb8436 100644
>> >> --- a/drivers/pci/host-bridge.c
>> >> +++ b/drivers/pci/host-bridge.c
>> >> @@ -6,9 +6,13 @@
>> >>  #include <linux/init.h>
>> >>  #include <linux/pci.h>
>> >>  #include <linux/module.h>
>> >> +#include <linux/of_address.h>
>> >> +#include <linux/of_pci.h>
>> >>
>> >>  #include "pci.h"
>> >>
>> >> +static int domain_nr;
>> >> +
>> >>  static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
>> >>  {
>> >>         while (bus->parent)
>> >> @@ -91,3 +95,133 @@ void pcibios_bus_to_resource(struct pci_bus *bus,
>> >> struct resource *res,
>> >>         res->end = region->end + offset;
>> >>  }
>> >>  EXPORT_SYMBOL(pcibios_bus_to_resource);
>> >> +
>> >> +/**
>> >> + * pci_host_bridge_of_get_ranges - Parse PCI host bridge resources from
>> >> DT
>> >> + * @dev: device node of the host bridge having the range property
>> >> + * @resources: list where the range of resources will be added after DT
>> >> parsing
>> >> + * @io_base: pointer to a variable that will contain the physical address
>> >> for
>> >> + * the start of the I/O range.
>> >> + *
>> >> + * If this function returns an error then the @resources list will be
>> >> freed.
>> >> + *
>> >> + * This function will parse the "ranges" property of a PCI host bridge
>> >> device
>> >> + * node and setup the resource mapping based on its content. It is
>> >> expected
>> >> + * that the property conforms with the Power ePAPR document.
>> >> + *
>> >> + * Each architecture is then offered the chance of applying their own
>> >> + * filtering of pci_host_bridge_windows based on their own restrictions
>> >> by
>> >> + * calling pcibios_fixup_bridge_ranges(). The filtered list of windows
>> >> + * can then be used when creating a pci_host_bridge structure.
>> >> + */
>> >> +static int pci_host_bridge_of_get_ranges(struct device_node *dev,
>> >> +               struct list_head *resources, resource_size_t *io_base)
>> >> +{
>> >> +       struct resource *res;
>> >> +       struct of_pci_range range;
>> >> +       struct of_pci_range_parser parser;
>> >> +       int err;
>> >> +
>> >> +       pr_info("PCI host bridge %s ranges:\n", dev->full_name);
>> >> +
>> >> +       /* Check for ranges property */
>> >> +       err = of_pci_range_parser_init(&parser, dev);
>> >> +       if (err)
>> >> +               return err;
>> >> +
>> >> +       pr_debug("Parsing ranges property...\n");
>> >> +       for_each_of_pci_range(&parser, &range) {
>> >> +               /* Read next ranges element */
>> >> +               pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
>> >> +                               range.pci_space, range.pci_addr);
>> >> +               pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
>> >> +                                       range.cpu_addr, range.size);
>> >> +
>> >> +               /*
>> >> +                * If we failed translation or got a zero-sized region
>> >> +                * then skip this range
>> >> +                */
>> >> +               if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
>> >> +                       continue;
>> >> +
>> >> +               res = kzalloc(sizeof(struct resource), GFP_KERNEL);
>> >> +               if (!res) {
>> >> +                       err = -ENOMEM;
>> >> +                       goto bridge_ranges_nomem;
>> >> +               }
>> >> +
>> >> +               of_pci_range_to_resource(&range, dev, res);
>> >> +
>> >> +               if (resource_type(res) == IORESOURCE_IO)
>> >> +                       *io_base = range.cpu_addr;
>> >> +
>> >> +               pci_add_resource_offset(resources, res,
>> >> +                               res->start - range.pci_addr);
>> >> +       }
>> >> +
>> >> +       /* Apply architecture specific fixups for the ranges */
>> >> +       pcibios_fixup_bridge_ranges(resources);
>> >> +
>> >> +       return 0;
>> >> +
>> >> +bridge_ranges_nomem:
>> >> +       pci_free_resource_list(resources);
>> >> +       return err;
>> >> +}
>> >> +
>> >> +/**
>> >> + * of_create_pci_host_bridge - Create a PCI host bridge structure using
>> >> + * information passed in the DT.
>> >> + * @parent: device owning this host bridge
>> >> + * @ops: pci_ops associated with the host controller
>> >> + * @host_data: opaque data structure used by the host controller.
>> >> + *
>> >> + * returns a pointer to the newly created pci_host_bridge structure, or
>> >> + * NULL if the call failed.
>> >> + *
>> >> + * This function will try to obtain the host bridge domain number by
>> >> + * using of_alias_get_id() call with "pci-domain" as a stem. If that
>> >> + * fails, a local allocator will be used that will put each host bridge
>> >> + * in a new domain.
>> >> + */
>> >> +struct pci_host_bridge *
>> >> +of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops,
>> >> void *host_data)
>> >> +{
>> >> +       int err, domain, busno;
>> >> +       struct resource bus_range;
>> >> +       struct pci_bus *root_bus;
>> >> +       struct pci_host_bridge *bridge;
>> >> +       resource_size_t io_base;
>> >> +       LIST_HEAD(res);
>> >> +
>> >> +       domain = of_alias_get_id(parent->of_node, "pci-domain");
>> >> +       if (domain == -ENODEV)
>> >> +               domain = domain_nr++;
>> >> +
>> >> +       err = of_pci_parse_bus_range(parent->of_node, &bus_range);
>> >> +       if (err) {
>> >> +               dev_info(parent, "No bus range for %s, using default
>> >> [0-255]\n",
>> >> +                       parent->of_node->full_name);
>> >> +               bus_range.start = 0;
>> >> +               bus_range.end = 255;
>> >> +               bus_range.flags = IORESOURCE_BUS;
>> >> +       }
>> >> +       busno = bus_range.start;
>> >> +       pci_add_resource(&res, &bus_range);
>> >> +
>> >> +       /* now parse the rest of host bridge bus ranges */
>> >> +       if (pci_host_bridge_of_get_ranges(parent->of_node, &res,
>> >> &io_base))
>> >> +               return NULL;
>> >> +
>> >> +       /* then create the root bus */
>> >> +       root_bus = pci_create_root_bus_in_domain(parent, domain, busno,
>> >> +                                               ops, host_data, &res);
>> >> +       if (!root_bus)
>> >> +               return NULL;
>> >> +
>> >> +       bridge = to_pci_host_bridge(root_bus->bridge);
>> >> +       bridge->io_base = io_base;
>> >> +
>> >> +       return bridge;
>> >> +}
>> >> +EXPORT_SYMBOL_GPL(of_create_pci_host_bridge);
>> >> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> >> index 1eed009..0c5e269 100644
>> >> --- a/include/linux/pci.h
>> >> +++ b/include/linux/pci.h
>> >> @@ -395,6 +395,7 @@ struct pci_host_bridge {
>> >>         struct device dev;
>> >>         struct pci_bus *bus;            /* root bus */
>> >>         int domain_nr;
>> >> +       resource_size_t io_base;        /* physical address for the start
>> >> of I/O area */
>> >>         struct list_head windows;       /* pci_host_bridge_windows */
>> >>         void (*release_fn)(struct pci_host_bridge *);
>> >>         void *release_data;
>> >> @@ -1786,11 +1787,23 @@ static inline struct device_node
>> >> *pci_bus_to_OF_node(struct pci_bus *bus)
>> >>         return bus ? bus->dev.of_node : NULL;
>> >>  }
>> >>
>> >> +struct pci_host_bridge *
>> >> +of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops,
>> >> +                       void *host_data);
>> >> +
>> >> +void pcibios_fixup_bridge_ranges(struct list_head *resources);
>> >>  #else /* CONFIG_OF */
>> >>  static inline void pci_set_of_node(struct pci_dev *dev) { }
>> >>  static inline void pci_release_of_node(struct pci_dev *dev) { }
>> >>  static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
>> >>  static inline void pci_release_bus_of_node(struct pci_bus *bus) { }
>> >> +
>> >> +static inline struct pci_host_bridge *
>> >> +pci_host_bridge_of_init(struct device *parent, struct pci_ops *ops,
>> >> +                       void *host_data)
>> >> +{
>> >> +       return NULL;
>> >> +}
>> >>  #endif  /* CONFIG_OF */
>> >>
>> >>  #ifdef CONFIG_EEH
>> >> --
>> >> 1.9.0
>> >>
>> >>
>> >> _______________________________________________
>> >> linux-arm-kernel mailing list
>> >> linux-arm-kernel@lists.infradead.org
>> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>> >
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> --
> -------------------
>    .oooO
>    (   )
>     \ (  Oooo.
>      \_) (   )
>           ) /
>          (_/
>
>  One small step
>    for me ...
>
> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
> is for the sole use of the intended recipient(s) and contains information
> that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries.
> It is to be used solely for the purpose of furthering the parties' business relationship.
> All unauthorized review, use, disclosure or distribution is prohibited.
> If you are not the intended recipient, please contact the sender by reply e-mail
> and destroy all copies of the original message.
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-03-01  8:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-28 13:08 [PATCH v3 0/5] [RFC] Support for creating generic host_bridge from device tree Liviu Dudau
2014-02-28 13:08 ` [PATCH v3 1/5] pci: Introduce pci_register_io_range() helper function Liviu Dudau
2014-02-28 13:08 ` [PATCH v3 2/5] pci: OF: Fix the conversion of IO ranges into IO resources Liviu Dudau
2014-02-28 13:08 ` [PATCH v3 3/5] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus Liviu Dudau
2014-02-28 13:08 ` [PATCH v3 4/5] pci: Introduce a domain number for pci_host_bridge Liviu Dudau
2014-02-28 13:08 ` [PATCH v3 5/5] pci: Add support for creating a generic host_bridge from device tree Liviu Dudau
     [not found]   ` <CACoXjck59jA1Ub8mHB0zn16krCxagUgH8BmJVnLQ8KGpFg7WpA@mail.gmail.com>
2014-03-01  2:07     ` Tanmay Inamdar
2014-03-01  2:39       ` Liviu Dudau
2014-03-01  8:30         ` Tanmay Inamdar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).