[PATCH 1/4] Simplify error logic in u3msi_setup_msi_irqs()

[PATCH 1/4] Simplify error logic in u3msi_setup_msi_irqs()

[Michael Ellerman]

u3msi_setup_msi_irqs() doesn't need to call teardown() itself,
the generic code will do this for us as long as we return a non
zero value.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/sysdev/mpic_u3msi.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
index 305b864..4e50d1c 100644
--- a/arch/powerpc/sysdev/mpic_u3msi.c
+++ b/arch/powerpc/sysdev/mpic_u3msi.c
@@ -124,7 +124,6 @@ static void u3msi_compose_msi_msg(struct pci_dev *pdev, int virq,
 static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 {
 	irq_hw_number_t hwirq;
-	int rc;
 	unsigned int virq;
 	struct msi_desc *entry;
 	struct msi_msg msg;
@@ -132,17 +131,15 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 	list_for_each_entry(entry, &pdev->msi_list, list) {
 		hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
 		if (hwirq < 0) {
-			rc = hwirq;
 			pr_debug("u3msi: failed allocating hwirq\n");
-			goto out_free;
+			return hwirq;
 		}
 
 		virq = irq_create_mapping(msi_mpic->irqhost, hwirq);
 		if (virq == NO_IRQ) {
 			pr_debug("u3msi: failed mapping hwirq 0x%lx\n", hwirq);
 			mpic_msi_free_hwirqs(msi_mpic, hwirq, 1);
-			rc = -ENOSPC;
-			goto out_free;
+			return -ENOSPC;
 		}
 
 		set_irq_msi(virq, entry);
@@ -156,10 +153,6 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 	}
 
 	return 0;
-
- out_free:
-	u3msi_teardown_msi_irqs(pdev);
-	return rc;
 }
 
 int mpic_u3msi_init(struct mpic *mpic)
-- 
1.5.1.3.g7a33b

[PATCH 2/4] Simplify error logic in rtas_setup_msi_irqs()

[Michael Ellerman]

rtas_setup_msi_irqs() doesn't need to call teardown() itself,
the generic code will do this for us as long as we return a non
zero value.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/msi.c |   17 +++--------------
 1 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 6063ea2..9c3bcfe 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -189,29 +189,22 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 
 	if (rc != nvec) {
 		pr_debug("rtas_msi: rtas_change_msi() failed\n");
-
-		/*
-		 * In case of an error it's not clear whether the device is
-		 * left with MSI enabled or not, so we explicitly disable.
-		 */
-		goto out_free;
+		return rc;
 	}
 
 	i = 0;
 	list_for_each_entry(entry, &pdev->msi_list, list) {
 		hwirq = rtas_query_irq_number(pdn, i);
 		if (hwirq < 0) {
-			rc = hwirq;
 			pr_debug("rtas_msi: error (%d) getting hwirq\n", rc);
-			goto out_free;
+			return hwirq;
 		}
 
 		virq = irq_create_mapping(NULL, hwirq);
 
 		if (virq == NO_IRQ) {
 			pr_debug("rtas_msi: Failed mapping hwirq %d\n", hwirq);
-			rc = -ENOSPC;
-			goto out_free;
+			return -ENOSPC;
 		}
 
 		dev_dbg(&pdev->dev, "rtas_msi: allocated virq %d\n", virq);
@@ -220,10 +213,6 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 	}
 
 	return 0;
-
- out_free:
-	rtas_teardown_msi_irqs(pdev);
-	return rc;
 }
 
 static void rtas_msi_pci_irq_fixup(struct pci_dev *pdev)
-- 
1.5.1.3.g7a33b

Re: [PATCH 2/4] Simplify error logic in rtas_setup_msi_irqs()

[Benjamin Herrenschmidt]

