[PATCH] pata-rb532-cf: platform_get_irq() failure ignored

------------------------------>8-------------8<---------------------------------
platform_get_irq() can return -ENXIO, but since 'irq' is an unsigned int, it
does not show when the IRQ resource wasn't found.

Signed-off-by: Roel Kluin <redacted>
---

diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
index ebfcda2..fe8178c 100644
--- a/drivers/ata/pata_rb532_cf.c
+++ b/drivers/ata/pata_rb532_cf.c

@@ -173,11 +173,12 @@ static __devinit int rb532_pata_driver_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
+	ret = platform_get_irq(pdev, 0);
+	if (ret <= 0) {
 		dev_err(&pdev->dev, "no IRQ resource found\n");
 		return -ENOENT;
 	}
+	irq = ret;
 
 	gpio = irq_to_gpio(irq);
 	if (gpio < 0) {

Roel Kluin wrote:

ACK from Phil or Florian?

Hi,

On Thu, Mar 05, 2009 at 07:23:22AM -0500, Jeff Garzik wrote:

Roel Kluin wrote:

quoted

------------------------------>8-------------8<---------------------------------
platform_get_irq() can return -ENXIO, but since 'irq' is an unsigned int, 
it
does not show when the IRQ resource wasn't found.

Signed-off-by: Roel Kluin <redacted>
---

diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
index ebfcda2..fe8178c 100644
--- a/drivers/ata/pata_rb532_cf.c
+++ b/drivers/ata/pata_rb532_cf.c

@@ -173,11 +173,12 @@ static __devinit int rb532_pata_driver_probe(struct 

platform_device *pdev)
		return -EINVAL;
	}

-	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
+	ret = platform_get_irq(pdev, 0);
+	if (ret <= 0) {
		dev_err(&pdev->dev, "no IRQ resource found\n");
		return -ENOENT;
	}
+	irq = ret;

	gpio = irq_to_gpio(irq);

ACK from Phil or Florian?

That's fine for me, though I didn't test it (I managed to break my
build-machine somehow, in an attempt to having the mainboard properly
grounded).

One could simplify the patch by making the variable 'irq' be signed
instead of unsigned. This would also prevent "abusing" the variable
'ret'. What do you think?

Greetings, Phil

Hi,

Le Friday 06 March 2009 17:43:18 Phil Sutter, vous avez écrit :

Hi,

On Thu, Mar 05, 2009 at 07:23:22AM -0500, Jeff Garzik wrote:

quoted

Roel Kluin wrote:

quoted

------------------------------>8-------------8<-------------------------
-------- platform_get_irq() can return -ENXIO, but since 'irq' is an
unsigned int, it
does not show when the IRQ resource wasn't found.

Signed-off-by: Roel Kluin <redacted>
---

diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
index ebfcda2..fe8178c 100644
--- a/drivers/ata/pata_rb532_cf.c
+++ b/drivers/ata/pata_rb532_cf.c

@@ -173,11 +173,12 @@ static __devinit int

rb532_pata_driver_probe(struct platform_device *pdev)
		return -EINVAL;
	}

-	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
+	ret = platform_get_irq(pdev, 0);
+	if (ret <= 0) {
		dev_err(&pdev->dev, "no IRQ resource found\n");
		return -ENOENT;
	}
+	irq = ret;

	gpio = irq_to_gpio(irq);

ACK from Phil or Florian?

That's fine for me, though I didn't test it (I managed to break my
build-machine somehow, in an attempt to having the mainboard properly
grounded).

One could simplify the patch by making the variable 'irq' be signed
instead of unsigned. This would also prevent "abusing" the variable
'ret'. What do you think?

I agree with this proposal, abusing the ret value is not that good. What about the patch below :
--
From: Florian Fainelli <florian@openwrt.org>
Subject: [PATCH v2] pata-rb532-cf: platform_get_irq() failure ignored

platform_get_irq() can return -ENXIO, but since 'irq' is an
unsigned int, it does not show when the IRQ resource wasn't found.
Make irq an int so that we can use a single variable to test the
platform_get_irq() return value.

Signed-off-by: Roel Kluin <redacted>
Signed-off-by: Phil Sutter <redacted>
Signed-off-by: Florian Fainelli <florian@openwrt.org>
--

diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
index ebfcda2..da05518 100644
--- a/drivers/ata/pata_rb532_cf.c
+++ b/drivers/ata/pata_rb532_cf.c

@@ -160,7 +160,7 @@ static void rb532_pata_setup_ports(struct ata_host *ah)
 
 static __devinit int rb532_pata_driver_probe(struct platform_device *pdev)
 {
-	unsigned int irq;
+	int irq;
 	int gpio;
 	struct resource *res;
 	struct ata_host *ah;

Hi,

On Fri, Mar 13, 2009 at 03:41:43PM +0100, Florian Fainelli wrote:

I agree with this proposal, abusing the ret value is not that good. What about the patch below :

That's exactly what I meant. Thanks for preparing the patch!

Greetings, Phil

Florian Fainelli wrote:

applied