[PATCH 00/15] wlcore: prepare to multi-channel

[PATCH 00/15] wlcore: prepare to multi-channel

[Eliad Peller]

Make the connection flow simpler by using only the sta
role (i.e. no need for dev role).

This is possible now, as we know the AP's BSSID even
before authentication.

This means we no longer start dev role when the device
is non-idle, so now we use the .remain_on_channel()
and start device role only then.

Finally, implement the chanctx callbacks in order to
have a clear per-vif channel (instead of the global
hw->channel)

(this patchset depends on
"mac80211: make remain_on_channel() op pass vif param")

Eliad Peller (15):
  wlcore: start sta role on CHANGED_BSSID
  wlcore: set ssid before starting station role
  wlcore: always use sta.hlid
  wlcore: workaround start_sta problem in wl12xx fw
  wlcore: implement .remain_on_channel() callback
  wlcore: use dev_hlid if the tx is offchannel
  wlcore: get channel from bss_conf instead of hw->conf
  wlcore: add chanctx implementation
  wlcore: remove channel handling from op_config
  wlcore: initiate ROC/CROC on sta state updates
  wlcore: set active psm on association
  wlcore: specify correct supported_rates
  wlcore: reconfigure rate policy on association
  wlcore: refactor CHANGED_HT handling
  wlcore: configure the remote rates with our own rates

 drivers/net/wireless/ti/wl12xx/main.c   |    9 +-
 drivers/net/wireless/ti/wlcore/cmd.c    |   56 ++--
 drivers/net/wireless/ti/wlcore/cmd.h    |    6 +-
 drivers/net/wireless/ti/wlcore/event.c  |    7 +
 drivers/net/wireless/ti/wlcore/main.c   |  663 ++++++++++++++++++-------------
 drivers/net/wireless/ti/wlcore/tx.c     |   14 +-
 drivers/net/wireless/ti/wlcore/wlcore.h |    6 +
 7 files changed, 454 insertions(+), 307 deletions(-)

-- 
1.7.6.401.g6a319

[PATCH 01/15] wlcore: start sta role on CHANGED_BSSID

[Eliad Peller]

Make the connection flow simpler by starting
sta role on bssid change.

Currently, we start dev role when going idle-off,
and start the sta role only after association
indication. This complicates the connection
flow with some possible intermediate states.

Make it simpler by starting sta role on bssid change,
which now happens *before* auth req get sent.

Update the handling of mac80211's notifications
and change wl1271_join/unjoin accordingly -
* Split wl1271_join() into wlcore_join (tuning on
  a channel/bssid) and wlcore_set_assoc (configure
  sta after association).
* Rename wl1271_unjoin() to wlcore_disconnect(), as
  it is no longer the inversion of wl1271_join()
  (now it's only used to disconnect associated sta /
  joined ibss, without stopping the role).

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/main.c |  233 +++++++++++++--------------------
 1 files changed, 88 insertions(+), 145 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 41ed1d5..41a8502 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2468,8 +2468,7 @@ static int wl12xx_op_change_interface(struct ieee80211_hw *hw,
 	return ret;
 }
 
-static int wl1271_join(struct wl1271 *wl, struct wl12xx_vif *wlvif,
-			  bool set_assoc)
+static int wlcore_join(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 {
 	int ret;
 	bool is_ibss = (wlvif->bss_type == BSS_TYPE_IBSS);
@@ -2489,18 +2488,17 @@ static int wl1271_join(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 	/* clear encryption type */
 	wlvif->encryption_type = KEY_NONE;
 
-	if (set_assoc)
-		set_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags);
-
 	if (is_ibss)
 		ret = wl12xx_cmd_role_start_ibss(wl, wlvif);
 	else
 		ret = wl12xx_cmd_role_start_sta(wl, wlvif);
-	if (ret < 0)
-		goto out;
 
-	if (!test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags))
-		goto out;
+	return ret;
+}
+
+static int wlcore_set_assoc(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+{
+	int ret;
 
 	/*
 	 * The join command disable the keep-alive mode, shut down its process,
@@ -2525,15 +2523,12 @@ static int wl1271_join(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 					   ACX_KEEP_ALIVE_TPL_VALID);
 	if (ret < 0)
 		goto out;
-
 out:
 	return ret;
 }
 
-static int wl1271_unjoin(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+static void wlcore_disconnect(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 {
-	int ret;
-
 	if (test_and_clear_bit(WLVIF_FLAG_CS_PROGRESS, &wlvif->flags)) {
 		struct ieee80211_vif *vif = wl12xx_wlvif_to_vif(wlvif);
 
@@ -2546,17 +2541,9 @@ static int wl1271_unjoin(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 				     wlvif->sta.klv_template_id,
 				     ACX_KEEP_ALIVE_TPL_INVALID);
 
-	/* to stop listening to a channel, we disconnect */
-	ret = wl12xx_cmd_role_stop_sta(wl, wlvif);
-	if (ret < 0)
-		goto out;
-
 	/* reset TX security counters on a clean disconnect */
 	wlvif->tx_security_last_seq_lsb = 0;
 	wlvif->tx_security_seq = 0;
-
-out:
-	return ret;
 }
 
 static void wl1271_set_band_rate(struct wl1271 *wl, struct wl12xx_vif *wlvif)
@@ -2565,45 +2552,6 @@ static void wl1271_set_band_rate(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 	wlvif->rate_set = wlvif->basic_rate_set;
 }
 
-static int wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
-				  bool idle)
-{
-	int ret;
-	bool cur_idle = !test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
-
-	if (idle == cur_idle)
-		return 0;
-
-	if (idle) {
-		/* no need to croc if we weren't busy (e.g. during boot) */
-		if (wl12xx_dev_role_started(wlvif)) {
-			ret = wl12xx_stop_dev(wl, wlvif);
-			if (ret < 0)
-				goto out;
-		}
-		wlvif->rate_set =
-			wl1271_tx_min_rate_get(wl, wlvif->basic_rate_set);
-		ret = wl1271_acx_sta_rate_policies(wl, wlvif);
-		if (ret < 0)
-			goto out;
-		clear_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
-	} else {
-		/* The current firmware only supports sched_scan in idle */
-		if (wl->sched_scanning) {
-			wl1271_scan_sched_scan_stop(wl, wlvif);
-			ieee80211_sched_scan_stopped(wl->hw);
-		}
-
-		ret = wl12xx_start_dev(wl, wlvif);
-		if (ret < 0)
-			goto out;
-		set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
-	}
-
-out:
-	return ret;
-}
-
 static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 			     struct ieee80211_conf *conf, u32 changed)
 {
@@ -3833,8 +3781,10 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
 			ibss_joined = true;
 		} else {
 			if (test_and_clear_bit(WLVIF_FLAG_IBSS_JOINED,
-					       &wlvif->flags))
-				wl1271_unjoin(wl, wlvif);
+					       &wlvif->flags)) {
+				wlcore_disconnect(wl, wlvif);
+				wl12xx_cmd_role_stop_sta(wl, wlvif);
+			}
 		}
 	}
 
@@ -3852,12 +3802,6 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
 		do_join = true;
 	}
 
-	if (changed & BSS_CHANGED_IDLE && !is_ibss) {
-		ret = wl1271_sta_handle_idle(wl, wlvif, bss_conf->idle);
-		if (ret < 0)
-			wl1271_warning("idle mode change failed %d", ret);
-	}
-
 	if ((changed & BSS_CHANGED_CQM)) {
 		bool enable = false;
 		if (bss_conf->cqm_rssi_thold)
@@ -3870,18 +3814,7 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
 		wlvif->rssi_thold = bss_conf->cqm_rssi_thold;
 	}
 
-	if (changed & BSS_CHANGED_BSSID)
-		if (!is_zero_ether_addr(bss_conf->bssid)) {
-			ret = wl12xx_cmd_build_null_data(wl, wlvif);
-			if (ret < 0)
-				goto out;
-
-			ret = wl1271_build_qos_null_data(wl, vif);
-			if (ret < 0)
-				goto out;
-		}
-
-	if (changed & (BSS_CHANGED_ASSOC | BSS_CHANGED_HT)) {
+	if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT)) {
 		rcu_read_lock();
 		sta = ieee80211_find_sta(vif, bss_conf->bssid);
 		if (!sta)
@@ -3900,20 +3833,16 @@ sta_not_found:
 		rcu_read_unlock();
 	}
 
-	if ((changed & BSS_CHANGED_ASSOC)) {
-		if (bss_conf->assoc) {
+	if (changed & BSS_CHANGED_BSSID) {
+		if (!is_zero_ether_addr(bss_conf->bssid)) {
 			u32 rates;
-			int ieoffset;
-			wlvif->aid = bss_conf->aid;
-			wlvif->channel_type = bss_conf->channel_type;
-			wlvif->beacon_int = bss_conf->beacon_int;
-			do_join = true;
-			set_assoc = true;
+			wl1271_debug(DEBUG_MAC80211,
+			     "changed_bssid: %pM, aid: %d, bcn_int: %d, brates: 0x%x sta_rate_set: 0x%x (%d)",
+			     bss_conf->bssid, bss_conf->aid,
+			     bss_conf->beacon_int,
+			     bss_conf->basic_rates, sta_rate_set, sta_exists);
 
-			/*
-			 * use basic rates from AP, and determine lowest rate
-			 * to use with control frames.
-			 */
+			wlvif->beacon_int = bss_conf->beacon_int;
 			rates = bss_conf->basic_rates;
 			wlvif->basic_rate_set =
 				wl1271_tx_enabled_rates_get(wl, rates,
@@ -3921,15 +3850,63 @@ sta_not_found:
 			wlvif->basic_rate =
 				wl1271_tx_min_rate_get(wl,
 						       wlvif->basic_rate_set);
+
 			if (sta_rate_set)
 				wlvif->rate_set =
 					wl1271_tx_enabled_rates_get(wl,
 								sta_rate_set,
 								wlvif->band);
+
+			/* we only supports sched_scan while not connected */
+			if (wl->sched_scanning) {
+				wl1271_scan_sched_scan_stop(wl, wlvif);
+				ieee80211_sched_scan_stopped(wl->hw);
+			}
+
 			ret = wl1271_acx_sta_rate_policies(wl, wlvif);
 			if (ret < 0)
 				goto out;
 
+			ret = wl12xx_cmd_build_null_data(wl, wlvif);
+			if (ret < 0)
+				goto out;
+
+			ret = wl1271_build_qos_null_data(wl, vif);
+			if (ret < 0)
+				goto out;
+
+			/* Need to update the BSSID (for filtering etc) */
+			set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
+			do_join = true;
+		} else {
+			/* revert back to minimum rates for the current band */
+			wl1271_set_band_rate(wl, wlvif);
+			wlvif->basic_rate =
+				wl1271_tx_min_rate_get(wl,
+						       wlvif->basic_rate_set);
+			ret = wl1271_acx_sta_rate_policies(wl, wlvif);
+			if (ret < 0)
+				goto out;
+
+			if (!is_ibss &&
+			    test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags)) {
+				ret = wl12xx_cmd_role_stop_sta(wl, wlvif);
+				if (ret < 0)
+					goto out;
+			}
+			clear_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
+		}
+	}
+
+	if ((changed & BSS_CHANGED_ASSOC)) {
+		if (bss_conf->assoc) {
+			int ieoffset;
+			wlvif->aid = bss_conf->aid;
+			wlvif->channel_type = bss_conf->channel_type;
+			wlvif->beacon_int = bss_conf->beacon_int;
+
+			set_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags);
+
 			/*
 			 * with wl1271, we don't need to update the
 			 * beacon_int and dtim_period, because the firmware
@@ -3955,64 +3932,32 @@ sta_not_found:
 			ret = wl1271_acx_conn_monit_params(wl, wlvif, true);
 			if (ret < 0)
 				goto out;
+
+			set_assoc = true;
 		} else {
-			/* use defaults when not associated */
 			bool was_assoc =
 			    !!test_and_clear_bit(WLVIF_FLAG_STA_ASSOCIATED,
 						 &wlvif->flags);
-			bool was_ifup =
-			    !!test_and_clear_bit(WLVIF_FLAG_STA_STATE_SENT,
-						 &wlvif->flags);
+
+			/* use defaults when not associated */
 			wlvif->aid = 0;
 
 			/* free probe-request template */
 			dev_kfree_skb(wlvif->probereq);
 			wlvif->probereq = NULL;
 
-			/* revert back to minimum rates for the current band */
-			wl1271_set_band_rate(wl, wlvif);
-			wlvif->basic_rate =
-				wl1271_tx_min_rate_get(wl,
-						       wlvif->basic_rate_set);
-			ret = wl1271_acx_sta_rate_policies(wl, wlvif);
-			if (ret < 0)
-				goto out;
-
 			/* disable connection monitor features */
 			ret = wl1271_acx_conn_monit_params(wl, wlvif, false);
+			if (ret < 0)
+				goto out;
 
 			/* Disable the keep-alive feature */
 			ret = wl1271_acx_keep_alive_mode(wl, wlvif, false);
 			if (ret < 0)
 				goto out;
 
-			/* restore the bssid filter and go to dummy bssid */
-			if (was_assoc) {
-				/*
-				 * we might have to disable roc, if there was
-				 * no IF_OPER_UP notification.
-				 */
-				if (!was_ifup) {
-					ret = wl12xx_croc(wl, wlvif->role_id);
-					if (ret < 0)
-						goto out;
-				}
-				/*
-				 * (we also need to disable roc in case of
-				 * roaming on the same channel. until we will
-				 * have a better flow...)
-				 */
-				if (test_bit(wlvif->dev_role_id, wl->roc_map)) {
-					ret = wl12xx_croc(wl,
-							  wlvif->dev_role_id);
-					if (ret < 0)
-						goto out;
-				}
-
-				wl1271_unjoin(wl, wlvif);
-				if (!bss_conf->idle)
-					wl12xx_start_dev(wl, wlvif);
-			}
+			if (was_assoc)
+				wlcore_disconnect(wl, wlvif);
 		}
 	}
 
@@ -4042,7 +3987,7 @@ sta_not_found:
 		goto out;
 
 	if (do_join) {
-		ret = wl1271_join(wl, wlvif, set_assoc);
+		ret = wlcore_join(wl, wlvif);
 		if (ret < 0) {
 			wl1271_warning("cmd join failed %d", ret);
 			goto out;
@@ -4053,21 +3998,18 @@ sta_not_found:
 			ret = wl12xx_roc(wl, wlvif, wlvif->role_id);
 			if (ret < 0)
 				goto out;
-
-			if (test_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags))
-				wl12xx_set_authorized(wl, wlvif);
-		}
-		/*
-		 * stop device role if started (we might already be in
-		 * STA/IBSS role).
-		 */
-		if (wl12xx_dev_role_started(wlvif)) {
-			ret = wl12xx_stop_dev(wl, wlvif);
-			if (ret < 0)
-				goto out;
 		}
 	}
 
