Patchwork tilegx: request_irq with a non-null device name

login
register
mail settings
Submitter Simon Marchi
Date Nov. 13, 2012, 8:58 p.m.
Message ID <1352840300-27814-1-git-send-email-simon.marchi@polymtl.ca>
Download mbox | patch
Permalink /patch/198786/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Simon Marchi - Nov. 13, 2012, 8:58 p.m.
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 <simon.marchi@polymtl.ca>
---
 drivers/net/ethernet/tile/tilegx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Chris Metcalf - Nov. 13, 2012, 9:37 p.m.
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 <simon.marchi@polymtl.ca>
> ---
>  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 <cmetcalf@tilera.com>
stephen hemminger - Nov. 13, 2012, 9:50 p.m.
On Tue, 13 Nov 2012 16:37:11 -0500
Chris Metcalf <cmetcalf@tilera.com> 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 <simon.marchi@polymtl.ca>
> > ---
> >  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 <cmetcalf@tilera.com>
> 

It really should be using dev->name since that helps out some
applications like irqbalance that read /proc/interrupts
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Marchi - Nov. 13, 2012, 9:54 p.m.
On Tue, Nov 13, 2012 at 1:37 PM, Chris Metcalf <cmetcalf@tilera.com> 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 <simon.marchi@polymtl.ca>
>> ---
>>  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 <cmetcalf@tilera.com>
>
> --
> Chris Metcalf, Tilera Corp.
> http://www.tilera.com
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Metcalf - Nov. 13, 2012, 9:59 p.m.
On 11/13/2012 4:54 PM, Simon Marchi wrote:
> On Tue, Nov 13, 2012 at 1:37 PM, Chris Metcalf <cmetcalf@tilera.com> 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 <simon.marchi@polymtl.ca>
>>> ---
>>>  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!

Patch

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