* [PATCH] gianfar: Fix tests of unsigneds in gfar_parse_group()
@ 2009-12-15 18:42 Roel Kluin
2009-12-16 5:02 ` Kumar Gopalpet-B05799
2009-12-16 5:04 ` Kumar Gopalpet-B05799
0 siblings, 2 replies; 4+ messages in thread
From: Roel Kluin @ 2009-12-15 18:42 UTC (permalink / raw)
To: netdev, David Miller, Andrew Morton, LKML
interruptTransmit, -Receive and -Error are unsigned so the tests did not work.
This also changes that if irq_of_parse_and_map(np, 0) returns NO_IRQ, that
gfar_parse_group() returns an -EINVAL.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
Found using coccinelle: http://coccinelle.lip6.fr/
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 6850dc0..c411682 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -522,6 +522,7 @@ static int gfar_parse_group(struct device_node *np,
{
u32 *queue_mask;
u64 addr, size;
+ int irq;
addr = of_translate_address(np,
of_get_address(np, 0, &size, NULL));
@@ -530,20 +531,22 @@ static int gfar_parse_group(struct device_node *np,
if (!priv->gfargrp[priv->num_grps].regs)
return -ENOMEM;
- priv->gfargrp[priv->num_grps].interruptTransmit =
- irq_of_parse_and_map(np, 0);
+ irq = irq_of_parse_and_map(np, 0);
+ if (irq < 0)
+ return -EINVAL;
+ priv->gfargrp[priv->num_grps].interruptTransmit = irq;
/* If we aren't the FEC we have multiple interrupts */
if (model && strcasecmp(model, "FEC")) {
- priv->gfargrp[priv->num_grps].interruptReceive =
- irq_of_parse_and_map(np, 1);
- priv->gfargrp[priv->num_grps].interruptError =
- irq_of_parse_and_map(np,2);
- if (priv->gfargrp[priv->num_grps].interruptTransmit < 0 ||
- priv->gfargrp[priv->num_grps].interruptReceive < 0 ||
- priv->gfargrp[priv->num_grps].interruptError < 0) {
+ irq = irq_of_parse_and_map(np, 1);
+ if (irq < 0)
return -EINVAL;
- }
+ priv->gfargrp[priv->num_grps].interruptReceive = irq;
+
+ irq = irq_of_parse_and_map(np, 2);
+ if (irq < 0)
+ return -EINVAL;
+ priv->gfargrp[priv->num_grps].interruptError = irq;
}
priv->gfargrp[priv->num_grps].grp_id = priv->num_grps;
^ permalink raw reply related [flat|nested] 4+ messages in thread* RE: [PATCH] gianfar: Fix tests of unsigneds in gfar_parse_group()
2009-12-15 18:42 [PATCH] gianfar: Fix tests of unsigneds in gfar_parse_group() Roel Kluin
@ 2009-12-16 5:02 ` Kumar Gopalpet-B05799
2009-12-16 5:04 ` Kumar Gopalpet-B05799
1 sibling, 0 replies; 4+ messages in thread
From: Kumar Gopalpet-B05799 @ 2009-12-16 5:02 UTC (permalink / raw)
To: Roel Kluin, netdev, David Miller, Andrew Morton, LKML
>interruptTransmit, -Receive and -Error are unsigned so the
>tests did not work.
>
>This also changes that if irq_of_parse_and_map(np, 0) returns
>NO_IRQ, that
>gfar_parse_group() returns an -EINVAL.
>
>Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>---
>Found using coccinelle: http://coccinelle.lip6.fr/
>
>diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
>index 6850dc0..c411682 100644
>--- a/drivers/net/gianfar.c
>+++ b/drivers/net/gianfar.c
>@@ -522,6 +522,7 @@ static int gfar_parse_group(struct
>device_node *np, {
> u32 *queue_mask;
> u64 addr, size;
>+ int irq;
>
> addr = of_translate_address(np,
> of_get_address(np, 0, &size, NULL)); @@
>-530,20 +531,22 @@ static int gfar_parse_group(struct device_node *np,
> if (!priv->gfargrp[priv->num_grps].regs)
> return -ENOMEM;
>
>- priv->gfargrp[priv->num_grps].interruptTransmit =
>- irq_of_parse_and_map(np, 0);
>+ irq = irq_of_parse_and_map(np, 0);
>+ if (irq < 0)
>+ return -EINVAL;
>+ priv->gfargrp[priv->num_grps].interruptTransmit = irq;
>
> /* If we aren't the FEC we have multiple interrupts */
> if (model && strcasecmp(model, "FEC")) {
>- priv->gfargrp[priv->num_grps].interruptReceive =
>- irq_of_parse_and_map(np, 1);
>- priv->gfargrp[priv->num_grps].interruptError =
>- irq_of_parse_and_map(np,2);
>- if
>(priv->gfargrp[priv->num_grps].interruptTransmit < 0 ||
>-
>priv->gfargrp[priv->num_grps].interruptReceive < 0 ||
>-
>priv->gfargrp[priv->num_grps].interruptError < 0) {
>+ irq = irq_of_parse_and_map(np, 1);
>+ if (irq < 0)
> return -EINVAL;
>- }
>+ priv->gfargrp[priv->num_grps].interruptReceive = irq;
>+
>+ irq = irq_of_parse_and_map(np, 2);
>+ if (irq < 0)
>+ return -EINVAL;
>+ priv->gfargrp[priv->num_grps].interruptError = irq;
> }
>
> priv->gfargrp[priv->num_grps].grp_id = priv->num_grps;
Won't it be better to change types of interruptTransmit, -Receive and
-Error
in struct gfar_priv_grp itself. Something like this:
struct gfar_priv_grp {
....
- unsigned int interruptTransmit;
- unsigned int interruptReceive;
- unsigned int interruptError;
+ unsigned int interruptTransmit;
+ unsigned int interruptReceive;
+ unsigned int interruptError;
....
}
--
Thanks
Sandeep
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH] gianfar: Fix tests of unsigneds in gfar_parse_group()
2009-12-15 18:42 [PATCH] gianfar: Fix tests of unsigneds in gfar_parse_group() Roel Kluin
2009-12-16 5:02 ` Kumar Gopalpet-B05799
@ 2009-12-16 5:04 ` Kumar Gopalpet-B05799
2009-12-16 15:48 ` roel kluin
1 sibling, 1 reply; 4+ messages in thread
From: Kumar Gopalpet-B05799 @ 2009-12-16 5:04 UTC (permalink / raw)
To: Roel Kluin, netdev, David Miller, Andrew Morton, LKML
>Won't it be better to change types of interruptTransmit,
>-Receive and -Error in struct gfar_priv_grp itself. Something
>like this:
>
>struct gfar_priv_grp {
>....
>- unsigned int interruptTransmit;
>- unsigned int interruptReceive;
>- unsigned int interruptError;
>
>+ unsigned int interruptTransmit;
>+ unsigned int interruptReceive;
>+ unsigned int interruptError;
>....
>}
>
Sorry for my earlier mail, What I meant is something like this:
struct gfar_priv_grp {
....
- unsigned int interruptTransmit;
- unsigned int interruptReceive;
- unsigned int interruptError;
+ int interruptTransmit;
+ int interruptReceive;
+ int interruptError;
....
}
--
Thanks
Sandeep
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] gianfar: Fix tests of unsigneds in gfar_parse_group()
2009-12-16 5:04 ` Kumar Gopalpet-B05799
@ 2009-12-16 15:48 ` roel kluin
0 siblings, 0 replies; 4+ messages in thread
From: roel kluin @ 2009-12-16 15:48 UTC (permalink / raw)
To: Kumar Gopalpet-B05799; +Cc: netdev, David Miller, Andrew Morton, LKML
On Wed, Dec 16, 2009 at 6:04 AM, Kumar Gopalpet-B05799
<B05799@freescale.com> wrote:
>
>>Won't it be better to change types of interruptTransmit,
>>-Receive and -Error in struct gfar_priv_grp itself. Something
>>like this:
> struct gfar_priv_grp {
> ....
> - unsigned int interruptTransmit;
> - unsigned int interruptReceive;
> - unsigned int interruptError;
>
> + int interruptTransmit;
> + int interruptReceive;
> + int interruptError;
> ....
> }
>
> --
>
> Thanks
> Sandeep
I don't think that's better. Only the error returned is signed,
If none occurred the irq thereafter is unsigned.
Thanks,
Roel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-12-16 15:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-15 18:42 [PATCH] gianfar: Fix tests of unsigneds in gfar_parse_group() Roel Kluin
2009-12-16 5:02 ` Kumar Gopalpet-B05799
2009-12-16 5:04 ` Kumar Gopalpet-B05799
2009-12-16 15:48 ` roel kluin
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).