* [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources
@ 2011-09-01 2:48 ` Deng-Cheng Zhu
0 siblings, 0 replies; 13+ messages in thread
From: Deng-Cheng Zhu @ 2011-09-01 2:48 UTC (permalink / raw)
To: bhelgaas, jbarnes, ralf, monstr, benh, paulus, davem, tglx,
mingo, hpa, x86
Cc: linux-pci, linux-kernel, linux-mips, eyal, zenon, dczhu, dengcheng.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(-)
^ permalink raw reply [flat|nested] 13+ messages in thread* [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources @ 2011-09-01 2:48 ` Deng-Cheng Zhu 0 siblings, 0 replies; 13+ messages in thread From: Deng-Cheng Zhu @ 2011-09-01 2:48 UTC (permalink / raw) To: bhelgaas, jbarnes, ralf, monstr, benh, paulus, davem, tglx, mingo, hpa, x86 Cc: linux-pci, linux-kernel, linux-mips, eyal, zenon, dczhu, dengcheng.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(-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* [RESEND PATCH v3 1/2] PCI: Pass available resources into pci_create_bus() @ 2011-09-01 2:48 ` Deng-Cheng Zhu 0 siblings, 0 replies; 13+ messages in thread From: Deng-Cheng Zhu @ 2011-09-01 2:48 UTC (permalink / raw) To: bhelgaas, jbarnes, ralf, monstr, benh, paulus, davem, tglx, mingo, hpa, x86 Cc: linux-pci, linux-kernel, linux-mips, eyal, zenon, dczhu, dengcheng.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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [RESEND PATCH v3 1/2] PCI: Pass available resources into pci_create_bus() @ 2011-09-01 2:48 ` Deng-Cheng Zhu 0 siblings, 0 replies; 13+ messages in thread From: Deng-Cheng Zhu @ 2011-09-01 2:48 UTC (permalink / raw) To: bhelgaas, jbarnes, ralf, monstr, benh, paulus, davem, tglx, mingo, hpa, x86 Cc: linux-pci, linux-kernel, linux-mips, eyal, zenon, dczhu, dengcheng.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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [RESEND PATCH v3 2/2] MIPS: PCI: Pass controller's resources to pci_create_bus() in pcibios_scanbus() @ 2011-09-01 2:48 ` Deng-Cheng Zhu 0 siblings, 0 replies; 13+ messages in thread From: Deng-Cheng Zhu @ 2011-09-01 2:48 UTC (permalink / raw) To: bhelgaas, jbarnes, ralf, monstr, benh, paulus, davem, tglx, mingo, hpa, x86 Cc: linux-pci, linux-kernel, linux-mips, eyal, zenon, dczhu, dengcheng.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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [RESEND PATCH v3 2/2] MIPS: PCI: Pass controller's resources to pci_create_bus() in pcibios_scanbus() @ 2011-09-01 2:48 ` Deng-Cheng Zhu 0 siblings, 0 replies; 13+ messages in thread From: Deng-Cheng Zhu @ 2011-09-01 2:48 UTC (permalink / raw) To: bhelgaas, jbarnes, ralf, monstr, benh, paulus, davem, tglx, mingo, hpa, x86 Cc: linux-pci, linux-kernel, linux-mips, eyal, zenon, dczhu, dengcheng.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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources 2011-09-01 2:48 ` Deng-Cheng Zhu ` (2 preceding siblings ...) (?) @ 2011-10-07 21:50 ` Bjorn Helgaas 2011-10-10 21:04 ` Bjorn Helgaas -1 siblings, 1 reply; 13+ messages in thread From: Bjorn Helgaas @ 2011-10-07 21:50 UTC (permalink / raw) To: Deng-Cheng Zhu Cc: jbarnes, ralf, monstr, benh, paulus, davem, tglx, mingo, hpa, x86, linux-pci, linux-kernel, linux-mips, eyal, zenon, dengcheng.zhu 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(-) > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources 2011-10-07 21:50 ` [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources Bjorn Helgaas @ 2011-10-10 21:04 ` Bjorn Helgaas 2011-10-10 23:49 ` Zhu, DengCheng 0 siblings, 1 reply; 13+ messages in thread From: Bjorn Helgaas @ 2011-10-10 21:04 UTC (permalink / raw) To: Deng-Cheng Zhu Cc: jbarnes, ralf, monstr, benh, paulus, davem, tglx, mingo, hpa, x86, linux-pci, linux-kernel, linux-mips, eyal, zenon, dengcheng.zhu 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources 2011-10-10 21:04 ` Bjorn Helgaas @ 2011-10-10 23:49 ` Zhu, DengCheng 2011-10-11 7:48 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 13+ messages in thread From: Zhu, DengCheng @ 2011-10-10 23:49 UTC (permalink / raw) To: Bjorn Helgaas Cc: jbarnes, ralf, monstr, benh, paulus, davem, tglx, mingo, hpa, x86, linux-pci, linux-kernel, linux-mips, Barzilay, Eyal, Fortuna, Zenon, dengcheng.zhu 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources 2011-10-10 23:49 ` Zhu, DengCheng @ 2011-10-11 7:48 ` Benjamin Herrenschmidt 2011-10-11 16:15 ` Bjorn Helgaas 0 siblings, 1 reply; 13+ messages in thread From: Benjamin Herrenschmidt @ 2011-10-11 7:48 UTC (permalink / raw) To: Zhu, DengCheng Cc: Bjorn Helgaas, jbarnes, ralf, monstr, paulus, davem, tglx, mingo, hpa, x86, linux-pci, linux-kernel, linux-mips, Barzilay, Eyal, Fortuna, Zenon, dengcheng.zhu 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources 2011-10-11 7:48 ` Benjamin Herrenschmidt @ 2011-10-11 16:15 ` Bjorn Helgaas 2011-11-02 21:14 ` Jesse Barnes 0 siblings, 1 reply; 13+ messages in thread From: Bjorn Helgaas @ 2011-10-11 16:15 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Zhu, DengCheng, jbarnes, ralf, monstr, paulus, davem, tglx, mingo, hpa, x86, linux-pci, linux-kernel, linux-mips, Barzilay, Eyal, Fortuna, Zenon, dengcheng.zhu 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources 2011-10-11 16:15 ` Bjorn Helgaas @ 2011-11-02 21:14 ` Jesse Barnes 2011-11-02 21:37 ` Bjorn Helgaas 0 siblings, 1 reply; 13+ messages in thread From: Jesse Barnes @ 2011-11-02 21:14 UTC (permalink / raw) To: Bjorn Helgaas Cc: Benjamin Herrenschmidt, Zhu, DengCheng, ralf, monstr, paulus, davem, tglx, mingo, hpa, x86, linux-pci, linux-kernel, linux-mips, Barzilay, Eyal, Fortuna, Zenon, dengcheng.zhu [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources 2011-11-02 21:14 ` Jesse Barnes @ 2011-11-02 21:37 ` Bjorn Helgaas 0 siblings, 0 replies; 13+ messages in thread From: Bjorn Helgaas @ 2011-11-02 21:37 UTC (permalink / raw) To: Jesse Barnes Cc: Benjamin Herrenschmidt, Zhu, DengCheng, ralf, monstr, paulus, davem, tglx, mingo, hpa, x86, linux-pci, linux-kernel, linux-mips, Barzilay, Eyal, Fortuna, Zenon, dengcheng.zhu 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-11-02 21:38 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-09-01 2:48 [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources Deng-Cheng Zhu 2011-09-01 2:48 ` Deng-Cheng Zhu 2011-09-01 2:48 ` [RESEND PATCH v3 1/2] PCI: Pass available resources into pci_create_bus() Deng-Cheng Zhu 2011-09-01 2:48 ` Deng-Cheng Zhu 2011-09-01 2:48 ` [RESEND PATCH v3 2/2] MIPS: PCI: Pass controller's resources to pci_create_bus() in pcibios_scanbus() Deng-Cheng Zhu 2011-09-01 2:48 ` Deng-Cheng Zhu 2011-10-07 21:50 ` [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources Bjorn Helgaas 2011-10-10 21:04 ` Bjorn Helgaas 2011-10-10 23:49 ` Zhu, DengCheng 2011-10-11 7:48 ` Benjamin Herrenschmidt 2011-10-11 16:15 ` Bjorn Helgaas 2011-11-02 21:14 ` Jesse Barnes 2011-11-02 21:37 ` Bjorn Helgaas
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.