All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Staging: rtl8712: Use mod_timer
@ 2015-02-23 10:56 Vaishali Thakkar
  2015-02-23 19:39 ` [Outreachy kernel] " Jes Sorensen
  0 siblings, 1 reply; 7+ messages in thread
From: Vaishali Thakkar @ 2015-02-23 10:56 UTC (permalink / raw)
  To: outreachy-kernel

This patch introduces the use of API function mod_timer
instead of driver specific function as it is a more
efficient and standard way to update the expire field of
an active timer.

Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
---
Changes since v1:
	-Edit commit message
	-Remove unnecessory parenthesis
	-Align second parameter of function
	 mod_timer properly

 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 9bb364f..0ce8e9d 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -129,7 +129,8 @@ static inline void handle_pairwise_key(struct sta_info *psta,
 		memcpy(psta->tkiprxmickey. skey, &(param->u.crypt.
 			key[24]), 8);
 		padapter->securitypriv. busetkipkey = false;
-		_set_timer(&padapter->securitypriv.tkip_timer, 50);
+		mod_timer(&padapter->securitypriv.tkip_timer,
+			  jiffies + msecs_to_jiffies(50));
 	}
 	r8712_setstakey_cmd(padapter, (unsigned char *)psta, true);
 }
@@ -153,8 +154,8 @@ static inline void handle_group_key(struct ieee_param *param,
 		if (padapter->registrypriv.power_mgnt > PS_MODE_ACTIVE) {
 			if (padapter->registrypriv.power_mgnt != padapter->
 			    pwrctrlpriv.pwr_mode)
-				_set_timer(&(padapter->mlmepriv.dhcp_timer),
-					   60000);
+				mod_timer(&(padapter->mlmepriv.dhcp_timer),
+					  jiffies + msecs_to_jiffies(60000));
 		}
 	}
 }
-- 
1.9.1



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

* Re: [Outreachy kernel] [PATCH v2] Staging: rtl8712: Use mod_timer
  2015-02-23 10:56 [PATCH v2] Staging: rtl8712: Use mod_timer Vaishali Thakkar
@ 2015-02-23 19:39 ` Jes Sorensen
  2015-02-23 19:49   ` Vaishali Thakkar
  0 siblings, 1 reply; 7+ messages in thread
From: Jes Sorensen @ 2015-02-23 19:39 UTC (permalink / raw)
  To: Vaishali Thakkar, outreachy-kernel

On 02/23/15 05:56, Vaishali Thakkar wrote:
> This patch introduces the use of API function mod_timer
> instead of driver specific function as it is a more
> efficient and standard way to update the expire field of
> an active timer.
> 
> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
> ---
> Changes since v1:
> 	-Edit commit message
> 	-Remove unnecessory parenthesis
> 	-Align second parameter of function
> 	 mod_timer properly
> 
>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

I like this patch!

However if you start getting rid of _set_timer() from a driver like
this, you should try and remove all instances, and then get rid of all
uses of _set_timer().

In addition you are still leaving some unnecessary parenthesis in place
- see below.

Cheers,
Jes


> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index 9bb364f..0ce8e9d 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -129,7 +129,8 @@ static inline void handle_pairwise_key(struct sta_info *psta,
>  		memcpy(psta->tkiprxmickey. skey, &(param->u.crypt.
>  			key[24]), 8);
>  		padapter->securitypriv. busetkipkey = false;
> -		_set_timer(&padapter->securitypriv.tkip_timer, 50);
> +		mod_timer(&padapter->securitypriv.tkip_timer,
> +			  jiffies + msecs_to_jiffies(50));
>  	}
>  	r8712_setstakey_cmd(padapter, (unsigned char *)psta, true);
>  }
> @@ -153,8 +154,8 @@ static inline void handle_group_key(struct ieee_param *param,
>  		if (padapter->registrypriv.power_mgnt > PS_MODE_ACTIVE) {
>  			if (padapter->registrypriv.power_mgnt != padapter->
>  			    pwrctrlpriv.pwr_mode)
> -				_set_timer(&(padapter->mlmepriv.dhcp_timer),
> -					   60000);
> +				mod_timer(&(padapter->mlmepriv.dhcp_timer),
                                      here ^
> +					  jiffies + msecs_to_jiffies(60000));
>  		}
>  	}
>  }
> 



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

* Re: [Outreachy kernel] [PATCH v2] Staging: rtl8712: Use mod_timer
  2015-02-23 19:39 ` [Outreachy kernel] " Jes Sorensen
