[PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Liviu Dudau]

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

Changes from v4:
  - Export pci_find_host_bridge() to be used by arch code. There is scope for
    making the arch/arm64 version of pci_domain_nr the default weak implementation
    but that would double the size of this series in order to handle all #define
    versions of the pci_domain_nr() function, so I suggest keeping that for a separate
    cleanup series.

Changes from v3:
  - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
  - Fix the domain number used when creating child busses.
  - Changed domain number allocator to use atomic operations.
  - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
    and of_create_pci_host_bridge().

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.


v4 thread here: https://lkml.org/lkml/2014/3/3/301
v3 thread here: https://lkml.org/lkml/2014/2/28/216
v2 thread here: https://lkml.org/lkml/2014/2/27/245
v1 thread here: https://lkml.org/lkml/2014/2/3/380

Best regards,
Liviu

Liviu Dudau (7):
  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: Use parent domain number when allocating child busses.
  pci: Export find_pci_host_bridge() function.
  pci: Add support for creating a generic host_bridge from device tree

 drivers/of/address.c       |  39 +++++++++++++
 drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/pci/probe.c        |  72 +++++++++++++++--------
 include/linux/of_address.h |  14 +----
 include/linux/pci.h        |  17 ++++++
 5 files changed, 248 insertions(+), 36 deletions(-)

-- 
1.9.0

[PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Liviu Dudau]

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

Changes from v4:
  - Export pci_find_host_bridge() to be used by arch code. There is scope for
    making the arch/arm64 version of pci_domain_nr the default weak implementation
    but that would double the size of this series in order to handle all #define
    versions of the pci_domain_nr() function, so I suggest keeping that for a separate
    cleanup series.

Changes from v3:
  - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
  - Fix the domain number used when creating child busses.
  - Changed domain number allocator to use atomic operations.
  - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
    and of_create_pci_host_bridge().

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.


v4 thread here: https://lkml.org/lkml/2014/3/3/301
v3 thread here: https://lkml.org/lkml/2014/2/28/216
v2 thread here: https://lkml.org/lkml/2014/2/27/245
v1 thread here: https://lkml.org/lkml/2014/2/3/380

Best regards,
Liviu

Liviu Dudau (7):
  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: Use parent domain number when allocating child busses.
  pci: Export find_pci_host_bridge() function.
  pci: Add support for creating a generic host_bridge from device tree

 drivers/of/address.c       |  39 +++++++++++++
 drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/pci/probe.c        |  72 +++++++++++++++--------
 include/linux/of_address.h |  14 +----
 include/linux/pci.h        |  17 ++++++
 5 files changed, 248 insertions(+), 36 deletions(-)

-- 
1.9.0

[PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Liviu Dudau]

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

[PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Liviu Dudau]

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

[PATCH v5 2/7] pci: OF: Fix the conversion of IO ranges into IO resources.

[Liviu Dudau]

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

[PATCH v5 2/7] pci: OF: Fix the conversion of IO ranges into IO resources.

[Liviu Dudau]

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

[PATCH v5 3/7] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.

[Liviu Dudau]

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

[PATCH v5 3/7] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.

[Liviu Dudau]

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

[PATCH v5 4/7] pci: Introduce a domain number for pci_host_bridge.

[Liviu Dudau]

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>

---
Arnd, I know you have Acked the previous version of this patch, but now I have
added ERR_PTR() calls to it. Do I get to keep the Ack?

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 78ccba0..26237a0 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;
@@ -1728,14 +1729,15 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 
 	bridge = pci_alloc_host_bridge();
 	if (!bridge)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	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);
-		return NULL;
+		return ERR_PTR(error);
 	}
 
 	b = pci_alloc_bus();
@@ -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;
@@ -1818,7 +1820,27 @@ err_bus_out:
 	kfree(b);
 err_out:
 	kfree(bridge);
-	return NULL;
+	return ERR_PTR(error);
+}
+
+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);
+
+	b = pci_create_root_bus_in_domain(parent, domain_nr, bus,
+				ops, sysdata, resources);
+	if (IS_ERR(b))
+		return NULL;
+
+	return b;
 }
 
 int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
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

[PATCH v5 4/7] pci: Introduce a domain number for pci_host_bridge.

[Liviu Dudau]

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>

---
Arnd, I know you have Acked the previous version of this patch, but now I have
added ERR_PTR() calls to it. Do I get to keep the Ack?

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 78ccba0..26237a0 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;
@@ -1728,14 +1729,15 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 
 	bridge = pci_alloc_host_bridge();
 	if (!bridge)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	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);
-		return NULL;
+		return ERR_PTR(error);
 	}
 
 	b = pci_alloc_bus();
@@ -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;
@@ -1818,7 +1820,27 @@ err_bus_out:
 	kfree(b);
 err_out:
 	kfree(bridge);
-	return NULL;
+	return ERR_PTR(error);
+}
+
+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);
+
+	b = pci_create_root_bus_in_domain(parent, domain_nr, bus,
+				ops, sysdata, resources);
+	if (IS_ERR(b))
+		return NULL;
+
+	return b;
 }
 
 int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
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

[PATCH v5 5/7] pci: Use parent domain number when allocating child busses.

[Liviu Dudau]

pci_alloc_child_bus() uses the newly allocated child bus to figure
out the domain number that is going to use for setting the device
name. A better option is to use the parent bus domain number.

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

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 26237a0..a12cda5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -677,7 +677,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 	 * now as the parent is not properly set up yet.
 	 */
 	child->dev.class = &pcibus_class;
-	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
+	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(parent), busnr);
 
 	/*
 	 * Set up the primary, secondary and subordinate
-- 
1.9.0

[PATCH v5 5/7] pci: Use parent domain number when allocating child busses.

[Liviu Dudau]

pci_alloc_child_bus() uses the newly allocated child bus to figure
out the domain number that is going to use for setting the device
name. A better option is to use the parent bus domain number.

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

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 26237a0..a12cda5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -677,7 +677,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 	 * now as the parent is not properly set up yet.
 	 */
 	child->dev.class = &pcibus_class;
-	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
+	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(parent), busnr);
 
 	/*
 	 * Set up the primary, secondary and subordinate
-- 
1.9.0

[PATCH v5 5/7] pci: Use parent domain number when allocating child busses.

[Liviu Dudau]

pci_alloc_child_bus() uses the newly allocated child bus to figure
out the domain number that is going to use for setting the device
name. A better option is to use the parent bus domain number.

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

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 26237a0..a12cda5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -677,7 +677,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 	 * now as the parent is not properly set up yet.
 	 */
 	child->dev.class = &pcibus_class;
