Patchwork [V3,1/2] PCI: retrieve host bridge by PCI bus

login
register
mail settings
Submitter Gavin Shan
Date June 25, 2012, 3:10 a.m.
Message ID <1340593821-19011-1-git-send-email-shangw@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/166928/
State Superseded
Headers show

Comments

Gavin Shan - June 25, 2012, 3:10 a.m.
With current implementation, there is one function to retrieve
the corresponding host bridge (struct pci_host_bridge) according
to the given PCI device (struct pci_dev) and that function has
been declared as "static". Further, we don't have the public
function to retrieve host bridge from PCI bus yet. The function
is useful somewhere.

The additional information like minimal resource alignment for I/O
and MMIO bars of p2p bridges will be put into the PCI host bridge.
The patch introduces the public function pci_bus_host_bridge() to
retrieve the corresponding PCI host bridge according to the specified
PCI bus, then accessing the information regarding the minimal resource
alignment for I/O and MMIO bars of p2p bridges.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
Reviewed-by: Richard Yang <weiyang@linux.vnet.ibm.com>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/pci/host-bridge.c |   13 +++++++++++++
 include/linux/pci.h       |    2 +-
 2 files changed, 14 insertions(+), 1 deletion(-)
Yinghai Lu - June 25, 2012, 5:30 p.m.
On Sun, Jun 24, 2012 at 8:10 PM, Gavin Shan <shangw@linux.vnet.ibm.com> wrote:
> With current implementation, there is one function to retrieve
> the corresponding host bridge (struct pci_host_bridge) according
> to the given PCI device (struct pci_dev) and that function has
> been declared as "static". Further, we don't have the public
> function to retrieve host bridge from PCI bus yet. The function
> is useful somewhere.
>
> The additional information like minimal resource alignment for I/O
> and MMIO bars of p2p bridges will be put into the PCI host bridge.
> The patch introduces the public function pci_bus_host_bridge() to
> retrieve the corresponding PCI host bridge according to the specified
> PCI bus, then accessing the information regarding the minimal resource
> alignment for I/O and MMIO bars of p2p bridges.
>
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> Reviewed-by: Ram Pai <linuxram@us.ibm.com>
> Reviewed-by: Richard Yang <weiyang@linux.vnet.ibm.com>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  drivers/pci/host-bridge.c |   13 +++++++++++++
>  include/linux/pci.h       |    2 +-
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> index a68dc61..b95f0ce 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c
> @@ -27,6 +27,19 @@ static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev)
>        return to_pci_host_bridge(bus->bridge);
>  }
>
> +struct pci_host_bridge *pci_bus_host_bridge(struct pci_bus *bus)
> +{
> +       struct pci_bus *b = bus;
> +
> +       /* Find the PCI root bus */
> +       while (b->parent)
> +               b = b->parent;
> +
> +       return to_pci_host_bridge(b->bridge);
> +}
> +
> +EXPORT_SYMBOL(pci_bus_host_bridge);

why do you need to export it?

> +
>  void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>                                 void (*release_fn)(struct pci_host_bridge *),
>                                 void *release_data)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index fefb4e1..6d5bb1c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -656,7 +656,7 @@ void pcibios_update_irq(struct pci_dev *, int irq);
>  void pci_fixup_cardbus(struct pci_bus *);
>
>  /* Generic PCI functions used internally */
> -
> +struct pci_host_bridge *pci_bus_host_bridge(struct pci_bus *bus);
>  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,

have one patch will change parameter all to bus instead of bridge.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=3dc90c8f31e5e6153e1ac9a903189d3013690e80

in that case, we can just make find_pci_host_bridge() global.

Thanks

Yinghai
--
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 26, 2012, 12:30 a.m.
>> With current implementation, there is one function to retrieve
>> the corresponding host bridge (struct pci_host_bridge) according
>> to the given PCI device (struct pci_dev) and that function has
>> been declared as "static". Further, we don't have the public
>> function to retrieve host bridge from PCI bus yet. The function
>> is useful somewhere.
>>
>> The additional information like minimal resource alignment for I/O
>> and MMIO bars of p2p bridges will be put into the PCI host bridge.
>> The patch introduces the public function pci_bus_host_bridge() to
>> retrieve the corresponding PCI host bridge according to the specified
>> PCI bus, then accessing the information regarding the minimal resource
>> alignment for I/O and MMIO bars of p2p bridges.
>>
>> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>> Reviewed-by: Ram Pai <linuxram@us.ibm.com>
>> Reviewed-by: Richard Yang <weiyang@linux.vnet.ibm.com>
>> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> ---
>>  drivers/pci/host-bridge.c |   13 +++++++++++++
>>  include/linux/pci.h       |    2 +-
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>> index a68dc61..b95f0ce 100644
>> --- a/drivers/pci/host-bridge.c
>> +++ b/drivers/pci/host-bridge.c
>> @@ -27,6 +27,19 @@ static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev)
>>        return to_pci_host_bridge(bus->bridge);
>>  }
>>
>> +struct pci_host_bridge *pci_bus_host_bridge(struct pci_bus *bus)
>> +{
>> +       struct pci_bus *b = bus;
>> +
>> +       /* Find the PCI root bus */
>> +       while (b->parent)
>> +               b = b->parent;
>> +
>> +       return to_pci_host_bridge(b->bridge);
>> +}
>> +
>> +EXPORT_SYMBOL(pci_bus_host_bridge);