+	if (set_assoc) {
+		ret = wlcore_set_assoc(wl, wlvif);
+		if (ret < 0)
+			goto out;
+
+		if (test_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags))
+			wl12xx_set_authorized(wl, wlvif);
+	}
+
 	/* Handle new association with HT. Do this after join. */
 	if (sta_exists) {
 		if ((changed & BSS_CHANGED_HT) &&
@@ -4434,6 +4376,7 @@ static int wl12xx_update_sta_state(struct wl1271 *wl,
 	    old_state == IEEE80211_STA_AUTHORIZED &&
 	    new_state == IEEE80211_STA_ASSOC) {
 		clear_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags);
+		clear_bit(WLVIF_FLAG_STA_STATE_SENT, &wlvif->flags);
 		return 0;
 	}
 
-- 
1.7.6.401.g6a319

Re: [PATCH 01/15] wlcore: start sta role on CHANGED_BSSID

[Luciano Coelho]

On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> Make the connection flow simpler by starting
> sta role on bssid change.
> 
> Currently, we start dev role when going idle-off,
> and start the sta role only after association
> indication. This complicates the connection
> flow with some possible intermediate states.
> 
> Make it simpler by starting sta role on bssid change,
> which now happens *before* auth req get sent.
> 
> Update the handling of mac80211's notifications
> and change wl1271_join/unjoin accordingly -
> * Split wl1271_join() into wlcore_join (tuning on
>   a channel/bssid) and wlcore_set_assoc (configure
>   sta after association).
> * Rename wl1271_unjoin() to wlcore_disconnect(), as
>   it is no longer the inversion of wl1271_join()
>   (now it's only used to disconnect associated sta /
>   joined ibss, without stopping the role).
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---

[...]

> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
> index 41ed1d5..41a8502 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -2468,8 +2468,7 @@ static int wl12xx_op_change_interface(struct ieee80211_hw *hw,

[...]

> -static int wl1271_unjoin(struct wl1271 *wl, struct wl12xx_vif *wlvif)
> +static void wlcore_disconnect(struct wl1271 *wl, struct wl12xx_vif *wlvif)

Why not wlcore_unset_assoc()? Or change the wlcore_set_assoc() to
wlcore_connect()?

[...]

> @@ -3921,15 +3850,63 @@ sta_not_found:
>  			wlvif->basic_rate =
>  				wl1271_tx_min_rate_get(wl,
>  						       wlvif->basic_rate_set);
> +
>  			if (sta_rate_set)
>  				wlvif->rate_set =
>  					wl1271_tx_enabled_rates_get(wl,
>  								sta_rate_set,
>  								wlvif->band);
> +
> +			/* we only supports sched_scan while not connected */

s/supports/support/

> +			if (wl->sched_scanning) {
> +				wl1271_scan_sched_scan_stop(wl, wlvif);
> +				ieee80211_sched_scan_stopped(wl->hw);
> +			}
> +
>  			ret = wl1271_acx_sta_rate_policies(wl, wlvif);
>  			if (ret < 0)
>  				goto out;
>  
> +			ret = wl12xx_cmd_build_null_data(wl, wlvif);
> +			if (ret < 0)
> +				goto out;
> +
> +			ret = wl1271_build_qos_null_data(wl, vif);
> +			if (ret < 0)
> +				goto out;
> +
> +			/* Need to update the BSSID (for filtering etc) */
> +			set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
> +			do_join = true;
> +		} else {
> +			/* revert back to minimum rates for the current band */
> +			wl1271_set_band_rate(wl, wlvif);
> +			wlvif->basic_rate =
> +				wl1271_tx_min_rate_get(wl,
> +						       wlvif->basic_rate_set);
> +			ret = wl1271_acx_sta_rate_policies(wl, wlvif);
> +			if (ret < 0)
> +				goto out;
> +
> +			if (!is_ibss &&
> +			    test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags)) {
> +				ret = wl12xx_cmd_role_stop_sta(wl, wlvif);
> +				if (ret < 0)
> +					goto out;
> +			}
> +			clear_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
> +		}
> +	}

It would be nice to keep all this stuff in a separate function instead
of bringing it back into this already-huge wl1271_bss_info_changed_sta()
function.


> +	if ((changed & BSS_CHANGED_ASSOC)) {

Same thing for this if, but you already did a great job removing things
from here. ;)

> +		if (bss_conf->assoc) {
> +			int ieoffset;
> +			wlvif->aid = bss_conf->aid;
> +			wlvif->channel_type = bss_conf->channel_type;
> +			wlvif->beacon_int = bss_conf->beacon_int;
> +
> +			set_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags);

Wouldn't it be better to put this in the set_assoc() function?

--
Luca.

Re: [PATCH 01/15] wlcore: start sta role on CHANGED_BSSID

[Eliad Peller]

On Mon, Nov 19, 2012 at 8:14 PM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> Make the connection flow simpler by starting
>> sta role on bssid change.
>>
>> Currently, we start dev role when going idle-off,
>> and start the sta role only after association
>> indication. This complicates the connection
>> flow with some possible intermediate states.
>>
>> Make it simpler by starting sta role on bssid change,
>> which now happens *before* auth req get sent.
>>
>> Update the handling of mac80211's notifications
>> and change wl1271_join/unjoin accordingly -
>> * Split wl1271_join() into wlcore_join (tuning on
>>   a channel/bssid) and wlcore_set_assoc (configure
>>   sta after association).
>> * Rename wl1271_unjoin() to wlcore_disconnect(), as
>>   it is no longer the inversion of wl1271_join()
>>   (now it's only used to disconnect associated sta /
>>   joined ibss, without stopping the role).
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>
> [...]
>
>> -static int wl1271_unjoin(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>> +static void wlcore_disconnect(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>
> Why not wlcore_unset_assoc()? Or change the wlcore_set_assoc() to
> wlcore_connect()?
>
i'll pick the first option.

>> +                     if (wl->sched_scanning) {
>> +                             wl1271_scan_sched_scan_stop(wl, wlvif);
>> +                             ieee80211_sched_scan_stopped(wl->hw);
>> +                     }
>> +
>>                       ret = wl1271_acx_sta_rate_policies(wl, wlvif);
>>                       if (ret < 0)
>>                               goto out;
>>
>> +                     ret = wl12xx_cmd_build_null_data(wl, wlvif);
>> +                     if (ret < 0)
>> +                             goto out;
>> +
>> +                     ret = wl1271_build_qos_null_data(wl, vif);
>> +                     if (ret < 0)
>> +                             goto out;
>> +
>> +                     /* Need to update the BSSID (for filtering etc) */
>> +                     set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
>> +                     do_join = true;
>> +             } else {
>> +                     /* revert back to minimum rates for the current band */
>> +                     wl1271_set_band_rate(wl, wlvif);
>> +                     wlvif->basic_rate =
>> +                             wl1271_tx_min_rate_get(wl,
>> +                                                    wlvif->basic_rate_set);
>> +                     ret = wl1271_acx_sta_rate_policies(wl, wlvif);
>> +                     if (ret < 0)
>> +                             goto out;
>> +
>> +                     if (!is_ibss &&
>> +                         test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags)) {
>> +                             ret = wl12xx_cmd_role_stop_sta(wl, wlvif);
>> +                             if (ret < 0)
>> +                                     goto out;
>> +                     }
>> +                     clear_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
>> +             }
>> +     }
>
> It would be nice to keep all this stuff in a separate function instead
> of bringing it back into this already-huge wl1271_bss_info_changed_sta()
> function.
>
sure.
i'll give this function some more overhaul.
this applies also for the other similar comments.

Eliad.

[PATCH 02/15] wlcore: set ssid before starting station role

[Eliad Peller]

start_role(station) takes the ssid as parameter, so set
the correct ssid on BSS_CHANGED_BSSID (just before
calling start_role(sta))

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/main.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 41a8502..2cb978e 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3390,6 +3390,23 @@ static int wl1271_ssid_set(struct ieee80211_vif *vif, struct sk_buff *skb,
 	return 0;
 }
 