-	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
+	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(parent), busnr);
 
 	/*
 	 * Set up the primary, secondary and subordinate
-- 
1.9.0

[PATCH v5 6/7] pci: Export find_pci_host_bridge() function.

[Liviu Dudau]

This is a useful function and we should make it visible outside the
generic PCI code. Export it as a GPL symbol.

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..8708b652 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -17,12 +17,13 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
 	return bus;
 }
 
-static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
 {
 	struct pci_bus *root_bus = find_pci_root_bus(bus);
 
 	return to_pci_host_bridge(root_bus->bridge);
 }
+EXPORT_SYMBOL_GPL(find_pci_host_bridge);
 
 void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
 				 void (*release_fn)(struct pci_host_bridge *),
-- 
1.9.0

[PATCH v5 6/7] pci: Export find_pci_host_bridge() function.

[Liviu Dudau]

This is a useful function and we should make it visible outside the
generic PCI code. Export it as a GPL symbol.

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..8708b652 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -17,12 +17,13 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
 	return bus;
 }
 
-static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
 {
 	struct pci_bus *root_bus = find_pci_root_bus(bus);
 
 	return to_pci_host_bridge(root_bus->bridge);
 }
+EXPORT_SYMBOL_GPL(find_pci_host_bridge);
 
 void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
 				 void (*release_fn)(struct pci_host_bridge *),
-- 
1.9.0

[PATCH v5 7/7] pci: Add support for creating a generic host_bridge from device tree

[Liviu Dudau]

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 8708b652..800678a 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 atomic_t domain_nr = ATOMIC_INIT(-1);
+
 static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
 {
 	while (bus->parent)
@@ -92,3 +96,138 @@ 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);
+
+	bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
+	if (!bus_range)
+		return ERR_PTR(-ENOMEM);
+
+	domain = of_alias_get_id(parent->of_node, "pci-domain");
+	if (domain == -ENODEV)
+		domain = atomic_inc_return(&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 */
+	err = pci_host_bridge_of_get_ranges(parent->of_node, &res, &io_base);
+	if (err)
+		return ERR_PTR(err);
+
+	/* 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

[PATCH v5 7/7] pci: Add support for creating a generic host_bridge from device tree

[Liviu Dudau]

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 8708b652..800678a 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 atomic_t domain_nr = ATOMIC_INIT(-1);
+
 static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
 {
 	while (bus->parent)
@@ -92,3 +96,138 @@ 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);
+
+	bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
+	if (!bus_range)
+		return ERR_PTR(-ENOMEM);
+
+	domain = of_alias_get_id(parent->of_node, "pci-domain");
+	if (domain == -ENODEV)
+		domain = atomic_inc_return(&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 */
+	err = pci_host_bridge_of_get_ranges(parent->of_node, &res, &io_base);
+	if (err)
+		return ERR_PTR(err);
+
+	/* 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

Re: [PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Arnd Bergmann]

On Tuesday 04 March 2014, Liviu Dudau wrote:
> +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> +{
> +       return 0;
> +}
> +

How about returning an error here? You don't actually register the range.

	Arnd

[PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Arnd Bergmann]

On Tuesday 04 March 2014, Liviu Dudau wrote:
> +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> +{
> +       return 0;
> +}
> +

How about returning an error here? You don't actually register the range.

	Arnd

Re: [PATCH v5 7/7] pci: Add support for creating a generic host_bridge from device tree

[Jingoo Han]

On Wednesday, March 05, 2014 12:50 AM, Liviu Dudau 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 8708b652..800678a 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c

[.....]

> +		res = kzalloc(sizeof(struct resource), GFP_KERNEL);

It makes build error with exynos_defconfig. (ARM32)
Thus, 'slab.h' is necessary in order to fix the build error.

./drivers/pci/host-bridge.c
@@ -8,6 +8,7 @@
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/slab.h>

 #include "pci.h"


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

It also makes compile problem with exynos_defconfig as below:

drivers/built-in.o: In function `pci_host_bridge_of_get_ranges':
drivers/pci/host-bridge.c:157: undefined reference to `pcibios_fixup_bridge_ranges'

Best regards,
Jingoo Han

[PATCH v5 7/7] pci: Add support for creating a generic host_bridge from device tree

[Jingoo Han]

On Wednesday, March 05, 2014 12:50 AM, Liviu Dudau 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 8708b652..800678a 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c

[.....]

> +		res = kzalloc(sizeof(struct resource), GFP_KERNEL);

It makes build error with exynos_defconfig. (ARM32)
Thus, 'slab.h' is necessary in order to fix the build error.

./drivers/pci/host-bridge.c
@@ -8,6 +8,7 @@
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/slab.h>

 #include "pci.h"


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

It also makes compile problem with exynos_defconfig as below:

drivers/built-in.o: In function `pci_host_bridge_of_get_ranges':
drivers/pci/host-bridge.c:157: undefined reference to `pcibios_fixup_bridge_ranges'

Best regards,
Jingoo Han

Re: [PATCH v5 5/7] pci: Use parent domain number when allocating child busses.

[Tanmay Inamdar]

Hello,

On Tue, Mar 4, 2014 at 7:50 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> pci_alloc_child_bus() uses the newly allocated child bus to figure
> out the domain number that is going to use for setting the device
> name. A better option is to use the parent bus domain number.
>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 26237a0..a12cda5 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -677,7 +677,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>          * now as the parent is not properly set up yet.
>          */
>         child->dev.class = &pcibus_class;
> -       dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
> +       dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(parent), busnr);
>
>         /*
>          * Set up the primary, secondary and subordinate
> --
> 1.9.0
>

This patch is not required after the fix in pci_domain_nr for arm64.

[PATCH v5 5/7] pci: Use parent domain number when allocating child busses.

[Tanmay Inamdar]

Hello,

On Tue, Mar 4, 2014 at 7:50 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> pci_alloc_child_bus() uses the newly allocated child bus to figure
> out the domain number that is going to use for setting the device
> name. A better option is to use the parent bus domain number.
>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 26237a0..a12cda5 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -677,7 +677,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>          * now as the parent is not properly set up yet.
>          */
>         child->dev.class = &pcibus_class;
> -       dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
> +       dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(parent), busnr);
>
>         /*
>          * Set up the primary, secondary and subordinate
> --
> 1.9.0
>

This patch is not required after the fix in pci_domain_nr for arm64.

Re: [PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Tanmay Inamdar]

Hello,

Thanks for the patch set.

On Tue, Mar 4, 2014 at 7:49 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> This is v5 of my attempt to add support for a generic pci_host_bridge controller created
> from a description passed in the device tree.
>
> Changes from v4:
>   - Export pci_find_host_bridge() to be used by arch code. There is scope for
>     making the arch/arm64 version of pci_domain_nr the default weak implementation
>     but that would double the size of this series in order to handle all #define
>     versions of the pci_domain_nr() function, so I suggest keeping that for a separate
>     cleanup series.
>
> Changes from v3:
>   - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
>   - Fix the domain number used when creating child busses.
>   - Changed domain number allocator to use atomic operations.
>   - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
>     and of_create_pci_host_bridge().
>
> 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.
>
>
> v4 thread here: https://lkml.org/lkml/2014/3/3/301
> v3 thread here: https://lkml.org/lkml/2014/2/28/216
> v2 thread here: https://lkml.org/lkml/2014/2/27/245
> v1 thread here: https://lkml.org/lkml/2014/2/3/380
>
> Best regards,
> Liviu
>
> Liviu Dudau (7):
>   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: Use parent domain number when allocating child busses.
>   pci: Export find_pci_host_bridge() function.
>   pci: Add support for creating a generic host_bridge from device tree
>
>  drivers/of/address.c       |  39 +++++++++++++
>  drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/pci/probe.c        |  72 +++++++++++++++--------
>  include/linux/of_address.h |  14 +----
>  include/linux/pci.h        |  17 ++++++
>  5 files changed, 248 insertions(+), 36 deletions(-)
>
> --
> 1.9.0
>

This series is working fine on X-Gene with multiple PCIe ports. You
can add my Tested-by if you want.

Thanks,
Tanmay

[PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Tanmay Inamdar]

Hello,

Thanks for the patch set.

On Tue, Mar 4, 2014 at 7:49 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> This is v5 of my attempt to add support for a generic pci_host_bridge controller created
> from a description passed in the device tree.
>
> Changes from v4:
>   - Export pci_find_host_bridge() to be used by arch code. There is scope for
>     making the arch/arm64 version of pci_domain_nr the default weak implementation
>     but that would double the size of this series in order to handle all #define
>     versions of the pci_domain_nr() function, so I suggest keeping that for a separate
>     cleanup series.
>
> Changes from v3:
>   - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
>   - Fix the domain number used when creating child busses.
>   - Changed domain number allocator to use atomic operations.
>   - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
>     and of_create_pci_host_bridge().
>
> 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.
>
>
> v4 thread here: https://lkml.org/lkml/2014/3/3/301
> v3 thread here: https://lkml.org/lkml/2014/2/28/216
> v2 thread here: https://lkml.org/lkml/2014/2/27/245
> v1 thread here: https://lkml.org/lkml/2014/2/3/380
>
> Best regards,
> Liviu
>
> Liviu Dudau (7):
>   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: Use parent domain number when allocating child busses.
>   pci: Export find_pci_host_bridge() function.
>   pci: Add support for creating a generic host_bridge from device tree
>
>  drivers/of/address.c       |  39 +++++++++++++
>  drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/pci/probe.c        |  72 +++++++++++++++--------
>  include/linux/of_address.h |  14 +----
>  include/linux/pci.h        |  17 ++++++
>  5 files changed, 248 insertions(+), 36 deletions(-)
>
> --
> 1.9.0
>

This series is working fine on X-Gene with multiple PCIe ports. You
can add my Tested-by if you want.

Thanks,
Tanmay

Re: [PATCH v5 3/7] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.

[Yijing Wang]

On 2014/3/4 23:50, Liviu Dudau wrote:
> 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;

What about use goto err_out?

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


-- 
Thanks!
Yijing

[PATCH v5 3/7] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.

[Yijing Wang]

On 2014/3/4 23:50, Liviu Dudau wrote:
> 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;

What about use goto err_out?

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


-- 
Thanks!
Yijing

Re: [PATCH v5 3/7] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.

[Jingoo Han]

On Wednesday, March 05, 2014 12:48 PM, Yijing Wang wrote:
> On 2014/3/4 23:50, Liviu Dudau wrote:
> > 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;
> 
> What about use goto err_out?

+1

I agree with your opinion.
It makes the code simpler.

Best regards,
Jingoo Han

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

[PATCH v5 3/7] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.

[Jingoo Han]

On Wednesday, March 05, 2014 12:48 PM, Yijing Wang wrote:
> On 2014/3/4 23:50, Liviu Dudau wrote:
> > 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;
> 
> What about use goto err_out?

+1

I agree with your opinion.
It makes the code simpler.

Best regards,
Jingoo Han

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

Re: [PATCH v5 5/7] pci: Use parent domain number when allocating child busses.

[Liviu Dudau]

On Wed, Mar 05, 2014 at 01:49:13AM +0000, Tanmay Inamdar wrote:
> Hello,
> 
> On Tue, Mar 4, 2014 at 7:50 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > pci_alloc_child_bus() uses the newly allocated child bus to figure
> > out the domain number that is going to use for setting the device
> > name. A better option is to use the parent bus domain number.
> >
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 26237a0..a12cda5 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -677,7 +677,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
> >          * now as the parent is not properly set up yet.
> >          */
> >         child->dev.class = &pcibus_class;
> > -       dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
> > +       dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(parent), busnr);
> >
> >         /*
> >          * Set up the primary, secondary and subordinate
> > --
> > 1.9.0
> >
> 
> This patch is not required after the fix in pci_domain_nr for arm64.
> 

True. Thanks for testing this!

Liviu

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

[PATCH v5 5/7] pci: Use parent domain number when allocating child busses.

[Liviu Dudau]

On Wed, Mar 05, 2014 at 01:49:13AM +0000, Tanmay Inamdar wrote:
> Hello,
> 
> On Tue, Mar 4, 2014 at 7:50 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > pci_alloc_child_bus() uses the newly allocated child bus to figure
> > out the domain number that is going to use for setting the device
> > name. A better option is to use the parent bus domain number.
> >
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 26237a0..a12cda5 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -677,7 +677,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
> >          * now as the parent is not properly set up yet.
> >          */
> >         child->dev.class = &pcibus_class;
> > -       dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
> > +       dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(parent), busnr);
> >
> >         /*
> >          * Set up the primary, secondary and subordinate
> > --
> > 1.9.0
> >
> 
> This patch is not required after the fix in pci_domain_nr for arm64.
> 

True. Thanks for testing this!

Liviu

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

Re: [PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Liviu Dudau]

On Wed, Mar 05, 2014 at 01:53:55AM +0000, Tanmay Inamdar wrote:
> Hello,
> 
> Thanks for the patch set.
> 
> On Tue, Mar 4, 2014 at 7:49 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > This is v5 of my attempt to add support for a generic pci_host_bridge controller created
> > from a description passed in the device tree.
> >
> > Changes from v4:
> >   - Export pci_find_host_bridge() to be used by arch code. There is scope for
> >     making the arch/arm64 version of pci_domain_nr the default weak implementation
> >     but that would double the size of this series in order to handle all #define
> >     versions of the pci_domain_nr() function, so I suggest keeping that for a separate
> >     cleanup series.
> >
> > Changes from v3:
> >   - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
> >   - Fix the domain number used when creating child busses.
> >   - Changed domain number allocator to use atomic operations.
> >   - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
> >     and of_create_pci_host_bridge().
> >
> > 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.
> >
> >
> > v4 thread here: https://lkml.org/lkml/2014/3/3/301
> > v3 thread here: https://lkml.org/lkml/2014/2/28/216
> > v2 thread here: https://lkml.org/lkml/2014/2/27/245
> > v1 thread here: https://lkml.org/lkml/2014/2/3/380
> >
> > Best regards,
> > Liviu
> >
> > Liviu Dudau (7):
> >   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: Use parent domain number when allocating child busses.
> >   pci: Export find_pci_host_bridge() function.
> >   pci: Add support for creating a generic host_bridge from device tree
> >
> >  drivers/of/address.c       |  39 +++++++++++++
> >  drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
> >  drivers/pci/probe.c        |  72 +++++++++++++++--------
> >  include/linux/of_address.h |  14 +----
> >  include/linux/pci.h        |  17 ++++++
> >  5 files changed, 248 insertions(+), 36 deletions(-)
> >
> > --
> > 1.9.0
> >
> 
> This series is working fine on X-Gene with multiple PCIe ports. You
> can add my Tested-by if you want.

Cheers! Any ETA on your updated patches?

I might get the required signatures on the legal approval next week and
then I will publish my host controller driver.

Best regards,
Liviu

> 
> Thanks,
> Tanmay
> 

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

[PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Liviu Dudau]

On Wed, Mar 05, 2014 at 01:53:55AM +0000, Tanmay Inamdar wrote:
> Hello,
> 
> Thanks for the patch set.
> 
> On Tue, Mar 4, 2014 at 7:49 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > This is v5 of my attempt to add support for a generic pci_host_bridge controller created
> > from a description passed in the device tree.
> >
> > Changes from v4:
> >   - Export pci_find_host_bridge() to be used by arch code. There is scope for
> >     making the arch/arm64 version of pci_domain_nr the default weak implementation
> >     but that would double the size of this series in order to handle all #define
> >     versions of the pci_domain_nr() function, so I suggest keeping that for a separate
> >     cleanup series.
> >
> > Changes from v3:
> >   - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
> >   - Fix the domain number used when creating child busses.
> >   - Changed domain number allocator to use atomic operations.
> >   - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
> >     and of_create_pci_host_bridge().
> >
> > 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.
> >
> >
> > v4 thread here: https://lkml.org/lkml/2014/3/3/301
> > v3 thread here: https://lkml.org/lkml/2014/2/28/216
> > v2 thread here: https://lkml.org/lkml/2014/2/27/245
> > v1 thread here: https://lkml.org/lkml/2014/2/3/380
> >
> > Best regards,
> > Liviu
> >
> > Liviu Dudau (7):
> >   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: Use parent domain number when allocating child busses.
> >   pci: Export find_pci_host_bridge() function.
> >   pci: Add support for creating a generic host_bridge from device tree
> >
> >  drivers/of/address.c       |  39 +++++++++++++
> >  drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
> >  drivers/pci/probe.c        |  72 +++++++++++++++--------
> >  include/linux/of_address.h |  14 +----
> >  include/linux/pci.h        |  17 ++++++
> >  5 files changed, 248 insertions(+), 36 deletions(-)
> >
> > --
> > 1.9.0
> >
> 
> This series is working fine on X-Gene with multiple PCIe ports. You
> can add my Tested-by if you want.

Cheers! Any ETA on your updated patches?

I might get the required signatures on the legal approval next week and
then I will publish my host controller driver.

Best regards,
Liviu

> 
> Thanks,
> Tanmay
> 

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

Re: [PATCH v5 3/7] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.

[Liviu Dudau]

On Wed, Mar 05, 2014 at 04:41:35AM +0000, Jingoo Han wrote:
> On Wednesday, March 05, 2014 12:48 PM, Yijing Wang wrote:
> > On 2014/3/4 23:50, Liviu Dudau wrote:
> > > 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;
> > 
> > What about use goto err_out?
> 
> +1
> 
> I agree with your opinion.
> It makes the code simpler.

Hi Yijing and Jingoo,

Thanks for reviewing these patches, I will make the change for the next version.

Best regards,
Liviu

> 
> Best regards,
> Jingoo Han
> 
> > 
> > > +	}
> > > +
> > >  	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;
> > >  }
> > >
> > >
> 
> 

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

Re: [PATCH v5 3/7] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.

[Liviu Dudau]

On Wed, Mar 05, 2014 at 04:41:35AM +0000, Jingoo Han wrote:
> On Wednesday, March 05, 2014 12:48 PM, Yijing Wang wrote:
> > On 2014/3/4 23:50, Liviu Dudau wrote:
> > > 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-5wv7dgnIgG8@public.gmane.org>
> > >
> > > 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;
> > 
> > What about use goto err_out?
> 
> +1
> 
> I agree with your opinion.
> It makes the code simpler.

Hi Yijing and Jingoo,

Thanks for reviewing these patches, I will make the change for the next version.

Best regards,
Liviu

> 
> Best regards,
> Jingoo Han
> 
> > 
> > > +	}
> > > +
> > >  	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;
> > >  }
> > >
> > >
> 
> 

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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 3/7] pci: Create pci_host_bridge before its associated bus in pci_create_root_bus.

[Liviu Dudau]

On Wed, Mar 05, 2014 at 04:41:35AM +0000, Jingoo Han wrote:
> On Wednesday, March 05, 2014 12:48 PM, Yijing Wang wrote:
> > On 2014/3/4 23:50, Liviu Dudau wrote:
> > > 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;
> > 
> > What about use goto err_out?
> 
> +1
> 
> I agree with your opinion.
> It makes the code simpler.

Hi Yijing and Jingoo,

Thanks for reviewing these patches, I will make the change for the next version.

Best regards,
Liviu

> 
> Best regards,
> Jingoo Han
> 
> > 
> > > +	}
> > > +
> > >  	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;
> > >  }
> > >
> > >
> 
> 

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

Re: [PATCH v5 7/7] pci: Add support for creating a generic host_bridge from device tree

[Liviu Dudau]

On Wed, Mar 05, 2014 at 10:20:28AM +0900, Jingoo Han wrote:
> On Wednesday, March 05, 2014 12:50 AM, Liviu Dudau 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 8708b652..800678a 100644
> > --- a/drivers/pci/host-bridge.c
> > +++ b/drivers/pci/host-bridge.c
> 
> [.....]
> 
> > +		res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> 
> It makes build error with exynos_defconfig. (ARM32)
> Thus, 'slab.h' is necessary in order to fix the build error.
> 
> ./drivers/pci/host-bridge.c
> @@ -8,6 +8,7 @@
>  #include <linux/module.h>
>  #include <linux/of_address.h>
>  #include <linux/of_pci.h>
> +#include <linux/slab.h>

Will add, thanks!

> 
>  #include "pci.h"
> 
> 
> > +		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);
> 
> It also makes compile problem with exynos_defconfig as below:
> 
> drivers/built-in.o: In function `pci_host_bridge_of_get_ranges':
> drivers/pci/host-bridge.c:157: undefined reference to `pcibios_fixup_bridge_ranges'

Does that mean that exynos_defconfig doesn't define CONFIG_OF? How do you
compile all the .dts files then? Should CONFIG_OF not be added to the default config
file?

Other than that, your comment is correct. drivers/pci/host-bridge.c gets compiled
in regardless of CONFIG_OF and I need to provide an empty implementation for
pcibios_fixup_bridge_ranges().

Thanks!

Liviu

> 
> Best regards,
> Jingoo Han
> 
> --
> 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 ...

Re: [PATCH v5 7/7] pci: Add support for creating a generic host_bridge from device tree

[Liviu Dudau]

On Wed, Mar 05, 2014 at 10:20:28AM +0900, Jingoo Han wrote:
> On Wednesday, March 05, 2014 12:50 AM, Liviu Dudau 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 8708b652..800678a 100644
> > --- a/drivers/pci/host-bridge.c
> > +++ b/drivers/pci/host-bridge.c
> 
> [.....]
> 
> > +		res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> 
> It makes build error with exynos_defconfig. (ARM32)
> Thus, 'slab.h' is necessary in order to fix the build error.
> 
> ./drivers/pci/host-bridge.c
> @@ -8,6 +8,7 @@
>  #include <linux/module.h>
>  #include <linux/of_address.h>
>  #include <linux/of_pci.h>
> +#include <linux/slab.h>

Will add, thanks!

> 
>  #include "pci.h"
> 
> 
> > +		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);
> 
> It also makes compile problem with exynos_defconfig as below:
> 
> drivers/built-in.o: In function `pci_host_bridge_of_get_ranges':
> drivers/pci/host-bridge.c:157: undefined reference to `pcibios_fixup_bridge_ranges'

Does that mean that exynos_defconfig doesn't define CONFIG_OF? How do you
compile all the .dts files then? Should CONFIG_OF not be added to the default config
file?

Other than that, your comment is correct. drivers/pci/host-bridge.c gets compiled
in regardless of CONFIG_OF and I need to provide an empty implementation for
pcibios_fixup_bridge_ranges().

Thanks!

Liviu

> 
> Best regards,
> Jingoo Han
> 
> --
> 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 ...

[PATCH v5 7/7] pci: Add support for creating a generic host_bridge from device tree

[Liviu Dudau]

On Wed, Mar 05, 2014 at 10:20:28AM +0900, Jingoo Han wrote:
> On Wednesday, March 05, 2014 12:50 AM, Liviu Dudau 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 8708b652..800678a 100644
> > --- a/drivers/pci/host-bridge.c
> > +++ b/drivers/pci/host-bridge.c
> 
> [.....]
> 
> > +		res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> 
> It makes build error with exynos_defconfig. (ARM32)
> Thus, 'slab.h' is necessary in order to fix the build error.
> 
> ./drivers/pci/host-bridge.c
> @@ -8,6 +8,7 @@
>  #include <linux/module.h>
>  #include <linux/of_address.h>
>  #include <linux/of_pci.h>
> +#include <linux/slab.h>

Will add, thanks!

> 
>  #include "pci.h"
> 
> 
> > +		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);
> 
> It also makes compile problem with exynos_defconfig as below:
> 
> drivers/built-in.o: In function `pci_host_bridge_of_get_ranges':
> drivers/pci/host-bridge.c:157: undefined reference to `pcibios_fixup_bridge_ranges'

Does that mean that exynos_defconfig doesn't define CONFIG_OF? How do you
compile all the .dts files then? Should CONFIG_OF not be added to the default config
file?

Other than that, your comment is correct. drivers/pci/host-bridge.c gets compiled
in regardless of CONFIG_OF and I need to provide an empty implementation for
pcibios_fixup_bridge_ranges().

Thanks!

Liviu

> 
> Best regards,
> Jingoo Han
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

 One small step
   for me ...

Re: [PATCH v5 7/7] pci: Add support for creating a generic host_bridge from device tree

[Jingoo Han]

On Wednesday, March 05, 2014 5:33 PM, Liviu Dudau wrote:
> On Wed, Mar 05, 2014 at 10:20:28AM +0900, Jingoo Han wrote:
> > On Wednesday, March 05, 2014 12:50 AM, Liviu Dudau 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 8708b652..800678a 100644
> > > --- a/drivers/pci/host-bridge.c
> > > +++ b/drivers/pci/host-bridge.c

[.....]

> > > +
> > > +	/* Apply architecture specific fixups for the ranges */
> > > +	pcibios_fixup_bridge_ranges(resources);
> >
> > It also makes compile problem with exynos_defconfig as below:
> >
> > drivers/built-in.o: In function `pci_host_bridge_of_get_ranges':
> > drivers/pci/host-bridge.c:157: undefined reference to `pcibios_fixup_bridge_ranges'
> 
> Does that mean that exynos_defconfig doesn't define CONFIG_OF? How do you
> compile all the .dts files then? Should CONFIG_OF not be added to the default config
> file?

