On 21-11-2016 12:30, Arend Van Spriel wrote: > On 21-11-2016 12:19, Arend Van Spriel wrote: >> Hi Johannes, Luca, >> >> The gscan work made me look at scheduled scan and the implementation of >> it in brcmfmac. The driver ignored the interval parameter from >> user-space. Now I am fixing that. One thing is that our firmware has a >> minimum interval which can not be indicated in struct wiphy. The other >> issue is how the maximum interval is used in the nl80211.c. >> >> In nl80211_parse_sched_scan_plans() it is used against value passed in >> NL80211_ATTR_SCHED_SCAN_INTERVAL and NL80211_SCHED_SCAN_PLAN_INTERVAL. >> For the first one it caps the value to the maximum, but for the second >> one it returns -EINVAL. I suspect this is done because maximum interval >> was introduced with schedule scan plans, but it feels inconsistent. > > It also maybe simply wrong to cap. At least brcmfmac does not set the > maximum so it will always get interval being zero. Maybe better to do: > > if (wiphy->max_sched_scan_plan_interval && > request->scan_plans[0].interval > > wiphy->max_sched_scan_plan_interval) > return -EINVAL; > >> Thoughts? Digging deeper. Looking at v4.3 before introduction of sched_scan_plans. struct sched_scan_request::interval was specified in milliseconds! Below the drivers that I see having scheduled scan support: iwlmvm: cap interval, convert to seconds. ath6kl: cap to 1sec minimum, no max check, convert to seconds. wl12xx: no checking in driver, fw need milliseconds. wl18xx: no checking in driver, fw need milliseconds. The milliseconds conversion seems to be taken care of by multiplying with MSEC_PER_SEC in wl{12,18}xx drivers. It seems in 4.8 only iwlmvm set wiphy->max_sched_scan_plan_interval so other drivers will get interval of zero which only ath6kl handles. Regards, Arend