Yinghai, thanks for your time on this :-)

>
>why do you need to export it?
>

The reason is that we have introduced extra fields to "struct pci_host_bridge"
in [PATCH 2/2] and platform want to access those extra fields.

	int io_align_shift;             /* P2P I/O bar minimal alignment shift  */
	int mem_align_shift;            /* P2P MMIO bar minimal alignment shift */
	int pmem_align_shift;           /* P2P prefetchable MMIO bar minimal alignment shift */


>> +
>>  void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>>                                 void (*release_fn)(struct pci_host_bridge *),
>>                                 void *release_data)
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index fefb4e1..6d5bb1c 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -656,7 +656,7 @@ void pcibios_update_irq(struct pci_dev *, int irq);
>>  void pci_fixup_cardbus(struct pci_bus *);
>>
>>  /* Generic PCI functions used internally */
>> -
>> +struct pci_host_bridge *pci_bus_host_bridge(struct pci_bus *bus);
>>  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,
>
>have one patch will change parameter all to bus instead of bridge.
>
>http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=3dc90c8f31e5e6153e1ac9a903189d3013690e80
>
>in that case, we can just make find_pci_host_bridge() global.
>

Yeah, I think your patch meets the requirement: access "struct pci_host_bridge",
and when will you merge your patch into mainline?

By the way, could you please take your a litle bit time to review [PATCH 2/2]?

>Thanks
>
>Yinghai
>

Thanks,
Gavin

--
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 26, 2012, 2:50 a.m.
On Tue, 2012-06-26 at 08:30 +0800, Gavin Shan wrote:
> >> +EXPORT_SYMBOL(pci_bus_host_bridge);
> 
> Yinghai, thanks for your time on this :-)
> 
> >
> >why do you need to export it?
> >
> 
> The reason is that we have introduced extra fields to "struct
> pci_host_bridge"
> in [PATCH 2/2] and platform want to access those extra fields. 

But the platform code is built-in, not modular, so it doesn't need
EXPORT_SYMBOL (additionally even if we wanted to export it we probably
also want it _GPL only, so EXPORT_SYMBOL_GPL, but I don't think we need
to).

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
Gavin Shan - June 26, 2012, 3:01 a.m.
>> >> +EXPORT_SYMBOL(pci_bus_host_bridge);
>> 
>> Yinghai, thanks for your time on this :-)
>> 
>> >
>> >why do you need to export it?
>> >
>> 
>> The reason is that we have introduced extra fields to "struct
>> pci_host_bridge"
>> in [PATCH 2/2] and platform want to access those extra fields. 
>
>But the platform code is built-in, not modular, so it doesn't need
>EXPORT_SYMBOL (additionally even if we wanted to export it we probably
>also want it _GPL only, so EXPORT_SYMBOL_GPL, but I don't think we need
>to).
>

Ben, thanks for correcting me :-)

Let me remove EXPORT_SYMBOL() is next revision.

>Cheers,
>Ben.
>
>

Thanks,
Gavin

--
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
Yinghai Lu - June 26, 2012, 6:29 p.m.
On Mon, Jun 25, 2012 at 5:30 PM, Gavin Shan <shangw@linux.vnet.ibm.com> wrote:
> Yeah, I think your patch meets the requirement: access "struct pci_host_bridge",
> and when will you merge your patch into mainline?

not sure.

but i extract that patch as attached.
and with that could make your first patch only make find_pci_host_bridge()
global.

>
> By the way, could you please take your a litle bit time to review [PATCH 2/2]?
>

ok, will look at that.

Thanks

Yinghai

Patch

diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index a68dc61..b95f0ce 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -27,6 +27,19 @@  static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev)
 	return to_pci_host_bridge(bus->bridge);
 }
 
+struct pci_host_bridge *pci_bus_host_bridge(struct pci_bus *bus)
+{
+	struct pci_bus *b = bus;
+
+	/* Find the PCI root bus */
+	while (b->parent)
+		b = b->parent;
+
+	return to_pci_host_bridge(b->bridge);
+}
+
+EXPORT_SYMBOL(pci_bus_host_bridge);
+
 void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
 				 void (*release_fn)(struct pci_host_bridge *),
 				 void *release_data)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fefb4e1..6d5bb1c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -656,7 +656,7 @@  void pcibios_update_irq(struct pci_dev *, int irq);
 void pci_fixup_cardbus(struct pci_bus *);
 
 /* Generic PCI functions used internally */
-
+struct pci_host_bridge *pci_bus_host_bridge(struct pci_bus *bus);
 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,