Patchwork [V2,3/3] powerpc/pci: Merge ppc32 and ppc64 versions of phb_scan()

login
register
mail settings
Submitter Grant Likely
Date Aug. 26, 2009, 6:07 a.m.
Message ID <20090826060716.30936.40578.stgit@localhost.localdomain>
Download mbox | patch
Permalink /patch/32110/
State Superseded
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

Grant Likely - Aug. 26, 2009, 6:07 a.m.
From: Grant Likely <grant.likely@secretlab.ca>

The two versions are doing almost exactly the same thing.  No need to
maintain them as separate files.  This patch also has the side effect
of making the PCI device tree scanning code available to 32 bit powerpc
machines, but no board ports actually make use of this feature at this
point.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 arch/powerpc/include/asm/pci.h   |    2 ++
 arch/powerpc/kernel/pci-common.c |   48 ++++++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/pci_32.c     |   25 ++------------------
 arch/powerpc/kernel/pci_64.c     |   46 +++++-------------------------------
 4 files changed, 58 insertions(+), 63 deletions(-)
Kumar Gala - Aug. 26, 2009, 1:29 p.m.
On Aug 26, 2009, at 1:07 AM, Grant Likely wrote:

> +/**
> + * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
> + * @hose: Pointer to the PCI host controller instance structure
> + * @data: value to use for sysdata pointer.  ppc32 and ppc64 differ  
> here
> + *
> + * Note: the 'data' pointer is a temporary measure.  As 32 and 64 bit
> + * pci code gets merged, this parameter should become unnecessary  
> because
> + * both will use the same value.
> + */
> +void __devinit pcibios_scan_phb(struct pci_controller *hose, void  
> *data)
> +{
> +

Just a nit, but why not data -> sysdata since that's what we using it  
as.

- k
Grant Likely - Aug. 26, 2009, 5:52 p.m.
On Wed, Aug 26, 2009 at 7:29 AM, Kumar Gala<galak@kernel.crashing.org> wrote:
>
> On Aug 26, 2009, at 1:07 AM, Grant Likely wrote:
>
>> +/**
>> + * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
>> + * @hose: Pointer to the PCI host controller instance structure
>> + * @data: value to use for sysdata pointer.  ppc32 and ppc64 differ here
>> + *
>> + * Note: the 'data' pointer is a temporary measure.  As 32 and 64 bit
>> + * pci code gets merged, this parameter should become unnecessary because
>> + * both will use the same value.
>> + */
>> +void __devinit pcibios_scan_phb(struct pci_controller *hose, void *data)
>> +{
>> +
>
> Just a nit, but why not data -> sysdata since that's what we using it as.

okay.

g.
Benjamin Herrenschmidt - Aug. 27, 2009, 5:46 a.m.
On Wed, 2009-08-26 at 00:07 -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> The two versions are doing almost exactly the same thing.  No need to
> maintain them as separate files.  This patch also has the side effect
> of making the PCI device tree scanning code available to 32 bit powerpc
> machines, but no board ports actually make use of this feature at this
> point.

You missed various calls to scan_phb() in arch/powerpc :-)

At least pSeries with dynamic LPAR is broken, I think iSeries might
break too. Try a ppc64_defconfig.

I tentatively applied the other patches to -test

Cheers,
Ben.

> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> 
>  arch/powerpc/include/asm/pci.h   |    2 ++
>  arch/powerpc/kernel/pci-common.c |   48 ++++++++++++++++++++++++++++++++++++++
>  arch/powerpc/kernel/pci_32.c     |   25 ++------------------
>  arch/powerpc/kernel/pci_64.c     |   46 +++++-------------------------------
>  4 files changed, 58 insertions(+), 63 deletions(-)
> 
> 
> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
> index 9ae2e3e..feebfed 100644
> --- a/arch/powerpc/include/asm/pci.h
> +++ b/arch/powerpc/include/asm/pci.h
> @@ -233,6 +233,8 @@ extern void pci_resource_to_user(const struct pci_dev *dev, int bar,
>  
>  extern void pcibios_setup_bus_devices(struct pci_bus *bus);
>  extern void pcibios_setup_bus_self(struct pci_bus *bus);
> +extern void pcibios_setup_phb_io_space(struct pci_controller *hose);
> +extern void pcibios_scan_phb(struct pci_controller *hose, void *data);
>  
>  #endif	/* __KERNEL__ */
>  #endif /* __ASM_POWERPC_PCI_H */
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 44497a8..8a16dbe 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1617,3 +1617,51 @@ void __devinit pcibios_setup_phb_resources(struct pci_controller *hose)
>  		 (unsigned long)hose->io_base_virt - _IO_BASE);
>  
>  }
> +
> +/**
> + * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
> + * @hose: Pointer to the PCI host controller instance structure
> + * @data: value to use for sysdata pointer.  ppc32 and ppc64 differ here
> + *
> + * Note: the 'data' pointer is a temporary measure.  As 32 and 64 bit
> + * pci code gets merged, this parameter should become unnecessary because
> + * both will use the same value.
> + */
> +void __devinit pcibios_scan_phb(struct pci_controller *hose, void *data)
> +{
> +	struct pci_bus *bus;
> +	struct device_node *node = hose->dn;
> +	int mode;
> +
> +	pr_debug("PCI: Scanning PHB %s\n",
> +		 node ? node->full_name : "<NO NAME>");
> +
> +	/* Create an empty bus for the toplevel */
> +	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, data);
> +	if (bus == NULL) {
> +		pr_err("Failed to create bus for PCI domain %04x\n",
> +			hose->global_number);
> +		return;
> +	}
> +	bus->secondary = hose->first_busno;
> +	hose->bus = bus;
> +
> +	/* Get some IO space for the new PHB */
> +	pcibios_setup_phb_io_space(hose);
> +
> +	/* Wire up PHB bus resources */
> +	pcibios_setup_phb_resources(hose);
> +
> +	/* Get probe mode and perform scan */
> +	mode = PCI_PROBE_NORMAL;
> +	if (node && ppc_md.pci_probe_mode)
> +		mode = ppc_md.pci_probe_mode(bus);
> +	pr_debug("    probe mode: %d\n", mode);
> +	if (mode == PCI_PROBE_DEVTREE) {
> +		bus->subordinate = hose->last_busno;
> +		of_scan_bus(node, bus);
> +	}
> +
> +	if (mode == PCI_PROBE_NORMAL)
> +		hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
> +}
> diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
> index 1e807fe..4e415e1 100644
> --- a/arch/powerpc/kernel/pci_32.c
> +++ b/arch/powerpc/kernel/pci_32.c
> @@ -354,36 +354,15 @@ pci_create_OF_bus_map(void)
>  	}
>  }
>  
> -static void __devinit pcibios_scan_phb(struct pci_controller *hose)
> +void __devinit pcibios_setup_phb_io_space(struct pci_controller *hose)
>  {
> -	struct pci_bus *bus;
> -	struct device_node *node = hose->dn;
>  	unsigned long io_offset;
>  	struct resource *res = &hose->io_resource;
>  
> -	pr_debug("PCI: Scanning PHB %s\n",
> -		 node ? node->full_name : "<NO NAME>");
> -
> -	/* Create an empty bus for the toplevel */
> -	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, hose);
> -	if (bus == NULL) {
> -		printk(KERN_ERR "Failed to create bus for PCI domain %04x\n",
> -		       hose->global_number);
> -		return;
> -	}
> -	bus->secondary = hose->first_busno;
> -	hose->bus = bus;
> -
>  	/* Fixup IO space offset */
>  	io_offset = (unsigned long)hose->io_base_virt - isa_io_base;
>  	res->start = (res->start + io_offset) & 0xffffffffu;
>  	res->end = (res->end + io_offset) & 0xffffffffu;
> -
> -	/* Wire up PHB bus resources */
> -	pcibios_setup_phb_resources(hose);
> -
> -	/* Scan children */
> -	hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
>  }
>  
>  static int __init pcibios_init(void)
> @@ -401,7 +380,7 @@ static int __init pcibios_init(void)
>  		if (pci_assign_all_buses)
>  			hose->first_busno = next_busno;
>  		hose->last_busno = 0xff;
> -		pcibios_scan_phb(hose);
> +		pcibios_scan_phb(hose, hose);
>  		pci_bus_add_devices(hose->bus);
>  		if (pci_assign_all_buses || next_busno <= hose->last_busno)
>  			next_busno = hose->last_busno + pcibios_assign_bus_offset;
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 4d5b4ce..ba949a2 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -43,45 +43,6 @@ unsigned long pci_probe_only = 1;
>  unsigned long pci_io_base = ISA_IO_BASE;
>  EXPORT_SYMBOL(pci_io_base);
>  
> -void __devinit scan_phb(struct pci_controller *hose)
> -{
> -	struct pci_bus *bus;
> -	struct device_node *node = hose->dn;
> -	int mode;
> -
> -	pr_debug("PCI: Scanning PHB %s\n",
> -		 node ? node->full_name : "<NO NAME>");
> -
> -	/* Create an empty bus for the toplevel */
> -	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, node);
> -	if (bus == NULL) {
> -		printk(KERN_ERR "Failed to create bus for PCI domain %04x\n",
> -		       hose->global_number);
> -		return;
> -	}
> -	bus->secondary = hose->first_busno;
> -	hose->bus = bus;
> -
> -	/* Get some IO space for the new PHB */
> -	pcibios_map_io_space(bus);
> -
> -	/* Wire up PHB bus resources */
> -	pcibios_setup_phb_resources(hose);
> -
> -	/* Get probe mode and perform scan */
> -	mode = PCI_PROBE_NORMAL;
> -	if (node && ppc_md.pci_probe_mode)
> -		mode = ppc_md.pci_probe_mode(bus);
> -	pr_debug("    probe mode: %d\n", mode);
> -	if (mode == PCI_PROBE_DEVTREE) {
> -		bus->subordinate = hose->last_busno;
> -		of_scan_bus(node, bus);
> -	}
> -
> -	if (mode == PCI_PROBE_NORMAL)
> -		hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
> -}
> -
>  static int __init pcibios_init(void)
>  {
>  	struct pci_controller *hose, *tmp;
> @@ -103,7 +64,7 @@ static int __init pcibios_init(void)
>  
>  	/* Scan all of the recorded PCI controllers.  */
>  	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> -		scan_phb(hose);
> +		pcibios_scan_phb(hose, hose->dn);
>  		pci_bus_add_devices(hose->bus);
>  	}
>  
> @@ -237,6 +198,11 @@ int __devinit pcibios_map_io_space(struct pci_bus *bus)
>  }
>  EXPORT_SYMBOL_GPL(pcibios_map_io_space);
>  
> +void __devinit pcibios_setup_phb_io_space(struct pci_controller *hose)
> +{
> +	pcibios_map_io_space(hose->bus);
> +}
> +
>  #define IOBASE_BRIDGE_NUMBER	0
>  #define IOBASE_MEMORY		1
>  #define IOBASE_IO		2
Grant Likely - Aug. 28, 2009, 6:42 p.m.
On Wed, Aug 26, 2009 at 11:46 PM, Benjamin
Herrenschmidt<benh@kernel.crashing.org> wrote:
> On Wed, 2009-08-26 at 00:07 -0600, Grant Likely wrote:
>> From: Grant Likely <grant.likely@secretlab.ca>
>>
>> The two versions are doing almost exactly the same thing.  No need to
>> maintain them as separate files.  This patch also has the side effect
>> of making the PCI device tree scanning code available to 32 bit powerpc
>> machines, but no board ports actually make use of this feature at this
>> point.
>
> You missed various calls to scan_phb() in arch/powerpc :-)
>
> At least pSeries with dynamic LPAR is broken

