This is v6 of my attempt to add support for a generic pci_host_bridge controller created from a description passed in the device tree.
Changes from v5: - Tested by Tanmay Inamdar, thanks Tanmay! - dropped v5 5/7 pci: Use parent domain number when allocating child busses. - Added weak implementation of pcibios_fixup_bridge_ranges() in drivers/pci/host-bridge.c so that architectures that enable CONFIG_OF and CONFIG_PCI don't suddenly get compilation errors. While at this, changed the signature of the function so that an error can be returned. - With the new error code in pcibios_fixup_bridge_ranges(), reworked the error handling in pci_host_bridge_of_get_ranges() and of_create_pci_host_bridge(). - Add linux/slab.h to the #include list - Revisit the error path in pci_create_root_bus[_in_domain]() and fixed the case where failing to allocate the bus will not return an error.
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.
v5 thread here: https://lkml.org/lkml/2014/3/4/318 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 (6): 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: 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 | 159 ++++++++++++++++++++++++++++++++++++++++++++- drivers/pci/probe.c | 70 +++++++++++++------- include/linux/of_address.h | 14 +--- include/linux/pci.h | 17 +++++ 5 files changed, 264 insertions(+), 35 deletions(-)
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 Tested-by: Tanmay Inamdar tinamdar@apm.com --- drivers/of/address.c | 5 +++++ include/linux/of_address.h | 1 + 2 files changed, 6 insertions(+)
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,
On Wed, 5 Mar 2014 11:48:52 +0000, Liviu Dudau Liviu.Dudau@arm.com wrote:
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 Tested-by: Tanmay Inamdar tinamdar@apm.com
Acked-by: Grant Likely grant.likely@linaro.org
drivers/of/address.c | 5 +++++ include/linux/of_address.h | 1 + 2 files changed, 6 insertions(+)
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
linaro-kernel mailing list linaro-kernel@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-kernel
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 Tested-by: Tanmay Inamdar tinamdar@apm.com --- drivers/of/address.c | 34 ++++++++++++++++++++++++++++++++++ include/linux/of_address.h | 13 ++----------- 2 files changed, 36 insertions(+), 11 deletions(-)
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);
On Wed, 5 Mar 2014 11:48:53 +0000, Liviu Dudau Liviu.Dudau@arm.com wrote:
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 Tested-by: Tanmay Inamdar tinamdar@apm.com
I think this is okay, but I'll need an ack from Arnd or Ben.
g.
drivers/of/address.c | 34 ++++++++++++++++++++++++++++++++++ include/linux/of_address.h | 13 ++----------- 2 files changed, 36 insertions(+), 11 deletions(-)
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
linaro-kernel mailing list linaro-kernel@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-kernel
On Wednesday 05 March 2014, Liviu Dudau wrote:
unsigned long port = -1;if (!pci_register_io_range(range->cpu_addr, range->size))port = pci_address_to_pio(range->cpu_addr);
I don't remember the outcome of my suggestion to just return 'port' from pci_register_io_range(). In my mind that would be much simpler than adding the range in a data structure just so we can look it up in the very next line.
Arnd
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 Tested-by: Tanmay Inamdar tinamdar@apm.com --- drivers/pci/probe.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 6e34498..fd11c12 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,19 @@ 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) + goto err_out; + b = pci_alloc_bus(); if (!b) - return NULL; + goto err_out;
b->sysdata = sysdata; b->ops = ops; @@ -1738,26 +1747,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 +1812,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; }
On Wed, 5 Mar 2014 11:48:54 +0000, Liviu Dudau Liviu.Dudau@arm.com 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 Tested-by: Tanmay Inamdar tinamdar@apm.com
Acked-by: Grant Likely grant.likely@linaro.org
drivers/pci/probe.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 6e34498..fd11c12 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,19 @@ 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)
goto err_out;- b = pci_alloc_bus(); if (!b)
return NULL;
goto err_out;b->sysdata = sysdata; b->ops = ops; @@ -1738,26 +1747,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;@@ -1814,8 +1812,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
linaro-kernel mailing list linaro-kernel@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-kernel
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 Tested-by: Tanmay Inamdar tinamdar@apm.com --- drivers/pci/probe.c | 41 +++++++++++++++++++++++++++++++++-------- include/linux/pci.h | 4 ++++ 2 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index fd11c12..172c615 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,30 +1729,34 @@ 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) goto err_out;
b = pci_alloc_bus(); - if (!b) + if (!b) { + error = -ENOMEM; goto err_out; + }
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"); + error = -EEXIST; goto err_bus_out; }
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); @@ -1766,7 +1771,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; @@ -1816,7 +1821,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);
On Wed, 5 Mar 2014 11:48:55 +0000, Liviu Dudau Liviu.Dudau@arm.com wrote:
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 Tested-by: Tanmay Inamdar tinamdar@apm.com
Looks okay to me structure wise, again I'd like an ack from Ben or Arnd.
g.
drivers/pci/probe.c | 41 +++++++++++++++++++++++++++++++++-------- include/linux/pci.h | 4 ++++ 2 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index fd11c12..172c615 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,30 +1729,34 @@ 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) goto err_out;
b = pci_alloc_bus();
- if (!b)
- if (!b) {
error = -ENOMEM;- }
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");
error = -EEXIST;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);
@@ -1766,7 +1771,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;
@@ -1816,7 +1821,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
linaro-kernel mailing list linaro-kernel@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-kernel
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 Tested-by: Tanmay Inamdar tinamdar@apm.com --- drivers/pci/host-bridge.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
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 *),
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 Tested-by: Tanmay Inamdar tinamdar@apm.com --- drivers/pci/host-bridge.c | 156 ++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 13 +++ 2 files changed, 169 insertions(+)
diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c index 8708b652..db9f51a 100644 --- a/drivers/pci/host-bridge.c +++ b/drivers/pci/host-bridge.c @@ -6,9 +6,14 @@ #include <linux/init.h> #include <linux/pci.h> #include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_pci.h> +#include <linux/slab.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 +97,154 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res, res->end = region->end + offset; } EXPORT_SYMBOL(pcibios_bus_to_resource); + +#ifdef CONFIG_OF +/** + * Simple version of the platform specific code for filtering the list + * of resources obtained from the ranges declaration in DT. + * + * Platforms can override this function in order to impose stronger + * constraints onto the list of resources that a host bridge can use. + * The filtered list will then be used to create a root bus and associate + * it with the host bridge. + * + */ +int __weak pcibios_fixup_bridge_ranges(struct list_head *resources) +{ + return 0; +} + +/** + * 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. + * + * It is the callers job to free the @resources list if an error is returned. + * + * 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) + return -ENOMEM; + + 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 */ + return pcibios_fixup_bridge_ranges(resources); +} + +/** + * 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) + goto err_create; + + /* then create the root bus */ + root_bus = pci_create_root_bus_in_domain(parent, domain, busno, + ops, host_data, &res); + if (IS_ERR(root_bus)) { + err = PTR_ERR(root_bus); + goto err_create; + } + + bridge = to_pci_host_bridge(root_bus->bridge); + bridge->io_base = io_base; + + return bridge; + +err_create: + pci_free_resource_list(&res); + return ERR_PTR(err); +} +EXPORT_SYMBOL_GPL(of_create_pci_host_bridge); + +#endif /* CONFIG_OF */ diff --git a/include/linux/pci.h b/include/linux/pci.h index 1eed009..40ddd3d 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); + +int 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 * +of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops, + void *host_data) +{ + return NULL; +} #endif /* CONFIG_OF */
#ifdef CONFIG_EEH
On Wed, 5 Mar 2014 11:48:57 +0000, Liviu Dudau Liviu.Dudau@arm.com wrote:
Several platforms use a rather generic version of parsing the device tree to find the host bridge ranges. Move the common code into the generic PCI code and use it to create a pci_host_bridge structure that can be used by arch code.
Based on early attempts by Andrew Murray to unify the code. Used powerpc and microblaze PCI code as starting point.
Signed-off-by: Liviu Dudau Liviu.Dudau@arm.com Tested-by: Tanmay Inamdar tinamdar@apm.com
Tentative ack for the whole series conditional on Arnd or Ben
g.
drivers/pci/host-bridge.c | 156 ++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 13 +++ 2 files changed, 169 insertions(+)
diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c index 8708b652..db9f51a 100644 --- a/drivers/pci/host-bridge.c +++ b/drivers/pci/host-bridge.c @@ -6,9 +6,14 @@ #include <linux/init.h> #include <linux/pci.h> #include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_pci.h> +#include <linux/slab.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 +97,154 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res, res->end = region->end + offset; } EXPORT_SYMBOL(pcibios_bus_to_resource);
+#ifdef CONFIG_OF +/**
- Simple version of the platform specific code for filtering the list
- of resources obtained from the ranges declaration in DT.
- Platforms can override this function in order to impose stronger
- constraints onto the list of resources that a host bridge can use.
- The filtered list will then be used to create a root bus and associate
- it with the host bridge.
- */
+int __weak pcibios_fixup_bridge_ranges(struct list_head *resources) +{
- return 0;
+}
+/**
- 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.
- It is the callers job to free the @resources list if an error is returned.
- 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)return -ENOMEM;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 */
- return pcibios_fixup_bridge_ranges(resources);
+}
+/**
- 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)
goto err_create;- /* then create the root bus */
- root_bus = pci_create_root_bus_in_domain(parent, domain, busno,
ops, host_data, &res);- if (IS_ERR(root_bus)) {
err = PTR_ERR(root_bus);goto err_create;- }
- bridge = to_pci_host_bridge(root_bus->bridge);
- bridge->io_base = io_base;
- return bridge;
+err_create:
- pci_free_resource_list(&res);
- return ERR_PTR(err);
+} +EXPORT_SYMBOL_GPL(of_create_pci_host_bridge);
+#endif /* CONFIG_OF */ diff --git a/include/linux/pci.h b/include/linux/pci.h index 1eed009..40ddd3d 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);+int 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 * +of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops,
void *host_data)+{
- return NULL;
+} #endif /* CONFIG_OF */
#ifdef CONFIG_EEH
1.9.0
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Fri, Mar 07, 2014 at 09:14:27PM +0000, Grant Likely wrote:
On Wed, 5 Mar 2014 11:48:57 +0000, Liviu Dudau Liviu.Dudau@arm.com wrote:
Several platforms use a rather generic version of parsing the device tree to find the host bridge ranges. Move the common code into the generic PCI code and use it to create a pci_host_bridge structure that can be used by arch code.
Based on early attempts by Andrew Murray to unify the code. Used powerpc and microblaze PCI code as starting point.
Signed-off-by: Liviu Dudau Liviu.Dudau@arm.com Tested-by: Tanmay Inamdar tinamdar@apm.com
Tentative ack for the whole series conditional on Arnd or Ben
Thanks Grant!
Liviu
g.
drivers/pci/host-bridge.c | 156 ++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 13 +++ 2 files changed, 169 insertions(+)
diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c index 8708b652..db9f51a 100644 --- a/drivers/pci/host-bridge.c +++ b/drivers/pci/host-bridge.c @@ -6,9 +6,14 @@ #include <linux/init.h> #include <linux/pci.h> #include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_pci.h> +#include <linux/slab.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 +97,154 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res, res->end = region->end + offset; } EXPORT_SYMBOL(pcibios_bus_to_resource);
+#ifdef CONFIG_OF +/**
- Simple version of the platform specific code for filtering the list
- of resources obtained from the ranges declaration in DT.
- Platforms can override this function in order to impose stronger
- constraints onto the list of resources that a host bridge can use.
- The filtered list will then be used to create a root bus and associate
- it with the host bridge.
- */
+int __weak pcibios_fixup_bridge_ranges(struct list_head *resources) +{
- return 0;
+}
+/**
- 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.
- It is the callers job to free the @resources list if an error is returned.
- 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)return -ENOMEM;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 */
- return pcibios_fixup_bridge_ranges(resources);
+}
+/**
- 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)
goto err_create;- /* then create the root bus */
- root_bus = pci_create_root_bus_in_domain(parent, domain, busno,
ops, host_data, &res);- if (IS_ERR(root_bus)) {
err = PTR_ERR(root_bus);goto err_create;- }
- bridge = to_pci_host_bridge(root_bus->bridge);
- bridge->io_base = io_base;
- return bridge;
+err_create:
- pci_free_resource_list(&res);
- return ERR_PTR(err);
+} +EXPORT_SYMBOL_GPL(of_create_pci_host_bridge);
+#endif /* CONFIG_OF */ diff --git a/include/linux/pci.h b/include/linux/pci.h index 1eed009..40ddd3d 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);+int 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 * +of_create_pci_host_bridge(struct device *parent, struct pci_ops *ops,
void *host_data)+{
- return NULL;
+} #endif /* CONFIG_OF */
#ifdef CONFIG_EEH
1.9.0
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
-- 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
On Wednesday 05 March 2014, Liviu Dudau wrote:
- 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)return -ENOMEM;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);- }
As mentioned regarding the pci_register_io_range() helper, x86 would not enter the 'resource_type(res) == IORESOURCE_IO' code path, which on the one hand is fine so we can return an error from pci_register_io_range() there, but I think it will lead to io_base getting an uninitialized content.
There could also be other reasons why pci_register_io_range() fails, e.g. because the space is exhausted, and I think we should try to catch that here and skip the pci_add_resource_offset() and io_base assignment then.
Arnd
On Sat, Mar 08, 2014 at 05:15:08PM +0000, Arnd Bergmann wrote:
On Wednesday 05 March 2014, Liviu Dudau wrote:
- 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)return -ENOMEM;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);- }
As mentioned regarding the pci_register_io_range() helper, x86 would not enter the 'resource_type(res) == IORESOURCE_IO' code path, which on the one hand is fine so we can return an error from pci_register_io_range() there, but I think it will lead to io_base getting an uninitialized content.
There could also be other reasons why pci_register_io_range() fails, e.g. because the space is exhausted, and I think we should try to catch that here and skip the pci_add_resource_offset() and io_base assignment then.
Hi Arnd,
I will try to improve the error handling in the next patchset version. However I am still confused about the earlier discussion on pci_register_io_range(). Your suggestion initially was to return an error in the default weak implementation, but in your last email you are talking about returning 'port'. My idea when I've introduced the helper function was that it would return an error if it fails to register the IO range and zero otherwise. I agree that we can treat the default 'do nothing with the IO range' case as an error, with the caveat that will force architectures that use this code to provide their own implementation of pci_register_io_range() in order to avoid failure, even for the cases where the architecture has a 1:1 mapping between IO and CPU addresses.
I've just noticed that my home server has silently dropped my reply to you from the 7th of March, so I'm going to resend it using ARM's setup.
Best regards, Liviu
Arnd
On Monday 10 March 2014 14:44:14 Liviu Dudau wrote:
I will try to improve the error handling in the next patchset version. However I am still confused about the earlier discussion on pci_register_io_range(). Your suggestion initially was to return an error in the default weak implementation, but in your last email you are talking about returning 'port'.
You can do either one: 'port' should be positive or zero, while the error would always be negative. We do the same thing in many interfaces in the kernel.
My idea when I've introduced the helper function was that it would return an error if it fails to register the IO range and zero otherwise. I agree that we can treat the default 'do nothing with the IO range' case as an error, with the caveat that will force architectures that use this code to provide their own implementation of pci_register_io_range() in order to avoid failure, even for the cases where the architecture has a 1:1 mapping between IO and CPU addresses.
Which architectures are you thinking of? The only one I know that does this is ia64, and we won't ever have to support this helper on that architecture.
I did not ask to treat 'do nothing with the IO range' as an error, what I meant is that we should treat 'architecture cannot translate from I/O space to memory space but DT lists a translation anyway' as an error. On x86, you should never see an entry for the I/O space in "ranges", so we will not call this function unless there is a bug in DT.
Arnd
On Mon, Mar 10, 2014 at 03:21:01PM +0000, Arnd Bergmann wrote:
On Monday 10 March 2014 14:44:14 Liviu Dudau wrote:
I will try to improve the error handling in the next patchset version. However I am still confused about the earlier discussion on pci_register_io_range(). Your suggestion initially was to return an error in the default weak implementation, but in your last email you are talking about returning 'port'.
You can do either one: 'port' should be positive or zero, while the error would always be negative. We do the same thing in many interfaces in the kernel.
My idea when I've introduced the helper function was that it would return an error if it fails to register the IO range and zero otherwise. I agree that we can treat the default 'do nothing with the IO range' case as an error, with the caveat that will force architectures that use this code to provide their own implementation of pci_register_io_range() in order to avoid failure, even for the cases where the architecture has a 1:1 mapping between IO and CPU addresses.
Which architectures are you thinking of? The only one I know that does this is ia64, and we won't ever have to support this helper on that architecture.
I was thinking about architectures that have IO_SPACE_LIMIT >= 0xffffffff. While not an absolute indicator, with the default pci_address_to_pio() that means that they can use the CPU MMIO address as IO address directly.
$ git grep IO_SPACE_LIMIT | grep -i ffffffff arch/arm/include/asm/io.h:#define IO_SPACE_LIMIT ((resource_size_t)0xffffffff) arch/arm/mach-at91/include/mach/io.h:#define IO_SPACE_LIMIT 0xFFFFFFFF arch/arm/mach-omap1/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/arm/mach-pxa/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/arm/mach-s3c24xx/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/avr32/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/frv/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/ia64/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffffffffffffUL arch/m32r/include/asm/io.h:#define IO_SPACE_LIMIT 0xFFFFFFFF arch/m68k/include/asm/io_no.h:#define IO_SPACE_LIMIT 0xffffffff arch/microblaze/include/asm/io.h:#define IO_SPACE_LIMIT (0xFFFFFFFF) arch/mn10300/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/sh/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/sparc/include/asm/io_32.h:#define IO_SPACE_LIMIT 0xffffffff arch/sparc/include/asm/io_64.h:#define IO_SPACE_LIMIT 0xffffffffffffffffUL arch/tile/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
I did not ask to treat 'do nothing with the IO range' as an error, what I meant is that we should treat 'architecture cannot translate from I/O space to memory space but DT lists a translation anyway' as an error. On x86, you should never see an entry for the I/O space in "ranges", so we will not call this function unless there is a bug in DT.
Ok, it's just that there is no "architecture cannot translate from I/O space to memory space" indicator anywhere and I don't want to make x86 a special case.
So, my proposal is this: default weak implementation of pci_register_io_range() returns an error (meaning I have no idea how to translate IO addresses to memory space) and anyone that wants this function to return success will have to provide their implementation.
I will send an updated series.
Best regards, Liviu
Arnd
On Monday 10 March 2014 16:33:44 Liviu Dudau wrote:
On Mon, Mar 10, 2014 at 03:21:01PM +0000, Arnd Bergmann wrote:
On Monday 10 March 2014 14:44:14 Liviu Dudau wrote:
I will try to improve the error handling in the next patchset version. However I am still confused about the earlier discussion on pci_register_io_range(). Your suggestion initially was to return an error in the default weak implementation, but in your last email you are talking about returning 'port'.
You can do either one: 'port' should be positive or zero, while the error would always be negative. We do the same thing in many interfaces in the kernel.
My idea when I've introduced the helper function was that it would return an error if it fails to register the IO range and zero otherwise. I agree that we can treat the default 'do nothing with the IO range' case as an error, with the caveat that will force architectures that use this code to provide their own implementation of pci_register_io_range() in order to avoid failure, even for the cases where the architecture has a 1:1 mapping between IO and CPU addresses.
Which architectures are you thinking of? The only one I know that does this is ia64, and we won't ever have to support this helper on that architecture.
I was thinking about architectures that have IO_SPACE_LIMIT >= 0xffffffff. While not an absolute indicator, with the default pci_address_to_pio() that means that they can use the CPU MMIO address as IO address directly.
Not really, that would only work if they also have instructions to do raw accesses to physical memory addresses rather than virtual memory pointers that most architectures do.
$ git grep IO_SPACE_LIMIT | grep -i ffffffff arch/arm/include/asm/io.h:#define IO_SPACE_LIMIT ((resource_size_t)0xffffffff) arch/arm/mach-at91/include/mach/io.h:#define IO_SPACE_LIMIT 0xFFFFFFFF arch/arm/mach-omap1/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/arm/mach-pxa/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/arm/mach-s3c24xx/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff
These use a special trick where an __iomem pointer is the same as the port number. This works most of the time, but breaks anything that assumes that port numbers are low, such as /dev/port or broken devices. Moreover, it means your code won't work because it depends on passing the virtual start address of the PIO mapping window as io_offset.
arch/avr32/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/frv/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
They have no MMU, and the code relies on the port number to match both the virtual and the physical address. You could be right about these, but I would guess that the code also needs some other changes if we want to make it work on nommu kernels. It also depends on whether the I/O bus address is the same as the CPU address, or if it starts at bus address 0.
arch/ia64/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffffffffffffUL
Here, the definition is special, the token is just used to encode a space number and an offset within the I/O space.
arch/m32r/include/asm/io.h:#define IO_SPACE_LIMIT 0xFFFFFFFF
no PCI here.
arch/m68k/include/asm/io_no.h:#define IO_SPACE_LIMIT 0xffffffff
This looks like a mistake, it should be smaller
arch/microblaze/include/asm/io.h:#define IO_SPACE_LIMIT (0xFFFFFFFF)
I suspect it doesn't actually work. microblaze copied large parts of this from PowerPC, but the parts that differ apparently get it wrong for the I/O space.
arch/mn10300/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
Same category as frv. We should ask David Howells whether he thinks I/O space actually works on these.
arch/sh/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
I think this should just be 0xffff.
arch/sparc/include/asm/io_32.h:#define IO_SPACE_LIMIT 0xffffffff arch/sparc/include/asm/io_64.h:#define IO_SPACE_LIMIT 0xffffffffffffffffUL
Sparc actually accesses the physical addresses, so in theory it could always work. In the 64-bit case it would however have to check that the port number is smaller than 0xffffffff, otherwise you couldn't set the BAR. This means you still need a custom function.
arch/tile/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
tile seems to support only ioport_map() but not inb/outb, if I'm reading this right.
I did not ask to treat 'do nothing with the IO range' as an error, what I meant is that we should treat 'architecture cannot translate from I/O space to memory space but DT lists a translation anyway' as an error. On x86, you should never see an entry for the I/O space in "ranges", so we will not call this function unless there is a bug in DT.
Ok, it's just that there is no "architecture cannot translate from I/O space to memory space" indicator anywhere and I don't want to make x86 a special case.
Right.
So, my proposal is this: default weak implementation of pci_register_io_range() returns an error (meaning I have no idea how to translate IO addresses to memory space) and anyone that wants this function to return success will have to provide their implementation.
Another idea: make this conditional on the definition of PCI_IOBASE: If this is defined, we can use the arm64 version that uses this number. Otherwise we fall back to returning an error, which means that either on the architecture we shouldn't be calling that function, or we need a custom implementation.
Arnd
On Mon, Mar 10, 2014 at 7:59 PM, Arnd Bergmann arnd@arndb.de wrote:
arch/avr32/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/frv/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
They have no MMU, and the code relies on the port number to match both the virtual and the physical address. You could be right about these, but I would guess that the code also needs some other changes if we want to make it work on nommu kernels. It also depends on whether the I/O bus address is the same as the CPU address, or if it starts at bus address 0.
arch/m68k/include/asm/io_no.h:#define IO_SPACE_LIMIT 0xffffffff
This looks like a mistake, it should be smaller
io_no.h is for nommu.
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Monday 10 March 2014 20:16:25 Geert Uytterhoeven wrote:
On Mon, Mar 10, 2014 at 7:59 PM, Arnd Bergmann arnd@arndb.de wrote:
arch/avr32/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/frv/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
They have no MMU, and the code relies on the port number to match both the virtual and the physical address. You could be right about these, but I would guess that the code also needs some other changes if we want to make it work on nommu kernels. It also depends on whether the I/O bus address is the same as the CPU address, or if it starts at bus address 0.
arch/m68k/include/asm/io_no.h:#define IO_SPACE_LIMIT 0xffffffff
This looks like a mistake, it should be smaller
io_no.h is for nommu.
Ah, I missed that. In that case I assume it doesn't matter because the only m68k with PCI is M54xx and that always has an MMU.
Arnd
On Mon, Mar 10, 2014 at 07:59:59PM +0100, Arnd Bergmann wrote:
On Monday 10 March 2014 16:33:44 Liviu Dudau wrote:
On Mon, Mar 10, 2014 at 03:21:01PM +0000, Arnd Bergmann wrote:
On Monday 10 March 2014 14:44:14 Liviu Dudau wrote:
I will try to improve the error handling in the next patchset version. However I am still confused about the earlier discussion on pci_register_io_range(). Your suggestion initially was to return an error in the default weak implementation, but in your last email you are talking about returning 'port'.
You can do either one: 'port' should be positive or zero, while the error would always be negative. We do the same thing in many interfaces in the kernel.
My idea when I've introduced the helper function was that it would return an error if it fails to register the IO range and zero otherwise. I agree that we can treat the default 'do nothing with the IO range' case as an error, with the caveat that will force architectures that use this code to provide their own implementation of pci_register_io_range() in order to avoid failure, even for the cases where the architecture has a 1:1 mapping between IO and CPU addresses.
Which architectures are you thinking of? The only one I know that does this is ia64, and we won't ever have to support this helper on that architecture.
I was thinking about architectures that have IO_SPACE_LIMIT >= 0xffffffff. While not an absolute indicator, with the default pci_address_to_pio() that means that they can use the CPU MMIO address as IO address directly.
Not really, that would only work if they also have instructions to do raw accesses to physical memory addresses rather than virtual memory pointers that most architectures do.
$ git grep IO_SPACE_LIMIT | grep -i ffffffff arch/arm/include/asm/io.h:#define IO_SPACE_LIMIT ((resource_size_t)0xffffffff) arch/arm/mach-at91/include/mach/io.h:#define IO_SPACE_LIMIT 0xFFFFFFFF arch/arm/mach-omap1/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/arm/mach-pxa/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/arm/mach-s3c24xx/include/mach/io.h:#define IO_SPACE_LIMIT 0xffffffff
These use a special trick where an __iomem pointer is the same as the port number. This works most of the time, but breaks anything that assumes that port numbers are low, such as /dev/port or broken devices. Moreover, it means your code won't work because it depends on passing the virtual start address of the PIO mapping window as io_offset.
arch/avr32/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff arch/frv/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
They have no MMU, and the code relies on the port number to match both the virtual and the physical address. You could be right about these, but I would guess that the code also needs some other changes if we want to make it work on nommu kernels. It also depends on whether the I/O bus address is the same as the CPU address, or if it starts at bus address 0.
arch/ia64/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffffffffffffUL
Here, the definition is special, the token is just used to encode a space number and an offset within the I/O space.
arch/m32r/include/asm/io.h:#define IO_SPACE_LIMIT 0xFFFFFFFF
no PCI here.
arch/m68k/include/asm/io_no.h:#define IO_SPACE_LIMIT 0xffffffff
This looks like a mistake, it should be smaller
arch/microblaze/include/asm/io.h:#define IO_SPACE_LIMIT (0xFFFFFFFF)
I suspect it doesn't actually work. microblaze copied large parts of this from PowerPC, but the parts that differ apparently get it wrong for the I/O space.
arch/mn10300/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
Same category as frv. We should ask David Howells whether he thinks I/O space actually works on these.
arch/sh/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
I think this should just be 0xffff.
arch/sparc/include/asm/io_32.h:#define IO_SPACE_LIMIT 0xffffffff arch/sparc/include/asm/io_64.h:#define IO_SPACE_LIMIT 0xffffffffffffffffUL
Sparc actually accesses the physical addresses, so in theory it could always work. In the 64-bit case it would however have to check that the port number is smaller than 0xffffffff, otherwise you couldn't set the BAR. This means you still need a custom function.
arch/tile/include/asm/io.h:#define IO_SPACE_LIMIT 0xffffffff
tile seems to support only ioport_map() but not inb/outb, if I'm reading this right.
I did not ask to treat 'do nothing with the IO range' as an error, what I meant is that we should treat 'architecture cannot translate from I/O space to memory space but DT lists a translation anyway' as an error. On x86, you should never see an entry for the I/O space in "ranges", so we will not call this function unless there is a bug in DT.
Ok, it's just that there is no "architecture cannot translate from I/O space to memory space" indicator anywhere and I don't want to make x86 a special case.
Right.
So, my proposal is this: default weak implementation of pci_register_io_range() returns an error (meaning I have no idea how to translate IO addresses to memory space) and anyone that wants this function to return success will have to provide their implementation.
Another idea: make this conditional on the definition of PCI_IOBASE: If this is defined, we can use the arm64 version that uses this number. Otherwise we fall back to returning an error, which means that either on the architecture we shouldn't be calling that function, or we need a custom implementation.
PCI_IOBASE is always defined. See the discussion with Russell on this subject.
include/asm-generic/io.h has at line 118:
#ifndef PCI_IOBASE #define PCI_IOBASE ((void __iomem *) 0) #endif
I will go with my idea tomorrow. arm64 overwrite the implementation anyway, I find it cleaner rather than having to do #ifdefs and/or ifs.
Best regards, 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
On Monday 10 March 2014 21:56:00 Liviu Dudau wrote:
PCI_IOBASE is always defined. See the discussion with Russell on this subject.
include/asm-generic/io.h has at line 118:
#ifndef PCI_IOBASE #define PCI_IOBASE ((void __iomem *) 0) #endif
That is only defined for those that use asm-generic/pci.h, which most architectures don't.
I will go with my idea tomorrow. arm64 overwrite the implementation anyway, I find it cleaner rather than having to do #ifdefs and/or ifs.
I'd really hope we can get to a point where arm64 doesn't need any architecture specific code for this. It doesn't do anything special.
Arnd
On Tue, Mar 11, 2014 at 06:50:24AM +0000, Arnd Bergmann wrote:
On Monday 10 March 2014 21:56:00 Liviu Dudau wrote:
PCI_IOBASE is always defined. See the discussion with Russell on this subject.
include/asm-generic/io.h has at line 118:
#ifndef PCI_IOBASE #define PCI_IOBASE ((void __iomem *) 0) #endif
That is only defined for those that use asm-generic/pci.h, which most architectures don't.
I think it is defined for anyone that #includes <asm-generic/io.h>. There is no other #ifdef around that.
I will go with my idea tomorrow. arm64 overwrite the implementation anyway, I find it cleaner rather than having to do #ifdefs and/or ifs.
I'd really hope we can get to a point where arm64 doesn't need any architecture specific code for this. It doesn't do anything special.
I agree.
Best regards, Liviu
Arnd
On Tuesday 11 March 2014 09:46:41 Liviu Dudau wrote:
On Tue, Mar 11, 2014 at 06:50:24AM +0000, Arnd Bergmann wrote:
On Monday 10 March 2014 21:56:00 Liviu Dudau wrote:
PCI_IOBASE is always defined. See the discussion with Russell on this subject.
include/asm-generic/io.h has at line 118:
#ifndef PCI_IOBASE #define PCI_IOBASE ((void __iomem *) 0) #endif
That is only defined for those that use asm-generic/pci.h, which most architectures don't.
I think it is defined for anyone that #includes <asm-generic/io.h>. There is no other #ifdef around that.
My mistake, I meant to write asm-generic/io.h.
On a related note, I would actually prefer to get rid of this PCI_IOBASE default and move it into the architectures that really want it like this. The default when PCI_IOBASE is not set IMHO should be to also not provide inb/outb and ioport_map() helpers, but we need a little more infrastructure to actually make the kernel build in all valid configuration when we remove them.
Arnd
linaro-kernel@lists.linaro.org
-
Arnd Bergmann -
Geert Uytterhoeven -
Grant Likely -
Liviu Dudau -
Liviu Dudau