@ 2015-02-23 19:49   ` Vaishali Thakkar
  2015-02-23 19:52     ` Jes Sorensen
  0 siblings, 1 reply; 7+ messages in thread
From: Vaishali Thakkar @ 2015-02-23 19:49 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 2878 bytes --]

On Feb 24, 2015 1:09 AM, "Jes Sorensen" <jes.sorensen@gmail.com> wrote:
>
> On 02/23/15 05:56, Vaishali Thakkar wrote:
> > This patch introduces the use of API function mod_timer
> > instead of driver specific function as it is a more
> > efficient and standard way to update the expire field of
> > an active timer.
> >
> > Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
> > ---
> > Changes since v1:
> >       -Edit commit message
> >       -Remove unnecessory parenthesis
> >       -Align second parameter of function
> >        mod_timer properly
> >
> >  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
>
> I like this patch!
>
> However if you start getting rid of _set_timer() from a driver like
> this, you should try and remove all instances, and then get rid of all
> uses of _set_timer().

Yes. Actually I sent this patch for mentors' reviews. I thought if this
will be accepted then I will go for sending more patches as there are many
instances of _set_timer() in this driver and in other network drivers as
well. And then I can remove those definitions once all cases are handled.

Also, network drivers are using driver specific functions where we can use
setup_ timer() function. So, I will go for it as well.

> In addition you are still leaving some unnecessary parenthesis in place
> - see below.

Oops! yeah. I will remove them in next version.

> Cheers,
> Jes
>
>
> > diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > index 9bb364f..0ce8e9d 100644
> > --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > @@ -129,7 +129,8 @@ static inline void handle_pairwise_key(struct
sta_info *psta,
> >               memcpy(psta->tkiprxmickey. skey, &(param->u.crypt.
> >                       key[24]), 8);
> >               padapter->securitypriv. busetkipkey = false;
> > -             _set_timer(&padapter->securitypriv.tkip_timer, 50);
> > +             mod_timer(&padapter->securitypriv.tkip_timer,
> > +                       jiffies + msecs_to_jiffies(50));
> >       }
> >       r8712_setstakey_cmd(padapter, (unsigned char *)psta, true);
> >  }
> > @@ -153,8 +154,8 @@ static inline void handle_group_key(struct
ieee_param *param,
> >               if (padapter->registrypriv.power_mgnt > PS_MODE_ACTIVE) {
> >                       if (padapter->registrypriv.power_mgnt !=
padapter->
> >                           pwrctrlpriv.pwr_mode)
> > -
 _set_timer(&(padapter->mlmepriv.dhcp_timer),
> > -                                        60000);
> > +
 mod_timer(&(padapter->mlmepriv.dhcp_timer),
>                                       here ^
> > +                                       jiffies +
msecs_to_jiffies(60000));
> >               }
> >       }
> >  }
> >
>

