pata-rb532-cf: platform_get_irq() failure ignored

PATCH Applied Roel Kluin 4 participants 6 messages Mar 3, 2009 – Mar 25, 2009 libata

Share & tools

Roel Kluin

author

2009-03-03 13:04 UTC #0 lore raw

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

---

drivers/ata/pata_rb532_cf.c +3 -2

index ebfcda2..fe8178c 100644
@@ -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) {

Jeff Garzik

2009-03-05 12:23 UTC #1 lore raw

Quoted 14 lines · Roel Kluin

------------------------------>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 <[email protected]>
---
drivers/ata/pata_rb532_cf.c +0 -0
index ebfcda2..fe8178c 100644
@@ -173,11 +173,12 @@ static __devinit int rb532_pata_driver_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 

drivers/ata/pata_rb532_cf.c static __devinit int rb532_pata_driver_probe(struct platform_device *pdev) · line 183

Quoted 10 lines

drivers/ata/pata_rb532_cf.c +3 -2

-	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?

Phil Sutter

2009-03-06 16:53 UTC #2 lore raw

Hi,

Quoted 26 lines · Jeff Garzik

Quoted 25 lines · Roel Kluin
------------------------------>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 <[email protected]>
---
drivers/ata/pata_rb532_cf.c +3 -2
index ebfcda2..fe8178c 100644
@@ -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;
 

Quoted 3 lines

drivers/ata/pata_rb532_cf.c static __devinit int rb532_pata_driver_probe(struct platform_device *pdev) · line 183
Quoted 1 line
drivers/ata/pata_rb532_cf.c +0 -0
 	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

Florian Fainelli

2009-03-13 14:41 UTC #3 lore raw

Hi,

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

Quoted 33 lines

Hi,
Quoted 26 lines · Jeff Garzik
Quoted 25 lines · Roel Kluin
------------------------------>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 <[email protected]>
---
drivers/ata/pata_rb532_cf.c +3 -2
index ebfcda2..fe8178c 100644
@@ -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;
 
Quoted 3 lines
drivers/ata/pata_rb532_cf.c static __devinit int · line 184
Quoted 1 line
drivers/ata/pata_rb532_cf.c +0 -0
 	gpio = irq_to_gpio(irq);
ACK from Phil or Florian?

Quoted 7 lines

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

Signed-off-by Phil Sutter <[email protected]>

Signed-off-by Florian Fainelli <[email protected]>

drivers/ata/pata_rb532_cf.c +1 -1

index ebfcda2..da05518 100644
@@ -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;

Phil Sutter

2009-03-14 00:51 UTC #4 lore raw

Hi,

Quoted 1 line · Florian Fainelli

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

Jeff Garzik

Applied

2009-03-25 02:17 UTC #5 lore raw

In reply to Florian Fainelli

Quoted 61 lines · Florian Fainelli

Hi,
Le Friday 06 March 2009 17:43:18 Phil Sutter, vous avez écrit :
Quoted 31 lines
Hi,
Quoted 26 lines · Jeff Garzik
Quoted 25 lines · Roel Kluin
------------------------------>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 <[email protected]>
---
drivers/ata/pata_rb532_cf.c +3 -2
index ebfcda2..fe8178c 100644
@@ -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;
 
Quoted 2 lines
drivers/ata/pata_rb532_cf.c static __devinit int · line 184
Quoted 1 line
drivers/ata/pata_rb532_cf.c +0 -0
 	gpio = irq_to_gpio(irq);
ACK from Phil or Florian?
Quoted 7 lines
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 <[email protected]>
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 <[email protected]>
Signed-off-by Phil Sutter <[email protected]>
Signed-off-by Florian Fainelli <[email protected]>
--
drivers/ata/pata_rb532_cf.c +0 -0
index ebfcda2..da05518 100644

drivers/ata/pata_rb532_cf.c static void rb532_pata_setup_ports(struct ata_host *ah) · line 166

Quoted 8 lines

drivers/ata/pata_rb532_cf.c +1 -1

@@ -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;

applied