On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> rtas_setup_msi_irqs() doesn't need to call teardown() itself,
> the generic code will do this for us as long as we return a non
> zero value.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  arch/powerpc/platforms/pseries/msi.c |   17 +++--------------
>  1 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index 6063ea2..9c3bcfe 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -189,29 +189,22 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  
>  	if (rc != nvec) {
>  		pr_debug("rtas_msi: rtas_change_msi() failed\n");
> -
> -		/*
> -		 * In case of an error it's not clear whether the device is
> -		 * left with MSI enabled or not, so we explicitly disable.
> -		 */
> -		goto out_free;
> +		return rc;
>  	}
>  
>  	i = 0;
>  	list_for_each_entry(entry, &pdev->msi_list, list) {
>  		hwirq = rtas_query_irq_number(pdn, i);
>  		if (hwirq < 0) {
> -			rc = hwirq;
>  			pr_debug("rtas_msi: error (%d) getting hwirq\n", rc);
> -			goto out_free;
> +			return hwirq;
>  		}
>  
>  		virq = irq_create_mapping(NULL, hwirq);
>  
>  		if (virq == NO_IRQ) {
>  			pr_debug("rtas_msi: Failed mapping hwirq %d\n", hwirq);
> -			rc = -ENOSPC;
> -			goto out_free;
> +			return -ENOSPC;
>  		}
>  
>  		dev_dbg(&pdev->dev, "rtas_msi: allocated virq %d\n", virq);
> @@ -220,10 +213,6 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  	}
>  
>  	return 0;
> -
> - out_free:
> -	rtas_teardown_msi_irqs(pdev);
> -	return rc;
>  }
>  
>  static void rtas_msi_pci_irq_fixup(struct pci_dev *pdev)

[PATCH 3/4] Simplify rtas_change_msi() error semantics

[Michael Ellerman]

Currently rtas_change_msi() returns either the error code from RTAS, or if
the RTAS call succeeded the number of irqs that were configured by RTAS.
This makes checking the return value more complicated than it needs to be.

Instead, have rtas_change_msi() check that the number of irqs configured by
RTAS is equal to what we requested - and return an error otherwise. This makes
the return semantics match the usual 0 for success, something else for error.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/msi.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 9c3bcfe..2793a1b 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -70,11 +70,15 @@ static int rtas_change_msi(struct pci_dn *pdn, u32 func, u32 num_irqs)
 		seq_num = rtas_ret[1];
 	} while (rtas_busy_delay(rc));
 
-	if (rc == 0) /* Success */
-		rc = rtas_ret[0];
+	/*
+	 * If the RTAS call succeeded, check the number of irqs is actually
+	 * what we asked for. If not, return an error.
+	 */
+	if (rc == 0 && rtas_ret[0] != num_irqs)
+		rc = -ENOSPC;
 
-	pr_debug("rtas_msi: ibm,change_msi(func=%d,num=%d) = (%d)\n",
-		 func, num_irqs, rc);
+	pr_debug("rtas_msi: ibm,change_msi(func=%d,num=%d), got %d rc = %d\n",
+		 func, num_irqs, rtas_ret[0], rc);
 
 	return rc;
 }
@@ -87,7 +91,7 @@ static void rtas_disable_msi(struct pci_dev *pdev)
 	if (!pdn)
 		return;
 
-	if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0)
+	if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0))
 		pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
 }
 
@@ -180,14 +184,14 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 	if (type == PCI_CAP_ID_MSI) {
 		rc = rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, nvec);
 
-		if (rc != nvec) {
+		if (rc) {
 			pr_debug("rtas_msi: trying the old firmware call.\n");
 			rc = rtas_change_msi(pdn, RTAS_CHANGE_FN, nvec);
 		}
 	} else
 		rc = rtas_change_msi(pdn, RTAS_CHANGE_MSIX_FN, nvec);
 