Now, I am testing your patches with ARM32 platform such as
Exynos SoCs. And the default 'exynos_defconfig' already defines
CONFIG_OF=y. (./arch/arm/configs/exynos_defconfig)

> 
> Other than that, your comment is correct. drivers/pci/host-bridge.c gets compiled
> in regardless of CONFIG_OF and I need to provide an empty implementation for
> pcibios_fixup_bridge_ranges().

There is no pcibios_fixup_bridge_ranges() in ./arch/arm/ directory.
So, it makes the compile problem.

I think that the empty implementation for pcibios_fixup_bridge_ranges()
may be necessary, in order to the compile problem.
Or, __weak pcibios_fixup_bridge_ranges() would be used.

Best regards,
Jingoo Han

[PATCH v5 7/7] pci: Add support for creating a generic host_bridge from device tree

[Jingoo Han]

On Wednesday, March 05, 2014 5:33 PM, Liviu Dudau wrote:
> On Wed, Mar 05, 2014 at 10:20:28AM +0900, Jingoo Han wrote:
> > On Wednesday, March 05, 2014 12:50 AM, Liviu Dudau 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 8708b652..800678a 100644
> > > --- a/drivers/pci/host-bridge.c
> > > +++ b/drivers/pci/host-bridge.c

[.....]

