WL12xx FW supports HW channel switch mechanism.
Add HW channel switch support via channel_switch ops.
Signed-off-by: Shahar Levi <[email protected]>
---
Dependencies:
- That patches have a runtime dependency on commit
"[PATCH] mac80211: Update device channel in case of HW channel
switch supported"
drivers/net/wireless/wl12xx/boot.c | 3 +-
drivers/net/wireless/wl12xx/cmd.c | 58 ++++++++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/cmd.h | 20 ++++++++++++
drivers/net/wireless/wl12xx/event.c | 15 +++++++++
drivers/net/wireless/wl12xx/main.c | 40 +++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl12xx.h | 1 +
6 files changed, 136 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/boot.c b/drivers/net/wireless/wl12xx/boot.c
index 6d5664b..9b40027 100644
--- a/drivers/net/wireless/wl12xx/boot.c
+++ b/drivers/net/wireless/wl12xx/boot.c
@@ -503,7 +503,8 @@ static int wl1271_boot_run_firmware(struct wl1271 *wl)
BA_SESSION_RX_CONSTRAINT_EVENT_ID |
REMAIN_ON_CHANNEL_COMPLETE_EVENT_ID |
INACTIVE_STA_EVENT_ID |
- MAX_TX_RETRY_EVENT_ID;
+ MAX_TX_RETRY_EVENT_ID |
+ CHANNEL_SWITCH_COMPLETE_EVENT_ID;
ret = wl1271_event_unmask(wl);
if (ret < 0) {
diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
index 817bc18..a94bb3a 100644
--- a/drivers/net/wireless/wl12xx/cmd.c
+++ b/drivers/net/wireless/wl12xx/cmd.c
@@ -1688,3 +1688,61 @@ int wl12xx_croc(struct wl1271 *wl, u8 role_id)
out:
return ret;
}
+
+int wl12xx_cmd_channel_switch(struct wl1271 *wl,
+ struct ieee80211_channel_switch *ch_switch)
+{
+ struct wl12xx_cmd_channel_switch *cmd;
+ int ret;
+
+ wl1271_debug(DEBUG_ACX, "cmd channel switch");
+
+ cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
+ if (!cmd) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ cmd->channel = ch_switch->channel->hw_value;
+ cmd->switch_time = ch_switch->count;
+ cmd->tx_suspend = ch_switch->block_tx;
+ cmd->flush = 0;
+
+ ret = wl1271_cmd_send(wl, CMD_CHANNEL_SWITCH, cmd, sizeof(*cmd), 0);
+ if (ret < 0) {
+ wl1271_error("failed to send channel switch command");
+ goto out_free;
+ }
+
+out_free:
+ kfree(cmd);
+
+out:
+ return ret;
+}
+
+int wl12xx_cmd_stop_channel_switch(struct wl1271 *wl)
+{
+ struct wl12xx_cmd_stop_channel_switch *cmd;
+ int ret;
+
+ wl1271_debug(DEBUG_ACX, "cmd stop channel switch");
+
+ cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
+ if (!cmd) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ ret = wl1271_cmd_send(wl, CMD_STOP_CHANNEL_SWICTH, cmd, sizeof(*cmd), 0);
+ if (ret < 0) {
+ wl1271_error("failed to stop channel switch command");
+ goto out_free;
+ }
+
+out_free:
+ kfree(cmd);
+
+out:
+ return ret;
+}
diff --git a/drivers/net/wireless/wl12xx/cmd.h b/drivers/net/wireless/wl12xx/cmd.h
index 22c2f37..8eff809 100644
--- a/drivers/net/wireless/wl12xx/cmd.h
+++ b/drivers/net/wireless/wl12xx/cmd.h
@@ -79,6 +79,9 @@ int wl12xx_cmd_remove_peer(struct wl1271 *wl, u8 hlid);
int wl12xx_cmd_config_fwlog(struct wl1271 *wl);
int wl12xx_cmd_start_fwlog(struct wl1271 *wl);
int wl12xx_cmd_stop_fwlog(struct wl1271 *wl);
+int wl12xx_cmd_channel_switch(struct wl1271 *wl,
+ struct ieee80211_channel_switch *ch_switch);
+int wl12xx_cmd_stop_channel_switch(struct wl1271 *wl);
enum wl1271_commands {
CMD_INTERROGATE = 1, /*use this to read information elements*/
@@ -670,4 +673,21 @@ struct wl12xx_cmd_stop_fwlog {
struct wl1271_cmd_header header;
} __packed;
+struct wl12xx_cmd_channel_switch {
+ struct wl1271_cmd_header header;
+
+ /* The new serving channel */
+ u8 channel;
+ /* Relative time of the serving channel switch in TBTT units */
+ u8 switch_time;
+ /* 1: Suspend TX till switch time; 0: Do not suspend TX */
+ u8 tx_suspend;
+ /* 1: Flush TX at switch time; 0: Do not flush */
+ u8 flush;
+} __packed;
+
+struct wl12xx_cmd_stop_channel_switch {
+ struct wl1271_cmd_header header;
+} __packed;
+
#endif /* __WL1271_CMD_H__ */
diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
index 0bd7b02..d173971 100644
--- a/drivers/net/wireless/wl12xx/event.c
+++ b/drivers/net/wireless/wl12xx/event.c
@@ -291,6 +291,21 @@ static int wl1271_event_process(struct wl1271 *wl, struct event_mailbox *mbox)
wl1271_stop_ba_event(wl, mbox->rx_ba_allowed);
}
+ if ((vector & CHANNEL_SWITCH_COMPLETE_EVENT_ID) && !is_ap) {
+ wl1271_debug(DEBUG_EVENT, "CHANNEL_SWITCH_COMPLETE_EVENT_ID. "
+ "status = 0x%x",
+ mbox->channel_switch_status);
+ /*
+ * That event uses for two cases:
+ * 1) channel switch complete with status=0
+ * 2) channel switch failed status=1
+ */
+ if (test_and_clear_bit(WL1271_FLAG_CS_PROGRESS, &wl->flags) &&
+ (wl->vif))
+ ieee80211_chswitch_done(wl->vif,
+ mbox->channel_switch_status ? false : true);
+ }
+
if ((vector & DUMMY_PACKET_EVENT_ID)) {
wl1271_debug(DEBUG_EVENT, "DUMMY_PACKET_ID_EVENT_ID");
if (wl->vif)
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index bde8402..cfdb719 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2183,6 +2183,11 @@ static int wl1271_unjoin(struct wl1271 *wl)
{
int ret;
+ if (test_and_clear_bit(WL1271_FLAG_CS_PROGRESS, &wl->flags)) {
+ wl12xx_cmd_stop_channel_switch(wl);
+ ieee80211_chswitch_done(wl->vif, false);
+ }
+
/* to stop listening to a channel, we disconnect */
ret = wl12xx_cmd_role_stop_sta(wl);
if (ret < 0)
@@ -3950,6 +3955,40 @@ out:
return ret;
}
+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);
+
+ ret = wl12xx_cmd_channel_switch(wl, ch_switch);
+
+ if (!ret)
+ set_bit(WL1271_FLAG_CS_PROGRESS, &wl->flags);
+
+ wl1271_ps_elp_sleep(wl);
+
+out:
+ mutex_unlock(&wl->mutex);
+}
+
static bool wl1271_tx_frames_pending(struct ieee80211_hw *hw)
{
struct wl1271 *wl = hw->priv;
@@ -4231,6 +4270,7 @@ static const struct ieee80211_ops wl1271_ops = {
.sta_remove = wl1271_op_sta_remove,
.ampdu_action = wl1271_op_ampdu_action,
.tx_frames_pending = wl1271_tx_frames_pending,
+ .channel_switch = wl12xx_op_channel_switch,
CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
};
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index fb2753c..8f9cb95 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -348,6 +348,7 @@ enum wl12xx_flags {
WL1271_FLAG_SOFT_GEMINI,
WL1271_FLAG_RX_STREAMING_STARTED,
WL1271_FLAG_RECOVERY_IN_PROGRESS,
+ WL1271_FLAG_CS_PROGRESS,
};
struct wl1271_link {
--
1.7.1
On Fri, Sep 23, 2011 at 1:59 PM, Luciano Coelho <[email protected]> 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:
>> > WL12xx FW supports HW channel switch mechanism.
>> > Add HW channel switch support via channel_switch ops.
>> >
>> > Signed-off-by: Shahar Levi <[email protected]>
>> > ---
>>
>> [...]
Hi Luca,
>>
>> > +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.
>
> 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.
>
> 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?
>
> --
> Cheers,
> Luca.
Thanks for your comment.
All the best,
Shahar
On Thu, 2011-09-08 at 13:01 +0300, Shahar Levi wrote:
> WL12xx FW supports HW channel switch mechanism.
> Add HW channel switch support via channel_switch ops.
>
> Signed-off-by: Shahar Levi <[email protected]>
> ---
[...]
> +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?
--
Cheers,
Luca.
On Thu, 2011-09-22 at 16:36 +0300, Luciano Coelho wrote:
> On Thu, 2011-09-08 at 13:01 +0300, Shahar Levi wrote:
> > WL12xx FW supports HW channel switch mechanism.
> > Add HW channel switch support via channel_switch ops.
> >
> > Signed-off-by: Shahar Levi <[email protected]>
> > ---
>
> [...]
>
> > +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 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.
I'm leaving this patch out for now until I understand this better.
--
Cheers,
Luca.
On Mon, Sep 26, 2011 at 9:25 AM, Luciano Coelho <[email protected]> wrote:
> On Sun, 2011-09-25 at 19:08 +0300, Levi, Shahar wrote:
>> On Fri, Sep 23, 2011 at 1:59 PM, Luciano Coelho <[email protected]> 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
On Sun, 2011-09-25 at 19:08 +0300, Levi, Shahar wrote:
> On Fri, Sep 23, 2011 at 1:59 PM, Luciano Coelho <[email protected]> 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.
On Mon, 2011-09-26 at 13:29 +0300, Levi, Shahar wrote:
> On Mon, Sep 26, 2011 at 9:25 AM, Luciano Coelho <[email protected]> wrote:
> > On Sun, 2011-09-25 at 19:08 +0300, Levi, Shahar wrote:
> >> On Fri, Sep 23, 2011 at 1:59 PM, Luciano Coelho <[email protected]> wrote:
> >> > 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)
This is a good reason (fixing a bug) and should be mentioned in the
commit description. Do we have more detailed information on this?
> b) This is FW support for channel switch with one command instead of
> using several commands: rate_policies, roc \ join...
This is an optimization and not entirely mandatory. This is the only
thing I thought this patch was addressing (the commit message was not
very descriptive, so I could only assume). At this stage (ie. close to
the merge window), I didn't want to assume the risk of a change just for
to optimize things a bit. And, since I had my doubts about the block_tx
value, I decided not to include it.
> Our latest version pass full test cycle with that fix that solve CS issues.
Okay, once I get the answer from the firmware team about the block_tx
value, I'll fix up this patch and apply it.
Thanks for the explanantions.
--
Cheers,
Luca.
On 26/09/2011 09:25, Luciano Coelho wrote:
>>> 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.
>
As far as I know the block_tx is for the DFS channel switch
implementation, when block_tx=1, the firmware will not start tx on new
channel.
Victor.
On Mon, 2011-09-26 at 10:23 +0300, Victor Goldenshtein wrote:
> On 26/09/2011 09:25, Luciano Coelho wrote:
> >>> 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.
> >
>
> As far as I know the block_tx is for the DFS channel switch
> implementation, when block_tx=1, the firmware will not start tx on new
> channel.
I'll check this up with our firmware team.
--
Cheers,
Luca.
On Thu, 2011-09-08 at 13:01 +0300, Shahar Levi wrote:
> WL12xx FW supports HW channel switch mechanism.
> Add HW channel switch support via channel_switch ops.
>
> Signed-off-by: Shahar Levi <[email protected]>
> ---
Applied with some modifications and a rephrased commit message.
--
Cheers,
Luca.