| Message ID | 1340808525-24996-1-git-send-email-shangw@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Jun 27, 2012 at 8:48 AM, Gavin Shan <shangw@linux.vnet.ibm.com> wrote: > For allocating resource under bus path, we do have dev pass along, > and we could just use bus instead. Also, we'd like to make function > find_pci_host_bridge() global so that some platforms (e.g. PPC) can > access the pci host bridge directly. This patch appears to have multiple unrelated changes: - change "struct pci_bus *bus" to "struct pci_bus *root_bus" - change find_pci_host_bridge() argument from dev to bus - fiddle with pcibios_bus_to_resource() and pcibios_resource_to_bus() These should be split out to make your patches easier to review. What's the rationale for preferring the pci_bus over the pci_dev? > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > --- > drivers/pci/host-bridge.c | 34 +++++++++++++++++++++------------- > include/linux/pci.h | 4 ++++ > 2 files changed, 25 insertions(+), 13 deletions(-) > > diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c > index a68dc61..4ccf477 100644 > --- a/drivers/pci/host-bridge.c > +++ b/drivers/pci/host-bridge.c > @@ -9,22 +9,19 @@ > > #include "pci.h" > > -static struct pci_bus *find_pci_root_bus(struct pci_dev *dev) > +static struct pci_bus *find_pci_root_bus(struct pci_bus *bus) > { > - struct pci_bus *bus; > - > - bus = dev->bus; > while (bus->parent) > bus = bus->parent; > > return bus; > } > > -static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev) > +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus) > { > - struct pci_bus *bus = find_pci_root_bus(dev); > + struct pci_bus *root_bus = find_pci_root_bus(bus); > > - return to_pci_host_bridge(bus->bridge); > + return to_pci_host_bridge(root_bus->bridge); > } > > void pci_set_host_bridge_release(struct pci_host_bridge *bridge, > @@ -40,10 +37,11 @@ static bool resource_contains(struct resource *res1, struct resource *res2) > return res1->start <= res2->start && res1->end >= res2->end; > } > > -void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, > - struct resource *res) > +void __pcibios_resource_to_bus(struct pci_bus *bus, > + struct pci_bus_region *region, > + struct resource *res) > { > - struct pci_host_bridge *bridge = find_pci_host_bridge(dev); > + struct pci_host_bridge *bridge = find_pci_host_bridge(bus); > struct pci_host_bridge_window *window; > resource_size_t offset = 0; > > @@ -60,6 +58,11 @@ void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, > region->start = res->start - offset; > region->end = res->end - offset; > } > +void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, > + struct resource *res) > +{ > + __pcibios_resource_to_bus(dev->bus, region, res); > +} > EXPORT_SYMBOL(pcibios_resource_to_bus); > > static bool region_contains(struct pci_bus_region *region1, > @@ -68,10 +71,10 @@ static bool region_contains(struct pci_bus_region *region1, > return region1->start <= region2->start && region1->end >= region2->end; > } > > -void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, > - struct pci_bus_region *region) > +static void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res, > + struct pci_bus_region *region) > { > - struct pci_host_bridge *bridge = find_pci_host_bridge(dev); > + struct pci_host_bridge *bridge = find_pci_host_bridge(bus); > struct pci_host_bridge_window *window; > resource_size_t offset = 0; > > @@ -93,4 +96,9 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, > res->start = region->start + offset; > res->end = region->end + offset; > } > +void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, > + struct pci_bus_region *region) > +{ > + __pcibios_bus_to_resource(dev->bus, res, region); > +} > EXPORT_SYMBOL(pcibios_bus_to_resource); > diff --git a/include/linux/pci.h b/include/linux/pci.h > index fefb4e1..2b559f1 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -385,6 +385,7 @@ struct pci_host_bridge { > }; > > #define to_pci_host_bridge(n) container_of(n, struct pci_host_bridge, dev) > +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus); > void pci_set_host_bridge_release(struct pci_host_bridge *bridge, > void (*release_fn)(struct pci_host_bridge *), > void *release_data); > @@ -657,6 +658,9 @@ void pci_fixup_cardbus(struct pci_bus *); > > /* Generic PCI functions used internally */ > > +void __pcibios_resource_to_bus(struct pci_bus *bus, > + struct pci_bus_region *region, > + struct resource *res); > void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, > struct resource *res); > void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> For allocating resource under bus path, we do have dev pass along, >> and we could just use bus instead. Also, we'd like to make function >> find_pci_host_bridge() global so that some platforms (e.g. PPC) can >> access the pci host bridge directly. > >This patch appears to have multiple unrelated changes: > > - change "struct pci_bus *bus" to "struct pci_bus *root_bus" > - change find_pci_host_bridge() argument from dev to bus > - fiddle with pcibios_bus_to_resource() and pcibios_resource_to_bus() > Leave those questions to Yinghai. >These should be split out to make your patches easier to review. > I will split it for easy review :-) >What's the rationale for preferring the pci_bus over the pci_dev? > We probablly need retrieve the pci_host_bridge through pci_bus and pci_dev. When converting pci_dev to pci_host_bridge, we can simply pass "pci_dev->bus". Thanks, Gavin >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> --- >> drivers/pci/host-bridge.c | 34 +++++++++++++++++++++------------- >> include/linux/pci.h | 4 ++++ >> 2 files changed, 25 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c >> index a68dc61..4ccf477 100644 >> --- a/drivers/pci/host-bridge.c >> +++ b/drivers/pci/host-bridge.c >> @@ -9,22 +9,19 @@ >> >> #include "pci.h" >> >> -static struct pci_bus *find_pci_root_bus(struct pci_dev *dev) >> +static struct pci_bus *find_pci_root_bus(struct pci_bus *bus) >> { >> - struct pci_bus *bus; >> - >> - bus = dev->bus; >> while (bus->parent) >> bus = bus->parent; >> >> return bus; >> } >> >> -static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev) >> +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus) >> { >> - struct pci_bus *bus = find_pci_root_bus(dev); >> + struct pci_bus *root_bus = find_pci_root_bus(bus); >> >> - return to_pci_host_bridge(bus->bridge); >> + return to_pci_host_bridge(root_bus->bridge); >> } >> >> void pci_set_host_bridge_release(struct pci_host_bridge *bridge, >> @@ -40,10 +37,11 @@ static bool resource_contains(struct resource *res1, struct resource *res2) >> return res1->start <= res2->start && res1->end >= res2->end; >> } >> >> -void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, >> - struct resource *res) >> +void __pcibios_resource_to_bus(struct pci_bus *bus, >> + struct pci_bus_region *region, >> + struct resource *res) >> { >> - struct pci_host_bridge *bridge = find_pci_host_bridge(dev); >> + struct pci_host_bridge *bridge = find_pci_host_bridge(bus); >> struct pci_host_bridge_window *window; >> resource_size_t offset = 0; >> >> @@ -60,6 +58,11 @@ void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, >> region->start = res->start - offset; >> region->end = res->end - offset; >> } >> +void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, >> + struct resource *res) >> +{ >> + __pcibios_resource_to_bus(dev->bus, region, res); >> +} >> EXPORT_SYMBOL(pcibios_resource_to_bus); >> >> static bool region_contains(struct pci_bus_region *region1, >> @@ -68,10 +71,10 @@ static bool region_contains(struct pci_bus_region *region1, >> return region1->start <= region2->start && region1->end >= region2->end; >> } >> >> -void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, >> - struct pci_bus_region *region) >> +static void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res, >> + struct pci_bus_region *region) >> { >> - struct pci_host_bridge *bridge = find_pci_host_bridge(dev); >> + struct pci_host_bridge *bridge = find_pci_host_bridge(bus); >> struct pci_host_bridge_window *window; >> resource_size_t offset = 0; >> >> @@ -93,4 +96,9 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, >> res->start = region->start + offset; >> res->end = region->end + offset; >> } >> +void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, >> + struct pci_bus_region *region) >> +{ >> + __pcibios_bus_to_resource(dev->bus, res, region); >> +} >> EXPORT_SYMBOL(pcibios_bus_to_resource); >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index fefb4e1..2b559f1 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -385,6 +385,7 @@ struct pci_host_bridge { >> }; >> >> #define to_pci_host_bridge(n) container_of(n, struct pci_host_bridge, dev) >> +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus); >> void pci_set_host_bridge_release(struct pci_host_bridge *bridge, >> void (*release_fn)(struct pci_host_bridge *), >> void *release_data); >> @@ -657,6 +658,9 @@ void pci_fixup_cardbus(struct pci_bus *); >> >> /* Generic PCI functions used internally */ >> >> +void __pcibios_resource_to_bus(struct pci_bus *bus, >> + struct pci_bus_region *region, >> + struct resource *res); >> void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, >> struct resource *res); >> void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, >> -- >> 1.7.9.5 >> > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2012-06-27 at 12:15 -0600, Bjorn Helgaas wrote: > > This patch appears to have multiple unrelated changes: > > - change "struct pci_bus *bus" to "struct pci_bus *root_bus" > - change find_pci_host_bridge() argument from dev to bus > - fiddle with pcibios_bus_to_resource() and > pcibios_resource_to_bus() > > These should be split out to make your patches easier to review. > > What's the rationale for preferring the pci_bus over the pci_dev? We really want to stop doing that crap and put a pointer to the host bridge directly in each pci_dev (and maybe pci_bus). We already do that on powerpc via sysdata (pointing to our private structure but the plan is to move that toward struct pci_host_bridge eventually). We'll need host bridge access to deal with resource "offsets" in a few more places and having to do a lookup is just a pain. For example, see my earlier email about the issues with the way we calculate the minimum address to assign devices. This is currently broken when we have an offset and to fix it we'll probably want to deal with offsets all the way down in the resource allocation code. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" 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/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c index a68dc61..4ccf477 100644 --- a/drivers/pci/host-bridge.c +++ b/drivers/pci/host-bridge.c @@ -9,22 +9,19 @@ #include "pci.h" -static struct pci_bus *find_pci_root_bus(struct pci_dev *dev) +static struct pci_bus *find_pci_root_bus(struct pci_bus *bus) { - struct pci_bus *bus; - - bus = dev->bus; while (bus->parent) bus = bus->parent; return bus; } -static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev) +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus) { - struct pci_bus *bus = find_pci_root_bus(dev); + struct pci_bus *root_bus = find_pci_root_bus(bus); - return to_pci_host_bridge(bus->bridge); + return to_pci_host_bridge(root_bus->bridge); } void pci_set_host_bridge_release(struct pci_host_bridge *bridge, @@ -40,10 +37,11 @@ static bool resource_contains(struct resource *res1, struct resource *res2) return res1->start <= res2->start && res1->end >= res2->end; } -void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, - struct resource *res) +void __pcibios_resource_to_bus(struct pci_bus *bus, + struct pci_bus_region *region, + struct resource *res) { - struct pci_host_bridge *bridge = find_pci_host_bridge(dev); + struct pci_host_bridge *bridge = find_pci_host_bridge(bus); struct pci_host_bridge_window *window; resource_size_t offset = 0; @@ -60,6 +58,11 @@ void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, region->start = res->start - offset; region->end = res->end - offset; } +void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, + struct resource *res) +{ + __pcibios_resource_to_bus(dev->bus, region, res); +} EXPORT_SYMBOL(pcibios_resource_to_bus); static bool region_contains(struct pci_bus_region *region1, @@ -68,10 +71,10 @@ static bool region_contains(struct pci_bus_region *region1, return region1->start <= region2->start && region1->end >= region2->end; } -void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, - struct pci_bus_region *region) +static void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res, + struct pci_bus_region *region) { - struct pci_host_bridge *bridge = find_pci_host_bridge(dev); + struct pci_host_bridge *bridge = find_pci_host_bridge(bus); struct pci_host_bridge_window *window; resource_size_t offset = 0; @@ -93,4 +96,9 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, res->start = region->start + offset; res->end = region->end + offset; } +void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res, + struct pci_bus_region *region) +{ + __pcibios_bus_to_resource(dev->bus, res, region); +} EXPORT_SYMBOL(pcibios_bus_to_resource); diff --git a/include/linux/pci.h b/include/linux/pci.h index fefb4e1..2b559f1 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -385,6 +385,7 @@ struct pci_host_bridge { }; #define to_pci_host_bridge(n) container_of(n, struct pci_host_bridge, dev) +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus); void pci_set_host_bridge_release(struct pci_host_bridge *bridge, void (*release_fn)(struct pci_host_bridge *), void *release_data); @@ -657,6 +658,9 @@ void pci_fixup_cardbus(struct pci_bus *); /* Generic PCI functions used internally */ +void __pcibios_resource_to_bus(struct pci_bus *bus, + struct pci_bus_region *region, + struct resource *res); void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region, struct resource *res); void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
For allocating resource under bus path, we do have dev pass along, and we could just use bus instead. Also, we'd like to make function find_pci_host_bridge() global so that some platforms (e.g. PPC) can access the pci host bridge directly. Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- drivers/pci/host-bridge.c | 34 +++++++++++++++++++++------------- include/linux/pci.h | 4 ++++ 2 files changed, 25 insertions(+), 13 deletions(-)