Return-path: Received: from na3sys009aog122.obsmtp.com ([74.125.149.147]:49587 "EHLO na3sys009aog122.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750808Ab1IZGZ1 (ORCPT ); Mon, 26 Sep 2011 02:25:27 -0400 Received: by mail-bw0-f43.google.com with SMTP id s6so7227134bka.30 for ; Sun, 25 Sep 2011 23:25:26 -0700 (PDT) Subject: Re: [PATCH] wl12xx: Add support for HW channel switch From: Luciano Coelho To: "Levi, Shahar" Cc: linux-wireless@vger.kernel.org In-Reply-To: References: <1315476093-27583-1-git-send-email-shahar_levi@ti.com> <1316698619.2157.628.camel@cumari> <1316775583.2157.728.camel@cumari> Content-Type: text/plain; charset="UTF-8" Date: Mon, 26 Sep 2011 09:25:22 +0300 Message-ID: <1317018322.2171.17.camel@cumari> (sfid-20110926_082537_249534_5859173A) Mime-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Sun, 2011-09-25 at 19:08 +0300, Levi, Shahar wrote: > On Fri, Sep 23, 2011 at 1:59 PM, Luciano Coelho 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? > > 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. -- Cheers, Luca.