On 7 May 2014 10:59, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
> On Wed, 2014-05-07 at 09:04 +0200, Michal Kazior wrote:
>
>> > However, there's one more thing - one caller of ieee80211_csa_finalize()
>> > doesn't check the return value now, I think it probably should. Mind
>> > taking a look at that? It should *probably* be the same as the other
>> > caller (and not propagate the error), I'd say, so maybe that should be
>> > rolled into the function?
>>
>> Right - the return value of ieee80211_csa_finalize() in
>> __ieee80211_channel_switch() should be checked against. I think it
>> should be like:
>>
>> err = ieee80211_csa_finalize(sdata);
>> if (err) {
>> sdata_info(sdata, "failed to finalize immediate CSA, disconnecting\n");
>> cfg80211_stop_iface(local->hw.wiphy, &sdata->wdev, GFP_KERNEL);
>>
>> /* don't propagate the error here - stop the iface instead since
>> there's no way to revert CS now */
>> err = 0;
>> }
>>
>> Should I re-post the patch(set?), post a follow up, or are you going
>> to update the patch yourself?
>
> Can you post a separate patch on top? I already have it in my tree, but
> I can squash it I guess if I really want to.
>
>> I'm not sure what you mean by rolling "that" into the function. You
>> want to put the stop_iface() in csa_finalize()? I think it's going to
>> look a bit ugly that way.
>
> Well, the only other caller of it right now does the same thing:
>
> err = ieee80211_csa_finalize(sdata);
> if (err) {
> sdata_info(sdata, "failed to finalize CSA, disconnecting
> \n");
> cfg80211_stop_iface(local->hw.wiphy, &sdata->wdev,
> GFP_KERNEL);
> goto unlock;
> }
>
> so it seems it would make sense to have that as common code, maybe in
> the ieee80211_csa_finalize() function or maybe by changing
> ieee80211_csa_finalize() to be __ieee80211_csa_finalize() and making a
> new ieee80211_csa_finalize() that does this step?
Yeah, this sounds reasonable.
At times I'm just out of ideas on function naming for CSA. There's
like 5 functions that perform various phases of CSA finalization :-)
> In fact, then we could get rid of the int return value again, so maybe
> better post a new version of this particular patch after all.
I'll re-spin the patch later then.
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