+static int wlcore_set_ssid(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+{
+	struct ieee80211_vif *vif = wl12xx_wlvif_to_vif(wlvif);
+	struct sk_buff *skb;
+	int ieoffset;
+
+	skb = ieee80211_ap_probereq_get(wl->hw, vif);
+	if (!skb)
+		return -EINVAL;
+
+	ieoffset = offsetof(struct ieee80211_mgmt,
+			    u.probe_req.variable);
+	wl1271_ssid_set(vif, skb, ieoffset);
+	dev_kfree_skb(skb);
+
+	return 0;
+}
 static void wl12xx_remove_ie(struct sk_buff *skb, u8 eid, int ieoffset)
 {
 	int len;
@@ -3875,6 +3892,8 @@ sta_not_found:
 			if (ret < 0)
 				goto out;
 
+			wlcore_set_ssid(wl, wlvif);
+
 			/* Need to update the BSSID (for filtering etc) */
 			set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
 			do_join = true;
-- 
1.7.6.401.g6a319

[PATCH 03/15] wlcore: always use sta.hlid

[Eliad Peller]

since we no longer use dev role in the connection flow,
we should always use the hlid of the sta role.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/tx.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/tx.c b/drivers/net/wireless/ti/wlcore/tx.c
index a90d3cd..a555a70 100644
--- a/drivers/net/wireless/ti/wlcore/tx.c
+++ b/drivers/net/wireless/ti/wlcore/tx.c
@@ -155,21 +155,13 @@ static u8 wl12xx_tx_get_hlid_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 u8 wl12xx_tx_get_hlid(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 		      struct sk_buff *skb, struct ieee80211_sta *sta)
 {
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-
 	if (!wlvif || wl12xx_is_dummy_packet(wl, skb))
 		return wl->system_hlid;
 
 	if (wlvif->bss_type == BSS_TYPE_AP_BSS)
 		return wl12xx_tx_get_hlid_ap(wl, wlvif, skb, sta);
 
-	if ((test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags) ||
-	     test_bit(WLVIF_FLAG_IBSS_JOINED, &wlvif->flags)) &&
-	    !ieee80211_is_auth(hdr->frame_control) &&
-	    !ieee80211_is_assoc_req(hdr->frame_control))
-		return wlvif->sta.hlid;
-	else
-		return wlvif->dev_hlid;
+	return wlvif->sta.hlid;
 }
 
 unsigned int wlcore_calc_packet_alignment(struct wl1271 *wl,
-- 
1.7.6.401.g6a319

Re: [PATCH 03/15] wlcore: always use sta.hlid

[Julian Calaby]

Hi Eliad,

On Tue, Nov 20, 2012 at 3:39 AM, Eliad Peller <eliad@wizery.com> wrote:
> since we no longer use dev role in the connection flow,
> we should always use the hlid of the sta role.

I don't really know the hardware, but this seems like something
something that should be combined with the first patch.

> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
>  drivers/net/wireless/ti/wlcore/tx.c |   10 +---------
>  1 files changed, 1 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/tx.c b/drivers/net/wireless/ti/wlcore/tx.c
> index a90d3cd..a555a70 100644
> --- a/drivers/net/wireless/ti/wlcore/tx.c
> +++ b/drivers/net/wireless/ti/wlcore/tx.c
> @@ -155,21 +155,13 @@ static u8 wl12xx_tx_get_hlid_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif,
>  u8 wl12xx_tx_get_hlid(struct wl1271 *wl, struct wl12xx_vif *wlvif,
>                       struct sk_buff *skb, struct ieee80211_sta *sta)
>  {
> -       struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> -
>         if (!wlvif || wl12xx_is_dummy_packet(wl, skb))
>                 return wl->system_hlid;
>
>         if (wlvif->bss_type == BSS_TYPE_AP_BSS)
>                 return wl12xx_tx_get_hlid_ap(wl, wlvif, skb, sta);
>
> -       if ((test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags) ||
> -            test_bit(WLVIF_FLAG_IBSS_JOINED, &wlvif->flags)) &&
> -           !ieee80211_is_auth(hdr->frame_control) &&
> -           !ieee80211_is_assoc_req(hdr->frame_control))
> -               return wlvif->sta.hlid;
> -       else
> -               return wlvif->dev_hlid;
> +       return wlvif->sta.hlid;
>  }
>
>  unsigned int wlcore_calc_packet_alignment(struct wl1271 *wl,
> --
> 1.7.6.401.g6a319
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

Re: [PATCH 03/15] wlcore: always use sta.hlid

[Eliad Peller]

Hi Julian,

On Tue, Nov 20, 2012 at 12:39 AM, Julian Calaby <julian.calaby@gmail.com> wrote:
> On Tue, Nov 20, 2012 at 3:39 AM, Eliad Peller <eliad@wizery.com> wrote:
>> since we no longer use dev role in the connection flow,
>> we should always use the hlid of the sta role.
>
> I don't really know the hardware, but this seems like something
> something that should be combined with the first patch.
>
Thanks.
Johannes had a similar comment regarding another patch.
i split the patches in order to make them easier to review, even if it
temporarily breaks the driver.
As agreed with Luca, i'll send the next version (which will hopefully
be the last one ;), after squashing such patches together.

Eliad.

[PATCH 04/15] wlcore: workaround start_sta problem in wl12xx fw

[Eliad Peller]

for some reason, the wl12xx fw is not able to rx/tx
on the first start_sta cmd.
Workaround it by issuing a dummy start_sta + stop_sta
before starting the sta for the final time.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wl12xx/main.c   |    9 ++++++---
 drivers/net/wireless/ti/wlcore/main.c   |   15 ++++++++++++++-
 drivers/net/wireless/ti/wlcore/wlcore.h |    3 +++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ti/wl12xx/main.c b/drivers/net/wireless/ti/wl12xx/main.c
index dadf1db..6c9fba4 100644
--- a/drivers/net/wireless/ti/wl12xx/main.c
+++ b/drivers/net/wireless/ti/wl12xx/main.c
@@ -637,7 +637,8 @@ static int wl12xx_identify_chip(struct wl1271 *wl)
 
 		wl->quirks |= WLCORE_QUIRK_LEGACY_NVS |
 			      WLCORE_QUIRK_DUAL_PROBE_TMPL |
-			      WLCORE_QUIRK_TKIP_HEADER_SPACE;
+			      WLCORE_QUIRK_TKIP_HEADER_SPACE |
+			      WLCORE_QUIRK_START_STA_FAILS;
 		wl->sr_fw_name = WL127X_FW_NAME_SINGLE;
 		wl->mr_fw_name = WL127X_FW_NAME_MULTI;
 		memcpy(&wl->conf.mem, &wl12xx_default_priv_conf.mem_wl127x,
@@ -657,7 +658,8 @@ static int wl12xx_identify_chip(struct wl1271 *wl)
 
 		wl->quirks |= WLCORE_QUIRK_LEGACY_NVS |
 			      WLCORE_QUIRK_DUAL_PROBE_TMPL |
-			      WLCORE_QUIRK_TKIP_HEADER_SPACE;
+			      WLCORE_QUIRK_TKIP_HEADER_SPACE |
+			      WLCORE_QUIRK_START_STA_FAILS;
 		wl->plt_fw_name = WL127X_PLT_FW_NAME;
 		wl->sr_fw_name = WL127X_FW_NAME_SINGLE;
 		wl->mr_fw_name = WL127X_FW_NAME_MULTI;
@@ -682,7 +684,8 @@ static int wl12xx_identify_chip(struct wl1271 *wl)
 		/* wl128x requires TX blocksize alignment */
 		wl->quirks |= WLCORE_QUIRK_TX_BLOCKSIZE_ALIGN |
 			      WLCORE_QUIRK_DUAL_PROBE_TMPL |
-			      WLCORE_QUIRK_TKIP_HEADER_SPACE;
+			      WLCORE_QUIRK_TKIP_HEADER_SPACE |
+			      WLCORE_QUIRK_START_STA_FAILS;
 
 		wlcore_set_min_fw_ver(wl, WL128X_CHIP_VER, WL128X_IFTYPE_VER,
 				      WL128X_MAJOR_VER, WL128X_SUBTYPE_VER,
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 2cb978e..f4017f3 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2490,8 +2490,21 @@ static int wlcore_join(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 
 	if (is_ibss)
 		ret = wl12xx_cmd_role_start_ibss(wl, wlvif);
-	else
+	else {
+		if (wl->quirks & WLCORE_QUIRK_START_STA_FAILS) {
+			/*
+			 * TODO: this is an ugly workaround for wl12xx fw
+			 * bug - we are not able to tx/rx after the first
+			 * start_sta, so make dummy start+stop calls,
+			 * and then call start_sta again.
+			 * this should be fixed in the fw.
+			 */
+			wl12xx_cmd_role_start_sta(wl, wlvif);
+			wl12xx_cmd_role_stop_sta(wl, wlvif);
+		}
+
 		ret = wl12xx_cmd_role_start_sta(wl, wlvif);
+	}
 
 	return ret;
 }
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
index 68584aa..1030b6b 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore.h
@@ -450,6 +450,9 @@ wlcore_set_min_fw_ver(struct wl1271 *wl, unsigned int chip,
 /* Each RX/TX transaction requires an end-of-transaction transfer */
 #define WLCORE_QUIRK_END_OF_TRANSACTION		BIT(0)
 
+/* the first start_role(sta) sometimes doesn't work on wl12xx */
+#define WLCORE_QUIRK_START_STA_FAILS		BIT(1)
+
 /* wl127x and SPI don't support SDIO block size alignment */
 #define WLCORE_QUIRK_TX_BLOCKSIZE_ALIGN		BIT(2)
 
-- 
1.7.6.401.g6a319

[PATCH 05/15] wlcore: implement .remain_on_channel() callback

[Eliad Peller]

implement the reamin_on_channel() callback by starting
a dev role (already associated with the current vif)
on the requested channel/band.

This channel is usually different from the channel
of the sta role, so pass it to wl12xx_roc() as well,
and notify mac80211 (async) when the fw is ready
on the new channel.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/cmd.c    |   39 +++-----
 drivers/net/wireless/ti/wlcore/cmd.h    |    6 +-
 drivers/net/wireless/ti/wlcore/event.c  |    7 ++
 drivers/net/wireless/ti/wlcore/main.c   |  164 +++++++++++++++++++++++++++----
 drivers/net/wireless/ti/wlcore/wlcore.h |    3 +
 5 files changed, 174 insertions(+), 45 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index eaef3f4..28a235f 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -345,7 +345,9 @@ static u8 wlcore_get_native_channel_type(u8 nl_channel_type)
 }
 
 static int wl12xx_cmd_role_start_dev(struct wl1271 *wl,
-				     struct wl12xx_vif *wlvif)
+				     struct wl12xx_vif *wlvif,
+				     enum ieee80211_band band,
+				     int channel)
 {
 	struct wl12xx_cmd_role_start *cmd;
 	int ret;
@@ -359,9 +361,9 @@ static int wl12xx_cmd_role_start_dev(struct wl1271 *wl,
 	wl1271_debug(DEBUG_CMD, "cmd role start dev %d", wlvif->dev_role_id);
 
 	cmd->role_id = wlvif->dev_role_id;
-	if (wlvif->band == IEEE80211_BAND_5GHZ)
+	if (band == IEEE80211_BAND_5GHZ)
 		cmd->band = WLCORE_BAND_5GHZ;
-	cmd->channel = wlvif->channel;
+	cmd->channel = channel;
 
 	if (wlvif->dev_hlid == WL12XX_INVALID_LINK_ID) {
 		ret = wl12xx_allocate_link(wl, wlvif, &wlvif->dev_hlid);
@@ -1591,12 +1593,12 @@ out:
 }
 
 static int wl12xx_cmd_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif,
-			  u8 role_id)
+			  u8 role_id, u8 channel)
 {
 	struct wl12xx_cmd_roc *cmd;
 	int ret = 0;
 
-	wl1271_debug(DEBUG_CMD, "cmd roc %d (%d)", wlvif->channel, role_id);
+	wl1271_debug(DEBUG_CMD, "cmd roc %d (%d)", channel, role_id);
 
 	if (WARN_ON(role_id == WL12XX_INVALID_ROLE_ID))
 		return -EINVAL;
@@ -1608,7 +1610,7 @@ static int wl12xx_cmd_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 	}
 
 	cmd->role_id = role_id;
-	cmd->channel = wlvif->channel;
+	cmd->channel = channel;
 	switch (wlvif->band) {
 	case IEEE80211_BAND_2GHZ:
 		cmd->band = WLCORE_BAND_2_4GHZ;
@@ -1664,30 +1666,18 @@ out:
 	return ret;
 }
 
-int wl12xx_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 role_id)
+int wl12xx_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 role_id,
+	       u8 channel)
 {
 	int ret = 0;
-	bool is_first_roc;
 
 	if (WARN_ON(test_bit(role_id, wl->roc_map)))
 		return 0;
 
-	is_first_roc = (find_first_bit(wl->roc_map, WL12XX_MAX_ROLES) >=
-			WL12XX_MAX_ROLES);
-
-	ret = wl12xx_cmd_roc(wl, wlvif, role_id);
+	ret = wl12xx_cmd_roc(wl, wlvif, role_id, channel);
 	if (ret < 0)
 		goto out;
 
-	if (is_first_roc) {
-		ret = wl1271_cmd_wait_for_event(wl,
-					   REMAIN_ON_CHANNEL_COMPLETE_EVENT_ID);
-		if (ret < 0) {
-			wl1271_error("cmd roc event completion error");
-			goto out;
-		}
-	}
-
 	__set_bit(role_id, wl->roc_map);
 out:
 	return ret;
@@ -1780,7 +1770,8 @@ out:
 }
 
 /* start dev role and roc on its channel */
-int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif,
+		     enum ieee80211_band band, int channel)
 {
 	int ret;
 
@@ -1795,11 +1786,11 @@ int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 	if (ret < 0)
 		goto out;
 
-	ret = wl12xx_cmd_role_start_dev(wl, wlvif);
+	ret = wl12xx_cmd_role_start_dev(wl, wlvif, band, channel);
 	if (ret < 0)
 		goto out_disable;
 
-	ret = wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+	ret = wl12xx_roc(wl, wlvif, wlvif->dev_role_id, channel);
 	if (ret < 0)
 		goto out_stop;
 
diff --git a/drivers/net/wireless/ti/wlcore/cmd.h b/drivers/net/wireless/ti/wlcore/cmd.h
index 2409f3d..2b63759 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.h
+++ b/drivers/net/wireless/ti/wlcore/cmd.h
@@ -39,7 +39,8 @@ int wl12xx_cmd_role_stop_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif);
 int wl12xx_cmd_role_start_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif);
 int wl12xx_cmd_role_stop_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif);
 int wl12xx_cmd_role_start_ibss(struct wl1271 *wl, struct wl12xx_vif *wlvif);
-int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
+int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif,
+		     enum ieee80211_band band, int channel);
 int wl12xx_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
 int wl1271_cmd_test(struct wl1271 *wl, void *buf, size_t buf_len, u8 answer);
 int wl1271_cmd_interrogate(struct wl1271 *wl, u16 id, void *buf, size_t len);
@@ -76,7 +77,8 @@ int wl1271_cmd_set_ap_key(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 			  u8 key_size, const u8 *key, u8 hlid, u32 tx_seq_32,
 			  u16 tx_seq_16);
 int wl12xx_cmd_set_peer_state(struct wl1271 *wl, u8 hlid);
-int wl12xx_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 role_id);
+int wl12xx_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 role_id,
+	       u8 channel);
 int wl12xx_croc(struct wl1271 *wl, u8 role_id);
 int wl12xx_cmd_add_peer(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 			struct ieee80211_sta *sta, u8 hlid);
diff --git a/drivers/net/wireless/ti/wlcore/event.c b/drivers/net/wireless/ti/wlcore/event.c
index 4890705..cf45aaf 100644
--- a/drivers/net/wireless/ti/wlcore/event.c
+++ b/drivers/net/wireless/ti/wlcore/event.c
@@ -250,6 +250,13 @@ static int wl1271_event_process(struct wl1271 *wl)
 		disconnect_sta = true;
 	}
 
+	if (vector & REMAIN_ON_CHANNEL_COMPLETE_EVENT_ID) {
+		wl1271_debug(DEBUG_EVENT,
+			     "REMAIN_ON_CHANNEL_COMPLETE_EVENT_ID");
+		if (wl->roc_vif)
+			ieee80211_ready_on_channel(wl->hw);
+	}
+
 	if (disconnect_sta) {
 		u32 num_packets = wl->conf.tx.max_tx_retries;
 		struct ieee80211_sta *sta;
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index f4017f3..ef299f9 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2610,24 +2610,6 @@ static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 			if (ret < 0)
 				wl1271_warning("rate policy for channel "
 					       "failed %d", ret);
-
-			/*
-			 * change the ROC channel. do it only if we are
-			 * not idle. otherwise, CROC will be called
-			 * anyway.
-			 */
-			if (!test_bit(WLVIF_FLAG_STA_ASSOCIATED,
-				      &wlvif->flags) &&
-			    wl12xx_dev_role_started(wlvif) &&
-			    !(conf->flags & IEEE80211_CONF_IDLE)) {
-				ret = wl12xx_stop_dev(wl, wlvif);
-				if (ret < 0)
-					return ret;
-
-				ret = wl12xx_start_dev(wl, wlvif);
-				if (ret < 0)
-					return ret;
-			}
 		}
 	}
 
@@ -4027,7 +4009,8 @@ sta_not_found:
 
 		/* ROC until connected (after EAPOL exchange) */
 		if (!is_ibss) {
-			ret = wl12xx_roc(wl, wlvif, wlvif->role_id);
+			ret = wl12xx_roc(wl, wlvif, wlvif->role_id,
+					 wlvif->channel);
 			if (ret < 0)
 				goto out;
 		}
@@ -4658,6 +4641,144 @@ static void wlcore_op_flush(struct ieee80211_hw *hw, bool drop)
 	wl1271_tx_flush(wl);
 }
 
