>>>>> On 07/25/2011 08:29 AM, Eliad Peller wrote: >>>>>> >>>>>> The hw is currently not configured when going >>>>>> back on-channel. >>>>> >>>>> I am less sure about this patch. With the existing code, >>>>> I think it should catch going from on channel to off >>>>> and do the hw config properly. >>>>> >>>> IIUC, this code is responsible for going back on-channel (if there is >>>> no started work on the tmp_channel). >>>> >>>>> With your change it will also reconfig the hardware, but it will >>>>> reconfig even if we were already on-channel (if, for instance, >>>>> local->tmp_channel is oper-channel), right? >>>>> >>>>> Can you please explain in more detail how this code is >>>>> broken? >>>>> >>>> we should reconfigure the hardware iff the hardware is not configured >>>> to the operational channel. >>>> the current code doesn't handle it (e.g. oper_channel=1, >>>> tmp_channel=11, hw_channel=11. since >>>> ieee80211_cfg_on_oper_channel(local) == 0, the hw won't go back >>>> on-channel). >>> >>> If we are off-channel when entering that block of code, then tmp_channel >>> != NULL, and on_oper_chan will be false. >>> >> right. >> >>> Then, we set tmp_channel to NULL, which should make >>> ieee80211_cfg_on_oper_channel >>> true. >>> >> tmp_channel is NULL, but ieee80211_cfg_on_oper_channel() also checks for: >> >> /* Check current hardware-config against oper_channel. */ >> if ((local->oper_channel != local->hw.conf.channel) || >> (local->_oper_channel_type != local->hw.conf.channel_type)) >> return false; >> >> >> so it will return false, and hw_config won't happen. > > Ahh, ok, I see your point. > > Your fix should be more correct than the current code, but > I think it might still could cause hardware config when not needed. > That isn't really a bug, just less efficient. And, I'd have to > look at the code in detail to be certain. I'm trying to be on > vacation this week, but will poke at it when I get a chance. > > In the meantime, your patch is probably worth applying, and > should probably go to stable. Hopefully Johannes can review > it as well, as I obviously didn't get it all right the first time! > as i'm not sure in this patch either, i guess we should better wait for Johannes' review. thanks, Eliad. -- 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
