On 5/16/2025 1:35 PM, Johannes Berg wrote:
> On Wed, 2025-05-14 at 16:58 +0530, Raj Kumar Bhagat wrote:
>>
>> +static bool
>> +__ieee80211_is_scan_ongoing(struct wiphy *wiphy,
>> + struct ieee80211_local *local,
>> + struct cfg80211_chan_def *chandef)
>
> Any particular reason or the __ name? We usually have that for internal
> locking-related things, but here doesn't matter, and there's no non-__
> version either?
>
Next version will rename this function to "ieee80211_is_scan_ongoing()".
>> +{
>> + struct cfg80211_scan_request *scan_req;
>> + int chan_radio_idx, req_radio_idx;
>> + struct ieee80211_roc_work *roc;
>> + bool ret = false;
>> +
>> + if (!list_empty(&local->roc_list) || local->scanning)
>> + ret = true;
>> +
>> + if (wiphy->n_radio < 2)
>> + return ret;
>> +
>> + /*
>> + * Multiple HWs are grouped under same wiphy. If not scanning then
>> + * return now itself
>> + */
>> + if (!ret)
>> + return ret;
>
> I don't fully understand this logic, and certainly not the comment. You
> can certainly "return false" here anyway or something. And initialize
> ret = list_empty || scanning or something, the whole thing is hard to
> follow?
>
Thanks for suggestion, will simplify the above logic in next version.
>
>> + if (!list_empty(&local->roc_list)) {
>> + list_for_each_entry(roc, &local->roc_list, list) {
>
> There's no point in checking first before iterating, it's perfectly fine
> to iterate an empty list and do nothing while doing so ...
>
Sure, will do in next version.
>
> Also patch-order wise, it seems this one really should go before the
> 2nd?
>
Sure will update the patch order as suggested.