+static int wlcore_op_remain_on_channel(struct ieee80211_hw *hw,
+				       struct ieee80211_vif *vif,
+				       struct ieee80211_channel *chan,
+				       enum nl80211_channel_type channel_type,
+				       int duration)
+{
+	struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
+	struct wl1271 *wl = hw->priv;
+	int channel, ret = 0;
+
+	channel = ieee80211_frequency_to_channel(chan->center_freq);
+
+	wl1271_debug(DEBUG_MAC80211, "mac80211 roc %d (%d)",
+		     channel, wlvif->role_id);
+
+	mutex_lock(&wl->mutex);
+
+	if (unlikely(wl->state != WLCORE_STATE_ON))
+		goto out;
+
+	/* return EBUSY if we can't ROC right now */
+	if (WARN_ON(wl->roc_vif ||
+		    find_first_bit(wl->roc_map,
+				   WL12XX_MAX_ROLES) < WL12XX_MAX_ROLES)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	/*
+	 * make sure the vif has dev role.
+	 * TODO: enable dev role dynamically instead of pre-allocating it.
+	 */
+	if (WARN_ON_ONCE(!(wlvif->bss_type == BSS_TYPE_STA_BSS ||
+			   wlvif->bss_type == BSS_TYPE_IBSS))) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = wl1271_ps_elp_wakeup(wl);
+	if (ret < 0)
+		goto out;
+
+	ret = wl12xx_start_dev(wl, wlvif, chan->band, channel);
+	if (ret < 0)
+		goto out_sleep;
+
+	wl->roc_vif = vif;
+	ieee80211_queue_delayed_work(hw, &wl->roc_complete_work,
+				     msecs_to_jiffies(duration));
+out_sleep:
+	wl1271_ps_elp_sleep(wl);
+out:
+	mutex_unlock(&wl->mutex);
+	return ret;
+}
+
+static int __wlcore_roc_completed(struct wl1271 *wl)
+{
+	struct wl12xx_vif *wlvif;
+	int ret;
+
+	/* already completed */
+	if (unlikely(!wl->roc_vif))
+		return 0;
+
+	wlvif = wl12xx_vif_to_data(wl->roc_vif);
+
+	if (!test_bit(WLVIF_FLAG_INITIALIZED, &wlvif->flags))
+		return -EBUSY;
+
+	ret = wl12xx_stop_dev(wl, wlvif);
+	if (ret < 0)
+		return ret;
+
+	wl->roc_vif = NULL;
+
+	return 0;
+}
+
+static int wlcore_roc_completed(struct wl1271 *wl)
+{
+	int ret;
+
+	wl1271_debug(DEBUG_MAC80211, "roc complete");
+
+	mutex_lock(&wl->mutex);
+
+	if (unlikely(wl->state != WLCORE_STATE_ON)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	ret = wl1271_ps_elp_wakeup(wl);
+	if (ret < 0)
+		goto out;
+
+	ret = __wlcore_roc_completed(wl);
+
+	wl1271_ps_elp_sleep(wl);
+out:
+	mutex_unlock(&wl->mutex);
+
+	return ret;
+}
+
+static void wlcore_roc_complete_work(struct work_struct *work)
+{
+	struct delayed_work *dwork;
+	struct wl1271 *wl;
+	int ret;
+
+	dwork = container_of(work, struct delayed_work, work);
+	wl = container_of(dwork, struct wl1271, roc_complete_work);
+
+	ret = wlcore_roc_completed(wl);
+	if (!ret)
+		ieee80211_remain_on_channel_expired(wl->hw);
+}
+
+static int wlcore_op_cancel_remain_on_channel(struct ieee80211_hw *hw)
+{
+	struct wl1271 *wl = hw->priv;
+
+	wl1271_debug(DEBUG_MAC80211, "mac80211 croc");
+
+	/* TODO: per-vif */
+	wl1271_tx_flush(wl);
+
+	/*
+	 * we can't just flush_work here, because it might deadlock
+	 * (as we might get called from the same workqueue)
+	 */
+	cancel_delayed_work_sync(&wl->roc_complete_work);
+	wlcore_roc_completed(wl);
+
+	return 0;
+}
+
 static bool wl1271_tx_frames_pending(struct ieee80211_hw *hw)
 {
 	struct wl1271 *wl = hw->priv;
@@ -4849,6 +4970,8 @@ static const struct ieee80211_ops wl1271_ops = {
 	.set_bitrate_mask = wl12xx_set_bitrate_mask,
 	.channel_switch = wl12xx_op_channel_switch,
 	.flush = wlcore_op_flush,
+	.remain_on_channel = wlcore_op_remain_on_channel,
+	.cancel_remain_on_channel = wlcore_op_cancel_remain_on_channel,
 	CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
 };
 
@@ -5245,6 +5368,8 @@ static int wl1271_init_ieee80211(struct wl1271 *wl)
 	wl->hw->wiphy->max_sched_scan_ie_len = WL1271_CMD_TEMPL_MAX_SIZE -
 		sizeof(struct ieee80211_header);
 
+	wl->hw->wiphy->max_remain_on_channel_duration = 5000;
+
 	wl->hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD |
 				WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
 
@@ -5343,6 +5468,7 @@ struct ieee80211_hw *wlcore_alloc_hw(size_t priv_size, u32 aggr_buf_size)
 	INIT_WORK(&wl->tx_work, wl1271_tx_work);
 	INIT_WORK(&wl->recovery_work, wl1271_recovery_work);
 	INIT_DELAYED_WORK(&wl->scan_complete_work, wl1271_scan_complete_work);
+	INIT_DELAYED_WORK(&wl->roc_complete_work, wlcore_roc_complete_work);
 	INIT_DELAYED_WORK(&wl->tx_watchdog_work, wl12xx_tx_watchdog_work);
 	INIT_DELAYED_WORK(&wl->connection_loss_work,
 			  wl1271_connection_loss_work);
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
index 1030b6b..b636f1d 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore.h
@@ -286,6 +286,9 @@ struct wl1271 {
 	/* Connection loss work */
 	struct delayed_work connection_loss_work;
 
+	struct ieee80211_vif *roc_vif;
+	struct delayed_work roc_complete_work;
+
 	bool sched_scanning;
 
 	/* The current band */
-- 
1.7.6.401.g6a319

Re: [PATCH 05/15] wlcore: implement .remain_on_channel() callback

[Arik Nemtsov]

On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <eliad@wizery.com> wrote:
> implement the reamin_on_channel() callback by starting
> a dev role (already associated with the current vif)
> on the requested channel/band.
>
> This channel is usually different from the channel
> of the sta role, so pass it to wl12xx_roc() as well,
> and notify mac80211 (async) when the fw is ready
> on the new channel.
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
[...]
> @@ -1608,7 +1610,7 @@ static int wl12xx_cmd_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif,
>         }
>
>         cmd->role_id = role_id;
> -       cmd->channel = wlvif->channel;
> +       cmd->channel = channel;
>         switch (wlvif->band) {
>         case IEEE80211_BAND_2GHZ:
>                 cmd->band = WLCORE_BAND_2_4GHZ;

This seems wrong... What happens if the roc is on another band from
the one the vif is connected on?

The dev role is started on the correct band, but the roc will be on
the band of the wlvif..

[...]
> +static int wlcore_op_remain_on_channel(struct ieee80211_hw *hw,
> +                                      struct ieee80211_vif *vif,
> +                                      struct ieee80211_channel *chan,
> +                                      enum nl80211_channel_type channel_type,
> +                                      int duration)
> +{
> +       struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
> +       struct wl1271 *wl = hw->priv;
> +       int channel, ret = 0;
> +
> +       channel = ieee80211_frequency_to_channel(chan->center_freq);
> +
> +       wl1271_debug(DEBUG_MAC80211, "mac80211 roc %d (%d)",
> +                    channel, wlvif->role_id);
> +
> +       mutex_lock(&wl->mutex);
> +
> +       if (unlikely(wl->state != WLCORE_STATE_ON))
> +               goto out;
> +
> +       /* return EBUSY if we can't ROC right now */
> +       if (WARN_ON(wl->roc_vif ||
> +                   find_first_bit(wl->roc_map,
> +                                  WL12XX_MAX_ROLES) < WL12XX_MAX_ROLES)) {
> +               ret = -EBUSY;
> +               goto out;
> +       }
> +
> +       /*
> +        * make sure the vif has dev role.
> +        * TODO: enable dev role dynamically instead of pre-allocating it.
> +        */

Not sure what you mean here. Seems pretty dynamic to me..

> +       if (WARN_ON_ONCE(!(wlvif->bss_type == BSS_TYPE_STA_BSS ||
> +                          wlvif->bss_type == BSS_TYPE_IBSS))) {
> +               ret = -EINVAL;
> +               goto out;
> +       }
> +
> +       ret = wl1271_ps_elp_wakeup(wl);
> +       if (ret < 0)
> +               goto out;
> +
> +       ret = wl12xx_start_dev(wl, wlvif, chan->band, channel);
> +       if (ret < 0)
> +               goto out_sleep;
> +
> +       wl->roc_vif = vif;
> +       ieee80211_queue_delayed_work(hw, &wl->roc_complete_work,
> +                                    msecs_to_jiffies(duration));
> +out_sleep:
> +       wl1271_ps_elp_sleep(wl);
> +out:
> +       mutex_unlock(&wl->mutex);
> +       return ret;
> +}
> +
> +static int __wlcore_roc_completed(struct wl1271 *wl)
> +{
> +       struct wl12xx_vif *wlvif;
> +       int ret;
> +
> +       /* already completed */
> +       if (unlikely(!wl->roc_vif))
> +               return 0;
> +
> +       wlvif = wl12xx_vif_to_data(wl->roc_vif);
> +
> +       if (!test_bit(WLVIF_FLAG_INITIALIZED, &wlvif->flags))
> +               return -EBUSY;
> +
> +       ret = wl12xx_stop_dev(wl, wlvif);
> +       if (ret < 0)
> +               return ret;
> +
> +       wl->roc_vif = NULL;
> +
> +       return 0;
> +}
> +
> +static int wlcore_roc_completed(struct wl1271 *wl)
> +{
> +       int ret;
> +
> +       wl1271_debug(DEBUG_MAC80211, "roc complete");
> +
> +       mutex_lock(&wl->mutex);
> +
> +       if (unlikely(wl->state != WLCORE_STATE_ON)) {
> +               ret = -EBUSY;
> +               goto out;
> +       }
> +
> +       ret = wl1271_ps_elp_wakeup(wl);
> +       if (ret < 0)
> +               goto out;
> +
> +       ret = __wlcore_roc_completed(wl);
> +
> +       wl1271_ps_elp_sleep(wl);
> +out:
> +       mutex_unlock(&wl->mutex);
> +
> +       return ret;
> +}
> +
> +static void wlcore_roc_complete_work(struct work_struct *work)
> +{
> +       struct delayed_work *dwork;
> +       struct wl1271 *wl;
> +       int ret;
> +
> +       dwork = container_of(work, struct delayed_work, work);
> +       wl = container_of(dwork, struct wl1271, roc_complete_work);
> +
> +       ret = wlcore_roc_completed(wl);
> +       if (!ret)
> +               ieee80211_remain_on_channel_expired(wl->hw);
> +}
> +
> +static int wlcore_op_cancel_remain_on_channel(struct ieee80211_hw *hw)
> +{
> +       struct wl1271 *wl = hw->priv;
> +
> +       wl1271_debug(DEBUG_MAC80211, "mac80211 croc");
> +
> +       /* TODO: per-vif */
> +       wl1271_tx_flush(wl);
> +
> +       /*
> +        * we can't just flush_work here, because it might deadlock
> +        * (as we might get called from the same workqueue)
> +        */
> +       cancel_delayed_work_sync(&wl->roc_complete_work);
> +       wlcore_roc_completed(wl);
> +
> +       return 0;
> +}
> +
>  static bool wl1271_tx_frames_pending(struct ieee80211_hw *hw)
>  {
>         struct wl1271 *wl = hw->priv;
> @@ -4849,6 +4970,8 @@ static const struct ieee80211_ops wl1271_ops = {
>         .set_bitrate_mask = wl12xx_set_bitrate_mask,
>         .channel_switch = wl12xx_op_channel_switch,
>         .flush = wlcore_op_flush,
> +       .remain_on_channel = wlcore_op_remain_on_channel,
> +       .cancel_remain_on_channel = wlcore_op_cancel_remain_on_channel,
>         CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
>  };
>
> @@ -5245,6 +5368,8 @@ static int wl1271_init_ieee80211(struct wl1271 *wl)
>         wl->hw->wiphy->max_sched_scan_ie_len = WL1271_CMD_TEMPL_MAX_SIZE -
>                 sizeof(struct ieee80211_header);
>
> +       wl->hw->wiphy->max_remain_on_channel_duration = 5000;

Isn't this the default?

> +
>         wl->hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD |
>                                 WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
>
> @@ -5343,6 +5468,7 @@ struct ieee80211_hw *wlcore_alloc_hw(size_t priv_size, u32 aggr_buf_size)
>         INIT_WORK(&wl->tx_work, wl1271_tx_work);
>         INIT_WORK(&wl->recovery_work, wl1271_recovery_work);
>         INIT_DELAYED_WORK(&wl->scan_complete_work, wl1271_scan_complete_work);
> +       INIT_DELAYED_WORK(&wl->roc_complete_work, wlcore_roc_complete_work);
>         INIT_DELAYED_WORK(&wl->tx_watchdog_work, wl12xx_tx_watchdog_work);
>         INIT_DELAYED_WORK(&wl->connection_loss_work,
>                           wl1271_connection_loss_work);
> diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
> index 1030b6b..b636f1d 100644
> --- a/drivers/net/wireless/ti/wlcore/wlcore.h
> +++ b/drivers/net/wireless/ti/wlcore/wlcore.h
> @@ -286,6 +286,9 @@ struct wl1271 {
>         /* Connection loss work */
>         struct delayed_work connection_loss_work;
>
> +       struct ieee80211_vif *roc_vif;
> +       struct delayed_work roc_complete_work;
> +
>         bool sched_scanning;
>
>         /* The current band */
> --
> 1.7.6.401.g6a319
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/15] wlcore: implement .remain_on_channel() callback

[Eliad Peller]

On Mon, Nov 19, 2012 at 11:13 PM, Arik Nemtsov <arik@wizery.com> wrote:
> On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <eliad@wizery.com> wrote:
>> implement the reamin_on_channel() callback by starting
>> a dev role (already associated with the current vif)
>> on the requested channel/band.
>>
>> This channel is usually different from the channel
>> of the sta role, so pass it to wl12xx_roc() as well,
>> and notify mac80211 (async) when the fw is ready
>> on the new channel.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
> [...]
>> @@ -1608,7 +1610,7 @@ static int wl12xx_cmd_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif,
>>         }
>>
>>         cmd->role_id = role_id;
>> -       cmd->channel = wlvif->channel;
>> +       cmd->channel = channel;
>>         switch (wlvif->band) {
>>         case IEEE80211_BAND_2GHZ:
>>                 cmd->band = WLCORE_BAND_2_4GHZ;
>
> This seems wrong... What happens if the roc is on another band from
> the one the vif is connected on?
>
> The dev role is started on the correct band, but the roc will be on
> the band of the wlvif..
>
good catch. i'll fix it.


>> +static int wlcore_op_remain_on_channel(struct ieee80211_hw *hw,
>> +                                      struct ieee80211_vif *vif,
>> +                                      struct ieee80211_channel *chan,
>> +                                      enum nl80211_channel_type channel_type,
>> +                                      int duration)
>> +{
>> +       struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
>> +       struct wl1271 *wl = hw->priv;
>> +       int channel, ret = 0;
>> +
>> +       channel = ieee80211_frequency_to_channel(chan->center_freq);
>> +
>> +       wl1271_debug(DEBUG_MAC80211, "mac80211 roc %d (%d)",
>> +                    channel, wlvif->role_id);
>> +
>> +       mutex_lock(&wl->mutex);
>> +
>> +       if (unlikely(wl->state != WLCORE_STATE_ON))
>> +               goto out;
>> +
>> +       /* return EBUSY if we can't ROC right now */
>> +       if (WARN_ON(wl->roc_vif ||
>> +                   find_first_bit(wl->roc_map,
>> +                                  WL12XX_MAX_ROLES) < WL12XX_MAX_ROLES)) {
>> +               ret = -EBUSY;
>> +               goto out;
>> +       }
>> +
>> +       /*
>> +        * make sure the vif has dev role.
>> +        * TODO: enable dev role dynamically instead of pre-allocating it.
>> +        */
>
> Not sure what you mean here. Seems pretty dynamic to me..
>
yeah... i referred to the lazy-enable of dev role, but forgot that we
already upstreamed it :)
so i'll just drop this check (as we can start dev role along with any
other role).

>> @@ -5245,6 +5368,8 @@ static int wl1271_init_ieee80211(struct wl1271 *wl)
>>         wl->hw->wiphy->max_sched_scan_ie_len = WL1271_CMD_TEMPL_MAX_SIZE -
>>                 sizeof(struct ieee80211_header);
>>
>> +       wl->hw->wiphy->max_remain_on_channel_duration = 5000;
>
> Isn't this the default?
>
only for drivers that don't implement .remain_on_channel (i.e.
implemented by mac80211).

Eliad.

[PATCH 06/15] wlcore: use dev_hlid if the tx is offchannel

[Eliad Peller]

In case of offchannel tx, we should use the dev role hlid
instead of the sta hlid.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/tx.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/tx.c b/drivers/net/wireless/ti/wlcore/tx.c
index a555a70..cf6dbee 100644
--- a/drivers/net/wireless/ti/wlcore/tx.c
+++ b/drivers/net/wireless/ti/wlcore/tx.c
@@ -155,12 +155,20 @@ static u8 wl12xx_tx_get_hlid_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 u8 wl12xx_tx_get_hlid(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 		      struct sk_buff *skb, struct ieee80211_sta *sta)
 {
+	struct ieee80211_tx_info *control;
+
 	if (!wlvif || wl12xx_is_dummy_packet(wl, skb))
 		return wl->system_hlid;
 
 	if (wlvif->bss_type == BSS_TYPE_AP_BSS)
 		return wl12xx_tx_get_hlid_ap(wl, wlvif, skb, sta);
 
+	control = IEEE80211_SKB_CB(skb);
+	if (control->flags & IEEE80211_TX_CTL_TX_OFFCHAN) {
+		wl1271_debug(DEBUG_TX, "tx offchannel");
+		return wlvif->dev_hlid;
+	}
+
 	return wlvif->sta.hlid;
 }
 
-- 
1.7.6.401.g6a319

[PATCH 07/15] wlcore: get channel from bss_conf instead of hw->conf

[Eliad Peller]

We care only about the operational channel, not
about the temporal hw channel (which won't have
any real meaning in multi-channel env anyway)

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index ef299f9..4f4a9e5 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3833,7 +3833,7 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
 			goto sta_not_found;
 
 		/* save the supp_rates of the ap */
-		sta_rate_set = sta->supp_rates[wl->hw->conf.channel->band];
+		sta_rate_set = sta->supp_rates[wlvif->band];
 		if (sta->ht_cap.ht_supported)
 			sta_rate_set |=
 			  (sta->ht_cap.mcs.rx_mask[0] << HW_HT_RATES_OFFSET) |
-- 
1.7.6.401.g6a319

[PATCH 08/15] wlcore: add chanctx implementation

[Eliad Peller]

Add some basic chanctx implementation.

Only add debug prints, and save the vif's channel/band/type.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/main.c |   66 +++++++++++++++++++++++++++++++++
 1 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 4f4a9e5..caafd8c 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -4150,6 +4150,67 @@ out:
 	mutex_unlock(&wl->mutex);
 }
 
+static int wlcore_op_add_chanctx(struct ieee80211_hw *hw,
+				 struct ieee80211_chanctx_conf *ctx)
+{
+	wl1271_debug(DEBUG_MAC80211, "mac80211 add chanctx %d (type %d)",
+		     ieee80211_frequency_to_channel(ctx->channel->center_freq),
+		     ctx->channel_type);
+	return 0;
+}
+
+static void wlcore_op_remove_chanctx(struct ieee80211_hw *hw,
+				     struct ieee80211_chanctx_conf *ctx)
+{
+	wl1271_debug(DEBUG_MAC80211, "mac80211 remove chanctx %d (type %d)",
+		     ieee80211_frequency_to_channel(ctx->channel->center_freq),
+		     ctx->channel_type);
+}
+
+static void wlcore_op_change_chanctx(struct ieee80211_hw *hw,
+				     struct ieee80211_chanctx_conf *ctx,
+				     u32 changed)
+{
+	wl1271_debug(DEBUG_MAC80211,
+		     "mac80211 change chanctx %d (type %d) changed 0x%x",
+		     ieee80211_frequency_to_channel(ctx->channel->center_freq),
+		     ctx->channel_type, changed);
+}
+
+static int wlcore_op_assign_vif_chanctx(struct ieee80211_hw *hw,
+					struct ieee80211_vif *vif,
+					struct ieee80211_chanctx_conf *ctx)
+{
+	struct wl1271 *wl = hw->priv;
+	struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
+	int channel = ieee80211_frequency_to_channel(
+		ctx->channel->center_freq);
+
+	wl1271_debug(DEBUG_MAC80211,
+		     "mac80211 assign chanctx (role %d) %d (type %d)",
+		     wlvif->role_id, channel, ctx->channel_type);
+
+	wlcore_tx_work_locked(wl);
+	wlvif->band = ctx->channel->band;
+	wlvif->channel = channel;
+	wlvif->channel_type = ctx->channel_type;
+
+	return 0;
+}
+
+static void wlcore_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
+					   struct ieee80211_vif *vif,
+					   struct ieee80211_chanctx_conf *ctx)
+{
+	struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
+
+	wl1271_debug(DEBUG_MAC80211,
+		     "mac80211 unassign chanctx (role %d) %d (type %d)",
+		     wlvif->role_id,
+		     ieee80211_frequency_to_channel(ctx->channel->center_freq),
+		     ctx->channel_type);
+}
+
 static int wl1271_op_conf_tx(struct ieee80211_hw *hw,
 			     struct ieee80211_vif *vif, u16 queue,
 			     const struct ieee80211_tx_queue_params *params)
@@ -4972,6 +5033,11 @@ static const struct ieee80211_ops wl1271_ops = {
 	.flush = wlcore_op_flush,
 	.remain_on_channel = wlcore_op_remain_on_channel,
 	.cancel_remain_on_channel = wlcore_op_cancel_remain_on_channel,
+	.add_chanctx = wlcore_op_add_chanctx,
+	.remove_chanctx = wlcore_op_remove_chanctx,
+	.change_chanctx = wlcore_op_change_chanctx,
+	.assign_vif_chanctx = wlcore_op_assign_vif_chanctx,
+	.unassign_vif_chanctx = wlcore_op_unassign_vif_chanctx,
 	CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
 };
 
-- 
1.7.6.401.g6a319

Re: [PATCH 08/15] wlcore: add chanctx implementation

[Johannes Berg]

On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> Add some basic chanctx implementation.

This patch will leave the driver in a broken state, and in fact probably
crashing trying to use hw.conf.channel.

johannes

Re: [PATCH 08/15] wlcore: add chanctx implementation

[Eliad Peller]

On Mon, Nov 19, 2012 at 7:03 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> Add some basic chanctx implementation.
>
> This patch will leave the driver in a broken state, and in fact probably
> crashing trying to use hw.conf.channel.
>
you are right. i'm taking care of these in the following patches.
i preferred splitting the patches for clarity instead of squashing
them all into one big patch (removing hw.conf.channel references
before implementing chanctx will break the driver as well).
i guess i can squash them in this case, but i'm not sure it will
really help, as other intermediate patches in this patchset might
still break some functionality of the driver.

i don't mind either way. whatever Luca will prefer :)
(maybe keeping it that way for review, and only squashing them on apply?)

Eliad.

Re: [PATCH 08/15] wlcore: add chanctx implementation

[Luciano Coelho]

On Mon, 2012-11-19 at 19:16 +0200, Eliad Peller wrote:
> On Mon, Nov 19, 2012 at 7:03 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> >> Add some basic chanctx implementation.
> >
> > This patch will leave the driver in a broken state, and in fact probably
> > crashing trying to use hw.conf.channel.
> >
> you are right. i'm taking care of these in the following patches.
> i preferred splitting the patches for clarity instead of squashing
> them all into one big patch (removing hw.conf.channel references
> before implementing chanctx will break the driver as well).
> i guess i can squash them in this case, but i'm not sure it will
> really help, as other intermediate patches in this patchset might
> still break some functionality of the driver.
> 
> i don't mind either way. whatever Luca will prefer :)
> (maybe keeping it that way for review, and only squashing them on apply?)

I prefer if we can do it so that the driver is not broken, to make it
easier to bisect.  But I also agree that for review, it's better to have
split up patches.

So, your suggestion sounds good to me.  Maybe you could send a final
squashed patchset when they're reviewed and ready to be applied?

--
Luca.

Re: [PATCH 08/15] wlcore: add chanctx implementation

[Eliad Peller]

On Mon, Nov 19, 2012 at 7:36 PM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2012-11-19 at 19:16 +0200, Eliad Peller wrote:
>> On Mon, Nov 19, 2012 at 7:03 PM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>> > On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> >> Add some basic chanctx implementation.
>> >
>> > This patch will leave the driver in a broken state, and in fact probably
>> > crashing trying to use hw.conf.channel.
>> >
>> you are right. i'm taking care of these in the following patches.
>> i preferred splitting the patches for clarity instead of squashing
>> them all into one big patch (removing hw.conf.channel references
>> before implementing chanctx will break the driver as well).
>> i guess i can squash them in this case, but i'm not sure it will
>> really help, as other intermediate patches in this patchset might
>> still break some functionality of the driver.
>>
>> i don't mind either way. whatever Luca will prefer :)
>> (maybe keeping it that way for review, and only squashing them on apply?)
>
> I prefer if we can do it so that the driver is not broken, to make it
> easier to bisect.  But I also agree that for review, it's better to have
> split up patches.
>
> So, your suggestion sounds good to me.  Maybe you could send a final
> squashed patchset when they're reviewed and ready to be applied?
>
sure.

Eliad.

Re: [PATCH 08/15] wlcore: add chanctx implementation

[Arik Nemtsov]

On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <eliad@wizery.com> wrote:
> Add some basic chanctx implementation.
>
> Only add debug prints, and save the vif's channel/band/type.
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
>  drivers/net/wireless/ti/wlcore/main.c |   66 +++++++++++++++++++++++++++++++++
>  1 files changed, 66 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
> index 4f4a9e5..caafd8c 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -4150,6 +4150,67 @@ out:
>         mutex_unlock(&wl->mutex);
>  }
>
> +static int wlcore_op_add_chanctx(struct ieee80211_hw *hw,
> +                                struct ieee80211_chanctx_conf *ctx)
> +{
> +       wl1271_debug(DEBUG_MAC80211, "mac80211 add chanctx %d (type %d)",
> +                    ieee80211_frequency_to_channel(ctx->channel->center_freq),
> +                    ctx->channel_type);
> +       return 0;
> +}
> +
> +static void wlcore_op_remove_chanctx(struct ieee80211_hw *hw,
> +                                    struct ieee80211_chanctx_conf *ctx)
> +{
> +       wl1271_debug(DEBUG_MAC80211, "mac80211 remove chanctx %d (type %d)",
> +                    ieee80211_frequency_to_channel(ctx->channel->center_freq),
> +                    ctx->channel_type);
> +}
> +
> +static void wlcore_op_change_chanctx(struct ieee80211_hw *hw,
> +                                    struct ieee80211_chanctx_conf *ctx,
> +                                    u32 changed)
> +{
> +       wl1271_debug(DEBUG_MAC80211,
> +                    "mac80211 change chanctx %d (type %d) changed 0x%x",
> +                    ieee80211_frequency_to_channel(ctx->channel->center_freq),
> +                    ctx->channel_type, changed);
> +}
> +
> +static int wlcore_op_assign_vif_chanctx(struct ieee80211_hw *hw,
> +                                       struct ieee80211_vif *vif,
> +                                       struct ieee80211_chanctx_conf *ctx)
> +{
> +       struct wl1271 *wl = hw->priv;
> +       struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
> +       int channel = ieee80211_frequency_to_channel(
> +               ctx->channel->center_freq);
> +
> +       wl1271_debug(DEBUG_MAC80211,
> +                    "mac80211 assign chanctx (role %d) %d (type %d)",
> +                    wlvif->role_id, channel, ctx->channel_type);
> +
> +       wlcore_tx_work_locked(wl);

is this some kind of a lame attempt at a flush? :)

