On 01/21/2010 10:18 AM, Alex Chiang wrote: > > Sorry, this is getting into bike-shed territory. > > I'm a lot happier with the code now, so may as well fix up a few > style issues before it goes in. > >> +static void pci_bridge_release_resources(struct pci_bus *bus, >> + unsigned long type) >> +{ > > [...] > >> + if (changed) { >> + if (type & IORESOURCE_PREFETCH) { >> + /* avoiding touch the one without PREF */ >> + type = IORESOURCE_PREFETCH; >> + } > > Strictly speaking, you don't need those curly braces. If you want > readability, how about moving the comment up? > > /* Only setup prefetch resources */ > if (type & IORESOURCE_PREFETCH) > type = IORESOURCE_PREFETCH; > > >> + __pci_setup_bridge(bus, type); >> + } >> +} >> + >> +static void __ref pci_bus_release_bridge_resources(struct pci_bus *bus, >> + unsigned long type, >> + enum release_type rel_type) >> +{ > > [...] > >> + >> + /* The root bus? */ > > Useless comment. > >> + if (pci_is_root_bus(bus)) >> + return; >> + >> + if ((bus->self->class >> 8) != PCI_CLASS_BRIDGE_PCI) >> + return; >> + >> + if (((rel_type == leaf_only) && is_leaf_bridge) || >> + (rel_type == whole_subtree)) >> + pci_bridge_release_resources(bus, type); >> +} > > Can clean this up a bit too with short-circuit logic. > > if ((rel_type == whole_subtree) || is_leaf_bridge) > pci_bridge_release_resources(bus, type); > > If you clean those up, you can add my: > > Reviewed-by: Alex Chiang <achiang@xxxxxx> ok thanks. -- 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