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

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

[Deng-Cheng Zhu]

(Resending the patch set to include more arch maintainers.)

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 --
v3 - v2:
o Do not do fixups for root buses in pcibios_fixup_bus().
o Skip bus creation when bus resources cannot be allocated.
o PCI domain/bus numbers added to the error info in controller_resources().

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              |   61 +++++++++++++++++++++++++++++++++-----
 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, 73 insertions(+), 17 deletions(-)

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

[Deng-Cheng Zhu]

(Resending the patch set to include more arch maintainers.)

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 --
v3 - v2:
o Do not do fixups for root buses in pcibios_fixup_bus().
o Skip bus creation when bus resources cannot be allocated.
o PCI domain/bus numbers added to the error info in controller_resources().

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              |   61 +++++++++++++++++++++++++++++++++-----
 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, 73 insertions(+), 17 deletions(-)

[RESEND PATCH v3 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

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Deng-Cheng Zhu <dczhu@mips.com>
---
Changes (v3 - v2):
None

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

[RESEND PATCH v3 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

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Deng-Cheng Zhu <dczhu@mips.com>
---
Changes (v3 - v2):
None

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

[RESEND PATCH v3 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().

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Deng-Cheng Zhu <dczhu@mips.com>
---
Changes (v3 - v2):
o Do not do fixups for root buses in pcibios_fixup_bus().
o Skip bus creation when bus resources cannot be allocated.
o PCI domain/bus numbers added to the error info in controller_resources().
o Patch description modified according to the changes above.

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 |   61 ++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 33bba7b..c76fb30 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -76,11 +76,42 @@ pcibios_align_resource(void *data, const struct resource *res,
 	return start;
 }
 
+static struct pci_bus_resource *
+controller_resources(const struct pci_controller *ctrl, int domain, int busno)
+{
+	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 "PCI bus %04x:%02x: Can't allocate bus resource.\n",
+		domain, busno);
+	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 *bus = NULL;
+	struct pci_bus_resource *bus_res;
 
 	if (!hose->iommu)
 		PCI_DMA_BUS_IS_PHYS = 1;
@@ -88,7 +119,22 @@ 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, hose->index, next_busno);
+	if (bus_res) {
+		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);
+		} else {
+			/* io_resource */
+			kfree(list_first_entry(&bus_res->list,
+				struct pci_bus_resource, list));
+			/* mem_resource */
+			kfree(bus_res);
+		}
+	}
+
 	hose->bus = bus;
 
 	need_domain_info = need_domain_info || hose->index;
@@ -265,15 +311,14 @@ void __devinit pcibios_fixup_bus(struct pci_bus *bus)
 {
 	/* Propagate hose info into the subordinate devices.  */
 
-	struct pci_controller *hose = bus->sysdata;
 	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 &&
-		   (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
+	/*
+	 * Root bus resources should already be set up correctly in
+	 * pci_create_bus(), so don't do fixups for it.
+	 */
+	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

[RESEND PATCH v3 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().

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Deng-Cheng Zhu <dczhu@mips.com>
---
Changes (v3 - v2):
o Do not do fixups for root buses in pcibios_fixup_bus().
o Skip bus creation when bus resources cannot be allocated.
o PCI domain/bus numbers added to the error info in controller_resources().
o Patch description modified according to the changes above.

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 |   61 ++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 33bba7b..c76fb30 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -76,11 +76,42 @@ pcibios_align_resource(void *data, const struct resource *res,
 	return start;
 }
 
+static struct pci_bus_resource *
+controller_resources(const struct pci_controller *ctrl, int domain, int busno)
+{
+	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 "PCI bus %04x:%02x: Can't allocate bus resource.\n",
+		domain, busno);
+	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 *bus = NULL;
+	struct pci_bus_resource *bus_res;
 
 	if (!hose->iommu)
 		PCI_DMA_BUS_IS_PHYS = 1;
@@ -88,7 +119,22 @@ 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, hose->index, next_busno);
+	if (bus_res) {
+		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);
+		} else {
+			/* io_resource */
+			kfree(list_first_entry(&bus_res->list,
+				struct pci_bus_resource, list));
+			/* mem_resource */
+			kfree(bus_res);
+		}
+	}
+
 	hose->bus = bus;
 
 	need_domain_info = need_domain_info || hose->index;