> > > +
> > > +	/* Apply architecture specific fixups for the ranges */
> > > +	pcibios_fixup_bridge_ranges(resources);
> >
> > It also makes compile problem with exynos_defconfig as below:
> >
> > drivers/built-in.o: In function `pci_host_bridge_of_get_ranges':
> > drivers/pci/host-bridge.c:157: undefined reference to `pcibios_fixup_bridge_ranges'
> 
> Does that mean that exynos_defconfig doesn't define CONFIG_OF? How do you
> compile all the .dts files then? Should CONFIG_OF not be added to the default config
> file?

Now, I am testing your patches with ARM32 platform such as
Exynos SoCs. And the default 'exynos_defconfig' already defines
CONFIG_OF=y. (./arch/arm/configs/exynos_defconfig)

> 
> Other than that, your comment is correct. drivers/pci/host-bridge.c gets compiled
> in regardless of CONFIG_OF and I need to provide an empty implementation for
> pcibios_fixup_bridge_ranges().

There is no pcibios_fixup_bridge_ranges() in ./arch/arm/ directory.
So, it makes the compile problem.

I think that the empty implementation for pcibios_fixup_bridge_ranges()
may be necessary, in order to the compile problem.
Or, __weak pcibios_fixup_bridge_ranges() would be used.

Best regards,
Jingoo Han

Re: [PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Liviu Dudau]