why not use flush here if the channel was set - or better yet, flush
stuff at wlcore_op_unassign_vif_chanctx, which is currently unused.
we'll need the TODO there for the per-vif-flush. once everything is
upstreamed (hw queues) it would be pretty trivial to implement.

also not sure what happened to the rest of the stuff done on channel
config - updating the rate_set according to band etc..

> +       wlvif->band = ctx->channel->band;
> +       wlvif->channel = channel;
> +       wlvif->channel_type = ctx->channel_type;
> +
> +       return 0;
> +}
> +
> +static void wlcore_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
> +                                          struct ieee80211_vif *vif,
> +                                          struct ieee80211_chanctx_conf *ctx)
> +{
> +       struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
> +
> +       wl1271_debug(DEBUG_MAC80211,
> +                    "mac80211 unassign chanctx (role %d) %d (type %d)",
> +                    wlvif->role_id,
> +                    ieee80211_frequency_to_channel(ctx->channel->center_freq),
> +                    ctx->channel_type);
> +}
> +
>  static int wl1271_op_conf_tx(struct ieee80211_hw *hw,
>                              struct ieee80211_vif *vif, u16 queue,
>                              const struct ieee80211_tx_queue_params *params)
> @@ -4972,6 +5033,11 @@ static const struct ieee80211_ops wl1271_ops = {
>         .flush = wlcore_op_flush,
>         .remain_on_channel = wlcore_op_remain_on_channel,
>         .cancel_remain_on_channel = wlcore_op_cancel_remain_on_channel,
> +       .add_chanctx = wlcore_op_add_chanctx,
> +       .remove_chanctx = wlcore_op_remove_chanctx,
> +       .change_chanctx = wlcore_op_change_chanctx,
> +       .assign_vif_chanctx = wlcore_op_assign_vif_chanctx,
> +       .unassign_vif_chanctx = wlcore_op_unassign_vif_chanctx,
>         CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
>  };
>
> --
> 1.7.6.401.g6a319
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/15] wlcore: add chanctx implementation

