linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] powerpc/powernv: Free PHB instance upon error
@ 2013-07-31  8:47 Gavin Shan
  2013-07-31  8:47 ` [PATCH 2/5] powerpc/powernv: Fetch PHB bus range from dev-tree Gavin Shan
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Gavin Shan @ 2013-07-31  8:47 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

We don't free PHB instance (struct pnv_phb) on error to creating
the associated PCI controler (struct pci_controller). The patch
fixes that. Also, the output messages have been cleaned for a bit
so that they looks unified for IODA_1/2 cases.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index d8140b1..9cccdc7 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1113,7 +1113,7 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
 	void *aux;
 	long rc;
 
-	pr_info(" Initializing IODA%d OPAL PHB %s\n", ioda_type, np->full_name);
+	pr_info("Initializing IODA%d OPAL PHB %s\n", ioda_type, np->full_name);
 
 	prop64 = of_get_property(np, "ibm,opal-phbid", NULL);
 	if (!prop64) {
@@ -1124,13 +1124,18 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
 	pr_debug("  PHB-ID  : 0x%016llx\n", phb_id);
 
 	phb = alloc_bootmem(sizeof(struct pnv_phb));
-	if (phb) {
-		memset(phb, 0, sizeof(struct pnv_phb));
-		phb->hose = hose = pcibios_alloc_controller(np);
+	if (!phb) {
+		pr_err("  Out of memory !\n");
+		return;
 	}
-	if (!phb || !phb->hose) {
-		pr_err("PCI: Failed to allocate PCI controller for %s\n",
+
+	/* Allocate PCI controller */
+	memset(phb, 0, sizeof(struct pnv_phb));
+	phb->hose = hose = pcibios_alloc_controller(np);
+	if (!phb->hose) {
+		pr_err("  Can't allocate PCI controller for %s\n",
 		       np->full_name);
+		free_bootmem((unsigned long)phb, sizeof(struct pnv_phb));
 		return;
 	}
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/5] powerpc/powernv: Fetch PHB bus range from dev-tree
  2013-07-31  8:47 [PATCH 1/5] powerpc/powernv: Free PHB instance upon error Gavin Shan
@ 2013-07-31  8:47 ` Gavin Shan
  2013-07-31  8:47 ` [PATCH 3/5] powerpc/powernv: Check primary PHB through ID Gavin Shan
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Gavin Shan @ 2013-07-31  8:47 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

The patch enables fetching bus range from device-tree for the
specific PHB. If we can't get that from device-tree, the default
range [0 255] will be used.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 9cccdc7..f472228 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1109,6 +1109,7 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
 	unsigned long size, m32map_off, iomap_off, pemap_off;
 	const u64 *prop64;
 	const u32 *prop32;
+	int len;
 	u64 phb_id;
 	void *aux;
 	long rc;
@@ -1140,9 +1141,15 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
 	}
 
 	spin_lock_init(&phb->lock);
-	/* XXX Use device-tree */
-	hose->first_busno = 0;
-	hose->last_busno = 0xff;
+	prop32 = of_get_property(np, "bus-range", &len);
+	if (prop32 && len == 8) {
+		hose->first_busno = prop32[0];
+		hose->last_busno = prop32[1];
+	} else {
+		pr_warn("  Broken <bus-range> on %s\n", np->full_name);
+		hose->first_busno = 0;
+		hose->last_busno = 0xff;
+	}
 	hose->private_data = phb;
 	phb->hub_id = hub_id;
 	phb->opal_id = phb_id;
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/5] powerpc/powernv: Check primary PHB through ID
  2013-07-31  8:47 [PATCH 1/5] powerpc/powernv: Free PHB instance upon error Gavin Shan
  2013-07-31  8:47 ` [PATCH 2/5] powerpc/powernv: Fetch PHB bus range from dev-tree Gavin Shan
@ 2013-07-31  8:47 ` Gavin Shan
  2013-07-31  8:47 ` [PATCH 4/5] powerpc/powernv: Pick up correct number of PEs Gavin Shan
  2013-07-31  8:47 ` [PATCH 5/5] powerpc/powernv: Needn't IO segment map for PHB3 Gavin Shan
  3 siblings, 0 replies; 8+ messages in thread
From: Gavin Shan @ 2013-07-31  8:47 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

The index of one specific PCI controller (struct pci_controller::
global_number) can tell that it's primary one or not. So we needn't
additional variable for that and just remove it.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index f472228..829047b 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1104,7 +1104,6 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
 				  u64 hub_id, int ioda_type)
 {
 	struct pci_controller *hose;
-	static int primary = 1;
 	struct pnv_phb *phb;
 	unsigned long size, m32map_off, iomap_off, pemap_off;
 	const u64 *prop64;
@@ -1164,8 +1163,7 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
 		phb->model = PNV_PHB_MODEL_UNKNOWN;
 
 	/* Parse 32-bit and IO ranges (if any) */
-	pci_process_bridge_OF_ranges(phb->hose, np, primary);
-	primary = 0;
+	pci_process_bridge_OF_ranges(hose, np, !hose->global_number);
 
 	/* Get registers */
 	phb->regs = of_iomap(np, 0);
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/5] powerpc/powernv: Pick up correct number of PEs
  2013-07-31  8:47 [PATCH 1/5] powerpc/powernv: Free PHB instance upon error Gavin Shan
  2013-07-31  8:47 ` [PATCH 2/5] powerpc/powernv: Fetch PHB bus range from dev-tree Gavin Shan
  2013-07-31  8:47 ` [PATCH 3/5] powerpc/powernv: Check primary PHB through ID Gavin Shan
@ 2013-07-31  8:47 ` Gavin Shan
  2013-07-31  9:18   ` Benjamin Herrenschmidt
  2013-07-31  8:47 ` [PATCH 5/5] powerpc/powernv: Needn't IO segment map for PHB3 Gavin Shan
  3 siblings, 1 reply; 8+ messages in thread