@@ -265,15 +311,14 @@ void __devinit pcibios_fixup_bus(struct pci_bus *bus)
 {
 	/* Propagate hose info into the subordinate devices.  */
 
-	struct pci_controller *hose = bus->sysdata;
 	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 &&
-		   (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
+	/*
+	 * Root bus resources should already be set up correctly in
+	 * pci_create_bus(), so don't do fixups for it.
+	 */
+	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: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources

[Bjorn Helgaas]

On Wed, Aug 31, 2011 at 8:48 PM, Deng-Cheng Zhu <dczhu@mips.com> wrote:
> (Resending the patch set to include more arch maintainers.)
>
> 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.

Jesse, I assume these are headed for the 3.2 merge window, right?

Do you have a git tree anywhere where we could check without having to
bother you?

Bjorn

> -- Changes --
> v3 - v2:
> o Do not do fixups for root buses in pcibios_fixup_bus().
> o Skip bus creation when bus resources cannot be allocated.
> o PCI domain/bus numbers added to the error info in controller_resources().
>
> 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              |   61 +++++++++++++++++++++++++++++++++-----
>  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, 73 insertions(+), 17 deletions(-)
>
>

Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources

[Bjorn Helgaas]

On Fri, Oct 7, 2011 at 3:50 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Wed, Aug 31, 2011 at 8:48 PM, Deng-Cheng Zhu <dczhu@mips.com> wrote:
>> (Resending the patch set to include more arch maintainers.)
>>
>> 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.
>
> Jesse, I assume these are headed for the 3.2 merge window, right?

I tried to build on these patches to convert x86 to using the new
pci_create_bus() interface, but I couldn't figure out a nice way to
handle an arbitrary number of resources.

We made pci_create_bus() take a "struct pci_bus_resource *"
(https://lkml.org/lkml/2011/8/26/88):

    struct pci_bus *pci_create_bus(struct device *parent, int bus,
			       struct pci_ops *ops, void *sysdata,
			       struct pci_bus_resource *bus_res);

Where pci_bus_resource looks like this:

    struct pci_bus_resource {
        struct list_head list;
        struct resource *res;
        unsigned int flags;
    };

Conceptually, we're passing a list of resources to pci_create_bus(),
which I think is the right thing.  But I think the best way to do that
is by passing a pointer to a struct list_head, not a pointer to a
pci_bus_resource.

If we pass a pci_bus_resource, we're basically using that as a
container for a list (as per include/linux/list.h), but it's not a
well-formed list.  Normally a list contains one list_head per element,
plus an extra list_head for the head of the list.  There's a nice
drawing on page 296 of http://lwn.net/images/pdf/LDD3/ch11.pdf.

The list built in your MIPS patch (https://lkml.org/lkml/2011/8/26/89)
consists of two pci_bus_resource structs (each with a list_head), but
there's no third list_head for the head of the list.  I think if you
tried to use list_for_each_entry() to iterate through the list, it
would not work correctly.

I'll post a slightly modified series to show what I mean.  Take a look
and see if it makes sense to you.

Bjorn

RE: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources

[Zhu, DengCheng]

2011/10/11 Bjorn Helgaas <bhelgaas@google.com>
>
> On Fri, Oct 7, 2011 at 3:50 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Wed, Aug 31, 2011 at 8:48 PM, Deng-Cheng Zhu <dczhu@mips.com> wrote:
> >> (Resending the patch set to include more arch maintainers.)
> >>
> >> 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.
> >
> > Jesse, I assume these are headed for the 3.2 merge window, right?
>
> I tried to build on these patches to convert x86 to using the new
> pci_create_bus() interface, but I couldn't figure out a nice way to
> handle an arbitrary number of resources.
>
> We made pci_create_bus() take a "struct pci_bus_resource *"
> (https://lkml.org/lkml/2011/8/26/88):
>
>    struct pci_bus *pci_create_bus(struct device *parent, int bus,
>                               struct pci_ops *ops, void *sysdata,
>                               struct pci_bus_resource *bus_res);
>
> Where pci_bus_resource looks like this:
>
>    struct pci_bus_resource {
>        struct list_head list;
>        struct resource *res;
>        unsigned int flags;
>    };
>
> Conceptually, we're passing a list of resources to pci_create_bus(),
> which I think is the right thing.  But I think the best way to do that
> is by passing a pointer to a struct list_head, not a pointer to a
> pci_bus_resource.
>
> If we pass a pci_bus_resource, we're basically using that as a
> container for a list (as per include/linux/list.h), but it's not a
> well-formed list.  Normally a list contains one list_head per element,
> plus an extra list_head for the head of the list.  There's a nice
> drawing on page 296 of http://lwn.net/images/pdf/LDD3/ch11.pdf.
>
> The list built in your MIPS patch (https://lkml.org/lkml/2011/8/26/89)
> consists of two pci_bus_resource structs (each with a list_head), but
> there's no third list_head for the head of the list.  I think if you
> tried to use list_for_each_entry() to iterate through the list, it
> would not work correctly.
>
> I'll post a slightly modified series to show what I mean.  Take a look
> and see if it makes sense to you.

Yes, I can easily understand what you mean, because this point was ever
considered while writing this patch series. We pass the element list as
opposed to a list_head for the head of the element list because we simply
want to link the elements into pci_bus->resources in pci_create_bus(). This
can be done by a single line:
    list_add_tail(&b->resources, &bus_res->list);

In addition, if we need to do list_for_each_entry() on the list, our target
should always be pci_bus->resources rather than the pci_bus_resource list
which is passed into pci_create_bus() to be part (the meat) of
pci_bus->resources.


Deng-Cheng

RE: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources

[Benjamin Herrenschmidt]

On Mon, 2011-10-10 at 23:49 +0000, Zhu, DengCheng wrote:
> Yes, I can easily understand what you mean, because this point was ever
> considered while writing this patch series. We pass the element list as
> opposed to a list_head for the head of the element list because we simply
> want to link the elements into pci_bus->resources in pci_create_bus(). This
> can be done by a single line:
>     list_add_tail(&b->resources, &bus_res->list);
> 
> In addition, if we need to do list_for_each_entry() on the list, our target
> should always be pci_bus->resources rather than the pci_bus_resource list
> which is passed into pci_create_bus() to be part (the meat) of
> pci_bus->resources. 

I must admit I don't completely understand what this patch is about,
other than it will most probably break the way we do resource management
on powerpc :-)

I don't understand the point about conflicts in scan_slot and I don't
see what you win by "settling down early". Also keep in mind that the
resources read from the device need to be remapped on some archs like
powerpc which we do from a header quirk at the moment.

We also have very different behaviour depending on the platform that we
can trigger ranging from ignoring all resources read from the devices
because we want the kernel to fully re-assign everything, to on the
contrary only every using what the firmware set (no re-assigning
possible), with variants such as re-assigning everything using a custom
algorithm etc....

We do rely in various places on the concept of scan first, alloc/fixup
next, then use. Merging scan & alloc/fixup of resources in one pass is a
major conceptual change. It might be ok but I need to convince myself it
is by understanding better what's going on and what problem you are
really trying to solve here.

Cheers,
Ben.

Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources

[Bjorn Helgaas]

On Tue, Oct 11, 2011 at 1:48 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:

> I must admit I don't completely understand what this patch is about,
> other than it will most probably break the way we do resource management
> on powerpc :-)
>
> I don't understand the point about conflicts in scan_slot and I don't
> see what you win by "settling down early". Also keep in mind that the
> resources read from the device need to be remapped on some archs like
> powerpc which we do from a header quirk at the moment.

These patches only deal with root bus resources, i.e., the
non-architected PCI host bridge windows.  They don't have anything to
do with normal PCI BARs.

MIPS sets up root buses differently than powerpc, so it has a problem
that powerpc doesn't have.  Here's the original MIPS flow (before this
series):

              pci_scan_bus
                pci_scan_bus_parented
                  pci_create_bus        <-- A create root bus
                  pci_scan_child_bus
                    pci_scan_slot
                      pci_scan_single_device
                        pci_scan_device
                          pci_setup_device
                            pci_fixup_device (pci_fixup_early)  <-- B
                        pci_device_add
                          pci_fixup_device (pci_fixup_header)   <-- C
                    pcibios_fixup_bus   <-- D fill in root bus resources

At point A, we allocate a struct pci_bus for the root bus.
pci_create_bus() fills in defaults for the resources available on that
bus: ioport_resource and iomem_resource, which cover all possible
address space.  Later at point D, we replace those defaults with the
correct resources (hose->io_resource and hose->mem_resource in this
MIPS case).

The problem is that the root bus resources are wrong during the
interval between A and D.  Anything that looks at them may break.  In
the case Deng-Cheng found, the quirk_piix4_acpi() fixup at point C
claimed a region, which incorrectly became the child of
ioport_resource instead of host->io_resource.

Deng-Cheng's patches close this window by basically combining the
fixup at D with the root bus creation at A.

Powerpc doesn't have the same problem because it calls
pci_create_bus() directly so it can fix the root bus resources with
pcibios_setup_phb_resources() *before* it scans the bus.

Even though powerpc and many other architectures don't have the MIPS
problem, I think it's worth changing the code because the existing
pattern is poor.  In almost all cases, we know what the host bridge
apertures are before we create the root bus.  It's error-prone to have
pci_create_bus() fill in default resources, then rely on the
architecture to fix that up later.  I think it's better to supply the
resources up front.

Bjorn

Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 3174 bytes --]

On Tue, 11 Oct 2011 10:15:34 -0600
Bjorn Helgaas <bhelgaas@google.com> wrote:

> On Tue, Oct 11, 2011 at 1:48 AM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> 
> > I must admit I don't completely understand what this patch is about,
> > other than it will most probably break the way we do resource management
> > on powerpc :-)
> >
> > I don't understand the point about conflicts in scan_slot and I don't
> > see what you win by "settling down early". Also keep in mind that the
> > resources read from the device need to be remapped on some archs like
> > powerpc which we do from a header quirk at the moment.
> 
> These patches only deal with root bus resources, i.e., the
> non-architected PCI host bridge windows.  They don't have anything to
> do with normal PCI BARs.
> 
> MIPS sets up root buses differently than powerpc, so it has a problem
> that powerpc doesn't have.  Here's the original MIPS flow (before this
> series):
> 
>               pci_scan_bus
>                 pci_scan_bus_parented
>                   pci_create_bus        <-- A create root bus
>                   pci_scan_child_bus
>                     pci_scan_slot
>                       pci_scan_single_device
>                         pci_scan_device
>                           pci_setup_device
>                             pci_fixup_device (pci_fixup_early)  <-- B
>                         pci_device_add
>                           pci_fixup_device (pci_fixup_header)   <-- C
>                     pcibios_fixup_bus   <-- D fill in root bus resources
> 
> At point A, we allocate a struct pci_bus for the root bus.
> pci_create_bus() fills in defaults for the resources available on that
> bus: ioport_resource and iomem_resource, which cover all possible
> address space.  Later at point D, we replace those defaults with the
> correct resources (hose->io_resource and hose->mem_resource in this
> MIPS case).
> 
> The problem is that the root bus resources are wrong during the
> interval between A and D.  Anything that looks at them may break.  In
> the case Deng-Cheng found, the quirk_piix4_acpi() fixup at point C
> claimed a region, which incorrectly became the child of
> ioport_resource instead of host->io_resource.
> 
> Deng-Cheng's patches close this window by basically combining the
> fixup at D with the root bus creation at A.
> 
> Powerpc doesn't have the same problem because it calls
> pci_create_bus() directly so it can fix the root bus resources with
> pcibios_setup_phb_resources() *before* it scans the bus.
> 
> Even though powerpc and many other architectures don't have the MIPS
> problem, I think it's worth changing the code because the existing
> pattern is poor.  In almost all cases, we know what the host bridge
> apertures are before we create the root bus.  It's error-prone to have
> pci_create_bus() fill in default resources, then rely on the
> architecture to fix that up later.  I think it's better to supply the
> resources up front.

Ben, with the above explained are you ok with this change?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources

[Bjorn Helgaas]

On Wed, Nov 2, 2011 at 3:14 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Tue, 11 Oct 2011 10:15:34 -0600
> Bjorn Helgaas <bhelgaas@google.com> wrote:
>
>> On Tue, Oct 11, 2011 at 1:48 AM, Benjamin Herrenschmidt
>> <benh@kernel.crashing.org> wrote:
>>
>> > I must admit I don't completely understand what this patch is about,
>> > other than it will most probably break the way we do resource management
>> > on powerpc :-)
>> >
>> > I don't understand the point about conflicts in scan_slot and I don't
>> > see what you win by "settling down early". Also keep in mind that the
>> > resources read from the device need to be remapped on some archs like
>> > powerpc which we do from a header quirk at the moment.
>>
>> These patches only deal with root bus resources, i.e., the
>> non-architected PCI host bridge windows.  They don't have anything to
>> do with normal PCI BARs.
>>
>> MIPS sets up root buses differently than powerpc, so it has a problem
>> that powerpc doesn't have.  Here's the original MIPS flow (before this
>> series):
>>
>>               pci_scan_bus
>>                 pci_scan_bus_parented
>>                   pci_create_bus        <-- A create root bus
>>                   pci_scan_child_bus
>>                     pci_scan_slot
>>                       pci_scan_single_device
>>                         pci_scan_device
>>                           pci_setup_device
>>                             pci_fixup_device (pci_fixup_early)  <-- B
>>                         pci_device_add
>>                           pci_fixup_device (pci_fixup_header)   <-- C
>>                     pcibios_fixup_bus   <-- D fill in root bus resources
>>
>> At point A, we allocate a struct pci_bus for the root bus.
>> pci_create_bus() fills in defaults for the resources available on that
>> bus: ioport_resource and iomem_resource, which cover all possible
>> address space.  Later at point D, we replace those defaults with the
>> correct resources (hose->io_resource and hose->mem_resource in this
>> MIPS case).
>>
>> The problem is that the root bus resources are wrong during the
>> interval between A and D.  Anything that looks at them may break.  In
>> the case Deng-Cheng found, the quirk_piix4_acpi() fixup at point C
>> claimed a region, which incorrectly became the child of
>> ioport_resource instead of host->io_resource.
>>
>> Deng-Cheng's patches close this window by basically combining the
>> fixup at D with the root bus creation at A.
>>
>> Powerpc doesn't have the same problem because it calls
>> pci_create_bus() directly so it can fix the root bus resources with
>> pcibios_setup_phb_resources() *before* it scans the bus.
>>
>> Even though powerpc and many other architectures don't have the MIPS
>> problem, I think it's worth changing the code because the existing
>> pattern is poor.  In almost all cases, we know what the host bridge
>> apertures are before we create the root bus.  It's error-prone to have
>> pci_create_bus() fill in default resources, then rely on the
>> architecture to fix that up later.  I think it's better to supply the
>> resources up front.
>
> Ben, with the above explained are you ok with this change?

Note that this thread is for an old version of these patches.  The
current version of this series (coincidentally also labelled "v3") is
here:

http://marc.info/?l=linux-pci&m=131984075431810&w=2