Oops, yup.  I also broke CONFIG_PPC_OF_PLATFORM_PCI in of_platform.c.
Fixed now.

> I think iSeries might break too.

iSeries has its own unrelated scan_phb() implementation.  I don't
think I broke it.

> Try a ppc64_defconfig.

Done.

> I tentatively applied the other patches to -test

Thanks.  New scan_phb() patch to follow.

g.

Patch

diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 9ae2e3e..feebfed 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -233,6 +233,8 @@  extern void pci_resource_to_user(const struct pci_dev *dev, int bar,
 
 extern void pcibios_setup_bus_devices(struct pci_bus *bus);
 extern void pcibios_setup_bus_self(struct pci_bus *bus);
+extern void pcibios_setup_phb_io_space(struct pci_controller *hose);
+extern void pcibios_scan_phb(struct pci_controller *hose, void *data);
 
 #endif	/* __KERNEL__ */
 #endif /* __ASM_POWERPC_PCI_H */
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 44497a8..8a16dbe 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1617,3 +1617,51 @@  void __devinit pcibios_setup_phb_resources(struct pci_controller *hose)
 		 (unsigned long)hose->io_base_virt - _IO_BASE);
 
 }
+
+/**
+ * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
+ * @hose: Pointer to the PCI host controller instance structure
+ * @data: value to use for sysdata pointer.  ppc32 and ppc64 differ here
+ *
+ * Note: the 'data' pointer is a temporary measure.  As 32 and 64 bit
+ * pci code gets merged, this parameter should become unnecessary because
+ * both will use the same value.
+ */
+void __devinit pcibios_scan_phb(struct pci_controller *hose, void *data)
+{
+	struct pci_bus *bus;
+	struct device_node *node = hose->dn;
+	int mode;
+
+	pr_debug("PCI: Scanning PHB %s\n",
+		 node ? node->full_name : "<NO NAME>");
+
+	/* Create an empty bus for the toplevel */
+	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, data);
+	if (bus == NULL) {
+		pr_err("Failed to create bus for PCI domain %04x\n",
+			hose->global_number);
+		return;
+	}
+	bus->secondary = hose->first_busno;
+	hose->bus = bus;
+
+	/* Get some IO space for the new PHB */
+	pcibios_setup_phb_io_space(hose);
+
+	/* Wire up PHB bus resources */
+	pcibios_setup_phb_resources(hose);
+
+	/* Get probe mode and perform scan */
+	mode = PCI_PROBE_NORMAL;
+	if (node && ppc_md.pci_probe_mode)
+		mode = ppc_md.pci_probe_mode(bus);
+	pr_debug("    probe mode: %d\n", mode);
+	if (mode == PCI_PROBE_DEVTREE) {
+		bus->subordinate = hose->last_busno;
+		of_scan_bus(node, bus);
+	}
+
+	if (mode == PCI_PROBE_NORMAL)
+		hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
+}
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 1e807fe..4e415e1 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -354,36 +354,15 @@  pci_create_OF_bus_map(void)
 	}
 }
 
