[PATCH v2 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources

[PATCH v2 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources

[Deng-Cheng Zhu]

Change the pci_create_bus() interface to pass in available resources to get them
settled down early. This is to avoid possible resource conflicts while doing
pci_scan_slot() in pci_scan_child_bus(). Note that pcibios_fixup_bus() can get
rid of such conflicts, but it's done AFTER scanning slots.

In addition, MIPS PCI resources are now fixed using this new interface.

-- Changes --
v2 - v1:
o Merge [PATCH 1/3] to [PATCH 3/3], so now 2 patches in total.
o Add more info to patch description.
o Fix arch breaks in default resource setup discovered by Bjorn Helgaas.

Deng-Cheng Zhu (2):
  PCI: Pass available resources into pci_create_bus()
  MIPS: PCI: Pass controller's resources to pci_create_bus() in
    pcibios_scanbus()

 arch/microblaze/pci/pci-common.c |    3 +-
 arch/mips/pci/pci.c              |   53 ++++++++++++++++++++++++++++++++++---
 arch/powerpc/kernel/pci-common.c |    3 +-
 arch/sparc/kernel/pci.c          |    3 +-
 arch/x86/pci/acpi.c              |    2 +-
 drivers/pci/probe.c              |   15 ++++++++---
 include/linux/pci.h              |    3 +-
 7 files changed, 68 insertions(+), 14 deletions(-)

[PATCH v2 1/2] PCI: Pass available resources into pci_create_bus()

[Deng-Cheng Zhu]

Currently, after pci_create_bus(), resources available on the bus could be
handled by pci_scan_child_bus(). The problem is that, in
pci_scan_child_bus(), before calling arch-dependent pcibios_fixup_bus(),
PCI quirks will probably conflict (while doing pci_claim_resource()) with
resources like system controller's I/O resource that have not yet been
added to the bus. One can see that, by default, ioport_resource and
iomem_resource are filled into the bus->resource[] array as the initial
resources in pci_create_bus(). So, to avoid such conflicts, add those
really available resources right before returning the newly created bus in
pci_create_bus() whose interface should then be extended to receive them.

A related discussion thread can be found here:
http://www.spinics.net/lists/mips/msg41654.html

Signed-off-by: Deng-Cheng Zhu <dczhu@mips.com>
---
Changes (v2 - v1):
o Add more info to the patch description.
o Fix arch breaks in default resource setup discovered by Bjorn Helgaas.

 arch/microblaze/pci/pci-common.c |    3 ++-
 arch/powerpc/kernel/pci-common.c |    3 ++-
 arch/sparc/kernel/pci.c          |    3 ++-
 arch/x86/pci/acpi.c              |    2 +-
 drivers/pci/probe.c              |   15 +++++++++++----
 include/linux/pci.h              |    3 ++-
 6 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 4cfae20..9c35aa6 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -1581,7 +1581,8 @@ static void __devinit pcibios_scan_phb(struct pci_controller *hose)
 		 node ? node->full_name : "<NO NAME>");
 
 	/* Create an empty bus for the toplevel */
-	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, hose);
+	bus = pci_create_bus(hose->parent, hose->first_busno,
+			     hose->ops, hose, NULL);
 	if (bus == NULL) {
 		printk(KERN_ERR "Failed to create bus for PCI domain %04x\n",
 		       hose->global_number);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 32656f1..2ede26a 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1703,7 +1703,8 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 		 node ? node->full_name : "<NO NAME>");
 
 	/* Create an empty bus for the toplevel */