[Eliad Peller]

On Mon, Nov 19, 2012 at 11:23 PM, Arik Nemtsov <arik@wizery.com> wrote:
> On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <eliad@wizery.com> wrote:
>> Add some basic chanctx implementation.
>>
>> Only add debug prints, and save the vif's channel/band/type.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---

>> +static int wlcore_op_assign_vif_chanctx(struct ieee80211_hw *hw,
>> +                                       struct ieee80211_vif *vif,
>> +                                       struct ieee80211_chanctx_conf *ctx)
>> +{
>> +       struct wl1271 *wl = hw->priv;
>> +       struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
>> +       int channel = ieee80211_frequency_to_channel(
>> +               ctx->channel->center_freq);
>> +
>> +       wl1271_debug(DEBUG_MAC80211,
>> +                    "mac80211 assign chanctx (role %d) %d (type %d)",
>> +                    wlvif->role_id, channel, ctx->channel_type);
>> +
>> +       wlcore_tx_work_locked(wl);
>
> is this some kind of a lame attempt at a flush? :)
>
> why not use flush here if the channel was set - or better yet, flush
> stuff at wlcore_op_unassign_vif_chanctx, which is currently unused.
> we'll need the TODO there for the per-vif-flush. once everything is
> upstreamed (hw queues) it would be pretty trivial to implement.
>
ok, you convinced me. i'll do a flush on unassign instead. :)

> also not sure what happened to the rest of the stuff done on channel
> config - updating the rate_set according to band etc..
>
well, actually you have a pending patch to fix it, and i planned
sending it in the next patchset. but since you already asked about it
i'll just squash it into this patch (and add your SOB). :)

Eliad.

Re: [PATCH 08/15] wlcore: add chanctx implementation

[Arik Nemtsov]

On Tue, Nov 20, 2012 at 10:38 AM, Eliad Peller <eliad@wizery.com> wrote:
> On Mon, Nov 19, 2012 at 11:23 PM, Arik Nemtsov <arik@wizery.com> wrote:
>> On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <eliad@wizery.com> wrote:
>>> Add some basic chanctx implementation.
>>>
>>> Only add debug prints, and save the vif's channel/band/type.
>>>
>>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>>> ---
>
>>> +static int wlcore_op_assign_vif_chanctx(struct ieee80211_hw *hw,
>>> +                                       struct ieee80211_vif *vif,
>>> +                                       struct ieee80211_chanctx_conf *ctx)
>>> +{
>>> +       struct wl1271 *wl = hw->priv;
>>> +       struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
>>> +       int channel = ieee80211_frequency_to_channel(
>>> +               ctx->channel->center_freq);
>>> +
>>> +       wl1271_debug(DEBUG_MAC80211,
>>> +                    "mac80211 assign chanctx (role %d) %d (type %d)",
>>> +                    wlvif->role_id, channel, ctx->channel_type);
>>> +
>>> +       wlcore_tx_work_locked(wl);
>>
>> is this some kind of a lame attempt at a flush? :)
>>
>> why not use flush here if the channel was set - or better yet, flush
>> stuff at wlcore_op_unassign_vif_chanctx, which is currently unused.
>> we'll need the TODO there for the per-vif-flush. once everything is
>> upstreamed (hw queues) it would be pretty trivial to implement.
>>
> ok, you convinced me. i'll do a flush on unassign instead. :)

btw, it's also a bug to call wlcore_tx_work_locked() since you didn't
take the mutex, which is another bug in your patch. you should take
the mutex before changing members of wlvif :)

>
>> also not sure what happened to the rest of the stuff done on channel
>> config - updating the rate_set according to band etc..
>>
> well, actually you have a pending patch to fix it, and i planned
> sending it in the next patchset. but since you already asked about it
> i'll just squash it into this patch (and add your SOB). :)

any form you prefer.

Re: [PATCH 08/15] wlcore: add chanctx implementation

[Johannes Berg]

On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:

> +static int wlcore_op_add_chanctx(struct ieee80211_hw *hw,
> +				 struct ieee80211_chanctx_conf *ctx)
> +{
> +	wl1271_debug(DEBUG_MAC80211, "mac80211 add chanctx %d (type %d)",
> +		     ieee80211_frequency_to_channel(ctx->channel->center_freq),
> +		     ctx->channel_type);

Ok so I just realized there's another small issue with this patch/these
patches -- my VHT changes, which I'm planning to merge soon, will change
the chanctx_conf structure contents, which might then be tricky with git
tree handling?

I'm not too worried about that, but might be something to keep in mind.
Right now my changes are easy because they only affect hwsim as the only
driver using channel contexts.

johannes

Re: [PATCH 08/15] wlcore: add chanctx implementation

[Eliad Peller]

On Tue, Nov 20, 2012 at 1:42 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>
>> +static int wlcore_op_add_chanctx(struct ieee80211_hw *hw,
>> +                              struct ieee80211_chanctx_conf *ctx)
>> +{
>> +     wl1271_debug(DEBUG_MAC80211, "mac80211 add chanctx %d (type %d)",
>> +                  ieee80211_frequency_to_channel(ctx->channel->center_freq),
>> +                  ctx->channel_type);
>
> Ok so I just realized there's another small issue with this patch/these
> patches -- my VHT changes, which I'm planning to merge soon, will change
> the chanctx_conf structure contents, which might then be tricky with git
> tree handling?
>
> I'm not too worried about that, but might be something to keep in mind.
> Right now my changes are easy because they only affect hwsim as the only
> driver using channel contexts.
>
yeah, we don't do anything fancy with the chanctx, so i guess the
merge should be fairly easy.
thanks for the heads up. we'll keep it in mind.

Eliad.

[PATCH 09/15] wlcore: remove channel handling from op_config

[Eliad Peller]

After setting the channel on chanctx callbacks, we
no longer need to handle channel change notifications
on op_config.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/main.c |   68 +-------------------------------
 1 files changed, 3 insertions(+), 65 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index caafd8c..fcdf381 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2569,49 +2569,7 @@ static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 			     struct ieee80211_conf *conf, u32 changed)
 {
 	bool is_ap = (wlvif->bss_type == BSS_TYPE_AP_BSS);
-	int channel, ret;
-
-	channel = ieee80211_frequency_to_channel(conf->channel->center_freq);
-
-	/* if the channel changes while joined, join again */
-	if (changed & IEEE80211_CONF_CHANGE_CHANNEL &&
-	    ((wlvif->band != conf->channel->band) ||
-	     (wlvif->channel != channel) ||
-	     (wlvif->channel_type != conf->channel_type))) {
-		/* send all pending packets */
-		ret = wlcore_tx_work_locked(wl);
-		if (ret < 0)
-			return ret;
-
-		wlvif->band = conf->channel->band;
-		wlvif->channel = channel;
-		wlvif->channel_type = conf->channel_type;
-
-		if (is_ap) {
-			wl1271_set_band_rate(wl, wlvif);
-			ret = wl1271_init_ap_rates(wl, wlvif);
-			if (ret < 0)
-				wl1271_error("AP rate policy change failed %d",
-					     ret);
-		} else {
-			/*
-			 * FIXME: the mac80211 should really provide a fixed
-			 * rate to use here. for now, just use the smallest
-			 * possible rate for the band as a fixed rate for
-			 * association frames and other control messages.
-			 */
-			if (!test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags))
-				wl1271_set_band_rate(wl, wlvif);
-
-			wlvif->basic_rate =
-				wl1271_tx_min_rate_get(wl,
-						       wlvif->basic_rate_set);
-			ret = wl1271_acx_sta_rate_policies(wl, wlvif);
-			if (ret < 0)
-				wl1271_warning("rate policy for channel "
-					       "failed %d", ret);
-		}
-	}
+	int ret;
 
 	if ((changed & IEEE80211_CONF_CHANGE_PS) && !is_ap) {
 
@@ -2666,37 +2624,17 @@ static int wl1271_op_config(struct ieee80211_hw *hw, u32 changed)
 	struct wl1271 *wl = hw->priv;
 	struct wl12xx_vif *wlvif;
 	struct ieee80211_conf *conf = &hw->conf;
-	int channel, ret = 0;
-
-	channel = ieee80211_frequency_to_channel(conf->channel->center_freq);
+	int ret = 0;
 
-	wl1271_debug(DEBUG_MAC80211, "mac80211 config ch %d psm %s power %d %s"
+	wl1271_debug(DEBUG_MAC80211, "mac80211 config psm %s power %d %s"
 		     " changed 0x%x",
-		     channel,
 		     conf->flags & IEEE80211_CONF_PS ? "on" : "off",
 		     conf->power_level,
 		     conf->flags & IEEE80211_CONF_IDLE ? "idle" : "in use",
 			 changed);
 
-	/*
-	 * mac80211 will go to idle nearly immediately after transmitting some
-	 * frames, such as the deauth. To make sure those frames reach the air,
-	 * wait here until the TX queue is fully flushed.
-	 */
-	if ((changed & IEEE80211_CONF_CHANGE_CHANNEL) ||
-	    ((changed & IEEE80211_CONF_CHANGE_IDLE) &&
-	     (conf->flags & IEEE80211_CONF_IDLE)))
-		wl1271_tx_flush(wl);
-
 	mutex_lock(&wl->mutex);
 
-	/* we support configuring the channel and band even while off */
-	if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
-		wl->band = conf->channel->band;
-		wl->channel = channel;
-		wl->channel_type = conf->channel_type;
-	}
-
 	if (changed & IEEE80211_CONF_CHANGE_POWER)
 		wl->power_level = conf->power_level;
 
-- 
1.7.6.401.g6a319

Re: [PATCH 09/15] wlcore: remove channel handling from op_config

[Arik Nemtsov]

On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <eliad@wizery.com> wrote:
> After setting the channel on chanctx callbacks, we
> no longer need to handle channel change notifications
> on op_config.
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
[...]
> -       /*
> -        * mac80211 will go to idle nearly immediately after transmitting some
> -        * frames, such as the deauth. To make sure those frames reach the air,
> -        * wait here until the TX queue is fully flushed.
> -        */
> -       if ((changed & IEEE80211_CONF_CHANGE_CHANNEL) ||
> -           ((changed & IEEE80211_CONF_CHANGE_IDLE) &&
> -            (conf->flags & IEEE80211_CONF_IDLE)))
> -               wl1271_tx_flush(wl);

This flush (and the comment above) is also a good reason for flushing
tx during wlcore_op_unassign_vif_chanctx...

[PATCH 10/15] wlcore: initiate ROC/CROC on sta state updates

[Eliad Peller]

Use the sta_state notifications to ROC when a station
is about to connect, and CROC respectively on
authorization (success) / deletion (failure).

Change the wl12xx_update_sta_state() flow to bail out
only on error, so multiple code blocks could refer
to the same state.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/main.c |   43 ++++++++++++++++++++------------
 1 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index fcdf381..e1f0606 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -83,8 +83,6 @@ static int wl12xx_set_authorized(struct wl1271 *wl,
 	if (ret < 0)
 		return ret;
 
-	wl12xx_croc(wl, wlvif->role_id);
-
 	wl1271_info("Association completed.");
 	return 0;
 }
@@ -3944,14 +3942,6 @@ sta_not_found:
 			wl1271_warning("cmd join failed %d", ret);
 			goto out;
 		}
-
-		/* ROC until connected (after EAPOL exchange) */
-		if (!is_ibss) {
-			ret = wl12xx_roc(wl, wlvif, wlvif->role_id,
-					 wlvif->channel);
-			if (ret < 0)
-				goto out;
-		}
 	}
 
 	if (set_assoc) {
@@ -4355,8 +4345,11 @@ static int wl12xx_update_sta_state(struct wl1271 *wl,
 	/* Add station (AP mode) */
 	if (is_ap &&
 	    old_state == IEEE80211_STA_NOTEXIST &&
-	    new_state == IEEE80211_STA_NONE)
-		return wl12xx_sta_add(wl, wlvif, sta);
+	    new_state == IEEE80211_STA_NONE) {
+		ret = wl12xx_sta_add(wl, wlvif, sta);
+		if (ret)
+			return ret;
+	}
 
 	/* Remove station (AP mode) */
 	if (is_ap &&
@@ -4364,7 +4357,6 @@ static int wl12xx_update_sta_state(struct wl1271 *wl,
 	    new_state == IEEE80211_STA_NOTEXIST) {
 		/* must not fail */
 		wl12xx_sta_remove(wl, wlvif, sta);
-		return 0;
 	}
 
 	/* Authorize station (AP mode) */
@@ -4376,14 +4368,17 @@ static int wl12xx_update_sta_state(struct wl1271 *wl,
 
 		ret = wl1271_acx_set_ht_capabilities(wl, &sta->ht_cap, true,
 						     hlid);
-		return ret;
+		if (ret)
+			return ret;
 	}
 
 	/* Authorize station */
 	if (is_sta &&
 	    new_state == IEEE80211_STA_AUTHORIZED) {
 		set_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags);
-		return wl12xx_set_authorized(wl, wlvif);
+		ret = wl12xx_set_authorized(wl, wlvif);
+		if (ret)
+			return ret;
 	}
 
 	if (is_sta &&
@@ -4391,9 +4386,25 @@ static int wl12xx_update_sta_state(struct wl1271 *wl,
 	    new_state == IEEE80211_STA_ASSOC) {
 		clear_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags);
 		clear_bit(WLVIF_FLAG_STA_STATE_SENT, &wlvif->flags);
-		return 0;
 	}
 
+	/* clear ROCs on failure or authorization */
+	if (is_sta &&
+	    (new_state == IEEE80211_STA_AUTHORIZED ||
+	     new_state == IEEE80211_STA_NOTEXIST)) {
+		if (test_bit(wlvif->role_id, wl->roc_map))
+			wl12xx_croc(wl, wlvif->role_id);
+	}
+
+	if (is_sta &&
+	    old_state == IEEE80211_STA_NOTEXIST &&
+	    new_state == IEEE80211_STA_NONE) {
+		if (find_first_bit(wl->roc_map,
+				   WL12XX_MAX_ROLES) >= WL12XX_MAX_ROLES) {
+			WARN_ON(wlvif->role_id == WL12XX_INVALID_ROLE_ID);
+			wl12xx_roc(wl, wlvif, wlvif->role_id, wlvif->channel);
+		}
+	}
 	return 0;
 }
 
