On Wed, Sep 19, 2018 at 02:17:30PM +0200, Tomislav Požega wrote: > On Wed, 19 Sep 2018 13:05:41 +0200, Stanislaw Gruszka wrote: > > >Driver should provide on what channels are supported to mac80211, but > >user space decide what channel to use and that imply band 2.4GHz or > >5GHz. ->curr_band is just shortcut for band of current channel. Is set > >in rt2x00lib_config() after we call rt2x00dev->ops->lib->config() > >[rt2800_config() for rt2800] . So patch is wrong. Either ->curr_band > >should be set before ->config call or we need to consistently use > >rf->channel <= 14 for band check in any rt2800_config() function and > >all it's subroutines. I prefer the second solution (i.e. rf->channel) > >and now I can see few places when we use ->curr_band, what is a bug. > > Works fine, no any kind of regression, especially not performance ones. Did you test on dual band devices ?
> So I don't see a reason to claim it is wrong or bug just because you > prefer current solution. It's not wrong bacause I prefer current solution. It's because ->curr_band is not updated when you call rt2800_config(). > >It's because ->curr_band initialize to 0 and NL80211_BAND_2GHZ > >happen to be 0. Also problem will not trigger on single band > >2.4GHz devices. > > Can you show us how will the problem trigger on dual band devices? When you switch from some 2.4GHz channel to 5GHz channel (or vice versa) ->curr_band will point to old band not the new one. To fix that you have to move curr_band assignemt before ->config() in rt2x00lib_config() i.e: rt2x00dev->curr_band = conf->chandef.chan->band; rt2x00dev->ops->lib->config(rt2x00dev, &libconf, ieee80211_flags); However I do not see the point of replacyng rf->channel check to ->curr_band check. What you can do is oposite thing, replace wrong usage of ->curr_band in very few places in rt2800_config() subroutines to rf->channel check. Also replacing spec->num_channels is wrong because this will stop reporting device support 5GHz band. Regards Stanislaw
