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

2014-02-28 Thread 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 

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

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

2014-02-28 Thread Tanmay Inamdar
Earlier email did not deliver to mailing lists because of plain text
setting problem on my side. Apologies for spamming. Sending it again.

Hello Liviu,

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

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

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

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

Thanks,
Tanmay

On Fri, Feb 28, 2014 at 6:01 PM, Tanmay Inamdar  wrote:
> Hello Liviu,
>
> While porting X-Gene PCIe driver to v2 series, following problems were
> observed.
>
> 1. In 'of_create_pci_host_bridge' function, bus_range is defined locally.
> So, while walking over list of resources in bridge->windows later, during
> X-Gene controller related setup, garbage values are found in the resource.
> Please allocate it dynamically.
>
> 2. 'domain_nr' problem is partially solved. There are still some places
> where functions are getting invalid domain_nr.  For example,
> 'pci_alloc_child_bus' tries to get domain_nr when bridge is not assigned to
> bus. You may want to look for all the places where pci_domain_nr is used.
> Please see below dump -->
>
> pci 0001:00:00.0: scanning [bus 00-00] behind bridge, pass 1
> [ cut here ]
> WARNING: CPU: 0 PID: 1 at
> /home/tinamdar/work/open-source/linux/fs/sysfs/dir.c:52
> sysfs_warn_dup+0x80/0xc0()
> sysfs: cannot create duplicate filename '/class/pci_bus/:01'
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.0-rc4+ #37
> Call trace:
> [] dump_backtrace+0x0/0x140
> [] show_stack+0x14/0x20
> [] dump_stack+0x78/0xc4
> [] warn_slowpath_common+0x88/0xc0
> [] warn_slowpath_fmt+0x50/0x60
> [] sysfs_warn_dup+0x80/0xc0
> [] sysfs_do_create_link_sd.isra.2+0xf8/0x100
> [] sysfs_create_link+0x20/0x40
> [] device_add+0x41c/0x520
> [] device_register+0x1c/0x40
> [] pci_add_new_bus+0x284/0x380
> [] pci_scan_bridge+0x4e0/0x540
> [] pci_scan_child_bus+0xb4/0x140
> [] pci_rescan_bus+0x14/0x40
> [] xgene_pcie_probe_bridge+0x688/0x750
> [] platform_drv_probe+0x24/0x60
> [] really_probe+0xf4/0x220
> [] __driver_attach+0xa4/0xc0
> [] bus_for_each_dev+0x58/0xa0
> [] driver_attach+0x20/0x40
> [] bus_add_driver+0x150/0x220
> [] driver_register+0x60/0x120
> [] __platform_driver_register+0x60/0x80
> [] xgene_pcie_driver_init+0x18/0x20
> [] do_one_initcall+0xe4/0x160
> [] kernel_init_freeable+0x138/0x1d8
> [] kernel_init+0x10/0xe0
> ---[ end trace 53db1c3a7fbdeb88 ]---
> [ cut here ]
> WARNING: CPU: 0 PID: 1 at
> /home/tinamdar/work/open-source/linux/drivers/pci/probe.c:711
> pci_add_new_bus+0x36c/0x380()
>
> Thanks,
> Tanmay
>
>
>
> On Fri, Feb 28, 2014 at 5:08 AM, Liviu Dudau  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 
>>
>> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>> index 06ace62

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

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

Hello Tanmay,

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

Thanks for trying it out.

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

Bah, sorry for that. Will fix.

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

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

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

Best regards,
Liviu