[-- Attachment #2: Type: text/html, Size: 3940 bytes --]

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

* Re: [Outreachy kernel] [PATCH v2] Staging: rtl8712: Use mod_timer
  2015-02-23 19:49   ` Vaishali Thakkar
@ 2015-02-23 19:52     ` Jes Sorensen
  2015-02-23 20:06       ` Vaishali Thakkar
  0 siblings, 1 reply; 7+ messages in thread
From: Jes Sorensen @ 2015-02-23 19:52 UTC (permalink / raw)
  To: Vaishali Thakkar; +Cc: outreachy-kernel

On 02/23/15 14:49, Vaishali Thakkar wrote:
> On Feb 24, 2015 1:09 AM, "Jes Sorensen" <jes.sorensen@gmail.com> wrote:
>>
>> On 02/23/15 05:56, Vaishali Thakkar wrote:
>>> This patch introduces the use of API function mod_timer
>>> instead of driver specific function as it is a more
>>> efficient and standard way to update the expire field of
>>> an active timer.
>>>
>>> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
>>> ---
>>> Changes since v1:
>>>       -Edit commit message
>>>       -Remove unnecessory parenthesis
>>>       -Align second parameter of function
>>>        mod_timer properly
>>>
>>>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 7 ++++---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> I like this patch!
>>
>> However if you start getting rid of _set_timer() from a driver like
>> this, you should try and remove all instances, and then get rid of all
>> uses of _set_timer().
> 
> Yes. Actually I sent this patch for mentors' reviews. I thought if this
> will be accepted then I will go for sending more patches as there are many
> instances of _set_timer() in this driver and in other network drivers as
> well. And then I can remove those definitions once all cases are handled.
> 
> Also, network drivers are using driver specific functions where we can use
> setup_ timer() function. So, I will go for it as well.

Sounds great - I recommend you focus on one change, so start out with
one patch that eliminates _set_timer() and then do follow-on patches
where you convert other portions of code to use setup_timer().

Cheers,
Jes

>> In addition you are still leaving some unnecessary parenthesis in place
>> - see below.
> 
> Oops! yeah. I will remove them in next version.
> 
>> Cheers,
>> Jes
>>
>>
>>> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>> index 9bb364f..0ce8e9d 100644
>>> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>> @@ -129,7 +129,8 @@ static inline void handle_pairwise_key(struct
> sta_info *psta,
>>>               memcpy(psta->tkiprxmickey. skey, &(param->u.crypt.
>>>                       key[24]), 8);
>>>               padapter->securitypriv. busetkipkey = false;
>>> -             _set_timer(&padapter->securitypriv.tkip_timer, 50);
>>> +             mod_timer(&padapter->securitypriv.tkip_timer,
>>> +                       jiffies + msecs_to_jiffies(50));
>>>       }
>>>       r8712_setstakey_cmd(padapter, (unsigned char *)psta, true);
>>>  }
>>> @@ -153,8 +154,8 @@ static inline void handle_group_key(struct
> ieee_param *param,
>>>               if (padapter->registrypriv.power_mgnt > PS_MODE_ACTIVE) {
>>>                       if (padapter->registrypriv.power_mgnt !=
> padapter->
>>>                           pwrctrlpriv.pwr_mode)
>>> -
>  _set_timer(&(padapter->mlmepriv.dhcp_timer),
>>> -                                        60000);
>>> +
>  mod_timer(&(padapter->mlmepriv.dhcp_timer),
>>                                       here ^
>>> +                                       jiffies +
> msecs_to_jiffies(60000));
>>>               }
>>>       }
>>>  }
>>>
>>
> 



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

* Re: [Outreachy kernel] [PATCH v2] Staging: rtl8712: Use mod_timer
  2015-02-23 19:52     ` Jes Sorensen
@ 2015-02-23 20:06       ` Vaishali Thakkar
  2015-02-23 20:12         ` Jes Sorensen
  0 siblings, 1 reply; 7+ messages in thread
From: Vaishali Thakkar @ 2015-02-23 20:06 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 3790 bytes --]

On Feb 24, 2015 1:22 AM, "Jes Sorensen" <jes.sorensen@gmail.com> wrote:
>
> On 02/23/15 14:49, Vaishali Thakkar wrote:
> > On Feb 24, 2015 1:09 AM, "Jes Sorensen" <jes.sorensen@gmail.com> wrote:
> >>
> >> On 02/23/15 05:56, Vaishali Thakkar wrote:
> >>> This patch introduces the use of API function mod_timer
> >>> instead of driver specific function as it is a more
> >>> efficient and standard way to update the expire field of
> >>> an active timer.
> >>>
> >>> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
> >>> ---
> >>> Changes since v1:
> >>>       -Edit commit message
> >>>       -Remove unnecessory parenthesis
> >>>       -Align second parameter of function
> >>>        mod_timer properly
> >>>
> >>>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 7 ++++---
> >>>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> I like this patch!
> >>
> >> However if you start getting rid of _set_timer() from a driver like
> >> this, you should try and remove all instances, and then get rid of all
> >> uses of _set_timer().
> >
> > Yes. Actually I sent this patch for mentors' reviews. I thought if this
> > will be accepted then I will go for sending more patches as there are
many
> > instances of _set_timer() in this driver and in other network drivers as
> > well. And then I can remove those definitions once all cases are
handled.
> >
> > Also, network drivers are using driver specific functions where we can
use
> > setup_ timer() function. So, I will go for it as well.
>
> Sounds great - I recommend you focus on one change, so start out with
> one patch that eliminates _set_timer() and then do follow-on patches
> where you convert other portions of code to use setup_timer().

