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

[Florian Fainelli <florian@openwrt.org>]

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:
> > Roel Kluin wrote:
> > >------------------------------>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 <roel.kluin@gmail.com>
> > >---
> > >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 <roel.kluin@gmail.com>
Signed-off-by: Phil Sutter <n0-1@freewrt.org>
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;