On Wed, Mar 05, 2014 at 01:53:55AM +0000, Tanmay Inamdar wrote:
> Hello,
> 
> Thanks for the patch set.
> 
> On Tue, Mar 4, 2014 at 7:49 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > This is v5 of my attempt to add support for a generic pci_host_bridge controller created
> > from a description passed in the device tree.
> >
> > Changes from v4:
> >   - Export pci_find_host_bridge() to be used by arch code. There is scope for
> >     making the arch/arm64 version of pci_domain_nr the default weak implementation
> >     but that would double the size of this series in order to handle all #define
> >     versions of the pci_domain_nr() function, so I suggest keeping that for a separate
> >     cleanup series.
> >
> > Changes from v3:
> >   - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
> >   - Fix the domain number used when creating child busses.
> >   - Changed domain number allocator to use atomic operations.
> >   - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
> >     and of_create_pci_host_bridge().
> >
> > 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.
> >
> >
> > v4 thread here: https://lkml.org/lkml/2014/3/3/301
> > v3 thread here: https://lkml.org/lkml/2014/2/28/216
> > v2 thread here: https://lkml.org/lkml/2014/2/27/245
> > v1 thread here: https://lkml.org/lkml/2014/2/3/380
> >
> > Best regards,
> > Liviu
> >
> > Liviu Dudau (7):
> >   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: Use parent domain number when allocating child busses.
> >   pci: Export find_pci_host_bridge() function.
> >   pci: Add support for creating a generic host_bridge from device tree
> >
> >  drivers/of/address.c       |  39 +++++++++++++
> >  drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
> >  drivers/pci/probe.c        |  72 +++++++++++++++--------
> >  include/linux/of_address.h |  14 +----
> >  include/linux/pci.h        |  17 ++++++
> >  5 files changed, 248 insertions(+), 36 deletions(-)
> >
> > --
> > 1.9.0
> >
> 
> This series is working fine on X-Gene with multiple PCIe ports. You
> can add my Tested-by if you want.

What about the arm64 series, can I add a Tested-by to that one as well?

Thanks,
Liviu

> 
> Thanks,
> Tanmay
> 

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

[PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Liviu Dudau]

On Wed, Mar 05, 2014 at 01:53:55AM +0000, Tanmay Inamdar wrote:
> Hello,
> 
> Thanks for the patch set.
> 
> On Tue, Mar 4, 2014 at 7:49 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > This is v5 of my attempt to add support for a generic pci_host_bridge controller created
> > from a description passed in the device tree.
> >
> > Changes from v4:
> >   - Export pci_find_host_bridge() to be used by arch code. There is scope for
> >     making the arch/arm64 version of pci_domain_nr the default weak implementation
> >     but that would double the size of this series in order to handle all #define
> >     versions of the pci_domain_nr() function, so I suggest keeping that for a separate
> >     cleanup series.
> >
> > Changes from v3:
> >   - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
> >   - Fix the domain number used when creating child busses.
> >   - Changed domain number allocator to use atomic operations.
> >   - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
> >     and of_create_pci_host_bridge().
> >
> > 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.
> >
> >
> > v4 thread here: https://lkml.org/lkml/2014/3/3/301
> > v3 thread here: https://lkml.org/lkml/2014/2/28/216
> > v2 thread here: https://lkml.org/lkml/2014/2/27/245
> > v1 thread here: https://lkml.org/lkml/2014/2/3/380
> >
> > Best regards,
> > Liviu
> >
> > Liviu Dudau (7):
> >   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: Use parent domain number when allocating child busses.
> >   pci: Export find_pci_host_bridge() function.
> >   pci: Add support for creating a generic host_bridge from device tree
> >
> >  drivers/of/address.c       |  39 +++++++++++++
> >  drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
> >  drivers/pci/probe.c        |  72 +++++++++++++++--------
> >  include/linux/of_address.h |  14 +----
> >  include/linux/pci.h        |  17 ++++++
> >  5 files changed, 248 insertions(+), 36 deletions(-)
> >
> > --
> > 1.9.0
> >
> 
> This series is working fine on X-Gene with multiple PCIe ports. You
> can add my Tested-by if you want.

What about the arm64 series, can I add a Tested-by to that one as well?

Thanks,
Liviu

> 
> Thanks,
> Tanmay
> 

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

Re: [PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Tanmay Inamdar]

On Wed, Mar 5, 2014 at 3:40 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Wed, Mar 05, 2014 at 01:53:55AM +0000, Tanmay Inamdar wrote:
>> Hello,
>>
>> Thanks for the patch set.
>>
>> On Tue, Mar 4, 2014 at 7:49 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> > This is v5 of my attempt to add support for a generic pci_host_bridge controller created
>> > from a description passed in the device tree.
>> >
>> > Changes from v4:
>> >   - Export pci_find_host_bridge() to be used by arch code. There is scope for
>> >     making the arch/arm64 version of pci_domain_nr the default weak implementation
>> >     but that would double the size of this series in order to handle all #define
>> >     versions of the pci_domain_nr() function, so I suggest keeping that for a separate
>> >     cleanup series.
>> >
>> > Changes from v3:
>> >   - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
>> >   - Fix the domain number used when creating child busses.
>> >   - Changed domain number allocator to use atomic operations.
>> >   - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
>> >     and of_create_pci_host_bridge().
>> >
>> > 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.
>> >
>> >
>> > v4 thread here: https://lkml.org/lkml/2014/3/3/301
>> > v3 thread here: https://lkml.org/lkml/2014/2/28/216
>> > v2 thread here: https://lkml.org/lkml/2014/2/27/245
>> > v1 thread here: https://lkml.org/lkml/2014/2/3/380
>> >
>> > Best regards,
>> > Liviu
>> >
>> > Liviu Dudau (7):
>> >   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: Use parent domain number when allocating child busses.
>> >   pci: Export find_pci_host_bridge() function.
>> >   pci: Add support for creating a generic host_bridge from device tree
>> >
>> >  drivers/of/address.c       |  39 +++++++++++++
>> >  drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
>> >  drivers/pci/probe.c        |  72 +++++++++++++++--------
>> >  include/linux/of_address.h |  14 +----
>> >  include/linux/pci.h        |  17 ++++++
>> >  5 files changed, 248 insertions(+), 36 deletions(-)
>> >
>> > --
>> > 1.9.0
>> >
>>
>> This series is working fine on X-Gene with multiple PCIe ports. You
>> can add my Tested-by if you want.
>
> What about the arm64 series, can I add a Tested-by to that one as well?

Sure.

>
> Thanks,
> Liviu
>
>>
>> Thanks,
>> Tanmay
>>
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
>

[PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Tanmay Inamdar]

On Wed, Mar 5, 2014 at 3:40 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Wed, Mar 05, 2014 at 01:53:55AM +0000, Tanmay Inamdar wrote:
>> Hello,
>>
>> Thanks for the patch set.
>>
>> On Tue, Mar 4, 2014 at 7:49 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> > This is v5 of my attempt to add support for a generic pci_host_bridge controller created
>> > from a description passed in the device tree.
>> >
>> > Changes from v4:
>> >   - Export pci_find_host_bridge() to be used by arch code. There is scope for
>> >     making the arch/arm64 version of pci_domain_nr the default weak implementation
>> >     but that would double the size of this series in order to handle all #define
>> >     versions of the pci_domain_nr() function, so I suggest keeping that for a separate
>> >     cleanup series.
>> >
>> > Changes from v3:
>> >   - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
>> >   - Fix the domain number used when creating child busses.
>> >   - Changed domain number allocator to use atomic operations.
>> >   - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
>> >     and of_create_pci_host_bridge().
>> >
>> > 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.
>> >
>> >
>> > v4 thread here: https://lkml.org/lkml/2014/3/3/301
>> > v3 thread here: https://lkml.org/lkml/2014/2/28/216
>> > v2 thread here: https://lkml.org/lkml/2014/2/27/245
>> > v1 thread here: https://lkml.org/lkml/2014/2/3/380
>> >
>> > Best regards,
>> > Liviu
>> >
>> > Liviu Dudau (7):
>> >   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: Use parent domain number when allocating child busses.
>> >   pci: Export find_pci_host_bridge() function.
>> >   pci: Add support for creating a generic host_bridge from device tree
>> >
>> >  drivers/of/address.c       |  39 +++++++++++++
>> >  drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
>> >  drivers/pci/probe.c        |  72 +++++++++++++++--------
>> >  include/linux/of_address.h |  14 +----
>> >  include/linux/pci.h        |  17 ++++++
>> >  5 files changed, 248 insertions(+), 36 deletions(-)
>> >
>> > --
>> > 1.9.0
>> >
>>
>> This series is working fine on X-Gene with multiple PCIe ports. You
>> can add my Tested-by if you want.
>
> What about the arm64 series, can I add a Tested-by to that one as well?

