On Sun, Feb 26, 2012 at 4:33 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote: > 1. change pci_host_bridge to find_pci_root_bridge. > 2. separate find_pci_root_bus(). > 3. on any possible path return NULL. > > Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx> > > --- > drivers/pci/host-bridge.c | 35 ++++++++++++++++++++++++++++------- > 1 file changed, 28 insertions(+), 7 deletions(-) > > Index: linux-2.6/drivers/pci/host-bridge.c > =================================================================== > --- linux-2.6.orig/drivers/pci/host-bridge.c > +++ linux-2.6/drivers/pci/host-bridge.c > @@ -16,19 +16,31 @@ void add_to_pci_host_bridges(struct pci_ > list_add_tail(&bridge->list, &pci_host_bridges); > } > > -static struct pci_host_bridge *pci_host_bridge(struct pci_dev *dev) > +static struct pci_bus *find_pci_root_bus(struct pci_dev *dev) > { > struct pci_bus *bus; > - struct pci_host_bridge *bridge; > > bus = dev->bus; > while (bus->parent) > bus = bus->parent; > > - list_for_each_entry(bridge, &pci_host_bridges, list) { > + if (!pci_is_root_bus(bus)) > + return NULL; pci_is_root_bus() returns "!(pbus->parent)", so the effect of this patch is: while (bus->parent) bus = bus->parent; if (bus->parent) return NULL; The only reason we exited the "while" loop is because "bus->parent == NULL", so what's the point of adding the "if" check afterwards? > + return bus; > +} > + > +static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev) > +{ > + struct pci_bus *bus = find_pci_root_bus(dev); > + struct pci_host_bridge *bridge; > + > + if (!bus) > + return NULL; This should never happen. If there's a way we can create a pci_dev that's not under a pci_host_bridge, I think that is a bug, and we should fix that rather than papering over it here. I don't think we should apply this patch. Bjorn > + list_for_each_entry(bridge, &pci_host_bridges, list) > if (bridge->bus == bus) > return bridge; > - } > > return NULL; > } > @@ -42,10 +54,13 @@ void __weak pcibios_resource_to_bus(stru > struct pci_bus_region *region, > struct resource *res) > { > - struct pci_host_bridge *bridge = pci_host_bridge(dev); > + struct pci_host_bridge *bridge = find_pci_host_bridge(dev); > struct pci_host_bridge_window *window; > resource_size_t offset = 0; > > + if (!bridge) > + goto no_bridge; > + > list_for_each_entry(window, &bridge->windows, list) { > if (resource_type(res) != resource_type(window->res)) > continue; > @@ -56,6 +71,7 @@ void __weak pcibios_resource_to_bus(stru > } > } > > +no_bridge: > region->start = res->start - offset; > region->end = res->end - offset; > } > @@ -70,12 +86,16 @@ static bool region_contains(struct pci_b > void __weak pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, > struct pci_bus_region *region) > { > - struct pci_host_bridge *bridge = pci_host_bridge(dev); > + struct pci_host_bridge *bridge = find_pci_host_bridge(dev); > struct pci_host_bridge_window *window; > - struct pci_bus_region bus_region; > resource_size_t offset = 0; > > + if (!bridge) > + goto no_bridge; > + > list_for_each_entry(window, &bridge->windows, list) { > + struct pci_bus_region bus_region; > + > if (resource_type(res) != resource_type(window->res)) > continue; > > @@ -88,6 +108,7 @@ void __weak pcibios_bus_to_resource(stru > } > } > > +no_bridge: > res->start = region->start + offset; > res->end = region->end + offset; > } -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html