ARM/PCI: move align_resource function pointer into pci_host_bridge structure

PATCH New Gabriele Paoloni 3 participants 5 messages Nov 11, 2015 – Nov 25, 2015

Subsystems (2)

Arm (32-bit) PCI

lore

Share & tools

lore

Gabriele Paoloni

author

2015-11-11 01:00 UTC #0 lore raw

PATCH 1/1 ARM/PCI: move align_resource function pointer into pci_host_bridge structure

From: gabpao01 <[email protected]>

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 <[email protected]>

---
 arch/arm/kernel/bios32.c | 19 +++++++++++--------
 drivers/pci/pci.h        |  2 --
 include/linux/pci.h      |  9 +++++++++
 3 files changed, 20 insertions(+), 10 deletions(-)

arch/arm/kernel/bios32.c +11 -8

index 6551d28..066f7f9 100644
@@ -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;
 }

drivers/pci/pci.h +0 -2

index fd2f03f..d390fc1 100644
@@ -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 */

include/linux/pci.h +9 -0

index e828e7b..980b7bc 100644
@@ -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);

Signature · 1 line

2.1.4

Arnd Bergmann

2015-11-11 08:47 UTC #1 lore raw

PATCH 1/1 ARM/PCI: move align_resource function pointer into pci_host_bridge structure

Quoted 1 line · On Wednesday 11 November 2015 09:12:25 Gabriele Paoloni wrote:

From: gabpao01 <[email protected]>

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

Quoted 12 lines

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.

Quoted 1 line

Signed-off-by Gabriele Paoloni <[email protected]>

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

author

2015-11-11 10:09 UTC #2 lore raw

PATCH 1/1 ARM/PCI: move align_resource function pointer into pci_host_bridge structure

Quoted 10 lines

-----Original Message-----
From: Arnd Bergmann [mailto:arnd at arndb.de]
Sent: Wednesday, November 11, 2015 4:48 PM
To: linux-arm-kernel at lists.infradead.org
Cc: Gabriele Paoloni; linux-pci at vger.kernel.org; lorenzo.pieralisi at arm.com; linux at arm.linux.org.uk; xuwei (O); Wangzhou (B); bhelgaas at google.com; Liguozhu (Kenneth)
Subject: Re: [PATCH] ARM/PCI: move align_resource function pointer into pci_host_bridge structure

Quoted 6 lines

Quoted 1 line · On Wednesday 11 November 2015 09:12:25 Gabriele Paoloni wrote:
From: gabpao01 <[email protected]>
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.

Quoted 26 lines

Quoted 13 lines
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.
Quoted 1 line
Signed-off-by Gabriele Paoloni <[email protected]>
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

2015-11-25 19:32 UTC #3 lore raw

PATCH 1/1 ARM/PCI: move align_resource function pointer into pci_host_bridge structure

In reply to Gabriele Paoloni

Quoted 14 lines · Gabriele Paoloni

From: gabpao01 <[email protected]>
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.

Quoted 1 line

Signed-off-by Gabriele Paoloni <[email protected]>

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 <[email protected]>

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.

include/linux/pci.h struct pci_host_bridge { · line 415

Quoted 1 line

include/linux/pci.h +1 -0
+	/* Resource alignement requirements     */

s/alignement/alignment/

Quoted 2 lines

include/linux/pci.h +1 -0

+	resource_size_t (*align_resource)(struct pci_dev *dev,
...

Arnd Bergmann

2015-11-25 19:55 UTC #4 lore raw

PATCH 1/1 ARM/PCI: move align_resource function pointer into pci_host_bridge structure

Quoted 23 lines · On Wednesday 25 November 2015 13:32:01 Bjorn Helgaas wrote:

Quoted 14 lines · Gabriele Paoloni
From: gabpao01 <[email protected]>
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.
Quoted 1 line
Signed-off-by Gabriele Paoloni <[email protected]>
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:

Quoted 12 lines

Acked-by Bjorn Helgaas <[email protected]>
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.
Quoted 1 line
+ /* Resource alignement requirements */
s/alignement/alignment/
Quoted 2 lines
+ 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 <[email protected]>

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