From: Luciano Coelho <coelho@ti.com>
To: "Levi, Shahar" <shahar_levi@ti.com>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH] wl12xx: Add support for HW channel switch
Date: Mon, 26 Sep 2011 09:25:22 +0300 [thread overview]
Message-ID: <1317018322.2171.17.camel@cumari> (raw)
In-Reply-To: <CAHiDVwkeMTYdVubk=YPp6XTfNtmr2+mTwuX-ED0TcmzmrvEfUw@mail.gmail.com>
On Sun, 2011-09-25 at 19:08 +0300, Levi, Shahar wrote:
> On Fri, Sep 23, 2011 at 1:59 PM, Luciano Coelho <coelho@ti.com> 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.
next prev parent reply other threads:[~2011-09-26 6:25 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-08 10:01 [PATCH] wl12xx: Add support for HW channel switch Shahar Levi
2011-09-22 13:36 ` Luciano Coelho
2011-09-23 10:59 ` Luciano Coelho
2011-09-25 16:08 ` Levi, Shahar
2011-09-26 6:25 ` Luciano Coelho [this message]
2011-09-26 7:23 ` Victor Goldenshtein
2011-09-27 5:28 ` Luciano Coelho
2011-09-26 10:29 ` Levi, Shahar
2011-09-27 5:33 ` Luciano Coelho
2011-10-06 13:17 ` Luciano Coelho
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1317018322.2171.17.camel@cumari \
--to=coelho@ti.com \
--cc=linux-wireless@vger.kernel.org \
--cc=shahar_levi@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).