* [PATCH] gianfar: irq_of_parse_and_map() error unnoticed
@ 2009-04-23 12:52 Roel Kluin
2009-04-27 10:11 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Roel Kluin @ 2009-04-23 12:52 UTC (permalink / raw)
To: David S. Miller, netdev
Not sure which irq_of_parse_and_map() is used, but I found definitions here:
vi arch/microblaze/kernel/irq.c +23
vi arch/powerpc/kernel/irq.c +727
vi arch/sparc/kernel/of_device_32.c +33
vi arch/sparc/kernel/of_device_64.c +59
They either return 0 or NO_IRQ - either defined 0, -1, 255 or INT_MAX.
------------------------------>8-------------8<---------------------------------
priv->interruptTransmit, -Receive and -Error are unsigned, so the error path
wasn't taken when irq_of_parse_and_map() returned an incorrect irq.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index b2c4967..e30d158 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -203,9 +203,12 @@ static int gfar_of_init(struct net_device *dev)
priv->interruptError = irq_of_parse_and_map(np, 2);
- if (priv->interruptTransmit < 0 ||
- priv->interruptReceive < 0 ||
- priv->interruptError < 0) {
+ if (priv->interruptTransmit == 0 ||
+ priv->interruptTransmit > 254 ||
+ priv->interruptReceive == 0 ||
+ priv->interruptReceive > 254 ||
+ priv->interruptError == 0 ||
+ priv->interruptReceive > 254) {
err = -EINVAL;
goto err_out;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] gianfar: irq_of_parse_and_map() error unnoticed
2009-04-23 12:52 [PATCH] gianfar: irq_of_parse_and_map() error unnoticed Roel Kluin
@ 2009-04-27 10:11 ` David Miller
2009-04-27 11:05 ` [PATCH 2/2] " Roel Kluin
0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2009-04-27 10:11 UTC (permalink / raw)
To: roel.kluin; +Cc: netdev
From: Roel Kluin <roel.kluin@gmail.com>
Date: Thu, 23 Apr 2009 14:52:44 +0200
> Not sure which irq_of_parse_and_map() is used, but I found definitions here:
>
> vi arch/microblaze/kernel/irq.c +23
> vi arch/powerpc/kernel/irq.c +727
> vi arch/sparc/kernel/of_device_32.c +33
> vi arch/sparc/kernel/of_device_64.c +59
>
> They either return 0 or NO_IRQ - either defined 0, -1, 255 or INT_MAX.
> ------------------------------>8-------------8<---------------------------------
> priv->interruptTransmit, -Receive and -Error are unsigned, so the error path
> wasn't taken when irq_of_parse_and_map() returned an incorrect irq.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
This test is way too convoluted for my taste.
I would rather that the implementations of this interface
use a consistent value for errors. Preferrably NO_IRQ.
Not applied.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] gianfar: irq_of_parse_and_map() error unnoticed
2009-04-27 10:11 ` David Miller
@ 2009-04-27 11:05 ` Roel Kluin
2009-04-27 19:23 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Roel Kluin @ 2009-04-27 11:05 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Andrew Morton
priv->interruptTransmit, -Receive and -Error are unsigned, so the error path
wasn't taken when irq_of_parse_and_map() returned an incorrect irq.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index b2c4967..55c2ce8 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -203,9 +203,9 @@ static int gfar_of_init(struct net_device *dev)
priv->interruptError = irq_of_parse_and_map(np, 2);
- if (priv->interruptTransmit < 0 ||
- priv->interruptReceive < 0 ||
- priv->interruptError < 0) {
+ if (priv->interruptTransmit == NO_IRQ ||
+ priv->interruptReceive == NO_IRQ ||
+ priv->interruptError == NO_IRQ) {
err = -EINVAL;
goto err_out;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] gianfar: irq_of_parse_and_map() error unnoticed
2009-04-27 11:05 ` [PATCH 2/2] " Roel Kluin
@ 2009-04-27 19:23 ` Andrew Morton
0 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2009-04-27 19:23 UTC (permalink / raw)
To: Roel Kluin; +Cc: davem, netdev
On Mon, 27 Apr 2009 13:05:30 +0200
Roel Kluin <roel.kluin@gmail.com> wrote:
> priv->interruptTransmit, -Receive and -Error are unsigned, so the error path
> wasn't taken when irq_of_parse_and_map() returned an incorrect irq.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index b2c4967..55c2ce8 100644
> --- a/drivers/net/gianfar.c
> +++ b/drivers/net/gianfar.c
> @@ -203,9 +203,9 @@ static int gfar_of_init(struct net_device *dev)
>
> priv->interruptError = irq_of_parse_and_map(np, 2);
>
> - if (priv->interruptTransmit < 0 ||
> - priv->interruptReceive < 0 ||
> - priv->interruptError < 0) {
> + if (priv->interruptTransmit == NO_IRQ ||
> + priv->interruptReceive == NO_IRQ ||
> + priv->interruptError == NO_IRQ) {
> err = -EINVAL;
> goto err_out;
> }
I guess that's OK, as gianfar.c appears to be a ppc-only thing, and ppc
does implement NO_IRQ.
However what a lot (all) rtc drivers do is
if (irq <= 0)
which (afaik) will dtrt in all cases: NO_IRQ==0, NO_IRQ<0,
!defined(NO_IRQ).
Then again, it will dtwt if the platform allows irq numbers of zero.
argh, how did we get into this pickle?
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-04-27 19:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-23 12:52 [PATCH] gianfar: irq_of_parse_and_map() error unnoticed Roel Kluin
2009-04-27 10:11 ` David Miller
2009-04-27 11:05 ` [PATCH 2/2] " Roel Kluin
2009-04-27 19:23 ` Andrew Morton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.