linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used
@ 2010-05-26 17:29 John Linn
  2010-05-27  3:44 ` David Miller
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: John Linn @ 2010-05-26 17:29 UTC (permalink / raw)
  To: netdev, linuxppc-dev, grant.likely, jwboyer
  Cc: michal.simek, John Linn, john.williams, Brian Hill

The code is not checking the interrupt for DMA correctly so that an
interrupt number of 0 will cause a false error.

Signed-off-by: Brian Hill <brian.hill@xilinx.com>
Signed-off-by: John Linn <john.linn@xilinx.com>
---
 drivers/net/ll_temac_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index fa7620e..0615737 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -950,7 +950,7 @@ temac_of_probe(struct of_device *op, const struct of_device_id *match)
 
 	lp->rx_irq = irq_of_parse_and_map(np, 0);
 	lp->tx_irq = irq_of_parse_and_map(np, 1);
-	if (!lp->rx_irq || !lp->tx_irq) {
+	if ((lp->rx_irq == NO_IRQ) || (lp->tx_irq == NO_IRQ)) {
 		dev_err(&op->dev, "could not determine irqs\n");
 		rc = -ENOMEM;
 		goto nodev;
-- 
1.6.2.1



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

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

* Re: [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used
  2010-05-26 17:29 [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used John Linn
@ 2010-05-27  3:44 ` David Miller
  2010-05-27  4:12 ` John Williams
  2010-06-06 21:57 ` Benjamin Herrenschmidt
  2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-05-27  3:44 UTC (permalink / raw)
  To: john.linn; +Cc: linuxppc-dev, netdev, brian.hill, michal.simek, john.williams

From: John Linn <john.linn@xilinx.com>
Date: Wed, 26 May 2010 11:29:18 -0600

> The code is not checking the interrupt for DMA correctly so that an
> interrupt number of 0 will cause a false error.
> 
> Signed-off-by: Brian Hill <brian.hill@xilinx.com>
> Signed-off-by: John Linn <john.linn@xilinx.com>

Applied.

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