-static void __devinit pcibios_scan_phb(struct pci_controller *hose)
+void __devinit pcibios_setup_phb_io_space(struct pci_controller *hose)
 {
-	struct pci_bus *bus;
-	struct device_node *node = hose->dn;
 	unsigned long io_offset;
 	struct resource *res = &hose->io_resource;
 
-	pr_debug("PCI: Scanning PHB %s\n",
-		 node ? node->full_name : "<NO NAME>");
-
-	/* Create an empty bus for the toplevel */
-	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, hose);
-	if (bus == NULL) {
-		printk(KERN_ERR "Failed to create bus for PCI domain %04x\n",
-		       hose->global_number);
-		return;
-	}
-	bus->secondary = hose->first_busno;
-	hose->bus = bus;
-
 	/* Fixup IO space offset */
 	io_offset = (unsigned long)hose->io_base_virt - isa_io_base;
 	res->start = (res->start + io_offset) & 0xffffffffu;
 	res->end = (res->end + io_offset) & 0xffffffffu;
-
-	/* Wire up PHB bus resources */
-	pcibios_setup_phb_resources(hose);
-
-	/* Scan children */
-	hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
 }
 
 static int __init pcibios_init(void)
@@ -401,7 +380,7 @@  static int __init pcibios_init(void)
 		if (pci_assign_all_buses)
 			hose->first_busno = next_busno;
 		hose->last_busno = 0xff;