-	if (rc != nvec) {
+	if (rc) {
 		pr_debug("rtas_msi: rtas_change_msi() failed\n");
 		return rc;
 	}
-- 
1.5.1.3.g7a33b

Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics

[Benjamin Herrenschmidt]

On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> Currently rtas_change_msi() returns either the error code from RTAS, or if
> the RTAS call succeeded the number of irqs that were configured by RTAS.
> This makes checking the return value more complicated than it needs to be.
> 
> Instead, have rtas_change_msi() check that the number of irqs configured by
> RTAS is equal to what we requested - and return an error otherwise. This makes
> the return semantics match the usual 0 for success, something else for error.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Looks allright, just a question tho... what do we do if it fails ? Do we
try to fallback to a lower number of MSIs ? Or what ? Dead device ?

Ben.

> ---
>  arch/powerpc/platforms/pseries/msi.c |   18 +++++++++++-------
>  1 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index 9c3bcfe..2793a1b 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -70,11 +70,15 @@ static int rtas_change_msi(struct pci_dn *pdn, u32 func, u32 num_irqs)
>  		seq_num = rtas_ret[1];
>  	} while (rtas_busy_delay(rc));
>  
> -	if (rc == 0) /* Success */
> -		rc = rtas_ret[0];
> +	/*
> +	 * If the RTAS call succeeded, check the number of irqs is actually
> +	 * what we asked for. If not, return an error.
> +	 */
> +	if (rc == 0 && rtas_ret[0] != num_irqs)
> +		rc = -ENOSPC;
>  
> -	pr_debug("rtas_msi: ibm,change_msi(func=%d,num=%d) = (%d)\n",
> -		 func, num_irqs, rc);
> +	pr_debug("rtas_msi: ibm,change_msi(func=%d,num=%d), got %d rc = %d\n",
> +		 func, num_irqs, rtas_ret[0], rc);
>  
>  	return rc;
>  }
> @@ -87,7 +91,7 @@ static void rtas_disable_msi(struct pci_dev *pdev)
>  	if (!pdn)
>  		return;
>  
> -	if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0)
> +	if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0))
>  		pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
>  }
>  
> @@ -180,14 +184,14 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  	if (type == PCI_CAP_ID_MSI) {
>  		rc = rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, nvec);
>  
> -		if (rc != nvec) {
> +		if (rc) {
>  			pr_debug("rtas_msi: trying the old firmware call.\n");
>  			rc = rtas_change_msi(pdn, RTAS_CHANGE_FN, nvec);
>  		}
>  	} else
>  		rc = rtas_change_msi(pdn, RTAS_CHANGE_MSIX_FN, nvec);
>  
> -	if (rc != nvec) {
> +	if (rc) {
>  		pr_debug("rtas_msi: rtas_change_msi() failed\n");
>  		return rc;
>  	}

Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]

On Tue, 2007-10-02 at 15:23 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> > Currently rtas_change_msi() returns either the error code from RTAS, or if
> > the RTAS call succeeded the number of irqs that were configured by RTAS.
> > This makes checking the return value more complicated than it needs to be.
> > 
> > Instead, have rtas_change_msi() check that the number of irqs configured by
> > RTAS is equal to what we requested - and return an error otherwise. This makes
> > the return semantics match the usual 0 for success, something else for error.
> > 
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> Looks allright, just a question tho... what do we do if it fails ? Do we
> try to fallback to a lower number of MSIs ? Or what ? Dead device ?

That's all up to the device driver. In theory the driver could try again
with a lower count - but that might require extra logic in the driver to
handle shared irq handlers etc. In practice I think the current drivers
will just fail.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics

[Benjamin Herrenschmidt]

On Tue, 2007-10-02 at 15:58 +1000, Michael Ellerman wrote:
> > Looks allright, just a question tho... what do we do if it fails ?
> Do we
> > try to fallback to a lower number of MSIs ? Or what ? Dead device ?
> 
> That's all up to the device driver. In theory the driver could try again
> with a lower count - but that might require extra logic in the driver to
> handle shared irq handlers etc. In practice I think the current drivers
> will just fail.

Question is badly phrased.. I meant something more like... what do we do
if RTAS returns a lower count ?

That is, we end up with that device with that lower count of MSIs
enabled, we fail at the driver level, do we still somewhat keep track ?
Drivers might assume that means it can use LSIs no ? which may not be
the case... Shouldn't we try to switch back to LSI mode (or does the
RTAS interface doesnt allow it ?)

Ben.

Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1795 bytes --]