From: Gavin Shan @ 2013-07-31  8:47 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

Usually, the property "ibm,opal-num-pes" of PHB dev-tree node
indicates the number of total PEs. If that property isn't existing
or valid, we should fall back to pick the correct number of total
PEs according to PHB type: IODA1 or IODA2.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 829047b..6386bb4 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1172,11 +1172,14 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
 
 	/* Initialize more IODA stuff */
 	prop32 = of_get_property(np, "ibm,opal-num-pes", NULL);
-	if (!prop32)
-		phb->ioda.total_pe = 1;
-	else
+	if (prop32)
 		phb->ioda.total_pe = *prop32;
-
+	else if (phb->type == PNV_PHB_IODA1)
+		phb->ioda.total_pe = 128;
+	else if (phb->type == PNV_PHB_IODA2)
+		phb->ioda.total_pe = 256;
+	else
+		phb->ioda.total_pe = 1;
 	phb->ioda.m32_size = resource_size(&hose->mem_resources[0]);
 	/* FW Has already off top 64k of M32 space (MSI space) */
 	phb->ioda.m32_size += 0x10000;
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/5] powerpc/powernv: Needn't IO segment map for PHB3
  2013-07-31  8:47 [PATCH 1/5] powerpc/powernv: Free PHB instance upon error Gavin Shan
                   ` (2 preceding siblings ...)
  2013-07-31  8:47 ` [PATCH 4/5] powerpc/powernv: Pick up correct number of PEs Gavin Shan
@ 2013-07-31  8:47 ` Gavin Shan
  3 siblings, 0 replies; 8+ messages in thread
From: Gavin Shan @ 2013-07-31  8:47 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

PHB3 doesn't support IO ports and we needn't IO segment map for that.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 6386bb4..147f9b7 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1190,22 +1190,23 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
 	phb->ioda.io_segsize = phb->ioda.io_size / phb->ioda.total_pe;
 	phb->ioda.io_pci_base = 0; /* XXX calculate this ? */
 
-	/* Allocate aux data & arrays
-	 *
-	 * XXX TODO: Don't allocate io segmap on PHB3
-	 */
+	/* Allocate aux data & arrays. We don't have IO ports on PHB3 */
 	size = _ALIGN_UP(phb->ioda.total_pe / 8, sizeof(unsigned long));
 	m32map_off = size;
 	size += phb->ioda.total_pe * sizeof(phb->ioda.m32_segmap[0]);
 	iomap_off = size;
-	size += phb->ioda.total_pe * sizeof(phb->ioda.io_segmap[0]);
+	if (phb->type == PNV_PHB_IODA1) {
+		iomap_off = size;
+		size += phb->ioda.total_pe * sizeof(phb->ioda.io_segmap[0]);
+	}
 	pemap_off = size;
 	size += phb->ioda.total_pe * sizeof(struct pnv_ioda_pe);
 	aux = alloc_bootmem(size);
 	memset(aux, 0, size);
 	phb->ioda.pe_alloc = aux;
 	phb->ioda.m32_segmap = aux + m32map_off;
-	phb->ioda.io_segmap = aux + iomap_off;
+	if (phb->type == PNV_PHB_IODA1)
+		phb->ioda.io_segmap = aux + iomap_off;
 	phb->ioda.pe_array = aux + pemap_off;
 	set_bit(0, phb->ioda.pe_alloc);
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 4/5] powerpc/powernv: Pick up correct number of PEs
  2013-07-31  8:47 ` [PATCH 4/5] powerpc/powernv: Pick up correct number of PEs Gavin Shan
@ 2013-07-31  9:18   ` Benjamin Herrenschmidt
  2013-08-01  4:24     ` Gavin Shan
  0 siblings, 1 reply; 8+ messages in thread
From: Benjamin Herrenschmidt @ 2013-07-31  9:18 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev

On Wed, 2013-07-31 at 16:47 +0800, Gavin Shan wrote:
> Usually, the property "ibm,opal-num-pes" of PHB dev-tree node
> indicates the number of total PEs. If that property isn't existing
> or valid, we should fall back to pick the correct number of total
> PEs according to PHB type: IODA1 or IODA2.