> [] kernel_init_freeable+0x138/0x1d8
> [] kernel_init+0x10/0xe0
> ---[ end trace 53db1c3a7fbdeb88 ]---
> [ cut here ]
> WARNING: CPU: 0 PID: 1 at
> /home/tinamdar/work/open-source/linux/drivers/pci/probe.c:711
> pci_add_new_bus+0x36c/0x380()
> 
> Thanks,
> Tanmay
> 
> On Fri, Feb 28, 2014 at 6:01 PM, Tanmay Inamdar  wrote:
> > Hello Liviu,
> >
> > While porting X-Gene PCIe driver to v2 series, following problems were
> > observed.
> >
> > 1. In 'of_create_pci_host_bridge' function, bus_range is defined locally.
> > So, while walking over list of resources in bridge->windows later, during
> > X-Gene controller related setup, garbage values are found in the resource.
> > Please allocate it dynamically.
> >
> > 2. 'domain_nr' problem is partially solved. There are still some places
> > where functions are getting invalid domain_nr.  For example,
> > 'pci_alloc_child_bus' tries to get domain_nr when bridge is not assigned to
> > bus. You may want to look for all the places where pci_domain_nr is used.
> > Please see below dump -->
> >
> > pci 0001:00:00.0: scanning [bus 00-00] behind bridge, pass 1
> > [ cut here ]
> > WARNING: CPU: 0 PID: 1 at
> > /home/tinamdar/work/open-source/linux/fs/sysfs/dir.c:52
> > sysfs_warn_dup+0x80/0xc0()
> > sysfs: cannot create duplicate filename '/class/pci_bus/:01'
> > Modules linked in:
> > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.0-rc4+ #37
> > Call trace:
> > [] dump_backtrace+0x0/0x140
> > [] show_stack+0x14/0x20
> > [] dump_stack+0x78/0xc4
> > [] warn_slowpath_common+0x88/0xc0
> > [] warn_slowpath_fmt+0x50/0x60
> > [] sysfs_warn_dup+0x80/0xc0
> > [] sysfs_do_create_link_sd.isra.2+0xf8/0x100
> > [] sysfs_create_link+0x20/0x40
> > [] device_add+0x41c/0x520
> > [] device_register+0x1c/0x40
> > [] pci_add_new_bus+0x284/0x380
> > [] pci_scan_bridge+0x4e0/0x540
> > [] pci_scan_child_bus+0xb4/0x140
> > [] pci_rescan_bus+0x14/0x40
> > [] xgene_pcie_probe_bridge+0x688/0x750
> > [] platform_drv_probe+0x24/0x60
> > [] really_probe+0xf4/0x220
> > [] __driver_attach+0xa4/0xc0
> > [] bus_for_each_dev+0x58/0xa0
> > [] driver_attach+0x20/0x40
> > [] bus_add_driver+0x150/0x220
> > [] driver_register+0x60/0x120
> > [] __platform_driver_register+0x60/0x80
> > [] xgene_pcie_driver_init+0x18/0x20
> > [] do_one_initcall+0xe4/0x160
> > [] kernel_init_freeable+0x138/0x1d8
> > [] kernel_init+0x10/0xe0
> > ---[ end trace 

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

2014-03-01 Thread Tanmay Inamdar
Hello,

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

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

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

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

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

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

>
> Best regards,
> Liviu
>
>> [] kernel_init_freeable+0x138/0x1d8
>> [] kernel_init+0x10/0xe0
>> ---[ end trace 53db1c3a7fbdeb88 ]---
>> [ cut here ]
>> WARNING: CPU: 0 PID: 1 at
>> /home/tinamdar/work/open-source/linux/drivers/pci/probe.c:711
>> pci_add_new_bus+0x36c/0x380()
>>
>> Thanks,
>> Tanmay
>>
>> On Fri, Feb 28, 2014 at 6:01 PM, Tanmay Inamdar  wrote:
>> > Hello Liviu,
>> >
>> > While porting X-Gene PCIe driver to v2 series, following problems were
>> > observed.
>> >
>> > 1. In 'of_create_pci_host_bridge' function, bus_range is defined locally.
>> > So, while walking over list of resources in bridge->windows later, during
>> > X-Gene controller related setup, garbage values are found in the resource.
>> > Please allocate it dynamically.
>> >
>> > 2. 'domain_nr' problem is partially solved. There are still some places
>> > where functions are getting invalid domain_nr.  For example,
>> > 'pci_alloc_child_bus' tries to get domain_nr when bridge is not assigned to
>> > bus. You may want to look for all the places where pci_domain_nr is used.
>> > Please see below dump -->
>> >
>> > pci 0001:00:00.0: scanning [bus 00-00] behind bridge, pass 1
>> > [ cut here ]
>> > WARNING: CPU: 0 PID: 1 at
>> > /home/tinamdar/work/open-source/linux/fs/sysfs/dir.c:52
>> > sysfs_warn_dup+0x80/0xc0()
>> > sysfs: cannot create duplicate filename '/class/pci_bus/:01'
>> > Modules linked in:
>> > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.0-rc4+ #37
>> > Call trace:
>> > [] dump_backtrace+0x0/0x140
>> > [] show_stack+0x14/0x20
>> > [] dump_stack+0x78/0xc4
>> > [] warn_slowpath_common+0x88/0xc0
>> > [] warn_slowpath_fmt+0x50/0x60
>> > [] sysfs_warn_dup+0x80/0xc0
>> > [] sysfs_do_create_link_sd.isra.2+0xf8/0x100
>> > [] sysfs_create_link+0x20/0x40
>> > [] de