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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel