Re: [Outreachy kernel] [PATCH v2] Staging: rtl8712: Use mod_timer

[Jes Sorensen <jes.sorensen@gmail.com>]

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