Sure.

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

Re: [PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Tanmay Inamdar]

On Wed, Mar 5, 2014 at 12:18 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Wed, Mar 05, 2014 at 01:53:55AM +0000, Tanmay Inamdar wrote:
>> Hello,
>>
>> Thanks for the patch set.
>>
>> On Tue, Mar 4, 2014 at 7:49 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> > This is v5 of my attempt to add support for a generic pci_host_bridge controller created
>> > from a description passed in the device tree.
>> >
>> > Changes from v4:
>> >   - Export pci_find_host_bridge() to be used by arch code. There is scope for
>> >     making the arch/arm64 version of pci_domain_nr the default weak implementation
>> >     but that would double the size of this series in order to handle all #define
>> >     versions of the pci_domain_nr() function, so I suggest keeping that for a separate
>> >     cleanup series.
>> >
>> > Changes from v3:
>> >   - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
>> >   - Fix the domain number used when creating child busses.
>> >   - Changed domain number allocator to use atomic operations.
>> >   - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
>> >     and of_create_pci_host_bridge().
>> >
>> > 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.
>> >
>> >
>> > v4 thread here: https://lkml.org/lkml/2014/3/3/301
>> > v3 thread here: https://lkml.org/lkml/2014/2/28/216
>> > v2 thread here: https://lkml.org/lkml/2014/2/27/245
>> > v1 thread here: https://lkml.org/lkml/2014/2/3/380
>> >
>> > Best regards,
>> > Liviu
>> >
>> > Liviu Dudau (7):
>> >   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: Use parent domain number when allocating child busses.
>> >   pci: Export find_pci_host_bridge() function.
>> >   pci: Add support for creating a generic host_bridge from device tree
>> >
>> >  drivers/of/address.c       |  39 +++++++++++++
>> >  drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
>> >  drivers/pci/probe.c        |  72 +++++++++++++++--------
>> >  include/linux/of_address.h |  14 +----
>> >  include/linux/pci.h        |  17 ++++++
>> >  5 files changed, 248 insertions(+), 36 deletions(-)
>> >
>> > --
>> > 1.9.0
>> >
>>
>> This series is working fine on X-Gene with multiple PCIe ports. You
>> can add my Tested-by if you want.
>
> Cheers! Any ETA on your updated patches?

I will apply your V6 and generate mine on top of them. I will send my
X-Gene patches out today.

>
> I might get the required signatures on the legal approval next week and
> then I will publish my host controller driver.
>
> Best regards,
> Liviu
>
>>
>> Thanks,
>> Tanmay
>>
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
>

[PATCH v5 0/7] [RFC] Support for creating generic host_bridge from device tree

[Tanmay Inamdar]

On Wed, Mar 5, 2014 at 12:18 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Wed, Mar 05, 2014 at 01:53:55AM +0000, Tanmay Inamdar wrote:
>> Hello,
>>
>> Thanks for the patch set.
>>
>> On Tue, Mar 4, 2014 at 7:49 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> > This is v5 of my attempt to add support for a generic pci_host_bridge controller created
>> > from a description passed in the device tree.
>> >
>> > Changes from v4:
>> >   - Export pci_find_host_bridge() to be used by arch code. There is scope for
>> >     making the arch/arm64 version of pci_domain_nr the default weak implementation
>> >     but that would double the size of this series in order to handle all #define
>> >     versions of the pci_domain_nr() function, so I suggest keeping that for a separate
>> >     cleanup series.
>> >
>> > Changes from v3:
>> >   - Dynamically allocate bus_range resource in of_create_pci_host_bridge()
>> >   - Fix the domain number used when creating child busses.
>> >   - Changed domain number allocator to use atomic operations.
>> >   - Use ERR_PTR() to propagate the error out of pci_create_root_bus_in_domain()
>> >     and of_create_pci_host_bridge().
>> >
>> > 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.
>> >
>> >
>> > v4 thread here: https://lkml.org/lkml/2014/3/3/301
>> > v3 thread here: https://lkml.org/lkml/2014/2/28/216
>> > v2 thread here: https://lkml.org/lkml/2014/2/27/245
>> > v1 thread here: https://lkml.org/lkml/2014/2/3/380
>> >
>> > Best regards,
>> > Liviu
>> >
>> > Liviu Dudau (7):
>> >   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: Use parent domain number when allocating child busses.
>> >   pci: Export find_pci_host_bridge() function.
>> >   pci: Add support for creating a generic host_bridge from device tree
>> >
>> >  drivers/of/address.c       |  39 +++++++++++++
>> >  drivers/pci/host-bridge.c  | 142 ++++++++++++++++++++++++++++++++++++++++++++-
>> >  drivers/pci/probe.c        |  72 +++++++++++++++--------
>> >  include/linux/of_address.h |  14 +----
>> >  include/linux/pci.h        |  17 ++++++
>> >  5 files changed, 248 insertions(+), 36 deletions(-)
>> >
>> > --
>> > 1.9.0
>> >
>>
>> This series is working fine on X-Gene with multiple PCIe ports. You
>> can add my Tested-by if you want.
>
> Cheers! Any ETA on your updated patches?

I will apply your V6 and generate mine on top of them. I will send my
X-Gene patches out today.

>
> I might get the required signatures on the legal approval next week and
> then I will publish my host controller driver.
>
> Best regards,
> Liviu
>
>>
>> Thanks,
>> Tanmay
>>
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ?\_(?)_/?
>

Re: [PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Liviu Dudau]

On Tue, Mar 04, 2014 at 10:30:09PM +0000, Arnd Bergmann wrote:
> On Tuesday 04 March 2014, Liviu Dudau wrote:
> > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > +{
> > +       return 0;
> > +}
> > +
> 
> How about returning an error here? You don't actually register the range.

That's not the intention here. I basically want a nop, as by default (read x86)
we do nothing with the IO range.

Best regards,
Liviu

> 
> 	Arnd
> 

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

Re: [PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Liviu Dudau]

On Tue, Mar 04, 2014 at 10:30:09PM +0000, Arnd Bergmann wrote:
> On Tuesday 04 March 2014, Liviu Dudau wrote:
> > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > +{
> > +       return 0;
> > +}
> > +
> 
> How about returning an error here? You don't actually register the range.

That's not the intention here. I basically want a nop, as by default (read x86)
we do nothing with the IO range.

Best regards,
Liviu

> 
> 	Arnd
> 

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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Liviu Dudau]

On Tue, Mar 04, 2014 at 10:30:09PM +0000, Arnd Bergmann wrote:
> On Tuesday 04 March 2014, Liviu Dudau wrote:
> > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > +{
> > +       return 0;
> > +}
> > +
> 
> How about returning an error here? You don't actually register the range.

That's not the intention here. I basically want a nop, as by default (read x86)
we do nothing with the IO range.

Best regards,
Liviu

> 
> 	Arnd
> 

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

Re: [PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Arnd Bergmann]

On Thursday 06 March 2014, Liviu Dudau wrote:
> On Tue, Mar 04, 2014 at 10:30:09PM +0000, Arnd Bergmann wrote:
> > On Tuesday 04 March 2014, Liviu Dudau wrote:
> > > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > > +{
> > > +       return 0;
> > > +}
> > > +
> > 
> > How about returning an error here? You don't actually register the range.
> 
> That's not the intention here. I basically want a nop, as by default (read x86)
> we do nothing with the IO range.

I think x86 is a bad default though, because that is the exception rather than
the rule. I also think that on x86, you shouldn't have an entry for the I/O
space in the "ranges" property since there is no translation, and then we don't
call this function.

PCI devices described in DT on x86 would still be able to list their I/O BARs
in DT, but you don't ever translate them into MMIO ranges.

	Arnd

[PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Arnd Bergmann]

On Thursday 06 March 2014, Liviu Dudau wrote:
> On Tue, Mar 04, 2014 at 10:30:09PM +0000, Arnd Bergmann wrote:
> > On Tuesday 04 March 2014, Liviu Dudau wrote:
> > > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > > +{
> > > +       return 0;
> > > +}
> > > +
> > 
> > How about returning an error here? You don't actually register the range.
> 
> That's not the intention here. I basically want a nop, as by default (read x86)
> we do nothing with the IO range.

I think x86 is a bad default though, because that is the exception rather than
the rule. I also think that on x86, you shouldn't have an entry for the I/O
space in the "ranges" property since there is no translation, and then we don't
call this function.

PCI devices described in DT on x86 would still be able to list their I/O BARs
in DT, but you don't ever translate them into MMIO ranges.

	Arnd

Re: [PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Liviu Dudau]

