Patchwork [V4,1/2] PCI: pcibus address to resource converting take bus directly

login
register
mail settings
Submitter Gavin Shan
Date June 27, 2012, 2:48 p.m.
Message ID <1340808525-24996-1-git-send-email-shangw@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/167684/
State Superseded
Headers show

Comments

Gavin Shan - June 27, 2012, 2:48 p.m.
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(-)
Bjorn Helgaas - June 27, 2012, 6:15 p.m.
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
Gavin Shan - June 28, 2012, 3:59 a.m.
>> 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
Benjamin Herrenschmidt - June 28, 2012, 7:03 a.m.
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

Patch

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,