Re: [PATCH 09/11] staging: wilc1000: remove line over 80 char in cfg_connect_result()

[Dan Carpenter <dan.carpenter@oracle.com>]

On Tue, Mar 20, 2018 at 10:25:42PM +0530, Ajay Singh wrote:
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index 1b6fe64..af1b8aa 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -460,6 +460,17 @@ static void cfg_scan_result(enum scan_event scan_event,
>  	}
>  }
>  
> +static inline bool wilc_wfi_cfg_scan_time_expired(int i)

"i" is the wrong parameter to pass.  The name is not useful.  Probably
the right parameter is either &last_scanned_shadow[i] or
last_scanned_shadow[i].time_scan_cached.

> +{
> +	unsigned long now = jiffies;
> +
> +	if (time_after(now, last_scanned_shadow[i].time_scan_cached +
> +		       (unsigned long)(nl80211_SCAN_RESULT_EXPIRE - (1 * HZ))))
> +		return true;
> +	else
> +		return false;

Also I think it you apply this patch and then run checkpatch.pl --strict
against the file it will complain that it should be:

	if (time_after(now, last_scanned_shadow[i].time_scan_cached +
		       (unsigned long)(nl80211_SCAN_RESULT_EXPIRE - (1 * HZ))))
		return true;
	return false;

> +}
> +
>  int wilc_connecting;
>  
>  static void cfg_connect_result(enum conn_event conn_disconn_evt,
> @@ -505,17 +516,14 @@ static void cfg_connect_result(enum conn_event conn_disconn_evt,
>  			bool scan_refresh = false;
>  			u32 i;
>  
> -			memcpy(priv->associated_bss, conn_info->bssid, ETH_ALEN);
> +			memcpy(priv->associated_bss, conn_info->bssid,
> +			       ETH_ALEN);
>  

It basically always helps me if the "new function" changes are in a
patch by themselves.  These lines are a pure white space changes and I
have a script that reviews those for me instantly, but when it's mixed
with this patch I have to do it by hand.

regards,
dan carpenter