linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] pata-rb532-cf: platform_get_irq() failure ignored
@ 2009-03-03 13:04 Roel Kluin
  2009-03-05 12:23 ` Jeff Garzik
  0 siblings, 1 reply; 6+ messages in thread
From: Roel Kluin @ 2009-03-03 13:04 UTC (permalink / raw)
  To: jgarzik; +Cc: linux-ide, Andrew Morton

------------------------------>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);
 	if (gpio < 0) {

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

* Re: [PATCH] pata-rb532-cf: platform_get_irq() failure ignored
  2009-03-03 13:04 [PATCH] pata-rb532-cf: platform_get_irq() failure ignored Roel Kluin
@ 2009-03-05 12:23 ` Jeff Garzik
  2009-03-06 16:43   ` Phil Sutter
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff Garzik @ 2009-03-05 12:23 UTC (permalink / raw)
  To: Roel Kluin
  Cc: jgarzik, linux-ide, Andrew Morton, Phil Sutter, Florian Fainelli

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?


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

* Re: [PATCH] pata-rb532-cf: platform_get_irq() failure ignored
  2009-03-05 12:23 ` Jeff Garzik
@ 2009-03-06 16:43   ` Phil Sutter
  2009-03-13 14:41     ` Florian Fainelli
  0 siblings, 1 reply; 6+ messages in thread
From: Phil Sutter @ 2009-03-06 16:43 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Roel Kluin, jgarzik, linux-ide, Andrew Morton, Florian Fainelli

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?

Greetings, Phil

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

* Re: [PATCH] pata-rb532-cf: platform_get_irq() failure ignored
  2009-03-06 16:43   ` Phil Sutter
@ 2009-03-13 14:41     ` Florian Fainelli
  2009-03-14  0:51       ` Phil Sutter
  2009-03-25  2:16       ` Jeff Garzik
  0 siblings, 2 replies; 6+ messages in thread
From: Florian Fainelli @ 2009-03-13 14:41 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Jeff Garzik, Roel Kluin, jgarzik, linux-ide, Andrew Morton

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;

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

* Re: [PATCH] pata-rb532-cf: platform_get_irq() failure ignored
  2009-03-13 14:41     ` Florian Fainelli
@ 2009-03-14  0:51       ` Phil Sutter
  2009-03-25  2:16       ` Jeff Garzik
  1 sibling, 0 replies; 6+ messages in thread
From: Phil Sutter @ 2009-03-14  0:51 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Jeff Garzik, Roel Kluin, jgarzik, linux-ide, Andrew Morton

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

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

* Re: [PATCH] pata-rb532-cf: platform_get_irq() failure ignored
  2009-03-13 14:41     ` Florian Fainelli
  2009-03-14  0:51       ` Phil Sutter
@ 2009-03-25  2:16       ` Jeff Garzik
  1 sibling, 0 replies; 6+ messages in thread
From: Jeff Garzik @ 2009-03-25  2:16 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Phil Sutter, Roel Kluin, jgarzik, linux-ide, Andrew Morton

Florian Fainelli wrote:
> 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;

applied



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

end of thread, other threads:[~2009-03-25  2:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-03 13:04 [PATCH] pata-rb532-cf: platform_get_irq() failure ignored Roel Kluin
2009-03-05 12:23 ` Jeff Garzik
2009-03-06 16:43   ` Phil Sutter
2009-03-13 14:41     ` Florian Fainelli
2009-03-14  0:51       ` Phil Sutter
2009-03-25  2:16       ` Jeff Garzik

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).