On Tue, 2007-10-02 at 16:24 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2007-10-02 at 15:58 +1000, Michael Ellerman wrote:
> > > Looks allright, just a question tho... what do we do if it fails ?
> > Do we
> > > try to fallback to a lower number of MSIs ? Or what ? Dead device ?
> > 
> > That's all up to the device driver. In theory the driver could try again
> > with a lower count - but that might require extra logic in the driver to
> > handle shared irq handlers etc. In practice I think the current drivers
> > will just fail.
> 
> Question is badly phrased.. I meant something more like... what do we do
> if RTAS returns a lower count ?
> 
> That is, we end up with that device with that lower count of MSIs
> enabled, we fail at the driver level, do we still somewhat keep track ?
> Drivers might assume that means it can use LSIs no ? which may not be
> the case... Shouldn't we try to switch back to LSI mode (or does the
> RTAS interface doesnt allow it ?)

OK I get you. So:

rtas_setup_msi_irqs() detects that it got fewer MSIs than it asked for
and returns an error.

The generic code (drivers/pci/msi.c) notices the error and calls
msi_free_irqs().

That calls back into rtas_teardown_msi_irqs() which disposes of the virq
mappings and calls rtas_disable_msi().

rtas_disable_msi() asks firmware to configure 0 MSIs on the device, that
hopefully succeeds. AFAIK configuring 0 MSIs is as close as we can get
to disabling MSI via RTAS.

Perhaps that should also (re)enable INTX?

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics

[Benjamin Herrenschmidt]

On Tue, 2007-10-02 at 17:40 +1000, Michael Ellerman wrote:
> 
> rtas_disable_msi() asks firmware to configure 0 MSIs on the device,
> that
> hopefully succeeds. AFAIK configuring 0 MSIs is as close as we can get
> to disabling MSI via RTAS.
> 
> Perhaps that should also (re)enable INTX?

Not sure... maybe. RTAS doesn't do it ? Then there,s the question of
what happens on machines that don't support INTx ...

Cheers.
Ben.

[PATCH 4/4] Inline u3msi_compose_msi_msg()

[Michael Ellerman]

In the MPIC U3 MSI code, we call u3msi_compose_msi_msg() once for each MSI.
This is overkill, as the address is per pci device, not per MSI. So setup
the address once, and just set the data per MSI.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/sysdev/mpic_u3msi.c |   24 +++++++++---------------
 1 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
index 4e50d1c..027fe01 100644
--- a/arch/powerpc/sysdev/mpic_u3msi.c
+++ b/arch/powerpc/sysdev/mpic_u3msi.c
@@ -107,26 +107,17 @@ static void u3msi_teardown_msi_irqs(struct pci_dev *pdev)
 	return;
 }
 