-		pcibios_scan_phb(hose);
+		pcibios_scan_phb(hose, hose);
 		pci_bus_add_devices(hose->bus);
 		if (pci_assign_all_buses || next_busno <= hose->last_busno)
 			next_busno = hose->last_busno + pcibios_assign_bus_offset;
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 4d5b4ce..ba949a2 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -43,45 +43,6 @@  unsigned long pci_probe_only = 1;
 unsigned long pci_io_base = ISA_IO_BASE;
 EXPORT_SYMBOL(pci_io_base);
 
-void __devinit scan_phb(struct pci_controller *hose)
-{
-	struct pci_bus *bus;
-	struct device_node *node = hose->dn;
-	int mode;
-
-	pr_debug("PCI: Scanning PHB %s\n",
-		 node ? node->full_name : "<NO NAME>");
-
-	/* Create an empty bus for the toplevel */
-	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, node);
-	if (bus == NULL) {
-		printk(KERN_ERR "Failed to create bus for PCI domain %04x\n",
-		       hose->global_number);
-		return;
-	}
-	bus->secondary = hose->first_busno;
-	hose->bus = bus;
-
-	/* Get some IO space for the new PHB */
-	pcibios_map_io_space(bus);
-
-	/* Wire up PHB bus resources */
-	pcibios_setup_phb_resources(hose);
-
-	/* Get probe mode and perform scan */
-	mode = PCI_PROBE_NORMAL;
-	if (node && ppc_md.pci_probe_mode)
-		mode = ppc_md.pci_probe_mode(bus);
-	pr_debug("    probe mode: %d\n", mode);
-	if (mode == PCI_PROBE_DEVTREE) {
-		bus->subordinate = hose->last_busno;
-		of_scan_bus(node, bus);
-	}
-
-	if (mode == PCI_PROBE_NORMAL)
-		hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
-}
-
 static int __init pcibios_init(void)
 {
 	struct pci_controller *hose, *tmp;
@@ -103,7 +64,7 @@  static int __init pcibios_init(void)
 
 	/* Scan all of the recorded PCI controllers.  */
 	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
-		scan_phb(hose);
+		pcibios_scan_phb(hose, hose->dn);
 		pci_bus_add_devices(hose->bus);
 	}
 
@@ -237,6 +198,11 @@  int __devinit pcibios_map_io_space(struct pci_bus *bus)
 }
 EXPORT_SYMBOL_GPL(pcibios_map_io_space);
 
+void __devinit pcibios_setup_phb_io_space(struct pci_controller *hose)
+{
+	pcibios_map_io_space(hose->bus);
+}
+
 #define IOBASE_BRIDGE_NUMBER	0
 #define IOBASE_MEMORY		1
 #define IOBASE_IO		2