-- 
1.7.6.401.g6a319

Re: [PATCH 10/15] wlcore: initiate ROC/CROC on sta state updates

[Arik Nemtsov]

On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <eliad@wizery.com> wrote:
> Use the sta_state notifications to ROC when a station
> is about to connect, and CROC respectively on
> authorization (success) / deletion (failure).
>
> Change the wl12xx_update_sta_state() flow to bail out
> only on error, so multiple code blocks could refer
> to the same state.
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
[...]
>
> +       /* clear ROCs on failure or authorization */
> +       if (is_sta &&
> +           (new_state == IEEE80211_STA_AUTHORIZED ||
> +            new_state == IEEE80211_STA_NOTEXIST)) {
> +               if (test_bit(wlvif->role_id, wl->roc_map))
> +                       wl12xx_croc(wl, wlvif->role_id);
> +       }
> +
> +       if (is_sta &&
> +           old_state == IEEE80211_STA_NOTEXIST &&
> +           new_state == IEEE80211_STA_NONE) {
> +               if (find_first_bit(wl->roc_map,
> +                                  WL12XX_MAX_ROLES) >= WL12XX_MAX_ROLES) {
> +                       WARN_ON(wlvif->role_id == WL12XX_INVALID_ROLE_ID);
> +                       wl12xx_roc(wl, wlvif, wlvif->role_id, wlvif->channel);
> +               }
> +       }

what about AP mode? we don't have an opportunistic ROC there as well
for connecting stations?

Re: [PATCH 10/15] wlcore: initiate ROC/CROC on sta state updates

[Eliad Peller]

On Mon, Nov 19, 2012 at 11:41 PM, Arik Nemtsov <arik@wizery.com> wrote:
> On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <eliad@wizery.com> wrote:
>> Use the sta_state notifications to ROC when a station
>> is about to connect, and CROC respectively on
>> authorization (success) / deletion (failure).
>>
>> Change the wl12xx_update_sta_state() flow to bail out
>> only on error, so multiple code blocks could refer
>> to the same state.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
> [...]
>>
>> +       /* clear ROCs on failure or authorization */
>> +       if (is_sta &&
>> +           (new_state == IEEE80211_STA_AUTHORIZED ||
>> +            new_state == IEEE80211_STA_NOTEXIST)) {
>> +               if (test_bit(wlvif->role_id, wl->roc_map))
>> +                       wl12xx_croc(wl, wlvif->role_id);
>> +       }
>> +
>> +       if (is_sta &&
>> +           old_state == IEEE80211_STA_NOTEXIST &&
>> +           new_state == IEEE80211_STA_NONE) {
>> +               if (find_first_bit(wl->roc_map,
>> +                                  WL12XX_MAX_ROLES) >= WL12XX_MAX_ROLES) {
>> +                       WARN_ON(wlvif->role_id == WL12XX_INVALID_ROLE_ID);
>> +                       wl12xx_roc(wl, wlvif, wlvif->role_id, wlvif->channel);
>> +               }
>> +       }
>
> what about AP mode? we don't have an opportunistic ROC there as well
> for connecting stations?

yeah, we do. but since in order to really be effective it requires
supplicant changes that weren't upstreamed yet (adding non-associated
station), i preferred postponing it to another patchset.
on the other hand, this might help protecting the EAPOL exchange even
in its current form, so i guess we can upstream it even before the
supplicant changes.

Eliad.

Re: [PATCH 10/15] wlcore: initiate ROC/CROC on sta state updates

[Arik Nemtsov]

On Tue, Nov 20, 2012 at 10:42 AM, Eliad Peller <eliad@wizery.com> wrote:
> On Mon, Nov 19, 2012 at 11:41 PM, Arik Nemtsov <arik@wizery.com> wrote:
>> On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <eliad@wizery.com> wrote:
>>> Use the sta_state notifications to ROC when a station
>>> is about to connect, and CROC respectively on
>>> authorization (success) / deletion (failure).
>>>
>>> Change the wl12xx_update_sta_state() flow to bail out
>>> only on error, so multiple code blocks could refer
>>> to the same state.
>>>
>>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> [...]
>>>
>>> +       /* clear ROCs on failure or authorization */
>>> +       if (is_sta &&
>>> +           (new_state == IEEE80211_STA_AUTHORIZED ||
>>> +            new_state == IEEE80211_STA_NOTEXIST)) {
>>> +               if (test_bit(wlvif->role_id, wl->roc_map))
>>> +                       wl12xx_croc(wl, wlvif->role_id);
>>> +       }
>>> +
>>> +       if (is_sta &&
>>> +           old_state == IEEE80211_STA_NOTEXIST &&
>>> +           new_state == IEEE80211_STA_NONE) {
>>> +               if (find_first_bit(wl->roc_map,
>>> +                                  WL12XX_MAX_ROLES) >= WL12XX_MAX_ROLES) {
>>> +                       WARN_ON(wlvif->role_id == WL12XX_INVALID_ROLE_ID);
>>> +                       wl12xx_roc(wl, wlvif, wlvif->role_id, wlvif->channel);
>>> +               }
>>> +       }
>>
>> what about AP mode? we don't have an opportunistic ROC there as well
>> for connecting stations?
>
> yeah, we do. but since in order to really be effective it requires
> supplicant changes that weren't upstreamed yet (adding non-associated
> station), i preferred postponing it to another patchset.
> on the other hand, this might help protecting the EAPOL exchange even
> in its current form, so i guess we can upstream it even before the
> supplicant changes.

yea. since this is opportunistic, it can't really hurt.

[PATCH 11/15] wlcore: set active psm on association

[Eliad Peller]

The default ps mode of the fw is auto, while the default
ps mode of mac80211 is active (ps off).
In order to sync them, configure active ps on association.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/main.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index e1f0606..0dd0ec1 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2534,6 +2534,15 @@ static int wlcore_set_assoc(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 					   ACX_KEEP_ALIVE_TPL_VALID);
 	if (ret < 0)
 		goto out;
+
+	/*
+	 * The default fw psm configuration is AUTO, while mac80211 default
+	 * setting is off (ACTIVE), so sync the fw with the correct value.
+	 */
+	ret = wl1271_ps_set_mode(wl, wlvif, STATION_ACTIVE_MODE);
+	if (ret < 0)
+		goto out;
+
 out:
 	return ret;
 }
-- 
1.7.6.401.g6a319

Re: [PATCH 11/15] wlcore: set active psm on association

[Luciano Coelho]

On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> The default ps mode of the fw is auto, while the default
> ps mode of mac80211 is active (ps off).
> In order to sync them, configure active ps on association.
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
>  drivers/net/wireless/ti/wlcore/main.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
> index e1f0606..0dd0ec1 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -2534,6 +2534,15 @@ static int wlcore_set_assoc(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>  					   ACX_KEEP_ALIVE_TPL_VALID);
>  	if (ret < 0)
>  		goto out;
> +
> +	/*
> +	 * The default fw psm configuration is AUTO, while mac80211 default
> +	 * setting is off (ACTIVE), so sync the fw with the correct value.
> +	 */
> +	ret = wl1271_ps_set_mode(wl, wlvif, STATION_ACTIVE_MODE);
> +	if (ret < 0)
> +		goto out;
> +

Does mac80211 enable PS soon after this by default?

--
Luca.

Re: [PATCH 11/15] wlcore: set active psm on association

[Eliad Peller]

On Tue, Nov 20, 2012 at 9:39 AM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> The default ps mode of the fw is auto, while the default
>> ps mode of mac80211 is active (ps off).
>> In order to sync them, configure active ps on association.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>>  drivers/net/wireless/ti/wlcore/main.c |    9 +++++++++
>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
>> index e1f0606..0dd0ec1 100644
>> --- a/drivers/net/wireless/ti/wlcore/main.c
>> +++ b/drivers/net/wireless/ti/wlcore/main.c
>> @@ -2534,6 +2534,15 @@ static int wlcore_set_assoc(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>>                                          ACX_KEEP_ALIVE_TPL_VALID);
>>       if (ret < 0)
>>               goto out;
>> +
>> +     /*
>> +      * The default fw psm configuration is AUTO, while mac80211 default
>> +      * setting is off (ACTIVE), so sync the fw with the correct value.
>> +      */
>> +     ret = wl1271_ps_set_mode(wl, wlvif, STATION_ACTIVE_MODE);
>> +     if (ret < 0)
>> +             goto out;
>> +
>
> Does mac80211 enable PS soon after this by default?
>
usually yes (unless userspace disabled it, etc.).
the problem is when mac80211 doesn't enable it - we won't get any
CHANGED_PSM notification, and then while mac80211 considers it as
active, the fw is configured (by default) to AUTO.

Eliad.

[PATCH 12/15] wlcore: specify correct supported_rates

[Eliad Peller]

The supported_rates field should contain all our supported
rates, even if the remote peer doesn't support them.

