2012-11-13 21:11:44

by Simon Marchi

Subject: [PATCH] tilegx: request_irq with a non-null device name

This patch simply makes the tilegx net driver call request_irq with a
non-null name. It makes the output in /proc/interrupts more obvious, but
also helps tools that don't expect to find null there.

Signed-off-by: Simon Marchi <[email protected]>
---
drivers/net/ethernet/tile/tilegx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
index 4e98100..66e025a 100644
--- a/drivers/net/ethernet/tile/tilegx.c
+++ b/drivers/net/ethernet/tile/tilegx.c
@@ -917,7 +917,7 @@ static int tile_net_setup_interrupts(struct net_device *dev)
ingress_irq = rc;
tile_irq_activate(ingress_irq, TILE_IRQ_PERCPU);
rc = request_irq(ingress_irq, tile_net_handle_ingress_irq,
- 0, NULL, NULL);
+ 0, "tile_net", NULL);
if (rc != 0) {
netdev_err(dev, "request_irq failed: %d\n", rc);
destroy_irq(ingress_irq);
--
1.7.1

2012-11-13 21:37:16

by Chris Metcalf

Subject: Re: [PATCH] tilegx: request_irq with a non-null device name

On 11/13/2012 3:58 PM, Simon Marchi wrote:
> This patch simply makes the tilegx net driver call request_irq with a
> non-null name. It makes the output in /proc/interrupts more obvious, but
> also helps tools that don't expect to find null there.
>
> Signed-off-by: Simon Marchi <[email protected]>
> ---
> drivers/net/ethernet/tile/tilegx.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
> index 4e98100..66e025a 100644
> --- a/drivers/net/ethernet/tile/tilegx.c
> +++ b/drivers/net/ethernet/tile/tilegx.c
> @@ -917,7 +917,7 @@ static int tile_net_setup_interrupts(struct net_device *dev)
> ingress_irq = rc;
> tile_irq_activate(ingress_irq, TILE_IRQ_PERCPU);
> rc = request_irq(ingress_irq, tile_net_handle_ingress_irq,
> - 0, NULL, NULL);
> + 0, "tile_net", NULL);

Good catch. If you can change it to dev->name instead of "tile_net", feel
free to add my:

Acked-by: Chris Metcalf <[email protected]>

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com

2012-11-13 21:51:21

by Stephen Hemminger

Subject: Re: [PATCH] tilegx: request_irq with a non-null device name

On Tue, 13 Nov 2012 16:37:11 -0500
Chris Metcalf <[email protected]> wrote:

> On 11/13/2012 3:58 PM, Simon Marchi wrote:
> > This patch simply makes the tilegx net driver call request_irq with a
> > non-null name. It makes the output in /proc/interrupts more obvious, but
> > also helps tools that don't expect to find null there.
> >
> > Signed-off-by: Simon Marchi <[email protected]>
> > ---
> > drivers/net/ethernet/tile/tilegx.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
> > index 4e98100..66e025a 100644
> > --- a/drivers/net/ethernet/tile/tilegx.c
> > +++ b/drivers/net/ethernet/tile/tilegx.c
> > @@ -917,7 +917,7 @@ static int tile_net_setup_interrupts(struct net_device *dev)
> > ingress_irq = rc;
> > tile_irq_activate(ingress_irq, TILE_IRQ_PERCPU);
> > rc = request_irq(ingress_irq, tile_net_handle_ingress_irq,
> > - 0, NULL, NULL);
> > + 0, "tile_net", NULL);
>
> Good catch. If you can change it to dev->name instead of "tile_net", feel
> free to add my:
>
> Acked-by: Chris Metcalf <[email protected]>
>

It really should be using dev->name since that helps out some
applications like irqbalance that read /proc/interrupts

2012-11-13 21:54:38

by Simon Marchi

Subject: Re: [PATCH] tilegx: request_irq with a non-null device name

On Tue, Nov 13, 2012 at 1:37 PM, Chris Metcalf <[email protected]> wrote:
> On 11/13/2012 3:58 PM, Simon Marchi wrote:
>> This patch simply makes the tilegx net driver call request_irq with a
>> non-null name. It makes the output in /proc/interrupts more obvious, but
>> also helps tools that don't expect to find null there.
>>
>> Signed-off-by: Simon Marchi <[email protected]>
>> ---
>> drivers/net/ethernet/tile/tilegx.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
>> index 4e98100..66e025a 100644
>> --- a/drivers/net/ethernet/tile/tilegx.c
>> +++ b/drivers/net/ethernet/tile/tilegx.c
>> @@ -917,7 +917,7 @@ static int tile_net_setup_interrupts(struct net_device *dev)
>> ingress_irq = rc;
>> tile_irq_activate(ingress_irq, TILE_IRQ_PERCPU);
>> rc = request_irq(ingress_irq, tile_net_handle_ingress_irq,
>> - 0, NULL, NULL);
>> + 0, "tile_net", NULL);
>
> Good catch. If you can change it to dev->name instead of "tile_net", feel
> free to add my:

I thought about that first, but it wouldn't be very logical. This
module is loaded once and for all when the first network device is
brought up, so this request_irq is for all the tile network
interfaces. Suppose you "up" gbe0, "up" gbe1 and "down" gbe0, the
device name would still be gbe0, even though it's down. That's why I
opted for a generic name instead.

If you still want to go with dev->name I have no problem with that.

> Acked-by: Chris Metcalf <[email protected]>
>
> --
> Chris Metcalf, Tilera Corp.
> http://www.tilera.com
>

2012-11-13 21:59:45

by Chris Metcalf

Subject: Re: [PATCH] tilegx: request_irq with a non-null device name

On 11/13/2012 4:54 PM, Simon Marchi wrote:
> On Tue, Nov 13, 2012 at 1:37 PM, Chris Metcalf <[email protected]> wrote:
>> On 11/13/2012 3:58 PM, Simon Marchi wrote:
>>> This patch simply makes the tilegx net driver call request_irq with a
>>> non-null name. It makes the output in /proc/interrupts more obvious, but
>>> also helps tools that don't expect to find null there.
>>>
>>> Signed-off-by: Simon Marchi <[email protected]>
>>> ---
>>> drivers/net/ethernet/tile/tilegx.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
>>> index 4e98100..66e025a 100644
>>> --- a/drivers/net/ethernet/tile/tilegx.c
>>> +++ b/drivers/net/ethernet/tile/tilegx.c
>>> @@ -917,7 +917,7 @@ static int tile_net_setup_interrupts(struct net_device *dev)
>>> ingress_irq = rc;
>>> tile_irq_activate(ingress_irq, TILE_IRQ_PERCPU);
>>> rc = request_irq(ingress_irq, tile_net_handle_ingress_irq,
>>> - 0, NULL, NULL);
>>> + 0, "tile_net", NULL);
>> Good catch. If you can change it to dev->name instead of "tile_net", feel
>> free to add my:
> I thought about that first, but it wouldn't be very logical. This
> module is loaded once and for all when the first network device is
> brought up, so this request_irq is for all the tile network
> interfaces. Suppose you "up" gbe0, "up" gbe1 and "down" gbe0, the
> device name would still be gbe0, even though it's down. That's why I
> opted for a generic name instead.
>
> If you still want to go with dev->name I have no problem with that.

No, you're right, that's a good point. It does probably make more sense to
use "tile_net". So apply my Acked-by to your patch as-is. Thanks!

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com