[PATCH 3/3] 3c59x: Use setup_timer()

[PATCH 3/3] 3c59x: Use setup_timer()

[Amitoj Kaur Chawla]

Convert a call to init_timer and accompanying intializations of
the timer's data and function fields to a call to setup_timer.

The Coccinelle semantic patch that fixes this problem is
as follows:
 
// <smpl>
@@
expression t,f,d;
@@

-init_timer(&t);
+setup_timer(&t,f,d);
 ...
-t.data = d;
-t.function = f;
// </smpl>

Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
 drivers/net/ethernet/3com/3c59x.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/3com/3c59x.c b/drivers/net/ethernet/3com/3c59x.c
index 79e1a02..c377607 100644
--- a/drivers/net/ethernet/3com/3c59x.c
+++ b/drivers/net/ethernet/3com/3c59x.c
@@ -1601,15 +1601,9 @@ vortex_up(struct net_device *dev)
 				dev->name, media_tbl[dev->if_port].name);
 	}
 
-	init_timer(&vp->timer);
+	setup_timer(&vp->timer, vortex_timer, (unsigned long)dev);
 	vp->timer.expires = RUN_AT(media_tbl[dev->if_port].wait);
-	vp->timer.data = (unsigned long)dev;
-	vp->timer.function = vortex_timer;		/* timer handler */
-	add_timer(&vp->timer);
-
-	init_timer(&vp->rx_oom_timer);
-	vp->rx_oom_timer.data = (unsigned long)dev;
-	vp->rx_oom_timer.function = rx_oom_timer;
+	setup_timer(&vp->rx_oom_timer, rx_oom_timer, (unsigned long)dev);
 
 	if (vortex_debug > 1)
 		pr_debug("%s: Initial media type %s.\n",
-- 
1.9.1

Re: [PATCH 3/3] 3c59x: Use setup_timer()

[David Miller]

From: Amitoj Kaur Chawla <amitoj1606@gmail.com>
Date: Wed, 24 Feb 2016 19:28:19 +0530

> Convert a call to init_timer and accompanying intializations of
> the timer's data and function fields to a call to setup_timer.
> 
> The Coccinelle semantic patch that fixes this problem is
> as follows:
>  
> // <smpl>
> @@
> expression t,f,d;
> @@
> 
> -init_timer(&t);
> +setup_timer(&t,f,d);
>  ...
> -t.data = d;
> -t.function = f;
> // </smpl>
> 
> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>

Applied.

Re: [PATCH 3/3] 3c59x: Use setup_timer()

[Stafford Horne]

On Thu, 25 Feb 2016, David Miller wrote:

> From: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> Date: Wed, 24 Feb 2016 19:28:19 +0530
>
>> Convert a call to init_timer and accompanying intializations of
>> the timer's data and function fields to a call to setup_timer.
>>
>> The Coccinelle semantic patch that fixes this problem is
>> as follows:
>>
>> // <smpl>
>> @@
>> expression t,f,d;
>> @@
>>
>> -init_timer(&t);
>> +setup_timer(&t,f,d);
>>  ...
>> -t.data = d;
>> -t.function = f;
>> // </smpl>
>>
>> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
>
> Applied.

Hi David, Amitoj,

The patch here seemed to remove the call to add_timer(&vp->timer) which
applies the expires time. Would that be an issue?

-Stafford

Re: [PATCH 3/3] 3c59x: Use setup_timer()

[Amitoj Kaur Chawla]

On Sun, Feb 28, 2016 at 12:18 AM, Stafford Horne <shorne@gmail.com> wrote:
>
>
> On Thu, 25 Feb 2016, David Miller wrote:
>
>> From: Amitoj Kaur Chawla <amitoj1606@gmail.com>
>> Date: Wed, 24 Feb 2016 19:28:19 +0530
>>
>>> Convert a call to init_timer and accompanying intializations of
>>> the timer's data and function fields to a call to setup_timer.
>>>
>>> The Coccinelle semantic patch that fixes this problem is
>>> as follows:
>>>
>>> // <smpl>
>>> @@
>>> expression t,f,d;
>>> @@
>>>
>>> -init_timer(&t);
>>> +setup_timer(&t,f,d);
>>>  ...
>>> -t.data = d;
>>> -t.function = f;
>>> // </smpl>
>>>
>>> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
>>
>>
>> Applied.
>
>
> Hi David, Amitoj,
>
> The patch here seemed to remove the call to add_timer(&vp->timer) which
> applies the expires time. Would that be an issue?
>
> -Stafford

I'm sorry. This is my mistake. How can I rectify it now that the patch
is applied?

Should I send a patch adding it back?

Amitoj

[PATCH] 3c59x: Ensure to apply the expires time

[Stafford Horne]

In commit 5b6490def9168af6a ("3c59x: Use setup_timer()") Amitoj
removed add_timer which sets up the epires timer.  In this patch
the behavior is restore but it uses mod_timer which is a bit more
compact.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---

I think a patch like this will help restore the behavior. Also, 
its small cleanup since we dont need to do separate set to expire
and call to add_timer. But thats a style preference. 

 drivers/net/ethernet/3com/3c59x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/3com/3c59x.c b/drivers/net/ethernet/3com/3c59x.c
index c377607..7b881ed 100644
--- a/drivers/net/ethernet/3com/3c59x.c
+++ b/drivers/net/ethernet/3com/3c59x.c
@@ -1602,7 +1602,7 @@ vortex_up(struct net_device *dev)
 	}
 
 	setup_timer(&vp->timer, vortex_timer, (unsigned long)dev);