Is that correct ? Don't we get the total number of PEs from a config
register on the bridge ? I didn't think the IODA architecture specified
the total number of PE of a given implementation...

For example, does Torrent implement 128 ?

I'd rather stick to safe here, if the firmware doesn't say, just use
one.

Now some of the PHB registers are actually architected in IODA afaik, so
we could just go look but let's not make a precedent here.

Cheers,
Ben.

> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/pci-ioda.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 829047b..6386bb4 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1172,11 +1172,14 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
>  
>  	/* Initialize more IODA stuff */
>  	prop32 = of_get_property(np, "ibm,opal-num-pes", NULL);
> -	if (!prop32)
> -		phb->ioda.total_pe = 1;
> -	else
> +	if (prop32)
>  		phb->ioda.total_pe = *prop32;
> -
> +	else if (phb->type == PNV_PHB_IODA1)
> +		phb->ioda.total_pe = 128;
> +	else if (phb->type == PNV_PHB_IODA2)
> +		phb->ioda.total_pe = 256;
> +	else
> +		phb->ioda.total_pe = 1;
>  	phb->ioda.m32_size = resource_size(&hose->mem_resources[0]);
>  	/* FW Has already off top 64k of M32 space (MSI space) */
>  	phb->ioda.m32_size += 0x10000;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 4/5] powerpc/powernv: Pick up correct number of PEs
  2013-07-31  9:18   ` Benjamin Herrenschmidt
@ 2013-08-01  4:24     ` Gavin Shan
  2013-08-01  4:42       ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 8+ messages in thread
From: Gavin Shan @ 2013-08-01  4:24 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Gavin Shan

On Wed, Jul 31, 2013 at 07:18:46PM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2013-07-31 at 16:47 +0800, Gavin Shan wrote:
>> Usually, the property "ibm,opal-num-pes" of PHB dev-tree node
>> indicates the number of total PEs. If that property isn't existing
>> or valid, we should fall back to pick the correct number of total
>> PEs according to PHB type: IODA1 or IODA2.
>
>Is that correct ? Don't we get the total number of PEs from a config
>register on the bridge ? I didn't think the IODA architecture specified
>the total number of PE of a given implementation...
>

For now, the firmware has fixed values (1/128/256), which isn't figured
out from EEH capability register. That might be something to do later
for the f/w.

>For example, does Torrent implement 128 ?
>

I don't know what's "Torrent" :-)

>I'd rather stick to safe here, if the firmware doesn't say, just use
>one.
>
>Now some of the PHB registers are actually architected in IODA afaik, so
>we could just go look but let's not make a precedent here.
>

Ok. Thanks, Ben. Please drop this one :-)

Thanks,
Gavin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 4/5] powerpc/powernv: Pick up correct number of PEs
  2013-08-01  4:24     ` Gavin Shan
@ 2013-08-01  4:42       ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 8+ messages in thread
From: Benjamin Herrenschmidt @ 2013-08-01  4:42 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev

On Thu, 2013-08-01 at 12:24 +0800, Gavin Shan wrote:
> at correct ? Don't we get the total number of PEs from a config
> >register on the bridge ? I didn't think the IODA architecture specified
> >the total number of PE of a given implementation...
> >
> 
> For now, the firmware has fixed values (1/128/256), which isn't figured
> out from EEH capability register. That might be something to do later
> for the f/w.

Sure but we can fix the firmware easily, we need per-chip code in there
anyway, while in Linux, we mostly avoid exposing the specifics of a
given implementation of the architecture, we only expose the
architectural version (IODA1 vs IODA2).

> >For example, does Torrent implement 128 ?
>
> 
> I don't know what's "Torrent" :-)

It's one of our IO chips for P7 :-) It has a built-in HFI (sort-of
infiniband thingy) and implements PCIe slots with IODA1. It has *some*
differences to P7IOC however.

> >I'd rather stick to safe here, if the firmware doesn't say, just use
> >one.
> >
> >Now some of the PHB registers are actually architected in IODA afaik, so
> >we could just go look but let's not make a precedent here.
> >
> 
> Ok. Thanks, Ben. Please drop this one :-)

Will do :-)

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-08-01  4:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-31  8:47 [PATCH 1/5] powerpc/powernv: Free PHB instance upon error Gavin Shan
2013-07-31  8:47 ` [PATCH 2/5] powerpc/powernv: Fetch PHB bus range from dev-tree Gavin Shan
2013-07-31  8:47 ` [PATCH 3/5] powerpc/powernv: Check primary PHB through ID Gavin Shan
2013-07-31  8:47 ` [PATCH 4/5] powerpc/powernv: Pick up correct number of PEs Gavin Shan
2013-07-31  9:18   ` Benjamin Herrenschmidt
2013-08-01  4:24     ` Gavin Shan
2013-08-01  4:42       ` Benjamin Herrenschmidt
2013-07-31  8:47 ` [PATCH 5/5] powerpc/powernv: Needn't IO segment map for PHB3 Gavin Shan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).