* [patch] net: make ndev->irq signed for error handling
@ 2013-11-07 7:48 Dan Carpenter
2013-11-07 12:14 ` Mugunthan V N
2013-11-08 0:13 ` David Miller
0 siblings, 2 replies; 7+ messages in thread
From: Dan Carpenter @ 2013-11-07 7:48 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, kernel-janitors
There is a bug in cpsw_probe() where we do:
ndev->irq = platform_get_irq(pdev, 0);
if (ndev->irq < 0) {
The problem is that "ndev->irq" is unsigned so the error handling
doesn't work. I have changed it to a regular int.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9da6a04..bb011f6 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1132,7 +1132,7 @@ struct net_device {
unsigned long mem_end; /* shared mem end */
unsigned long mem_start; /* shared mem start */
unsigned long base_addr; /* device I/O address */
- unsigned int irq; /* device IRQ number */
+ int irq; /* device IRQ number */
/*
* Some hardware also needs these fields, but they are not
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [patch] net: make ndev->irq signed for error handling
2013-11-07 7:48 [patch] net: make ndev->irq signed for error handling Dan Carpenter
@ 2013-11-07 12:14 ` Mugunthan V N
2013-11-07 12:22 ` Dan Carpenter
2013-11-08 0:13 ` David Miller
1 sibling, 1 reply; 7+ messages in thread
From: Mugunthan V N @ 2013-11-07 12:14 UTC (permalink / raw)
To: Dan Carpenter, David S. Miller; +Cc: netdev, linux-kernel, kernel-janitors
On Thursday 07 November 2013 01:18 PM, Dan Carpenter wrote:
> There is a bug in cpsw_probe() where we do:
>
> ndev->irq = platform_get_irq(pdev, 0);
> if (ndev->irq < 0) {
>
> The problem is that "ndev->irq" is unsigned so the error handling
> doesn't work. I have changed it to a regular int.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
ndev->irq is never used any where in the driver, I think its better to
remove this part of code from probe. If every one is ok, I can send a
patch to remove the code.
Regards
Mugunthan V N
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [patch] net: make ndev->irq signed for error handling
2013-11-07 12:14 ` Mugunthan V N
@ 2013-11-07 12:22 ` Dan Carpenter
2013-11-07 16:33 ` Mugunthan V N
0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2013-11-07 12:22 UTC (permalink / raw)
To: Mugunthan V N; +Cc: David S. Miller, netdev, linux-kernel, kernel-janitors
On Thu, Nov 07, 2013 at 05:44:38PM +0530, Mugunthan V N wrote:
> On Thursday 07 November 2013 01:18 PM, Dan Carpenter wrote:
> > There is a bug in cpsw_probe() where we do:
> >
> > ndev->irq = platform_get_irq(pdev, 0);
> > if (ndev->irq < 0) {
> >
> > The problem is that "ndev->irq" is unsigned so the error handling
> > doesn't work. I have changed it to a regular int.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ndev->irq is never used any where in the driver, I think its better to
> remove this part of code from probe. If every one is ok, I can send a
> patch to remove the code.
It seems like cpsw_ndo_poll_controller() uses it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [patch] net: make ndev->irq signed for error handling
2013-11-07 12:22 ` Dan Carpenter
@ 2013-11-07 16:33 ` Mugunthan V N
2013-11-08 8:41 ` Dan Carpenter
0 siblings, 1 reply; 7+ messages in thread
From: Mugunthan V N @ 2013-11-07 16:33 UTC (permalink / raw)
To: Dan Carpenter; +Cc: David S. Miller, netdev, linux-kernel, kernel-janitors
On Thursday 07 November 2013 05:52 PM, Dan Carpenter wrote:
> On Thu, Nov 07, 2013 at 05:44:38PM +0530, Mugunthan V N wrote:
>> On Thursday 07 November 2013 01:18 PM, Dan Carpenter wrote:
>>> There is a bug in cpsw_probe() where we do:
>>>
>>> ndev->irq = platform_get_irq(pdev, 0);
>>> if (ndev->irq < 0) {
>>>
>>> The problem is that "ndev->irq" is unsigned so the error handling
>>> doesn't work. I have changed it to a regular int.
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ndev->irq is never used any where in the driver, I think its better to
>> remove this part of code from probe. If every one is ok, I can send a
>> patch to remove the code.
> It seems like cpsw_ndo_poll_controller() uses it.
>
>
That can be changed to pass one of the interrupt numbers from priv as
the irq number is not used in interrupt service routine.
Regards
Mugunthan V N
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [patch] net: make ndev->irq signed for error handling
2013-11-07 16:33 ` Mugunthan V N
@ 2013-11-08 8:41 ` Dan Carpenter
2013-11-08 9:18 ` Mugunthan V N
0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2013-11-08 8:41 UTC (permalink / raw)
To: Mugunthan V N; +Cc: David S. Miller, netdev, linux-kernel, kernel-janitors
On Thu, Nov 07, 2013 at 10:03:19PM +0530, Mugunthan V N wrote:
> On Thursday 07 November 2013 05:52 PM, Dan Carpenter wrote:
> > On Thu, Nov 07, 2013 at 05:44:38PM +0530, Mugunthan V N wrote:
> >> On Thursday 07 November 2013 01:18 PM, Dan Carpenter wrote:
> >>> There is a bug in cpsw_probe() where we do:
> >>>
> >>> ndev->irq = platform_get_irq(pdev, 0);
> >>> if (ndev->irq < 0) {
> >>>
> >>> The problem is that "ndev->irq" is unsigned so the error handling
> >>> doesn't work. I have changed it to a regular int.
> >>>
> >>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >> ndev->irq is never used any where in the driver, I think its better to
> >> remove this part of code from probe. If every one is ok, I can send a
> >> patch to remove the code.
> > It seems like cpsw_ndo_poll_controller() uses it.
> >
> >
>
> That can be changed to pass one of the interrupt numbers from priv as
> the irq number is not used in interrupt service routine.
I don't know anything about this driver, so I'm fine with your idea.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [patch] net: make ndev->irq signed for error handling
2013-11-08 8:41 ` Dan Carpenter
@ 2013-11-08 9:18 ` Mugunthan V N
0 siblings, 0 replies; 7+ messages in thread
From: Mugunthan V N @ 2013-11-08 9:18 UTC (permalink / raw)
To: Dan Carpenter; +Cc: David S. Miller, netdev, linux-kernel, kernel-janitors
On Friday 08 November 2013 02:11 PM, Dan Carpenter wrote:
> On Thu, Nov 07, 2013 at 10:03:19PM +0530, Mugunthan V N wrote:
>> On Thursday 07 November 2013 05:52 PM, Dan Carpenter wrote:
>>> On Thu, Nov 07, 2013 at 05:44:38PM +0530, Mugunthan V N wrote:
>>>> On Thursday 07 November 2013 01:18 PM, Dan Carpenter wrote:
>>>>> There is a bug in cpsw_probe() where we do:
>>>>>
>>>>> ndev->irq = platform_get_irq(pdev, 0);
>>>>> if (ndev->irq < 0) {
>>>>>
>>>>> The problem is that "ndev->irq" is unsigned so the error handling
>>>>> doesn't work. I have changed it to a regular int.
>>>>>
>>>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>> ndev->irq is never used any where in the driver, I think its better to
>>>> remove this part of code from probe. If every one is ok, I can send a
>>>> patch to remove the code.
>>> It seems like cpsw_ndo_poll_controller() uses it.
>>>
>>>
>> That can be changed to pass one of the interrupt numbers from priv as
>> the irq number is not used in interrupt service routine.
> I don't know anything about this driver, so I'm fine with your idea.
>
Anyhow David has applied your patch, I will send a driver cleanup patch
seperately.
Regards
Mugunthan V N
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] net: make ndev->irq signed for error handling
2013-11-07 7:48 [patch] net: make ndev->irq signed for error handling Dan Carpenter
2013-11-07 12:14 ` Mugunthan V N
@ 2013-11-08 0:13 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2013-11-08 0:13 UTC (permalink / raw)
To: dan.carpenter; +Cc: netdev, linux-kernel, kernel-janitors
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Thu, 7 Nov 2013 10:48:49 +0300
> There is a bug in cpsw_probe() where we do:
>
> ndev->irq = platform_get_irq(pdev, 0);
> if (ndev->irq < 0) {
>
> The problem is that "ndev->irq" is unsigned so the error handling
> doesn't work. I have changed it to a regular int.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Even though some simplifications have been suggested wrt. how
this irq value is obtained in the one place where it is used,
I am applying Dan's patch for now.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-11-08 9:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-07 7:48 [patch] net: make ndev->irq signed for error handling Dan Carpenter
2013-11-07 12:14 ` Mugunthan V N
2013-11-07 12:22 ` Dan Carpenter
2013-11-07 16:33 ` Mugunthan V N
2013-11-08 8:41 ` Dan Carpenter
2013-11-08 9:18 ` Mugunthan V N
2013-11-08 0:13 ` David Miller
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).