Ok. Firstly, I am planning to eliminate use of _set_timer() completly in
all network drivers and then I will go for setup_timer(). Is it right
strategy?

I am just little confused about sending them in series. Can I send a patch
series [ one patch for each driver] for eliminating _set_timer()?? And then
one different patch series goes for setup_timer().

> Cheers,
> Jes
>
> >> In addition you are still leaving some unnecessary parenthesis in place
> >> - see below.
> >
> > Oops! yeah. I will remove them in next version.
> >
> >> Cheers,
> >> Jes
> >>
> >>
> >>> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >>> index 9bb364f..0ce8e9d 100644
> >>> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >>> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >>> @@ -129,7 +129,8 @@ static inline void handle_pairwise_key(struct
> > sta_info *psta,
> >>>               memcpy(psta->tkiprxmickey. skey, &(param->u.crypt.
> >>>                       key[24]), 8);
> >>>               padapter->securitypriv. busetkipkey = false;
> >>> -             _set_timer(&padapter->securitypriv.tkip_timer, 50);
> >>> +             mod_timer(&padapter->securitypriv.tkip_timer,
> >>> +                       jiffies + msecs_to_jiffies(50));
> >>>       }
> >>>       r8712_setstakey_cmd(padapter, (unsigned char *)psta, true);
> >>>  }
> >>> @@ -153,8 +154,8 @@ static inline void handle_group_key(struct
> > ieee_param *param,
> >>>               if (padapter->registrypriv.power_mgnt > PS_MODE_ACTIVE)
{
> >>>                       if (padapter->registrypriv.power_mgnt !=
> > padapter->
> >>>                           pwrctrlpriv.pwr_mode)
> >>> -
> >  _set_timer(&(padapter->mlmepriv.dhcp_timer),
> >>> -                                        60000);
> >>> +
> >  mod_timer(&(padapter->mlmepriv.dhcp_timer),
> >>                                       here ^
> >>> +                                       jiffies +
> > msecs_to_jiffies(60000));
> >>>               }
> >>>       }
> >>>  }
> >>>
> >>
> >
>