On Fri, Mar 07, 2014 at 01:24:12AM +0100, Arnd Bergmann wrote:
> On Thursday 06 March 2014, Liviu Dudau wrote:
> > On Tue, Mar 04, 2014 at 10:30:09PM +0000, Arnd Bergmann wrote:
> > > On Tuesday 04 March 2014, Liviu Dudau wrote:
> > > > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > > > +{
> > > > +       return 0;
> > > > +}
> > > > +
> > > 
> > > How about returning an error here? You don't actually register the range.
> > 
> > That's not the intention here. I basically want a nop, as by default (read x86)
> > we do nothing with the IO range.
> 
> I think x86 is a bad default though, because that is the exception rather than
> the rule. I also think that on x86, you shouldn't have an entry for the I/O
> space in the "ranges" property since there is no translation, and then we don't
> call this function.
> 
> PCI devices described in DT on x86 would still be able to list their I/O BARs
> in DT, but you don't ever translate them into MMIO ranges.

So, if I understand you correctly, you would prefer to fail here and hence stop the
parsing for the x86, rather than pretending everything is OK and going through the
motions?

That was not my original thinking when I've introduced this function here. The main
purpose of the function is to help the correct translation of IO addresses in
pci_address_to_pio(). As Jason has explained very nicely, we have 3 types of
architectures here that we try to support:
  - the ones that have separate IO address space (x86)
  - the ones that have 1:1 mapping between physical IO addresses and logical ports
  - the architectures that memory map the IO addresses in virtual address space
    and then translate the logical addresses into virtual based on a given offset.

For the first two types we don't want to do anything special. Architectures that
fall in the last category will have to provide their own version of this function,
with the arm64 version being generic enough to be used as de facto?

But I can see your point of view as well. I just don't know if that is good enough
for powerpc and microblaze. With the way things are in my patch, they should be able
to switch to of_create_pci_host_bridge() easily*, with your suggestion they will
have to provide their implementation for pci_register_io_range().

We really need to get another architecture converted. If there are no other takers I
will make a stab once the current push towards upstreaming AArch64 hardware support
slows down.

* says the newby with confidence in his voice ;)

Liviu

> 
> 	Arnd
> --
> 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
> 

Re: [PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Liviu Dudau]

On Fri, Mar 07, 2014 at 01:24:12AM +0100, Arnd Bergmann wrote:
> On Thursday 06 March 2014, Liviu Dudau wrote:
> > On Tue, Mar 04, 2014 at 10:30:09PM +0000, Arnd Bergmann wrote:
> > > On Tuesday 04 March 2014, Liviu Dudau wrote:
> > > > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > > > +{
> > > > +       return 0;
> > > > +}
> > > > +
> > > 
> > > How about returning an error here? You don't actually register the range.
> > 
> > That's not the intention here. I basically want a nop, as by default (read x86)
> > we do nothing with the IO range.
> 
> I think x86 is a bad default though, because that is the exception rather than
> the rule. I also think that on x86, you shouldn't have an entry for the I/O
> space in the "ranges" property since there is no translation, and then we don't
> call this function.
> 
> PCI devices described in DT on x86 would still be able to list their I/O BARs
> in DT, but you don't ever translate them into MMIO ranges.

So, if I understand you correctly, you would prefer to fail here and hence stop the
parsing for the x86, rather than pretending everything is OK and going through the
motions?

That was not my original thinking when I've introduced this function here. The main
purpose of the function is to help the correct translation of IO addresses in
pci_address_to_pio(). As Jason has explained very nicely, we have 3 types of
architectures here that we try to support:
  - the ones that have separate IO address space (x86)
  - the ones that have 1:1 mapping between physical IO addresses and logical ports
  - the architectures that memory map the IO addresses in virtual address space
    and then translate the logical addresses into virtual based on a given offset.

For the first two types we don't want to do anything special. Architectures that
fall in the last category will have to provide their own version of this function,
with the arm64 version being generic enough to be used as de facto?

But I can see your point of view as well. I just don't know if that is good enough
for powerpc and microblaze. With the way things are in my patch, they should be able
to switch to of_create_pci_host_bridge() easily*, with your suggestion they will
have to provide their implementation for pci_register_io_range().

We really need to get another architecture converted. If there are no other takers I
will make a stab once the current push towards upstreaming AArch64 hardware support
slows down.

* says the newby with confidence in his voice ;)

Liviu

> 
> 	Arnd
> --
> 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
> 

[PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Liviu Dudau]

On Fri, Mar 07, 2014 at 01:24:12AM +0100, Arnd Bergmann wrote:
> On Thursday 06 March 2014, Liviu Dudau wrote:
> > On Tue, Mar 04, 2014 at 10:30:09PM +0000, Arnd Bergmann wrote:
> > > On Tuesday 04 March 2014, Liviu Dudau wrote:
> > > > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > > > +{
> > > > +       return 0;
> > > > +}
> > > > +
> > > 
> > > How about returning an error here? You don't actually register the range.
> > 
> > That's not the intention here. I basically want a nop, as by default (read x86)
> > we do nothing with the IO range.
> 
> I think x86 is a bad default though, because that is the exception rather than
> the rule. I also think that on x86, you shouldn't have an entry for the I/O
> space in the "ranges" property since there is no translation, and then we don't
> call this function.
> 
> PCI devices described in DT on x86 would still be able to list their I/O BARs
> in DT, but you don't ever translate them into MMIO ranges.

So, if I understand you correctly, you would prefer to fail here and hence stop the
parsing for the x86, rather than pretending everything is OK and going through the
motions?

That was not my original thinking when I've introduced this function here. The main
purpose of the function is to help the correct translation of IO addresses in
pci_address_to_pio(). As Jason has explained very nicely, we have 3 types of
architectures here that we try to support:
  - the ones that have separate IO address space (x86)
  - the ones that have 1:1 mapping between physical IO addresses and logical ports
  - the architectures that memory map the IO addresses in virtual address space
    and then translate the logical addresses into virtual based on a given offset.

For the first two types we don't want to do anything special. Architectures that
fall in the last category will have to provide their own version of this function,
with the arm64 version being generic enough to be used as de facto?

But I can see your point of view as well. I just don't know if that is good enough
for powerpc and microblaze. With the way things are in my patch, they should be able
to switch to of_create_pci_host_bridge() easily*, with your suggestion they will
have to provide their implementation for pci_register_io_range().

We really need to get another architecture converted. If there are no other takers I
will make a stab once the current push towards upstreaming AArch64 hardware support
slows down.

* says the newby with confidence in his voice ;)

Liviu

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

Re: [PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Liviu Dudau]

On Fri, Mar 07, 2014 at 12:24:12AM +0000, Arnd Bergmann wrote:
> On Thursday 06 March 2014, Liviu Dudau wrote:
> > On Tue, Mar 04, 2014 at 10:30:09PM +0000, Arnd Bergmann wrote:
> > > On Tuesday 04 March 2014, Liviu Dudau wrote:
> > > > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > > > +{
> > > > +       return 0;
> > > > +}
> > > > +
> > > 
> > > How about returning an error here? You don't actually register the range.
> > 
> > That's not the intention here. I basically want a nop, as by default (read x86)
> > we do nothing with the IO range.
> 
> I think x86 is a bad default though, because that is the exception rather than
> the rule. I also think that on x86, you shouldn't have an entry for the I/O
> space in the "ranges" property since there is no translation, and then we don't
> call this function.
> 
> PCI devices described in DT on x86 would still be able to list their I/O BARs
> in DT, but you don't ever translate them into MMIO ranges.
> 
> 	Arnd
> 

So, if I understand you correctly, you would prefer to fail here and hence stop the
parsing for the x86, rather than pretending everything is OK and going through the
motions?

That was not my original thinking when I've introduced this function here. The main
purpose of the function is to help the correct translation of IO addresses in
pci_address_to_pio(). As Jason has explained very nicely, we have 3 types of
architectures here that we try to support:
  - the ones that have separate IO address space (x86)
  - the ones that have 1:1 mapping between physical IO addresses and logical ports
  - the architectures that memory map the IO addresses in virtual address space
    and then translate the logical addresses into virtual based on a given offset.

For the first two types we don't want to do anything special. Architectures that
fall in the last category will have to provide their own version of this function,
with the arm64 version being generic enough to be used as de facto?

But I can see your point of view as well. I just don't know if that is good enough
for powerpc and microblaze. With the way things are in my patch, they should be able
to switch to of_create_pci_host_bridge() easily*, with your suggestion they will
have to provide their implementation for pci_register_io_range().

We really need to get another architecture converted. If there are no other takers I
will make a stab once the current push towards upstreaming AArch64 hardware support
slows down.

* says the newby with confidence in his voice ;)

Liviu


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

[PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Liviu Dudau]