(use CONF_TX_AP_ENABLED_RATES for bg rates, as the possible
rates are the same for ap and sta)

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/cmd.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index 28a235f..4c2ed25 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -441,6 +441,7 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 {
 	struct ieee80211_vif *vif = wl12xx_wlvif_to_vif(wlvif);
 	struct wl12xx_cmd_role_start *cmd;
+	u32 supported_rates;
 	int ret;
 
 	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
@@ -461,7 +462,14 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 	cmd->sta.ssid_len = wlvif->ssid_len;
 	memcpy(cmd->sta.ssid, wlvif->ssid, wlvif->ssid_len);
 	memcpy(cmd->sta.bssid, vif->bss_conf.bssid, ETH_ALEN);
-	cmd->sta.local_rates = cpu_to_le32(wlvif->rate_set);
+
+	supported_rates = CONF_TX_AP_ENABLED_RATES | CONF_TX_MCS_RATES |
+			  wlcore_hw_sta_get_ap_rate_mask(wl, wlvif);
+	if (wlvif->p2p)
+		supported_rates &= ~CONF_TX_CCK_RATES;
+
+	cmd->sta.local_rates = cpu_to_le32(supported_rates);
+
 	cmd->channel_type = wlcore_get_native_channel_type(wlvif->channel_type);
 
 	if (wlvif->sta.hlid == WL12XX_INVALID_LINK_ID) {
-- 
1.7.6.401.g6a319

Re: [PATCH 12/15] wlcore: specify correct supported_rates

[Luciano Coelho]

On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> The supported_rates field should contain all our supported
> rates, even if the remote peer doesn't support them.
> 
> (use CONF_TX_AP_ENABLED_RATES for bg rates, as the possible
> rates are the same for ap and sta)

Maybe the macro should be renamed then?


> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
>  drivers/net/wireless/ti/wlcore/cmd.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
> index 28a235f..4c2ed25 100644
> --- a/drivers/net/wireless/ti/wlcore/cmd.c
> +++ b/drivers/net/wireless/ti/wlcore/cmd.c
> @@ -441,6 +441,7 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>  {
>  	struct ieee80211_vif *vif = wl12xx_wlvif_to_vif(wlvif);
>  	struct wl12xx_cmd_role_start *cmd;
> +	u32 supported_rates;
>  	int ret;
>  
>  	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
> @@ -461,7 +462,14 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>  	cmd->sta.ssid_len = wlvif->ssid_len;
>  	memcpy(cmd->sta.ssid, wlvif->ssid, wlvif->ssid_len);
>  	memcpy(cmd->sta.bssid, vif->bss_conf.bssid, ETH_ALEN);
> -	cmd->sta.local_rates = cpu_to_le32(wlvif->rate_set);
> +
> +	supported_rates = CONF_TX_AP_ENABLED_RATES | CONF_TX_MCS_RATES |
> +			  wlcore_hw_sta_get_ap_rate_mask(wl, wlvif);
> +	if (wlvif->p2p)
> +		supported_rates &= ~CONF_TX_CCK_RATES;

Why not do this when creating the vif (init_vif_data)? I think this kind
of code doesn't belong in the cmd function.  Seem better to adjust
wlvif->rate_set.

--
Luca.

Re: [PATCH 12/15] wlcore: specify correct supported_rates

[Eliad Peller]

On Tue, Nov 20, 2012 at 9:55 AM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> The supported_rates field should contain all our supported
>> rates, even if the remote peer doesn't support them.
>>
>> (use CONF_TX_AP_ENABLED_RATES for bg rates, as the possible
>> rates are the same for ap and sta)
>
> Maybe the macro should be renamed then?
>
sure.

>> @@ -461,7 +462,14 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>>       cmd->sta.ssid_len = wlvif->ssid_len;
>>       memcpy(cmd->sta.ssid, wlvif->ssid, wlvif->ssid_len);
>>       memcpy(cmd->sta.bssid, vif->bss_conf.bssid, ETH_ALEN);
>> -     cmd->sta.local_rates = cpu_to_le32(wlvif->rate_set);
>> +
>> +     supported_rates = CONF_TX_AP_ENABLED_RATES | CONF_TX_MCS_RATES |
>> +                       wlcore_hw_sta_get_ap_rate_mask(wl, wlvif);
>> +     if (wlvif->p2p)
>> +             supported_rates &= ~CONF_TX_CCK_RATES;
>
> Why not do this when creating the vif (init_vif_data)? I think this kind
> of code doesn't belong in the cmd function.  Seem better to adjust
> wlvif->rate_set.
>
well, this value is used only here. i don't think adding it to wl
struct and initializing it in a different place is really better.
however, i don't mind doing so if you insist :)

Eliad.

[PATCH 13/15] wlcore: reconfigure rate policy on association

[Eliad Peller]

When first configuring the rate policy, before auth,
we still don't have the correct rates that were
agreed during association.

Reconfigure the rate policy on association in order
to update them.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/main.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 0dd0ec1..a1ad326 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3771,7 +3771,8 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
 		wlvif->rssi_thold = bss_conf->cqm_rssi_thold;
 	}
 
-	if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT)) {
+	if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT |
+		       BSS_CHANGED_ASSOC)) {
 		rcu_read_lock();
 		sta = ieee80211_find_sta(vif, bss_conf->bssid);
 		if (!sta)
@@ -3958,6 +3959,17 @@ sta_not_found:
 		if (ret < 0)
 			goto out;
 
+		if (sta_rate_set) {
+			wlvif->rate_set =
+				wl1271_tx_enabled_rates_get(wl,
+							    sta_rate_set,
+							    wlvif->band);
+
+			ret = wl1271_acx_sta_rate_policies(wl, wlvif);
+			if (ret < 0)
+				goto out;
+		}
+
 		if (test_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags))
 			wl12xx_set_authorized(wl, wlvif);
 	}
-- 
1.7.6.401.g6a319

Re: [PATCH 13/15] wlcore: reconfigure rate policy on association

[Luciano Coelho]

On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> When first configuring the rate policy, before auth,
> we still don't have the correct rates that were
> agreed during association.
> 
> Reconfigure the rate policy on association in order
> to update them.
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
>  drivers/net/wireless/ti/wlcore/main.c |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
> index 0dd0ec1..a1ad326 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -3771,7 +3771,8 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
>  		wlvif->rssi_thold = bss_conf->cqm_rssi_thold;
>  	}
>  
> -	if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT)) {
> +	if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT |
> +		       BSS_CHANGED_ASSOC)) {
>  		rcu_read_lock();
>  		sta = ieee80211_find_sta(vif, bss_conf->bssid);
>  		if (!sta)
> @@ -3958,6 +3959,17 @@ sta_not_found:
>  		if (ret < 0)
>  			goto out;
>  
> +		if (sta_rate_set) {
> +			wlvif->rate_set =
> +				wl1271_tx_enabled_rates_get(wl,
> +							    sta_rate_set,
> +							    wlvif->band);
> +
> +			ret = wl1271_acx_sta_rate_policies(wl, wlvif);
> +			if (ret < 0)
> +				goto out;
> +		}
> +
>  		if (test_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags))
>  			wl12xx_set_authorized(wl, wlvif);
>  	}

Yes, it's messy that we use the same value for vif-supported rates and
current association negotiated rates... :(

--
Luca.

Re: [PATCH 13/15] wlcore: reconfigure rate policy on association

[Eliad Peller]

On Tue, Nov 20, 2012 at 10:13 AM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> When first configuring the rate policy, before auth,
>> we still don't have the correct rates that were
>> agreed during association.
>>
>> Reconfigure the rate policy on association in order
>> to update them.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>>  drivers/net/wireless/ti/wlcore/main.c |   14 +++++++++++++-
>>  1 files changed, 13 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
>> index 0dd0ec1..a1ad326 100644
>> --- a/drivers/net/wireless/ti/wlcore/main.c
>> +++ b/drivers/net/wireless/ti/wlcore/main.c
>> @@ -3771,7 +3771,8 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
>>               wlvif->rssi_thold = bss_conf->cqm_rssi_thold;
>>       }
>>
>> -     if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT)) {
>> +     if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT |
>> +                    BSS_CHANGED_ASSOC)) {
>>               rcu_read_lock();
>>               sta = ieee80211_find_sta(vif, bss_conf->bssid);
>>               if (!sta)
>> @@ -3958,6 +3959,17 @@ sta_not_found:
>>               if (ret < 0)
>>                       goto out;
>>
>> +             if (sta_rate_set) {
>> +                     wlvif->rate_set =
>> +                             wl1271_tx_enabled_rates_get(wl,
>> +                                                         sta_rate_set,
>> +                                                         wlvif->band);
>> +
>> +                     ret = wl1271_acx_sta_rate_policies(wl, wlvif);
>> +                     if (ret < 0)
>> +                             goto out;
>> +             }
>> +
>>               if (test_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags))
>>                       wl12xx_set_authorized(wl, wlvif);
>>       }
>
> Yes, it's messy that we use the same value for vif-supported rates and
> current association negotiated rates... :(
>
yeah, we have a total mess with all the configured/supported rates.
we'll have to reorganize it at some point.

Eliad.

[PATCH 14/15] wlcore: refactor CHANGED_HT handling

[Eliad Peller]

Pass a variable indicating whether HT is enabled,
instead of duplicating the function call with
different arguments.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/main.c |   48 ++++++++++++--------------------
 1 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index a1ad326..a81e50b 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3975,44 +3975,32 @@ sta_not_found:
 	}
 
 	/* Handle new association with HT. Do this after join. */
-	if (sta_exists) {
-		if ((changed & BSS_CHANGED_HT) &&
-		    (bss_conf->channel_type != NL80211_CHAN_NO_HT)) {
-			ret = wl1271_acx_set_ht_capabilities(wl,
-							     &sta_ht_cap,
-							     true,
-							     wlvif->sta.hlid);
-			if (ret < 0) {
-				wl1271_warning("Set ht cap true failed %d",
-					       ret);
-				goto out;
-			}
+	if (sta_exists &&
+	    (changed & BSS_CHANGED_HT)) {
+		bool enabled =
+			bss_conf->channel_type != NL80211_CHAN_NO_HT;
+
+		ret = wl1271_acx_set_ht_capabilities(wl,
+						     &sta_ht_cap,
+						     enabled,
+						     wlvif->sta.hlid);
+		if (ret < 0) {
+			wl1271_warning("Set ht cap failed %d", ret);
+			goto out;
+
 		}
-		/* handle new association without HT and disassociation */
-		else if (changed & BSS_CHANGED_ASSOC) {
-			ret = wl1271_acx_set_ht_capabilities(wl,
-							     &sta_ht_cap,
-							     false,
-							     wlvif->sta.hlid);
+
+		if (enabled) {
+			ret = wl1271_acx_set_ht_information(wl, wlvif,
+						bss_conf->ht_operation_mode);
 			if (ret < 0) {
-				wl1271_warning("Set ht cap false failed %d",
+				wl1271_warning("Set ht information failed %d",
 					       ret);
 				goto out;
 			}
 		}
 	}
 
-	/* Handle HT information change. Done after join. */
-	if ((changed & BSS_CHANGED_HT) &&
-	    (bss_conf->channel_type != NL80211_CHAN_NO_HT)) {
-		ret = wl1271_acx_set_ht_information(wl, wlvif,
-					bss_conf->ht_operation_mode);
-		if (ret < 0) {
-			wl1271_warning("Set ht information failed %d", ret);
-			goto out;
-		}
-	}
-
 	/* Handle arp filtering. Done after join. */
 	if ((changed & BSS_CHANGED_ARP_FILTER) ||
 	    (!is_ibss && (changed & BSS_CHANGED_QOS))) {
-- 
1.7.6.401.g6a319

[PATCH 15/15] wlcore: configure the remote rates with our own rates

[Eliad Peller]

With the new connection flow, start_sta is called before
the remote rates where updated. Use our own supported rates
instead to make sure we don't disable any potential rate
(the rate policies will be updated later, but there is
currently no way to update the remote rates)

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/cmd.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index 4c2ed25..ec7e8b3 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -479,7 +479,12 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 	}
 	cmd->sta.hlid = wlvif->sta.hlid;
 	cmd->sta.session = wl12xx_get_new_session_id(wl, wlvif);
-	cmd->sta.remote_rates = cpu_to_le32(wlvif->rate_set);
+	/*
+	 * We don't have the correct remote rates in this stage, and there
+	 * is no way to update them later, so use our supported rates instead.
+	 * The fw will take the configured rate policies into account anyway.
+	 */
+	cmd->sta.remote_rates = cpu_to_le32(supported_rates);
 
 	wl1271_debug(DEBUG_CMD, "role start: roleid=%d, hlid=%d, session=%d "
 		     "basic_rate_set: 0x%x, remote_rates: 0x%x",
-- 
1.7.6.401.g6a319

Re: [PATCH 15/15] wlcore: configure the remote rates with our own rates

[Luciano Coelho]

On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> With the new connection flow, start_sta is called before
> the remote rates where updated. Use our own supported rates
> instead to make sure we don't disable any potential rate
> (the rate policies will be updated later, but there is
> currently no way to update the remote rates)
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
>  drivers/net/wireless/ti/wlcore/cmd.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
> index 4c2ed25..ec7e8b3 100644
> --- a/drivers/net/wireless/ti/wlcore/cmd.c
> +++ b/drivers/net/wireless/ti/wlcore/cmd.c
> @@ -479,7 +479,12 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>  	}
>  	cmd->sta.hlid = wlvif->sta.hlid;
>  	cmd->sta.session = wl12xx_get_new_session_id(wl, wlvif);
> -	cmd->sta.remote_rates = cpu_to_le32(wlvif->rate_set);
> +	/*
> +	 * We don't have the correct remote rates in this stage, and there
> +	 * is no way to update them later, so use our supported rates instead.
> +	 * The fw will take the configured rate policies into account anyway.
> +	 */
> +	cmd->sta.remote_rates = cpu_to_le32(supported_rates);

Why do we even have to pass this value, then? Any hidden reason that we
may be overlooking?

--
Luca.

Re: [PATCH 15/15] wlcore: configure the remote rates with our own rates

[Eliad Peller]

On Tue, Nov 20, 2012 at 10:18 AM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> With the new connection flow, start_sta is called before
>> the remote rates where updated. Use our own supported rates
>> instead to make sure we don't disable any potential rate
>> (the rate policies will be updated later, but there is
>> currently no way to update the remote rates)
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>>  drivers/net/wireless/ti/wlcore/cmd.c |    7 ++++++-
>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
>> index 4c2ed25..ec7e8b3 100644
>> --- a/drivers/net/wireless/ti/wlcore/cmd.c
>> +++ b/drivers/net/wireless/ti/wlcore/cmd.c
>> @@ -479,7 +479,12 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>>       }
>>       cmd->sta.hlid = wlvif->sta.hlid;
>>       cmd->sta.session = wl12xx_get_new_session_id(wl, wlvif);
>> -     cmd->sta.remote_rates = cpu_to_le32(wlvif->rate_set);
>> +     /*
>> +      * We don't have the correct remote rates in this stage, and there
>> +      * is no way to update them later, so use our supported rates instead.
>> +      * The fw will take the configured rate policies into account anyway.
>> +      */
>> +     cmd->sta.remote_rates = cpu_to_le32(supported_rates);
>
> Why do we even have to pass this value, then? Any hidden reason that we
> may be overlooking?
>
the fw uses an intersection our local_supported_rates, the
remote_rates and the rate_policy in order to determine the rates.
i'm not too familiar with its whole logic, though.

Eliad.