* Re: [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used
  2010-05-26 17:29 [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used John Linn
  2010-05-27  3:44 ` David Miller
@ 2010-05-27  4:12 ` John Williams
  2010-06-06  3:33   ` Grant Likely
  2010-06-06 21:57 ` Benjamin Herrenschmidt
  2 siblings, 1 reply; 6+ messages in thread
From: John Williams @ 2010-05-27  4:12 UTC (permalink / raw)
  To: John Linn; +Cc: linuxppc-dev, netdev, Brian Hill, michal.simek

On Thu, May 27, 2010 at 3:29 AM, John Linn <john.linn@xilinx.com> wrote:
> The code is not checking the interrupt for DMA correctly so that an
> interrupt number of 0 will cause a false error.
>
> Signed-off-by: Brian Hill <brian.hill@xilinx.com>
> Signed-off-by: John Linn <john.linn@xilinx.com>
> ---
> =A0drivers/net/ll_temac_main.c | =A0 =A02 +-
> =A01 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
> index fa7620e..0615737 100644
> --- a/drivers/net/ll_temac_main.c
> +++ b/drivers/net/ll_temac_main.c
> @@ -950,7 +950,7 @@ temac_of_probe(struct of_device *op, const struct of_=
device_id *match)
>
> =A0 =A0 =A0 =A0lp->rx_irq =3D irq_of_parse_and_map(np, 0);
> =A0 =A0 =A0 =A0lp->tx_irq =3D irq_of_parse_and_map(np, 1);
> - =A0 =A0 =A0 if (!lp->rx_irq || !lp->tx_irq) {
> + =A0 =A0 =A0 if ((lp->rx_irq =3D=3D NO_IRQ) || (lp->tx_irq =3D=3D NO_IRQ=
)) {

Personally I think this is the right thing to do.  But, I thought the
IRQ 0 =3D=3D NO_IRQ (AKA "all-the-world's-an-x86-and-if-not-it-should-be")
holy war was already fought and won (or lost, depending on your
perspective)?

I seem to recall giving reluctant assent to a patch from Grant a few
months ago that touched MicroBlaze thus?

John

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

* Re: [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used
  2010-05-27  4:12 ` John Williams
@ 2010-06-06  3:33   ` Grant Likely
  0 siblings, 0 replies; 6+ messages in thread
From: Grant Likely @ 2010-06-06  3:33 UTC (permalink / raw)
  To: John Williams; +Cc: linuxppc-dev, netdev, Brian Hill, michal.simek, John Linn

On Wed, May 26, 2010 at 10:12 PM, John Williams
<john.williams@petalogix.com> wrote:
> On Thu, May 27, 2010 at 3:29 AM, John Linn <john.linn@xilinx.com> wrote:
>> The code is not checking the interrupt for DMA correctly so that an
>> interrupt number of 0 will cause a false error.
>>
>> Signed-off-by: Brian Hill <brian.hill@xilinx.com>
>> Signed-off-by: John Linn <john.linn@xilinx.com>
>> ---
>> =A0drivers/net/ll_temac_main.c | =A0 =A02 +-
>> =A01 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
>> index fa7620e..0615737 100644
>> --- a/drivers/net/ll_temac_main.c
>> +++ b/drivers/net/ll_temac_main.c
>> @@ -950,7 +950,7 @@ temac_of_probe(struct of_device *op, const struct of=
_device_id *match)
>>
>> =A0 =A0 =A0 =A0lp->rx_irq =3D irq_of_parse_and_map(np, 0);
>> =A0 =A0 =A0 =A0lp->tx_irq =3D irq_of_parse_and_map(np, 1);
>> - =A0 =A0 =A0 if (!lp->rx_irq || !lp->tx_irq) {
>> + =A0 =A0 =A0 if ((lp->rx_irq =3D=3D NO_IRQ) || (lp->tx_irq =3D=3D NO_IR=
Q)) {
>
> Personally I think this is the right thing to do. =A0But, I thought the
> IRQ 0 =3D=3D NO_IRQ (AKA "all-the-world's-an-x86-and-if-not-it-should-be"=
)
> holy war was already fought and won (or lost, depending on your
> perspective)?
>
> I seem to recall giving reluctant assent to a patch from Grant a few
> months ago that touched MicroBlaze thus?

I've still got the patch in my private queue.  I can reapply it, test
it and repost it.  I think what was still a bit up in the air was the
exact method to map hw irq numbers onto linux irqs.

g.

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

* Re: [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used
  2010-05-26 17:29 [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used John Linn
  2010-05-27  3:44 ` David Miller
  2010-05-27  4:12 ` John Williams
@ 2010-06-06 21:57 ` Benjamin Herrenschmidt
  2010-06-07  0:47   ` Grant Likely
  2 siblings, 1 reply; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2010-06-06 21:57 UTC (permalink / raw)
  To: John Linn; +Cc: linuxppc-dev, netdev, Brian Hill, michal.simek, john.williams

On Wed, 2010-05-26 at 11:29 -0600, John Linn wrote:
> The code is not checking the interrupt for DMA correctly so that an
> interrupt number of 0 will cause a false error.
> 
> Signed-off-by: Brian Hill <brian.hill@xilinx.com>
> Signed-off-by: John Linn <john.linn@xilinx.com>
> ---
>  drivers/net/ll_temac_main.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
> index fa7620e..0615737 100644
> --- a/drivers/net/ll_temac_main.c
> +++ b/drivers/net/ll_temac_main.c
> @@ -950,7 +950,7 @@ temac_of_probe(struct of_device *op, const struct of_device_id *match)
>  
>  	lp->rx_irq = irq_of_parse_and_map(np, 0);
>  	lp->tx_irq = irq_of_parse_and_map(np, 1);
> -	if (!lp->rx_irq || !lp->tx_irq) {
> +	if ((lp->rx_irq == NO_IRQ) || (lp->tx_irq == NO_IRQ)) {
>  		dev_err(&op->dev, "could not determine irqs\n");
>  		rc = -ENOMEM;
>  		goto nodev;

Hasn't NO_IRQ been deprecated ?

Linus made it clear a while back that interrupt 0 was not valid and
that's the way it should be.

We now have an interrupt remapping scheme on powerpc, so we ensure that
0 always mean no interrupt. Other archs might need some fixups. Which
are specifically needs this patch ?

Cheers,
Ben.

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

* Re: [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used
  2010-06-06 21:57 ` Benjamin Herrenschmidt
@ 2010-06-07  0:47   ` Grant Likely
  0 siblings, 0 replies; 6+ messages in thread
From: Grant Likely @ 2010-06-07  0:47 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev, netdev, Brian Hill, michal.simek, John Linn,
	john.williams

On Sun, Jun 6, 2010 at 3:57 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2010-05-26 at 11:29 -0600, John Linn wrote:
>> The code is not checking the interrupt for DMA correctly so that an
>> interrupt number of 0 will cause a false error.
>>
>> Signed-off-by: Brian Hill <brian.hill@xilinx.com>
>> Signed-off-by: John Linn <john.linn@xilinx.com>
>> ---
>> =A0drivers/net/ll_temac_main.c | =A0 =A02 +-
>> =A01 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
>> index fa7620e..0615737 100644
>> --- a/drivers/net/ll_temac_main.c
>> +++ b/drivers/net/ll_temac_main.c
>> @@ -950,7 +950,7 @@ temac_of_probe(struct of_device *op, const struct of=
_device_id *match)
>>
>> =A0 =A0 =A0 lp->rx_irq =3D irq_of_parse_and_map(np, 0);
>> =A0 =A0 =A0 lp->tx_irq =3D irq_of_parse_and_map(np, 1);
>> - =A0 =A0 if (!lp->rx_irq || !lp->tx_irq) {
>> + =A0 =A0 if ((lp->rx_irq =3D=3D NO_IRQ) || (lp->tx_irq =3D=3D NO_IRQ)) =
{
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&op->dev, "could not determine irqs\=
n");
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 rc =3D -ENOMEM;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto nodev;
>
> Hasn't NO_IRQ been deprecated ?

ARM still uses it, plus less than a handful of other arches.  There is
no reason for Microblaze to use NO_IRQ as -1.

In fact, on ARM, as part of the device tree work I'm hoping to piggy
back in irq remapping and make 0 no longer a valid irq.

> Linus made it clear a while back that interrupt 0 was not valid and
> that's the way it should be.
>
> We now have an interrupt remapping scheme on powerpc, so we ensure that
> 0 always mean no interrupt. Other archs might need some fixups. Which
> are specifically needs this patch ?

Microblaze.

BTW jlinn, for xilinx ARM support we should make sure 0 is never used
as a valid IRQ from day one.  It will result it far less pain in the
future.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

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

end of thread, other threads:[~2010-06-07  0:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-26 17:29 [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used John Linn
2010-05-27  3:44 ` David Miller
2010-05-27  4:12 ` John Williams
2010-06-06  3:33   ` Grant Likely
2010-06-06 21:57 ` Benjamin Herrenschmidt
2010-06-07  0:47   ` Grant Likely

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