diff mbox

ARM/PCI: move align_resource function pointer into pci_host_bridge structure

Message ID 1447204345-143793-1-git-send-email-gabriele.paoloni@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gabriele Paoloni Nov. 11, 2015, 1:12 a.m. UTC
From: gabpao01 <gabriele.paoloni@huawei.com>

commit b3a72384fe29 ("ARM/PCI: Replace
pci_sys_data->align_resource with global function pointer") has
introduced a global function pointer that makes the ARM specific
code not portable and broken in case any platform have a two
different HW IPs for the PCIe host bridge controller.
This patch moves align_resource function pointer into
pci_host_bridge structure so that the code is now suitable to be
reworked as we want to get rid of hw_pci structure (the host bridge
drivers can just set the align function pointer in the
pci_host_bridge structure) and there is no more broken code for a
SoC with two HW IPs.

Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
---
 arch/arm/kernel/bios32.c | 19 +++++++++++--------
 drivers/pci/pci.h        |  2 --
 include/linux/pci.h      |  9 +++++++++
 3 files changed, 20 insertions(+), 10 deletions(-)

Comments

Arnd Bergmann Nov. 11, 2015, 8:47 a.m. UTC | #1
On Wednesday 11 November 2015 09:12:25 Gabriele Paoloni wrote:
> From: gabpao01 <gabriele.paoloni@huawei.com>

Regardless of this patch, you might want to update your global git config
to use the proper name for your commits.

> commit b3a72384fe29 ("ARM/PCI: Replace
> pci_sys_data->align_resource with global function pointer") has
> introduced a global function pointer that makes the ARM specific
> code not portable and broken in case any platform have a two
> different HW IPs for the PCIe host bridge controller.
> This patch moves align_resource function pointer into
> pci_host_bridge structure so that the code is now suitable to be
> reworked as we want to get rid of hw_pci structure (the host bridge
> drivers can just set the align function pointer in the
> pci_host_bridge structure) and there is no more broken code for a
> SoC with two HW IPs.
> 
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>

Thanks for doing this, that addresses my main concerns, and is probably
small enough to still get it into 4.4 if Bjorn agrees with the general
idea of putting more function pointers into struct pci_host_bridge.

As a follow-on to this, we can now try to get rid of pcibios_align_resource
by moving the existing per-architecture implementations in there as well,
with a reasonable default for the common case.

	Arnd
Gabriele Paoloni Nov. 11, 2015, 10:09 a.m. UTC | #2
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Wednesday, November 11, 2015 4:48 PM
> To: linux-arm-kernel@lists.infradead.org
> Cc: Gabriele Paoloni; linux-pci@vger.kernel.org;
> lorenzo.pieralisi@arm.com; linux@arm.linux.org.uk; xuwei (O); Wangzhou
> (B); bhelgaas@google.com; Liguozhu (Kenneth)
> Subject: Re: [PATCH] ARM/PCI: move align_resource function pointer into
> pci_host_bridge structure
> 
> On Wednesday 11 November 2015 09:12:25 Gabriele Paoloni wrote:
> > From: gabpao01 <gabriele.paoloni@huawei.com>
> 
> Regardless of this patch, you might want to update your global git
> config
> to use the proper name for your commits.

Sorry, I am travelling and moved to another PC...I didn't notice
the wrong name, my bad.

> 
> > commit b3a72384fe29 ("ARM/PCI: Replace
> > pci_sys_data->align_resource with global function pointer") has
> > introduced a global function pointer that makes the ARM specific
> > code not portable and broken in case any platform have a two
> > different HW IPs for the PCIe host bridge controller.
> > This patch moves align_resource function pointer into
> > pci_host_bridge structure so that the code is now suitable to be
> > reworked as we want to get rid of hw_pci structure (the host bridge
> > drivers can just set the align function pointer in the
> > pci_host_bridge structure) and there is no more broken code for a
> > SoC with two HW IPs.
> >
> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> 
> Thanks for doing this, that addresses my main concerns, and is probably
> small enough to still get it into 4.4 if Bjorn agrees with the general
> idea of putting more function pointers into struct pci_host_bridge.
> 
> As a follow-on to this, we can now try to get rid of
> pcibios_align_resource
> by moving the existing per-architecture implementations in there as
> well,
> with a reasonable default for the common case.
> 
> 	Arnd
Bjorn Helgaas Nov. 25, 2015, 7:32 p.m. UTC | #3
On Wed, Nov 11, 2015 at 09:12:25AM +0800, Gabriele Paoloni wrote:
> From: gabpao01 <gabriele.paoloni@huawei.com>
> 
> commit b3a72384fe29 ("ARM/PCI: Replace
> pci_sys_data->align_resource with global function pointer") has
> introduced a global function pointer that makes the ARM specific
> code not portable and broken in case any platform have a two
> different HW IPs for the PCIe host bridge controller.
> This patch moves align_resource function pointer into
> pci_host_bridge structure so that the code is now suitable to be
> reworked as we want to get rid of hw_pci structure (the host bridge
> drivers can just set the align function pointer in the
> pci_host_bridge structure) and there is no more broken code for a
> SoC with two HW IPs.
> 
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>

This looks good to me.  I merged b3a72384fe29 so maybe it would make
sense for me to merge this follow-up as well.  I put it on my
for-linus branch for v4.4.

If you'd rather merge it via an ARM tree, here's my ack:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

If you want me to merge it, an ARM ack would be nice.  If you want to
merge it, let me know and I'll drop it from my tree.

> +	/* Resource alignement requirements     */

s/alignement/alignment/

> +	resource_size_t (*align_resource)(struct pci_dev *dev,
> ...
Arnd Bergmann Nov. 25, 2015, 7:55 p.m. UTC | #4
On Wednesday 25 November 2015 13:32:01 Bjorn Helgaas wrote:
> On Wed, Nov 11, 2015 at 09:12:25AM +0800, Gabriele Paoloni wrote:
> > From: gabpao01 <gabriele.paoloni@huawei.com>
> > 
> > commit b3a72384fe29 ("ARM/PCI: Replace
> > pci_sys_data->align_resource with global function pointer") has
> > introduced a global function pointer that makes the ARM specific
> > code not portable and broken in case any platform have a two
> > different HW IPs for the PCIe host bridge controller.
> > This patch moves align_resource function pointer into
> > pci_host_bridge structure so that the code is now suitable to be
> > reworked as we want to get rid of hw_pci structure (the host bridge
> > drivers can just set the align function pointer in the
> > pci_host_bridge structure) and there is no more broken code for a
> > SoC with two HW IPs.
> > 
> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> 
> This looks good to me.  I merged b3a72384fe29 so maybe it would make
> sense for me to merge this follow-up as well.  I put it on my
> for-linus branch for v4.4.
> 
> If you'd rather merge it via an ARM tree, here's my ack:
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> If you want me to merge it, an ARM ack would be nice.  If you want to
> merge it, let me know and I'll drop it from my tree.
> 
> > +     /* Resource alignement requirements     */
> 
> s/alignement/alignment/
> 
> > +     resource_size_t (*align_resource)(struct pci_dev *dev,
> > ...
> 

I was the one who asked for this change, so you can definitely have my

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

If Russell wants to take it through his tree, Gabriele can submit it
to http://www.arm.linux.org.uk/developer/patches/, otherwise I'd
suggest we merge it through your tree, as that was also the source
of the patch we are trying to fix up.

	Arnd
diff mbox

Patch

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 6551d28..066f7f9 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -17,11 +17,6 @@ 
 #include <asm/mach/pci.h>
 
 static int debug_pci;
-static resource_size_t (*align_resource)(struct pci_dev *dev,
-		  const struct resource *res,
-		  resource_size_t start,
-		  resource_size_t size,
-		  resource_size_t align) = NULL;
 
 /*
  * We can't use pci_get_device() here since we are
@@ -461,7 +456,6 @@  static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;
-		align_resource = hw->align_resource;
 		INIT_LIST_HEAD(&sys->resources);
 
 		if (hw->private_data)
@@ -470,6 +464,8 @@  static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		ret = hw->setup(nr, sys);
 
 		if (ret > 0) {
+			struct pci_host_bridge *host_bridge;
+
 			ret = pcibios_init_resources(nr, sys);
 			if (ret)  {
 				kfree(sys);
@@ -491,6 +487,9 @@  static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 			busnr = sys->bus->busn_res.end + 1;
 
 			list_add(&sys->node, head);
+
+			host_bridge = pci_find_host_bridge(sys->bus);
+			host_bridge->align_resource = hw->align_resource;
 		} else {
 			kfree(sys);
 			if (ret < 0)
@@ -578,14 +577,18 @@  resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 {
 	struct pci_dev *dev = data;
 	resource_size_t start = res->start;
+	struct pci_host_bridge *host_bridge;
 
 	if (res->flags & IORESOURCE_IO && start & 0x300)
 		start = (start + 0x3ff) & ~0x3ff;
 
 	start = (start + align - 1) & ~(align - 1);
 
-	if (align_resource)
-		return align_resource(dev, res, start, size, align);
+	host_bridge = pci_find_host_bridge(dev->bus);
+
+	if (host_bridge->align_resource)
+		return host_bridge->align_resource(dev, res,
+				start, size, align);
 
 	return start;
 }
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index fd2f03f..d390fc1 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -337,6 +337,4 @@  static inline int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 }
 #endif
 
-struct pci_host_bridge *pci_find_host_bridge(struct pci_bus *bus);
-
 #endif /* DRIVERS_PCI_H */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e828e7b..980b7bc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -412,9 +412,18 @@  struct pci_host_bridge {
 	void (*release_fn)(struct pci_host_bridge *);
 	void *release_data;
 	unsigned int ignore_reset_delay:1;	/* for entire hierarchy */
+	/* Resource alignement requirements     */
+	resource_size_t (*align_resource)(struct pci_dev *dev,
+			const struct resource *res,
+			resource_size_t start,
+			resource_size_t size,
+			resource_size_t align);
 };
 
 #define	to_pci_host_bridge(n) container_of(n, struct pci_host_bridge, dev)
+
+struct pci_host_bridge *pci_find_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);