-	vp->timer.expires = RUN_AT(media_tbl[dev->if_port].wait);
+	mod_timer(&vp->timer, RUN_AT(media_tbl[dev->if_port].wait));
 	setup_timer(&vp->rx_oom_timer, rx_oom_timer, (unsigned long)dev);
 
 	if (vortex_debug > 1)
-- 
2.5.0

Re: [PATCH 3/3] 3c59x: Use setup_timer()

[Stafford Horne]

On Sun, 28 Feb 2016, Amitoj Kaur Chawla wrote:

> On Sun, Feb 28, 2016 at 12:18 AM, Stafford Horne <shorne@gmail.com> wrote:
> >
> >
> > On Thu, 25 Feb 2016, David Miller wrote:
> >
> >> From: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> >> Date: Wed, 24 Feb 2016 19:28:19 +0530
> >>
> >>> Convert a call to init_timer and accompanying intializations of
> >>> the timer's data and function fields to a call to setup_timer.
> >>>
> >>> The Coccinelle semantic patch that fixes this problem is
> >>> as follows:
> >>>
> >>> // <smpl>
> >>> @@
> >>> expression t,f,d;
> >>> @@
> >>>
> >>> -init_timer(&t);
> >>> +setup_timer(&t,f,d);
> >>>  ...
> >>> -t.data = d;
> >>> -t.function = f;
> >>> // </smpl>
> >>>
> >>> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> >>
> >>
> >> Applied.
> >
> >
> > Hi David, Amitoj,
> >
> > The patch here seemed to remove the call to add_timer(&vp->timer) which
> > applies the expires time. Would that be an issue?
> >
> > -Stafford
> 
> I'm sorry. This is my mistake. How can I rectify it now that the patch
> is applied?
> 
> Should I send a patch adding it back?

I sent a patch just now which could help to restore the behavior.

This is applied on top of your patch which I pulled from Dave's
tree here:

git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git

-Stafford

Re: [PATCH 3/3] 3c59x: Use setup_timer()

[Amitoj Kaur Chawla]

On Sun, Feb 28, 2016 at 1:23 PM, Stafford Horne <shorne@gmail.com> wrote:
>
>
> On Sun, 28 Feb 2016, Amitoj Kaur Chawla wrote:
>
>> On Sun, Feb 28, 2016 at 12:18 AM, Stafford Horne <shorne@gmail.com> wrote:
>> >
>> >
>> > On Thu, 25 Feb 2016, David Miller wrote:
>> >
>> >> From: Amitoj Kaur Chawla <amitoj1606@gmail.com>
>> >> Date: Wed, 24 Feb 2016 19:28:19 +0530
>> >>
>> >>> Convert a call to init_timer and accompanying intializations of
>> >>> the timer's data and function fields to a call to setup_timer.
>> >>>
>> >>> The Coccinelle semantic patch that fixes this problem is
>> >>> as follows:
>> >>>
>> >>> // <smpl>
>> >>> @@
>> >>> expression t,f,d;
>> >>> @@
>> >>>
>> >>> -init_timer(&t);
>> >>> +setup_timer(&t,f,d);
>> >>>  ...
>> >>> -t.data = d;
>> >>> -t.function = f;
>> >>> // </smpl>
>> >>>
>> >>> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
>> >>
>> >>
>> >> Applied.
>> >
>> >
>> > Hi David, Amitoj,
>> >
>> > The patch here seemed to remove the call to add_timer(&vp->timer) which
>> > applies the expires time. Would that be an issue?
>> >
>> > -Stafford
>>
>> I'm sorry. This is my mistake. How can I rectify it now that the patch
>> is applied?
>>
>> Should I send a patch adding it back?
>
>
> I sent a patch just now which could help to restore the behavior.
>
> This is applied on top of your patch which I pulled from Dave's
> tree here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>
> -Stafford

Thanks!

Amitoj

Re: [PATCH] 3c59x: Ensure to apply the expires time

[David Miller]

From: Stafford Horne <shorne@gmail.com>
Date: Sun, 28 Feb 2016 16:49:29 +0900

> In commit 5b6490def9168af6a ("3c59x: Use setup_timer()") Amitoj
> removed add_timer which sets up the epires timer.  In this patch
> the behavior is restore but it uses mod_timer which is a bit more
> compact.
> 
> Signed-off-by: Stafford Horne <shorne@gmail.com>

Applied, thanks.