| Message ID | 1346622653-30741-5-git-send-email-yinghai@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Sep 2, 2012 at 3:50 PM, Yinghai Lu <yinghai@kernel.org> wrote: > So could use it with hot-added root bus. > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > --- > arch/x86/pci/i386.c | 58 ++++++++++++++++++++++++++++++++++++--------------- > 1 files changed, 41 insertions(+), 17 deletions(-) > > diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c > index 84696ed..4fdf0b2 100644 > --- a/arch/x86/pci/i386.c > +++ b/arch/x86/pci/i386.c > @@ -298,27 +298,51 @@ static void __init pcibios_allocate_resources(struct pci_bus *bus, int pass) > } > } > > -static int __init pcibios_assign_resources(void) > +static void __init pcibios_allocate_dev_rom_resource(struct pci_dev *dev) > { > - struct pci_dev *dev = NULL; > struct resource *r; > > - if (!(pci_probe & PCI_ASSIGN_ROMS)) { > - /* > - * Try to use BIOS settings for ROMs, otherwise let > - * pci_assign_unassigned_resources() allocate the new > - * addresses. > - */ > - for_each_pci_dev(dev) { > - r = &dev->resource[PCI_ROM_RESOURCE]; > - if (!r->flags || !r->start) > - continue; > - if (pci_claim_resource(dev, PCI_ROM_RESOURCE) < 0) { > - r->end -= r->start; > - r->start = 0; > - } > - } > + /* > + * Try to use BIOS settings for ROMs, otherwise let > + * pci_assign_unassigned_resources() allocate the new > + * addresses. > + */ > + r = &dev->resource[PCI_ROM_RESOURCE]; > + if (!r->flags || !r->start) > + return; > + > + if (pci_claim_resource(dev, PCI_ROM_RESOURCE) < 0) { > + r->end -= r->start; > + r->start = 0; > } > +} > +static void __init __pcibios_allocate_rom_resources(struct pci_bus *bus) > +{ > + struct pci_dev *dev; > + struct pci_bus *child; > + > + list_for_each_entry(dev, &bus->devices, bus_list) { > + pcibios_allocate_dev_rom_resource(dev); > + > + child = dev->subordinate; > + if (child) > + __pcibios_allocate_rom_resources(child); I really dislike the "__" prefix on the function names. It seems pointless to add another function and the "__" when all you need is a test of PCI_ASSIGN_ROMS. Also, it makes the structure of __pcibios_allocate_rom_resources() different from pcibios_allocate_resources() when they should be exactly the same. What if you made pcibios_assign_resources() look like this: if (!!(pci_probe & PCI_ASSIGN_ROMS)) { list_for_each_entry(bus, &pci_root_buses, node) pcibios_allocate_rom_resources(bus); } Then I don't think you'd need the extra function. > + } > +} > +static void __init pcibios_allocate_rom_resources(struct pci_bus *bus) > +{ > + if (pci_probe & PCI_ASSIGN_ROMS) > + return; > + > + __pcibios_allocate_rom_resources(bus); > +} > + > +static int __init pcibios_assign_resources(void) > +{ > + struct pci_bus *bus; > + > + list_for_each_entry(bus, &pci_root_buses, node) > + pcibios_allocate_rom_resources(bus); > > pci_assign_unassigned_resources(); > pcibios_fw_addr_list_del(); > -- > 1.7.7 > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 17, 2012 at 4:38 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Sun, Sep 2, 2012 at 3:50 PM, Yinghai Lu <yinghai@kernel.org> wrote: >> So could use it with hot-added root bus. >> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> --- >> arch/x86/pci/i386.c | 58 ++++++++++++++++++++++++++++++++++++--------------- >> 1 files changed, 41 insertions(+), 17 deletions(-) >> >> diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c >> index 84696ed..4fdf0b2 100644 >> --- a/arch/x86/pci/i386.c >> +++ b/arch/x86/pci/i386.c >> @@ -298,27 +298,51 @@ static void __init pcibios_allocate_resources(struct pci_bus *bus, int pass) >> } >> } >> >> +static void __init __pcibios_allocate_rom_resources(struct pci_bus *bus) >> +{ >> + struct pci_dev *dev; >> + struct pci_bus *child; >> + >> + list_for_each_entry(dev, &bus->devices, bus_list) { >> + pcibios_allocate_dev_rom_resource(dev); >> + >> + child = dev->subordinate; >> + if (child) >> + __pcibios_allocate_rom_resources(child); > > I really dislike the "__" prefix on the function names. It seems > pointless to add another function and the "__" when all you need is a > test of PCI_ASSIGN_ROMS. Also, it makes the structure of > __pcibios_allocate_rom_resources() different from > pcibios_allocate_resources() when they should be exactly the same. > > What if you made pcibios_assign_resources() look like this: > > if (!!(pci_probe & PCI_ASSIGN_ROMS)) { > list_for_each_entry(bus, &pci_root_buses, node) > pcibios_allocate_rom_resources(bus); > } > > Then I don't think you'd need the extra function. ok, will just move pci_probe checking back to pcibios_allocate_rom_resources directly. Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c index 84696ed..4fdf0b2 100644 --- a/arch/x86/pci/i386.c +++ b/arch/x86/pci/i386.c @@ -298,27 +298,51 @@ static void __init pcibios_allocate_resources(struct pci_bus *bus, int pass) } } -static int __init pcibios_assign_resources(void) +static void __init pcibios_allocate_dev_rom_resource(struct pci_dev *dev) { - struct pci_dev *dev = NULL; struct resource *r; - if (!(pci_probe & PCI_ASSIGN_ROMS)) { - /* - * Try to use BIOS settings for ROMs, otherwise let - * pci_assign_unassigned_resources() allocate the new - * addresses. - */ - for_each_pci_dev(dev) { - r = &dev->resource[PCI_ROM_RESOURCE]; - if (!r->flags || !r->start) - continue; - if (pci_claim_resource(dev, PCI_ROM_RESOURCE) < 0) { - r->end -= r->start; - r->start = 0; - } - } + /* + * Try to use BIOS settings for ROMs, otherwise let + * pci_assign_unassigned_resources() allocate the new + * addresses. + */ + r = &dev->resource[PCI_ROM_RESOURCE]; + if (!r->flags || !r->start) + return; + + if (pci_claim_resource(dev, PCI_ROM_RESOURCE) < 0) { + r->end -= r->start; + r->start = 0; } +} +static void __init __pcibios_allocate_rom_resources(struct pci_bus *bus) +{ + struct pci_dev *dev; + struct pci_bus *child; + + list_for_each_entry(dev, &bus->devices, bus_list) { + pcibios_allocate_dev_rom_resource(dev); + + child = dev->subordinate; + if (child) + __pcibios_allocate_rom_resources(child); + } +} +static void __init pcibios_allocate_rom_resources(struct pci_bus *bus) +{ + if (pci_probe & PCI_ASSIGN_ROMS) + return; + + __pcibios_allocate_rom_resources(bus); +} + +static int __init pcibios_assign_resources(void) +{ + struct pci_bus *bus; + + list_for_each_entry(bus, &pci_root_buses, node) + pcibios_allocate_rom_resources(bus); pci_assign_unassigned_resources(); pcibios_fw_addr_list_del();
So could use it with hot-added root bus. Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- arch/x86/pci/i386.c | 58 ++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 41 insertions(+), 17 deletions(-)