-static void u3msi_compose_msi_msg(struct pci_dev *pdev, int virq,
-				  struct msi_msg *msg)
-{
-	u64 addr;
-
-	addr = find_ht_magic_addr(pdev);
-	msg->address_lo = addr & 0xFFFFFFFF;
-	msg->address_hi = addr >> 32;
-	msg->data = virq_to_hw(virq);
-
-	pr_debug("u3msi: allocated virq 0x%x (hw 0x%lx) at address 0x%lx\n",
-		 virq, virq_to_hw(virq), addr);
-}
-
 static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 {
 	irq_hw_number_t hwirq;
 	unsigned int virq;
 	struct msi_desc *entry;
 	struct msi_msg msg;
+	u64 addr;
+
+	addr = find_ht_magic_addr(pdev);
+	msg.address_lo = addr & 0xFFFFFFFF;
+	msg.address_hi = addr >> 32;
 
 	list_for_each_entry(entry, &pdev->msi_list, list) {
 		hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
@@ -146,7 +137,10 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 		set_irq_chip(virq, &mpic_u3msi_chip);
 		set_irq_type(virq, IRQ_TYPE_EDGE_RISING);
 
-		u3msi_compose_msi_msg(pdev, virq, &msg);
+		pr_debug("u3msi: allocated virq 0x%x (hw 0x%lx) addr 0x%lx\n",
+			  virq, hwirq, addr);
+
+		msg.data = hwirq;
 		write_msi_msg(virq, &msg);
 
 		hwirq++;
-- 
1.5.1.3.g7a33b

Re: [PATCH 4/4] Inline u3msi_compose_msi_msg()

[Benjamin Herrenschmidt]

On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> In the MPIC U3 MSI code, we call u3msi_compose_msi_msg() once for each MSI.
> This is overkill, as the address is per pci device, not per MSI. So setup
> the address once, and just set the data per MSI.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  arch/powerpc/sysdev/mpic_u3msi.c |   24 +++++++++---------------
>  1 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
> index 4e50d1c..027fe01 100644
> --- a/arch/powerpc/sysdev/mpic_u3msi.c
> +++ b/arch/powerpc/sysdev/mpic_u3msi.c
> @@ -107,26 +107,17 @@ static void u3msi_teardown_msi_irqs(struct pci_dev *pdev)
>  	return;
>  }
>  
> -static void u3msi_compose_msi_msg(struct pci_dev *pdev, int virq,
> -				  struct msi_msg *msg)
> -{
> -	u64 addr;
> -
> -	addr = find_ht_magic_addr(pdev);
> -	msg->address_lo = addr & 0xFFFFFFFF;
> -	msg->address_hi = addr >> 32;
> -	msg->data = virq_to_hw(virq);
> -
> -	pr_debug("u3msi: allocated virq 0x%x (hw 0x%lx) at address 0x%lx\n",
> -		 virq, virq_to_hw(virq), addr);
> -}
> -
>  static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  {
>  	irq_hw_number_t hwirq;
>  	unsigned int virq;
>  	struct msi_desc *entry;
>  	struct msi_msg msg;
> +	u64 addr;
> +
> +	addr = find_ht_magic_addr(pdev);
> +	msg.address_lo = addr & 0xFFFFFFFF;
> +	msg.address_hi = addr >> 32;
>  
>  	list_for_each_entry(entry, &pdev->msi_list, list) {
>  		hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
> @@ -146,7 +137,10 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  		set_irq_chip(virq, &mpic_u3msi_chip);
>  		set_irq_type(virq, IRQ_TYPE_EDGE_RISING);
>  
> -		u3msi_compose_msi_msg(pdev, virq, &msg);
> +		pr_debug("u3msi: allocated virq 0x%x (hw 0x%lx) addr 0x%lx\n",
> +			  virq, hwirq, addr);
> +
> +		msg.data = hwirq;
>  		write_msi_msg(virq, &msg);
>  
>  		hwirq++;

Re: [PATCH 1/4] Simplify error logic in u3msi_setup_msi_irqs()

[Benjamin Herrenschmidt]

On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> u3msi_setup_msi_irqs() doesn't need to call teardown() itself,
> the generic code will do this for us as long as we return a non
> zero value.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  arch/powerpc/sysdev/mpic_u3msi.c |   11 ++---------
>  1 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
> index 305b864..4e50d1c 100644
> --- a/arch/powerpc/sysdev/mpic_u3msi.c
> +++ b/arch/powerpc/sysdev/mpic_u3msi.c
> @@ -124,7 +124,6 @@ static void u3msi_compose_msi_msg(struct pci_dev *pdev, int virq,
>  static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  {
>  	irq_hw_number_t hwirq;
> -	int rc;
>  	unsigned int virq;
>  	struct msi_desc *entry;
>  	struct msi_msg msg;
> @@ -132,17 +131,15 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  	list_for_each_entry(entry, &pdev->msi_list, list) {
>  		hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
>  		if (hwirq < 0) {
> -			rc = hwirq;
>  			pr_debug("u3msi: failed allocating hwirq\n");
> -			goto out_free;
> +			return hwirq;
>  		}
>  
>  		virq = irq_create_mapping(msi_mpic->irqhost, hwirq);
>  		if (virq == NO_IRQ) {
>  			pr_debug("u3msi: failed mapping hwirq 0x%lx\n", hwirq);
>  			mpic_msi_free_hwirqs(msi_mpic, hwirq, 1);
> -			rc = -ENOSPC;
> -			goto out_free;
> +			return -ENOSPC;
>  		}
>  
>  		set_irq_msi(virq, entry);
> @@ -156,10 +153,6 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  	}
>  
>  	return 0;
> -
> - out_free:
> -	u3msi_teardown_msi_irqs(pdev);
> -	return rc;
>  }
>  
>  int mpic_u3msi_init(struct mpic *mpic)