Johannes Berg wrote:
On Thu, 2012-06-28 at 08:04 +0200, Michal Kazior wrote:
In any case, I think you're turning it upside down. I think we should
get rid of local->oper_channel(_type) completely, and instead use the
channel contexts in mac80211 everywhere. If the driver doesn't implement
channel contexts it can only support a single channel. Thus, we can have
at most one channel context, so whenever a new context is added (there
could be zero) or any context is modified (the only one) we can set
hw.conf.channel and call hw_config() with the CHANNEL_CHANGE flag.
IOW, nothing in mac80211 would ever call hw_config() for the channel or
channel type change, it would all do channel contexts, but the channel
context code would see that if the driver doesn't support channel
contexts
1) there will be at most one context in mac80211
2) this context is programmed into the device by using hw_config()
instead of the context callbacks
Yes, this is more or less what I also had in mind. I was just thinking
about solving the issue of channel context and hw.conf.channel
consistency. If we switch a channel we either modify channel in channel
context directly (violating the immutability of channel contexts) or we
iterate and re-set the new channel on each interface (because
single-channel drivers may still have multiple interfaces and we
probably want to use sdata->vif.chanctx_conf->channel instead of
hw.conf.channel inside mac80211).
Now that I think about it I guess violating the immutability for the
single-channel case is okay. It would greatly simplify the code and we'd
just put a comment down in hw_config where the only violation would occur.
I'm not sure why we would violate it? The way I see it, you'd never
change the channel context channel since internally in mac80211 you'd
never want to see a different channel, just like today we use
local->oper_channel everywhere we'd then use sdata->vif.chanctx->channel
throughout, right?
I think the only thing we need to do is put something like this into
hw_config:
if (local->tmp_channel) {
local->hw.conf.channel = local->tmp_channel;
...
} else {
local->hw.conf.channel = chanctx->channel;
}
No?
Using sdata->vif.chanctx_conf->channel instead of local->oper_channel
doesn't make any sense to me.
Take ieee80211_tx() for example. It does:
tx.channel = local->hw.conf.channel;
We don't use oper_channel here, but hw.conf.channel. TX can happen on
different interfaces so for multi-channel operation it should be saying:
tx.channel = sdata->vif.chanctx_conf->channel;
In this case if we want to support the swscan/tmpchan through
hw_config() we need update the channel context's channel somehow.
I'm more thinking of hw.conf.channel becoming more of a backup value for
single-channel drivers while we internally focus on channel contexts.
--
Pozdrawiam / Best regards, Michal Kazior.
--
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
