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

Reply via email to