On Fri, Mar 07, 2014 at 12:24:12AM +0000, Arnd Bergmann wrote:
> On Thursday 06 March 2014, Liviu Dudau wrote:
> > On Tue, Mar 04, 2014 at 10:30:09PM +0000, Arnd Bergmann wrote:
> > > On Tuesday 04 March 2014, Liviu Dudau wrote:
> > > > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > > > +{
> > > > +       return 0;
> > > > +}
> > > > +
> > > 
> > > How about returning an error here? You don't actually register the range.
> > 
> > That's not the intention here. I basically want a nop, as by default (read x86)
> > we do nothing with the IO range.
> 
> I think x86 is a bad default though, because that is the exception rather than
> the rule. I also think that on x86, you shouldn't have an entry for the I/O
> space in the "ranges" property since there is no translation, and then we don't
> call this function.
> 
> PCI devices described in DT on x86 would still be able to list their I/O BARs
> in DT, but you don't ever translate them into MMIO ranges.
> 
> 	Arnd
> 

So, if I understand you correctly, you would prefer to fail here and hence stop the
parsing for the x86, rather than pretending everything is OK and going through the
motions?

That was not my original thinking when I've introduced this function here. The main
purpose of the function is to help the correct translation of IO addresses in
pci_address_to_pio(). As Jason has explained very nicely, we have 3 types of
architectures here that we try to support:
  - the ones that have separate IO address space (x86)
  - the ones that have 1:1 mapping between physical IO addresses and logical ports
  - the architectures that memory map the IO addresses in virtual address space
    and then translate the logical addresses into virtual based on a given offset.

For the first two types we don't want to do anything special. Architectures that
fall in the last category will have to provide their own version of this function,
with the arm64 version being generic enough to be used as de facto?

But I can see your point of view as well. I just don't know if that is good enough
for powerpc and microblaze. With the way things are in my patch, they should be able
to switch to of_create_pci_host_bridge() easily*, with your suggestion they will
have to provide their implementation for pci_register_io_range().

We really need to get another architecture converted. If there are no other takers I
will make a stab once the current push towards upstreaming AArch64 hardware support
slows down.

* says the newby with confidence in his voice ;)

Liviu


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

Re: [PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Arnd Bergmann]

On Monday 10 March 2014 14:45:19 Liviu Dudau wrote:
> 
> So, if I understand you correctly, you would prefer to fail here and hence stop the
> parsing for the x86, rather than pretending everything is OK and going through the
> motions?

Yes, on x86 it is clearly a bug if we end up calling this function.

> That was not my original thinking when I've introduced this function here. The main
> purpose of the function is to help the correct translation of IO addresses in
> pci_address_to_pio(). As Jason has explained very nicely, we have 3 types of
> architectures here that we try to support:
>   - the ones that have separate IO address space (x86)
>   - the ones that have 1:1 mapping between physical IO addresses and logical ports

Still not convinced this second one actually exists.

>   - the architectures that memory map the IO addresses in virtual address space
>     and then translate the logical addresses into virtual based on a given offset.
> 
> For the first two types we don't want to do anything special. Architectures that
> fall in the last category will have to provide their own version of this function,
> with the arm64 version being generic enough to be used as de facto?

The page flags might be different across architectures: arm32 currently uses
MT_DEVICE, which only exists on arm32 and derived architectures (unicore32,
arm64).

> But I can see your point of view as well. I just don't know if that is good enough
> for powerpc and microblaze. With the way things are in my patch, they should be able
> to switch to of_create_pci_host_bridge() easily*, with your suggestion they will
> have to provide their implementation for pci_register_io_range().

I think there would still be a lot to do have powerpc switch over to
of_create_pci_host_bridge(), the more likely thing to happen is to have
that architecture implement its own copy that calls the same internal helpers
and does some more things that we may not want on other architectures.

Microblaze can probably be changed to use of_create_pci_host_bridge()
and need no custom code at all, it should need only a subset of what
we need for arm64.

> We really need to get another architecture converted. If there are no other takers I
> will make a stab once the current push towards upstreaming AArch64 hardware support
> slows down.

Moving over microblaze I think would be a good start. It has rather
specific requirements since there is only one host driver, but then again
that PCI host implementation might be shared with zynq (or synthesizable
there). I also wonder whether it's actually related to the X-gene PCI,
since I know some of the ppc4xx use Xilinx PCI and X-gene is also
based on those.

	Arnd

Re: [PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Arnd Bergmann]

On Monday 10 March 2014 14:45:19 Liviu Dudau wrote:
> 
> So, if I understand you correctly, you would prefer to fail here and hence stop the
> parsing for the x86, rather than pretending everything is OK and going through the
> motions?

Yes, on x86 it is clearly a bug if we end up calling this function.

> That was not my original thinking when I've introduced this function here. The main
> purpose of the function is to help the correct translation of IO addresses in
> pci_address_to_pio(). As Jason has explained very nicely, we have 3 types of
> architectures here that we try to support:
>   - the ones that have separate IO address space (x86)
>   - the ones that have 1:1 mapping between physical IO addresses and logical ports

Still not convinced this second one actually exists.

>   - the architectures that memory map the IO addresses in virtual address space
>     and then translate the logical addresses into virtual based on a given offset.
> 
> For the first two types we don't want to do anything special. Architectures that
> fall in the last category will have to provide their own version of this function,
> with the arm64 version being generic enough to be used as de facto?

The page flags might be different across architectures: arm32 currently uses
MT_DEVICE, which only exists on arm32 and derived architectures (unicore32,
arm64).

> But I can see your point of view as well. I just don't know if that is good enough
> for powerpc and microblaze. With the way things are in my patch, they should be able
> to switch to of_create_pci_host_bridge() easily*, with your suggestion they will
> have to provide their implementation for pci_register_io_range().

I think there would still be a lot to do have powerpc switch over to
of_create_pci_host_bridge(), the more likely thing to happen is to have
that architecture implement its own copy that calls the same internal helpers
and does some more things that we may not want on other architectures.

Microblaze can probably be changed to use of_create_pci_host_bridge()
and need no custom code at all, it should need only a subset of what
we need for arm64.

> We really need to get another architecture converted. If there are no other takers I
> will make a stab once the current push towards upstreaming AArch64 hardware support
> slows down.

Moving over microblaze I think would be a good start. It has rather
specific requirements since there is only one host driver, but then again
that PCI host implementation might be shared with zynq (or synthesizable
there). I also wonder whether it's actually related to the X-gene PCI,
since I know some of the ppc4xx use Xilinx PCI and X-gene is also
based on those.

	Arnd

[PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

[Arnd Bergmann]

On Monday 10 March 2014 14:45:19 Liviu Dudau wrote:
> 
> So, if I understand you correctly, you would prefer to fail here and hence stop the
> parsing for the x86, rather than pretending everything is OK and going through the
> motions?

Yes, on x86 it is clearly a bug if we end up calling this function.

> That was not my original thinking when I've introduced this function here. The main
> purpose of the function is to help the correct translation of IO addresses in
> pci_address_to_pio(). As Jason has explained very nicely, we have 3 types of
> architectures here that we try to support:
>   - the ones that have separate IO address space (x86)
>   - the ones that have 1:1 mapping between physical IO addresses and logical ports

Still not convinced this second one actually exists.

>   - the architectures that memory map the IO addresses in virtual address space
>     and then translate the logical addresses into virtual based on a given offset.
> 
> For the first two types we don't want to do anything special. Architectures that
> fall in the last category will have to provide their own version of this function,
> with the arm64 version being generic enough to be used as de facto?

The page flags might be different across architectures: arm32 currently uses
MT_DEVICE, which only exists on arm32 and derived architectures (unicore32,
arm64).

> But I can see your point of view as well. I just don't know if that is good enough
> for powerpc and microblaze. With the way things are in my patch, they should be able
> to switch to of_create_pci_host_bridge() easily*, with your suggestion they will
> have to provide their implementation for pci_register_io_range().

I think there would still be a lot to do have powerpc switch over to
of_create_pci_host_bridge(), the more likely thing to happen is to have
that architecture implement its own copy that calls the same internal helpers
and does some more things that we may not want on other architectures.

Microblaze can probably be changed to use of_create_pci_host_bridge()
and need no custom code at all, it should need only a subset of what
we need for arm64.

> We really need to get another architecture converted. If there are no other takers I
> will make a stab once the current push towards upstreaming AArch64 hardware support
> slows down.

Moving over microblaze I think would be a good start. It has rather
specific requirements since there is only one host driver, but then again
that PCI host implementation might be shared with zynq (or synthesizable
there). I also wonder whether it's actually related to the X-gene PCI,
since I know some of the ppc4xx use Xilinx PCI and X-gene is also
based on those.

	Arnd