On Mon, Sep 26, 2011 at 9:25 AM, Luciano Coelho <coelho@xxxxxx> wrote:
> On Sun, 2011-09-25 at 19:08 +0300, Levi, Shahar wrote:
>> On Fri, Sep 23, 2011 at 1:59 PM, Luciano Coelho <coelho@xxxxxx> wrote:
>> > On Thu, 2011-09-22 at 16:36 +0300, Luciano Coelho wrote:
>> >> On Thu, 2011-09-08 at 13:01 +0300, Shahar Levi wrote:
>> >> > +static void wl12xx_op_channel_switch(struct ieee80211_hw *hw,
>> >> > + struct ieee80211_channel_switch *ch_switch)
>> >> > +{
>> >> > + struct wl1271 *wl = hw->priv;
>> >> > + int ret;
>> >> > +
>> >> > + wl1271_debug(DEBUG_MAC80211, "mac80211 channel switch");
>> >> > +
>> >> > + mutex_lock(&wl->mutex);
>> >> > +
>> >> > + if (unlikely(wl->state == WL1271_STATE_OFF)) {
>> >> > + mutex_unlock(&wl->mutex);
>> >> > + ieee80211_chswitch_done(wl->vif, false);
>> >> > + return;
>> >> > + }
>> >> > +
>> >> > + ret = wl1271_ps_elp_wakeup(wl);
>> >> > + if (ret < 0)
>> >> > + goto out;
>> >> > +
>> >> > + /* send all pending packets */
>> >> > + wl1271_tx_work_locked(wl);
>> >>
>> >> Is this really needed? Can anyone tell why?
>> I have re- discussed with the FW guys and it seems that when using HW
>> channel switch sending all TX packet isn't needed.
>
> As I suspected. ;)
>
>
>> > I checked this a bit further and I don't see the point in sending all
>> > packets here. In fact, if block_tx is set, we can't even send any more
>> > frames until the switch happens (or fails). So we should at least check
>> > this here.
>> The block_tx is taken into account in the mac layer.
>
> This is a bit strange, though. I checked the code and nobody seems to
> be using the block_tx element of ieee80211_channel_switch, so I wonder
> why it's there at all.
>
> In any case, wouldn't it be a good idea for us to use it and send it to
> the firmware, since our firmware supports it? This way we would also
> block TX packets that are queued in the firmware, wouldn't we?
The FW support blocking TX in case of block_tx. The sepc definition is
to block TX in case of block_tx. Is that peer respectability?
>
>> > I'm leaving this patch out for now until I understand this better.
>> Do you prefer me to set v2 without that line or you could fix that in
>> the apply stage?
>
> No need to send v2. It seems that Victor will need this change for
> something else he's working on, so I guess he can take it over once it
> is needed. I don't want to include this unless we have a good reason to
> do it.
There is a good reason to include this:
a) It solve STA calibration issue in the FW in case of SW channel
switch (Rx issues)
b) This is FW support for channel switch with one command instead of
using several commands: rate_policies, roc \ join...
Our latest version pass full test cycle with that fix that solve CS issues.
> --
> Cheers,
> Luca.
All the best,
Shahar
--
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
