On Sun, 2020-09-20 at 22:06 -0700, Thomas Pedersen wrote:
>
> > > + /* For scanning on the S1G band, ignore scan_width (which is
> > > constant
> > > + * across all channels) for now since channel width is specific to
> > > each
> > > + * channel. Detect the required channel width here and likely
> > > revisit
> > > + * later. Maybe scan_width could be used to build the channel scan
> > > list?
> > > + */
> > > + if (chan->band == NL80211_BAND_S1GHZ) {
> > > + local->scan_chandef.width = ieee80211_s1g_channel_width(chan);
> > > + goto set_channel;
> > > + }
> >
> > nit: double space after 'goto'
> >
> > but really I came to say that this probably changes then, if you don't
> > convince me about the stuff in the previous patch review? :)
> >
> > So I'm leaving this patch also for now - have applied 1-5 so far.
>
> Thanks. I'm not really sure what else would make sense here?
> scan_req->scan_width is constant across all channels in
> scan_req->channels so for S1G we can either filter the scan_req channels
> list based on scan_width (kind of strange and unexpected), or deduce the
> correct chanenl width for each channel in the list and ignore scan_width
> (mostly correct). It seems like scan_width is currently only used for
> scanning at 5 or 10MHz anyway?
Yeah, that's true, it's sort of undefined if you're not in 5 or 10, and
then we currently assume it's 20, but obviously for S1G we should assume
then it's 1 MHz or something.
FWIW, here's probably where I thought you have a unique (freq, bw) tuple
and thus shouldn't really need to have the parsing in the other patch?
But I never looked at the spec so far ...
Anyway, wrt. the code here, I think perhaps you should just simply
remove the reference to scan_width? I'm not sure what you'd really do
with it, since it's a 5/10 MHz thing?
TBH though, offhand I don't even know how the 5/10 MHz scanning is
supposed to work?
johannes