[-- Attachment #2: Type: text/html, Size: 5464 bytes --]

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

* Re: [Outreachy kernel] [PATCH v2] Staging: rtl8712: Use mod_timer
  2015-02-23 20:06       ` Vaishali Thakkar
@ 2015-02-23 20:12         ` Jes Sorensen
  2015-02-23 20:17           ` Vaishali Thakkar
  0 siblings, 1 reply; 7+ messages in thread
From: Jes Sorensen @ 2015-02-23 20:12 UTC (permalink / raw)
  To: Vaishali Thakkar; +Cc: outreachy-kernel

On 02/23/15 15:06, Vaishali Thakkar wrote:
> On Feb 24, 2015 1:22 AM, "Jes Sorensen" <jes.sorensen@gmail.com> wrote:
>>
>> On 02/23/15 14:49, Vaishali Thakkar wrote:
>>> On Feb 24, 2015 1:09 AM, "Jes Sorensen" <jes.sorensen@gmail.com> wrote:
>>>>
>>>> On 02/23/15 05:56, Vaishali Thakkar wrote:
>>>>> This patch introduces the use of API function mod_timer
>>>>> instead of driver specific function as it is a more
>>>>> efficient and standard way to update the expire field of
>>>>> an active timer.
>>>>>
>>>>> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
>>>>> ---
>>>>> Changes since v1:
>>>>>       -Edit commit message
>>>>>       -Remove unnecessory parenthesis
>>>>>       -Align second parameter of function
>>>>>        mod_timer properly
>>>>>
>>>>>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 7 ++++---
>>>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> I like this patch!
>>>>
>>>> However if you start getting rid of _set_timer() from a driver like
>>>> this, you should try and remove all instances, and then get rid of all
>>>> uses of _set_timer().
>>>
>>> Yes. Actually I sent this patch for mentors' reviews. I thought if this
>>> will be accepted then I will go for sending more patches as there are
> many
>>> instances of _set_timer() in this driver and in other network drivers as
>>> well. And then I can remove those definitions once all cases are
> handled.
>>>
>>> Also, network drivers are using driver specific functions where we can
> use
>>> setup_ timer() function. So, I will go for it as well.
>>
>> Sounds great - I recommend you focus on one change, so start out with
>> one patch that eliminates _set_timer() and then do follow-on patches
>> where you convert other portions of code to use setup_timer().
> 
> Ok. Firstly, I am planning to eliminate use of _set_timer() completly in
> all network drivers and then I will go for setup_timer(). Is it right
> strategy?
> 
> I am just little confused about sending them in series. Can I send a patch
> series [ one patch for each driver] for eliminating _set_timer()?? And then
> one different patch series goes for setup_timer().

I would recommend one patch series per driver, as opposed to one series
for all drivers. So for each driver I would do:

1: Patch to eliminate _set_timer()
2: Patch(es) to convert code to setup_timer()

It is better to do one patch set per driver, as it allows individual
driver maintainers to apply them without touching code they do not
maintainer.

Cheers,
Jes


>> Cheers,
>> Jes
>>
>>>> In addition you are still leaving some unnecessary parenthesis in place
>>>> - see below.
>>>
>>> Oops! yeah. I will remove them in next version.
>>>
>>>> Cheers,
>>>> Jes
>>>>
>>>>
>>>>> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>> b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>>>> index 9bb364f..0ce8e9d 100644
>>>>> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>>>> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>>>>> @@ -129,7 +129,8 @@ static inline void handle_pairwise_key(struct
>>> sta_info *psta,
>>>>>               memcpy(psta->tkiprxmickey. skey, &(param->u.crypt.
>>>>>                       key[24]), 8);
>>>>>               padapter->securitypriv. busetkipkey = false;
>>>>> -             _set_timer(&padapter->securitypriv.tkip_timer, 50);
>>>>> +             mod_timer(&padapter->securitypriv.tkip_timer,
>>>>> +                       jiffies + msecs_to_jiffies(50));
>>>>>       }
>>>>>       r8712_setstakey_cmd(padapter, (unsigned char *)psta, true);
>>>>>  }
>>>>> @@ -153,8 +154,8 @@ static inline void handle_group_key(struct
>>> ieee_param *param,
>>>>>               if (padapter->registrypriv.power_mgnt > PS_MODE_ACTIVE)
> {
>>>>>                       if (padapter->registrypriv.power_mgnt !=
>>> padapter->
>>>>>                           pwrctrlpriv.pwr_mode)
>>>>> -
>>>  _set_timer(&(padapter->mlmepriv.dhcp_timer),
>>>>> -                                        60000);
>>>>> +
>>>  mod_timer(&(padapter->mlmepriv.dhcp_timer),
>>>>                                       here ^
>>>>> +                                       jiffies +
>>> msecs_to_jiffies(60000));
>>>>>               }
>>>>>       }
>>>>>  }
>>>>>
>>>>
>>>
>>
> 



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

* Re: [Outreachy kernel] [PATCH v2] Staging: rtl8712: Use mod_timer
  2015-02-23 20:12         ` Jes Sorensen
@ 2015-02-23 20:17           ` Vaishali Thakkar
  0 siblings, 0 replies; 7+ messages in thread
From: Vaishali Thakkar @ 2015-02-23 20:17 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 4622 bytes --]

On Feb 24, 2015 1:42 AM, "Jes Sorensen" <jes.sorensen@gmail.com> wrote:
>
> On 02/23/15 15:06, Vaishali Thakkar wrote:
> > On Feb 24, 2015 1:22 AM, "Jes Sorensen" <jes.sorensen@gmail.com> wrote:
> >>
> >> On 02/23/15 14:49, Vaishali Thakkar wrote:
> >>> On Feb 24, 2015 1:09 AM, "Jes Sorensen" <jes.sorensen@gmail.com>
wrote:
> >>>>
> >>>> On 02/23/15 05:56, Vaishali Thakkar wrote:
> >>>>> This patch introduces the use of API function mod_timer
> >>>>> instead of driver specific function as it is a more
> >>>>> efficient and standard way to update the expire field of
> >>>>> an active timer.
> >>>>>
> >>>>> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
> >>>>> ---
> >>>>> Changes since v1:
> >>>>>       -Edit commit message
> >>>>>       -Remove unnecessory parenthesis
> >>>>>       -Align second parameter of function
> >>>>>        mod_timer properly
> >>>>>
> >>>>>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 7 ++++---
> >>>>>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>>>
> >>>> I like this patch!
> >>>>
> >>>> However if you start getting rid of _set_timer() from a driver like
> >>>> this, you should try and remove all instances, and then get rid of
all
> >>>> uses of _set_timer().
> >>>
> >>> Yes. Actually I sent this patch for mentors' reviews. I thought if
this
> >>> will be accepted then I will go for sending more patches as there are
> > many
> >>> instances of _set_timer() in this driver and in other network drivers
as
> >>> well. And then I can remove those definitions once all cases are
> > handled.
> >>>
> >>> Also, network drivers are using driver specific functions where we can
> > use
> >>> setup_ timer() function. So, I will go for it as well.
> >>
> >> Sounds great - I recommend you focus on one change, so start out with
> >> one patch that eliminates _set_timer() and then do follow-on patches
> >> where you convert other portions of code to use setup_timer().
> >
> > Ok. Firstly, I am planning to eliminate use of _set_timer() completly in
> > all network drivers and then I will go for setup_timer(). Is it right
> > strategy?
> >
> > I am just little confused about sending them in series. Can I send a
patch
> > series [ one patch for each driver] for eliminating _set_timer()?? And
then
> > one different patch series goes for setup_timer().
>
> I would recommend one patch series per driver, as opposed to one series
> for all drivers. So for each driver I would do:
>
> 1: Patch to eliminate _set_timer()
> 2: Patch(es) to convert code to setup_timer()
>
> It is better to do one patch set per driver, as it allows individual
> driver maintainers to apply them without touching code they do not
> maintainer.

Ok. Yes. This makes sense. I will send them as per your recommendation.

Thank You

> Cheers,
> Jes
>
>
> >> Cheers,
> >> Jes
> >>
> >>>> In addition you are still leaving some unnecessary parenthesis in
place
> >>>> - see below.
> >>>
> >>> Oops! yeah. I will remove them in next version.
> >>>
> >>>> Cheers,
> >>>> Jes
> >>>>
> >>>>
> >>>>> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >>> b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >>>>> index 9bb364f..0ce8e9d 100644
> >>>>> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >>>>> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >>>>> @@ -129,7 +129,8 @@ static inline void handle_pairwise_key(struct
> >>> sta_info *psta,
> >>>>>               memcpy(psta->tkiprxmickey. skey, &(param->u.crypt.
> >>>>>                       key[24]), 8);
> >>>>>               padapter->securitypriv. busetkipkey = false;
> >>>>> -             _set_timer(&padapter->securitypriv.tkip_timer, 50);
> >>>>> +             mod_timer(&padapter->securitypriv.tkip_timer,
> >>>>> +                       jiffies + msecs_to_jiffies(50));
> >>>>>       }
> >>>>>       r8712_setstakey_cmd(padapter, (unsigned char *)psta, true);
> >>>>>  }
> >>>>> @@ -153,8 +154,8 @@ static inline void handle_group_key(struct
> >>> ieee_param *param,
> >>>>>               if (padapter->registrypriv.power_mgnt >
PS_MODE_ACTIVE)
> > {
> >>>>>                       if (padapter->registrypriv.power_mgnt !=
> >>> padapter->
> >>>>>                           pwrctrlpriv.pwr_mode)
> >>>>> -
> >>>  _set_timer(&(padapter->mlmepriv.dhcp_timer),
> >>>>> -                                        60000);
> >>>>> +
> >>>  mod_timer(&(padapter->mlmepriv.dhcp_timer),
> >>>>                                       here ^
> >>>>> +                                       jiffies +
> >>> msecs_to_jiffies(60000));
> >>>>>               }
> >>>>>       }
> >>>>>  }
> >>>>>
> >>>>
> >>>
> >>
> >
>

[-- Attachment #2: Type: text/html, Size: 7168 bytes --]

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

end of thread, other threads:[~2015-02-23 20:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-23 10:56 [PATCH v2] Staging: rtl8712: Use mod_timer Vaishali Thakkar
2015-02-23 19:39 ` [Outreachy kernel] " Jes Sorensen
2015-02-23 19:49   ` Vaishali Thakkar
2015-02-23 19:52     ` Jes Sorensen
2015-02-23 20:06       ` Vaishali Thakkar
2015-02-23 20:12         ` Jes Sorensen
2015-02-23 20:17           ` Vaishali Thakkar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.