-	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, hose);
+	bus = pci_create_bus(hose->parent, hose->first_busno,
+			     hose->ops, hose, NULL);
 	if (bus == NULL) {
 		pr_err("Failed to create bus for PCI domain %04x\n",
 			hose->global_number);
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 1e94f94..77c38bb 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -689,7 +689,8 @@ struct pci_bus * __devinit pci_scan_one_pbm(struct pci_pbm_info *pbm,
 
 	printk("PCI: Scanning PBM %s\n", node->full_name);
 
-	bus = pci_create_bus(parent, pbm->pci_first_busno, pbm->pci_ops, pbm);
+	bus = pci_create_bus(parent, pbm->pci_first_busno,
+			     pbm->pci_ops, pbm, NULL);
 	if (!bus) {
 		printk(KERN_ERR "Failed to create bus for %s\n",
 		       node->full_name);
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index c953302..bab2113 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -353,7 +353,7 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
 		memcpy(bus->sysdata, sd, sizeof(*sd));
 		kfree(sd);
 	} else {
-		bus = pci_create_bus(NULL, busnum, &pci_root_ops, sd);
+		bus = pci_create_bus(NULL, busnum, &pci_root_ops, sd, NULL);
 		if (bus) {
 			get_current_resources(device, busnum, domain, bus);
 			bus->subordinate = pci_scan_child_bus(bus);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8473727..47a364c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1516,7 +1516,8 @@ unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
 }
 
 struct pci_bus * pci_create_bus(struct device *parent,
-		int bus, struct pci_ops *ops, void *sysdata)
+		int bus, struct pci_ops *ops, void *sysdata,
+		struct pci_bus_resource *bus_res)
 {
 	int error;
 	struct pci_bus *b, *b2;
@@ -1570,8 +1571,14 @@ struct pci_bus * pci_create_bus(struct device *parent,
 	pci_create_legacy_files(b);
 
 	b->number = b->secondary = bus;
-	b->resource[0] = &ioport_resource;
-	b->resource[1] = &iomem_resource;
+
+	/* Add initial resources to the bus */
+	if (bus_res != NULL)
+		list_add_tail(&b->resources, &bus_res->list);
+	else {
+		b->resource[0] = &ioport_resource;
+		b->resource[1] = &iomem_resource;
+	}
 
 	return b;
 
@@ -1592,7 +1599,7 @@ struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent,
 {
 	struct pci_bus *b;
 
-	b = pci_create_bus(parent, bus, ops, sysdata);
+	b = pci_create_bus(parent, bus, ops, sysdata, NULL);
 	if (b)
 		b->subordinate = pci_scan_child_bus(b);
 	return b;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8c230cb..5e1bacd 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -666,7 +666,8 @@ static inline struct pci_bus * __devinit pci_scan_bus(int bus, struct pci_ops *o
 	return root_bus;
 }
 struct pci_bus *pci_create_bus(struct device *parent, int bus,
-			       struct pci_ops *ops, void *sysdata);
+			       struct pci_ops *ops, void *sysdata,
+			       struct pci_bus_resource *bus_res);
 struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
 				int busnr);
 void pcie_update_link_speed(struct pci_bus *bus, u16 link_status);
-- 
1.7.1

[PATCH v2 2/2] MIPS: PCI: Pass controller's resources to pci_create_bus() in pcibios_scanbus()

[Deng-Cheng Zhu]

Use the new interface of pci_create_bus() so that system controller's
resources are added to the root bus upon bus creation, thereby avoiding
conflicts with PCI quirks before pcibios_fixup_bus() gets the chance to do
right things in pci_scan_child_bus(). Also, since we are passing resources
to pci_create_bus() and setting them up in the bus->resources list as
opposed to the bus->resource[] array, we have to adopt the list style while
doing bus fixups later on, or else, for example in this MIPS case, system
controller's resources will stay in both bus->resources and
bus->resource[].

Signed-off-by: Deng-Cheng Zhu <dczhu@mips.com>
---
Changes (v2 - v1):
o Merge [PATCH 1/3] to [PATCH 3/3] of v1.
o Add more info to patch description.

 arch/mips/pci/pci.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 33bba7b..30856d6 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -76,11 +76,41 @@ pcibios_align_resource(void *data, const struct resource *res,
 	return start;
 }
 
+static struct pci_bus_resource *
+controller_resources(const struct pci_controller *ctrl)
+{
+	struct pci_bus_resource *mem_res, *io_res;
+
+	mem_res = kzalloc(sizeof(struct pci_bus_resource), GFP_KERNEL);
+	if (!mem_res)
+		goto err_out;
+
+	mem_res->res = ctrl->mem_resource;
+	mem_res->flags = 0;
+	INIT_LIST_HEAD(&mem_res->list);
+
+	io_res = kzalloc(sizeof(struct pci_bus_resource), GFP_KERNEL);
+	if (!io_res) {
+		kfree(mem_res);
+		goto err_out;
+	}
+
+	io_res->res = ctrl->io_resource;
+	io_res->flags = 0;
+	list_add(&io_res->list, &mem_res->list);
+
+	return mem_res;
+err_out:
+	printk(KERN_ERR "Can't allocate PCI bus resource.\n");
+	return NULL;
+}
+
 static void __devinit pcibios_scanbus(struct pci_controller *hose)
 {
 	static int next_busno;
 	static int need_domain_info;
 	struct pci_bus *bus;
+	struct pci_bus_resource *bus_res;
 
 	if (!hose->iommu)
 		PCI_DMA_BUS_IS_PHYS = 1;
@@ -88,7 +118,13 @@ static void __devinit pcibios_scanbus(struct pci_controller *hose)
 	if (hose->get_busno && pci_probe_only)
 		next_busno = (*hose->get_busno)();
 
-	bus = pci_scan_bus(next_busno, hose->pci_ops, hose);
+	bus_res = controller_resources(hose);
+	bus = pci_create_bus(NULL, next_busno, hose->pci_ops, hose, bus_res);
+	if (bus) {
+		bus->subordinate = pci_scan_child_bus(bus);
+		pci_bus_add_devices(bus);
+	}
+
 	hose->bus = bus;
 
 	need_domain_info = need_domain_info || hose->index;
@@ -261,6 +297,14 @@ static void pcibios_fixup_device_resources(struct pci_dev *dev,
 	}
 }
 
+static void __devinit
+pcibios_setup_root_resources(struct pci_bus *bus, struct pci_controller *ctrl)
+{
+	pci_bus_remove_resources(bus);
+	pci_bus_add_resource(bus, ctrl->io_resource, 0);
+	pci_bus_add_resource(bus, ctrl->mem_resource, 0);
+}
+
 void __devinit pcibios_fixup_bus(struct pci_bus *bus)
 {
 	/* Propagate hose info into the subordinate devices.  */
@@ -269,10 +313,9 @@ void __devinit pcibios_fixup_bus(struct pci_bus *bus)
 	struct list_head *ln;
 	struct pci_dev *dev = bus->self;
 
-	if (!dev) {
-		bus->resource[0] = hose->io_resource;
-		bus->resource[1] = hose->mem_resource;
-	} else if (pci_probe_only &&
+	if (!dev)
+		pcibios_setup_root_resources(bus, hose);
+	else if (pci_probe_only &&
 		   (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
 		pci_read_bridge_bases(bus);
 		pcibios_fixup_device_resources(dev, bus);
-- 
1.7.1

Re: [PATCH v2 2/2] MIPS: PCI: Pass controller's resources to pci_create_bus() in pcibios_scanbus()

[Bjorn Helgaas]

On Thu, Aug 25, 2011 at 5:18 AM, Deng-Cheng Zhu <dczhu@mips.com> wrote:
> Use the new interface of pci_create_bus() so that system controller's
> resources are added to the root bus upon bus creation, thereby avoiding
> conflicts with PCI quirks before pcibios_fixup_bus() gets the chance to do
> right things in pci_scan_child_bus(). Also, since we are passing resources
> to pci_create_bus() and setting them up in the bus->resources list as
> opposed to the bus->resource[] array, we have to adopt the list style while
> doing bus fixups later on, or else, for example in this MIPS case, system
> controller's resources will stay in both bus->resources and
> bus->resource[].

> +static void __devinit
> +pcibios_setup_root_resources(struct pci_bus *bus, struct pci_controller *ctrl)
> +{
> +       pci_bus_remove_resources(bus);
> +       pci_bus_add_resource(bus, ctrl->io_resource, 0);
> +       pci_bus_add_resource(bus, ctrl->mem_resource, 0);
> +}
> +
>  void __devinit pcibios_fixup_bus(struct pci_bus *bus)
>  {
>        /* Propagate hose info into the subordinate devices.  */
> @@ -269,10 +313,9 @@ void __devinit pcibios_fixup_bus(struct pci_bus *bus)
>        struct list_head *ln;
>        struct pci_dev *dev = bus->self;
>
> -       if (!dev) {
> -               bus->resource[0] = hose->io_resource;
> -               bus->resource[1] = hose->mem_resource;
> -       } else if (pci_probe_only &&
> +       if (!dev)
> +               pcibios_setup_root_resources(bus, hose);

As I mentioned in my other response, I think you can just drop this
whole "if (!dev)" block since the root bus resources should already be
correct.  There's no need to move them from the bus->resource[] array
to the bus->resources list.

Someday, if all arches adopt your nice pci_create_bus() extension to
make root bus resources correct from the beginning, there should be no
arch references to the bus->resource[] array left, and then we can
remove it altogether.  At least, that's what I was hoping when I added
the list :)

In any case, all the *readers* of bus resources already use the
pci_bus_for_each_resource() interface, which knows how to look at both
the array and the list.

Bjorn