[PATCH 0/7] wl12xx: various (multi-vif) fixes

[PATCH 0/7] wl12xx: various (multi-vif) fixes

[Eliad Peller]

Fix some bugs related to multiple vifs support.

Eliad Peller (7):
  wl12xx: implement change_interface
  wl12xx: remove redundant code from wl1271_op_conf_tx
  wl12xx: make WL1271_FLAG_IDLE flag per-vif
  wl12xx: flush packets before stopping dev role
  wl12xx: fix checking of started dev role
  wl12xx: stop device role on remove_interface
  wl12xx: check the actual vif operstate in wl1271_dev_notify

 drivers/net/wireless/wl12xx/cmd.c    |    3 +
 drivers/net/wireless/wl12xx/main.c   |  100 +++++++++++++++++-----------------
 drivers/net/wireless/wl12xx/ps.c     |   10 +++-
 drivers/net/wireless/wl12xx/scan.c   |    2 +-
 drivers/net/wireless/wl12xx/wl12xx.h |    2 +-
 5 files changed, 63 insertions(+), 54 deletions(-)

-- 
1.7.6.401.g6a319

[PATCH 1/7] wl12xx: implement change_interface

[Eliad Peller]

Implement the change_interface callback by simply removing the
current vif and adding a new one after updating the vif type.

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

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index c305841..86a7ee3 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2269,6 +2269,16 @@ out:
 	cancel_work_sync(&wl->recovery_work);
 }
 
+static int wl12xx_op_change_interface(struct ieee80211_hw *hw,
+				      struct ieee80211_vif *vif,
+				      enum nl80211_iftype new_type, bool p2p)
+{
+	wl1271_op_remove_interface(hw, vif);
+
+	vif->type = ieee80211_iftype_p2p(new_type, p2p);
+	return wl1271_op_add_interface(hw, vif);
+}
+
 static int wl1271_join(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 			  bool set_assoc)
 {
@@ -4629,6 +4639,7 @@ static const struct ieee80211_ops wl1271_ops = {
 	.stop = wl1271_op_stop,
 	.add_interface = wl1271_op_add_interface,
 	.remove_interface = wl1271_op_remove_interface,
+	.change_interface = wl12xx_op_change_interface,
 #ifdef CONFIG_PM
 	.suspend = wl1271_op_suspend,
 	.resume = wl1271_op_resume,
-- 
1.7.6.401.g6a319

Re: [PATCH 1/7] wl12xx: implement change_interface

[Arik Nemtsov]

On Sun, Dec 18, 2011 at 20:25, Eliad Peller <eliad@wizery.com> wrote:
> Implement the change_interface callback by simply removing the
> current vif and adding a new one after updating the vif type.
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
>  drivers/net/wireless/wl12xx/main.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
> index c305841..86a7ee3 100644
> --- a/drivers/net/wireless/wl12xx/main.c
> +++ b/drivers/net/wireless/wl12xx/main.c
> @@ -2269,6 +2269,16 @@ out:
>        cancel_work_sync(&wl->recovery_work);
>  }
>
> +static int wl12xx_op_change_interface(struct ieee80211_hw *hw,
> +                                     struct ieee80211_vif *vif,
> +                                     enum nl80211_iftype new_type, bool p2p)
> +{
> +       wl1271_op_remove_interface(hw, vif);
> +
> +       vif->type = ieee80211_iftype_p2p(new_type, p2p);

Isn't this internal?

Re: [PATCH 1/7] wl12xx: implement change_interface

[Eliad Peller]

On Sun, Dec 18, 2011 at 8:37 PM, Arik Nemtsov <anamtsov@gmail.com> wrote:
> On Sun, Dec 18, 2011 at 20:25, Eliad Peller <eliad@wizery.com> wrote:
>> Implement the change_interface callback by simply removing the
>> current vif and adding a new one after updating the vif type.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>>  drivers/net/wireless/wl12xx/main.c |   11 +++++++++++
>>  1 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
>> index c305841..86a7ee3 100644
>> --- a/drivers/net/wireless/wl12xx/main.c
>> +++ b/drivers/net/wireless/wl12xx/main.c
>> @@ -2269,6 +2269,16 @@ out:
>>        cancel_work_sync(&wl->recovery_work);
>>  }
>>
>> +static int wl12xx_op_change_interface(struct ieee80211_hw *hw,
>> +                                     struct ieee80211_vif *vif,
>> +                                     enum nl80211_iftype new_type, bool p2p)
>> +{
>> +       wl1271_op_remove_interface(hw, vif);
>> +
>> +       vif->type = ieee80211_iftype_p2p(new_type, p2p);
>
> Isn't this internal?

it's usually read-only (for the low-level drivers), but here we have
to change it...
otoh, i've just noticed that i forgot to update vif->p2p as well :)

Eliad.

[PATCH 2/7] wl12xx: remove redundant code from wl1271_op_conf_tx

[Eliad Peller]

Since the conf_tx callback passes the vif as param,
we must have been added first (and mac80211 verifies it).

Remove the handling of such case.

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

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 86a7ee3..17e62e4 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -3958,31 +3958,8 @@ static int wl1271_op_conf_tx(struct ieee80211_hw *hw,
 	else
 		ps_scheme = CONF_PS_SCHEME_LEGACY;
 
-	if (wl->state == WL1271_STATE_OFF) {
-		/*
-		 * If the state is off, the parameters will be recorded and
-		 * configured on init. This happens in AP-mode.
-		 */
-		struct conf_tx_ac_category *conf_ac =
-			&wl->conf.tx.ac_conf[wl1271_tx_get_queue(queue)];
-		struct conf_tx_tid *conf_tid =
-			&wl->conf.tx.tid_conf[wl1271_tx_get_queue(queue)];
-
-		conf_ac->ac = wl1271_tx_get_queue(queue);
-		conf_ac->cw_min = (u8)params->cw_min;
-		conf_ac->cw_max = params->cw_max;
-		conf_ac->aifsn = params->aifs;
-		conf_ac->tx_op_limit = params->txop << 5;
-
-		conf_tid->queue_id = wl1271_tx_get_queue(queue);
-		conf_tid->channel_type = CONF_CHANNEL_TYPE_EDCF;
-		conf_tid->tsid = wl1271_tx_get_queue(queue);
-		conf_tid->ps_scheme = ps_scheme;
-		conf_tid->ack_policy = CONF_ACK_POLICY_LEGACY;
-		conf_tid->apsd_conf[0] = 0;
-		conf_tid->apsd_conf[1] = 0;
+	if (!test_bit(WLVIF_FLAG_INITIALIZED, &wlvif->flags))
 		goto out;
-	}
 
 	ret = wl1271_ps_elp_wakeup(wl);
 	if (ret < 0)
-- 
1.7.6.401.g6a319

[PATCH 3/7] wl12xx: make WL1271_FLAG_IDLE flag per-vif

[Eliad Peller]

This flag should be set per-vif, rather than globally.

Rename the flag to indicate IN_USE (rather than IDLE), as
in the default configuration (i.e. flag is clear) the vif
should be idle.

Change all the bit operations (and elp conditions) appropriately.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/wl12xx/main.c   |    8 ++++++--
 drivers/net/wireless/wl12xx/ps.c     |   10 ++++++++--
 drivers/net/wireless/wl12xx/scan.c   |    2 +-
 drivers/net/wireless/wl12xx/wl12xx.h |    2 +-
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 17e62e4..3a8fd33 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2383,6 +2383,10 @@ 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) */
@@ -2401,7 +2405,7 @@ static int wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 			ACX_KEEP_ALIVE_TPL_INVALID);
 		if (ret < 0)
 			goto out;
-		set_bit(WL1271_FLAG_IDLE, &wl->flags);
+		clear_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
 	} else {
 		/* The current firmware only supports sched_scan in idle */
 		if (wl->sched_scanning) {
@@ -2412,7 +2416,7 @@ static int wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 		ret = wl12xx_start_dev(wl, wlvif);
 		if (ret < 0)
 			goto out;
-		clear_bit(WL1271_FLAG_IDLE, &wl->flags);
+		set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
 	}
 
 out:
diff --git a/drivers/net/wireless/wl12xx/ps.c b/drivers/net/wireless/wl12xx/ps.c
index a7a1108..a2bdacd 100644
--- a/drivers/net/wireless/wl12xx/ps.c
+++ b/drivers/net/wireless/wl12xx/ps.c
@@ -53,8 +53,11 @@ void wl1271_elp_work(struct work_struct *work)
 		goto out;
 
 	wl12xx_for_each_wlvif(wl, wlvif) {
+		if (wlvif->bss_type == BSS_TYPE_AP_BSS)
+			goto out;
+
 		if (!test_bit(WLVIF_FLAG_PSM, &wlvif->flags) &&
-		    !test_bit(WL1271_FLAG_IDLE, &wl->flags))
+		    test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags))
 			goto out;
 	}
 
@@ -78,8 +81,11 @@ void wl1271_ps_elp_sleep(struct wl1271 *wl)
 		return;
 
 	wl12xx_for_each_wlvif(wl, wlvif) {
+		if (wlvif->bss_type == BSS_TYPE_AP_BSS)
+			return;
+
 		if (!test_bit(WLVIF_FLAG_PSM, &wlvif->flags) &&
-		    !test_bit(WL1271_FLAG_IDLE, &wl->flags))
+		    test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags))
 			return;
 	}
 
diff --git a/drivers/net/wireless/wl12xx/scan.c b/drivers/net/wireless/wl12xx/scan.c
index 8599dab..108765a 100644
--- a/drivers/net/wireless/wl12xx/scan.c
+++ b/drivers/net/wireless/wl12xx/scan.c
@@ -703,7 +703,7 @@ int wl1271_scan_sched_scan_start(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 	if (wlvif->bss_type != BSS_TYPE_STA_BSS)
 		return -EOPNOTSUPP;
 
-	if (!test_bit(WL1271_FLAG_IDLE, &wl->flags))
+	if (test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags))
 		return -EBUSY;
 
 	start = kzalloc(sizeof(*start), GFP_KERNEL);
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index d21f71f..b2b09cd 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -241,7 +241,6 @@ enum wl12xx_flags {
 	WL1271_FLAG_IN_ELP,
 	WL1271_FLAG_ELP_REQUESTED,
 	WL1271_FLAG_IRQ_RUNNING,
-	WL1271_FLAG_IDLE,
 	WL1271_FLAG_FW_TX_BUSY,
 	WL1271_FLAG_DUMMY_PACKET_PENDING,
 	WL1271_FLAG_SUSPENDED,
@@ -262,6 +261,7 @@ enum wl12xx_vif_flags {
 	WLVIF_FLAG_PSPOLL_FAILURE,
 	WLVIF_FLAG_CS_PROGRESS,
 	WLVIF_FLAG_AP_PROBE_RESP_SET,
+	WLVIF_FLAG_IN_USE,
 };
 
 struct wl1271_link {
-- 
1.7.6.401.g6a319

[PATCH 4/7] wl12xx: flush packets before stopping dev role

[Eliad Peller]

During sta disconnection, a deauth packet is being queued to
the dev role queue. However, the dev role is being stopped
before the packet was sent.

Flush the tx queue before stopping the dev role.

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

diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
index e0d2179..25990bd 100644
--- a/drivers/net/wireless/wl12xx/cmd.c
+++ b/drivers/net/wireless/wl12xx/cmd.c
@@ -1835,6 +1835,9 @@ int wl12xx_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 		      wlvif->bss_type == BSS_TYPE_IBSS)))
 		return -EINVAL;
 
+	/* flush all pending packets */
+	wl1271_tx_work_locked(wl);
+
 	if (test_bit(wlvif->dev_role_id, wl->roc_map)) {
 		ret = wl12xx_croc(wl, wlvif->dev_role_id);
 		if (ret < 0)
-- 
1.7.6.401.g6a319

[PATCH 5/7] wl12xx: fix checking of started dev role

[Eliad Peller]

dev_role_id only indicates whether the dev role
is enabled, not started (e.g. on IBSS merge,
the device role is enabled, but not started).

Checking for any role in ROC (in order to determine
whether dev role was started) is wrong as well,
especially in multi-vif env.

Check for started dev role only by checking the dev_hlid.

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

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 3a8fd33..7a2ed34 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2036,6 +2036,11 @@ out:
 	return booted;
 }
 
+static bool wl12xx_dev_role_started(struct wl12xx_vif *wlvif)
+{
+	return wlvif->dev_hlid != WL12XX_INVALID_LINK_ID;
+}
+
 static int wl1271_op_add_interface(struct ieee80211_hw *hw,
 				   struct ieee80211_vif *vif)
 {
@@ -2368,17 +2373,6 @@ static void wl1271_set_band_rate(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 	wlvif->rate_set = wlvif->basic_rate_set;
 }
 
-static bool wl12xx_is_roc(struct wl1271 *wl)
-{
-	u8 role_id;
-
-	role_id = find_first_bit(wl->roc_map, WL12XX_MAX_ROLES);
-	if (role_id >= WL12XX_MAX_ROLES)
-		return false;
-
-	return true;
-}
-
 static int wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 				  bool idle)
 {
@@ -2390,7 +2384,7 @@ static int wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 
 	if (idle) {
 		/* no need to croc if we weren't busy (e.g. during boot) */
-		if (wl12xx_is_roc(wl)) {
+		if (wl12xx_dev_role_started(wlvif)) {
 			ret = wl12xx_stop_dev(wl, wlvif);
 			if (ret < 0)
 				goto out;
@@ -2460,7 +2454,7 @@ static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 
 			if (test_bit(WLVIF_FLAG_STA_ASSOCIATED,
 				     &wlvif->flags)) {
-				if (wl12xx_is_roc(wl)) {
+				if (wl12xx_dev_role_started(wlvif)) {
 					/* roaming */
 					ret = wl12xx_croc(wl,
 							  wlvif->dev_role_id);
@@ -2477,7 +2471,7 @@ static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 				 * not idle. otherwise, CROC will be called
 				 * anyway.
 				 */
-				if (wl12xx_is_roc(wl) &&
+				if (wl12xx_dev_role_started(wlvif) &&
 				    !(conf->flags & IEEE80211_CONF_IDLE)) {
 					ret = wl12xx_stop_dev(wl, wlvif);
 					if (ret < 0)
@@ -3024,15 +3018,16 @@ static int wl1271_op_hw_scan(struct ieee80211_hw *hw,
 	if (ret < 0)
 		goto out;
 
+	if (test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags) &&
+	    test_bit(wlvif->role_id, wl->roc_map)) {
+		/* don't allow scanning right now */
+		ret = -EBUSY;
+		goto out_sleep;
+	}
+
 	/* cancel ROC before scanning */
-	if (wl12xx_is_roc(wl)) {
-		if (test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags)) {
-			/* don't allow scanning right now */
-			ret = -EBUSY;
-			goto out_sleep;
-		}
+	if (wl12xx_dev_role_started(wlvif))
 		wl12xx_stop_dev(wl, wlvif);
-	}
 
 	ret = wl1271_scan(hw->priv, vif, ssid, len, req);
 out_sleep:
@@ -3843,9 +3838,9 @@ sta_not_found:
 		}
 		/*
 		 * stop device role if started (we might already be in
-		 * STA role). TODO: make it better.
+		 * STA/IBSS role).
 		 */
-		if (wlvif->dev_role_id != WL12XX_INVALID_ROLE_ID) {
+		if (wl12xx_dev_role_started(wlvif)) {
 			ret = wl12xx_stop_dev(wl, wlvif);
 			if (ret < 0)
 				goto out;
-- 
1.7.6.401.g6a319

[PATCH 6/7] wl12xx: stop device role on remove_interface

[Eliad Peller]

When removing a sta/ibss role, the device role has to
stopped (and disabled) as well.

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

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 7a2ed34..4b68b2a 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2189,7 +2189,11 @@ static void __wl1271_op_remove_interface(struct wl1271 *wl,
 		if (ret < 0)
 			goto deinit;
 
-		if (wlvif->bss_type == BSS_TYPE_STA_BSS) {
+		if (wlvif->bss_type == BSS_TYPE_STA_BSS ||
+		    wlvif->bss_type == BSS_TYPE_IBSS) {
+			if (wl12xx_dev_role_started(wlvif))
+				wl12xx_stop_dev(wl, wlvif);
+
 			ret = wl12xx_cmd_role_disable(wl, &wlvif->dev_role_id);
 			if (ret < 0)
 				goto deinit;
-- 
1.7.6.401.g6a319

[PATCH 7/7] wl12xx: check the actual vif operstate in wl1271_dev_notify

[Eliad Peller]

The current wl1271_dev_notify implementation sets the
new operstate to all associated stations (while only
a specific vif was changed).

Until we'll have a method to get the actual vif from
the given dev, check the current operstate of each vif.

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

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 4b68b2a..59d3c55 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -450,7 +450,16 @@ static int wl1271_dev_notify(struct notifier_block *me, unsigned long what,
 	if (wl->state == WL1271_STATE_OFF)
 		goto out;
 
+	if (dev->operstate != IF_OPER_UP)
+		goto out;
+	/*
+	 * The correct behavior should be just getting the appropriate wlvif
+	 * from the given dev, but currently we don't have a mac80211
+	 * interface for it.
+	 */
 	wl12xx_for_each_wlvif_sta(wl, wlvif) {
+		struct ieee80211_vif *vif = wl12xx_wlvif_to_vif(wlvif);
+
 		if (!test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags))
 			continue;
 
@@ -458,7 +467,8 @@ static int wl1271_dev_notify(struct notifier_block *me, unsigned long what,
 		if (ret < 0)
 			goto out;
 
-		wl1271_check_operstate(wl, wlvif, dev->operstate);
+		wl1271_check_operstate(wl, wlvif,
+				       ieee80211_get_operstate(vif));
 
 		wl1271_ps_elp_sleep(wl);
 	}
-- 
1.7.6.401.g6a319

Re: [PATCH 0/7] wl12xx: various (multi-vif) fixes

[Luciano Coelho]

On Sun, 2011-12-18 at 20:25 +0200, Eliad Peller wrote: 
> Fix some bugs related to multiple vifs support.
> 
> Eliad Peller (7):
>   wl12xx: implement change_interface
>   wl12xx: remove redundant code from wl1271_op_conf_tx
>   wl12xx: make WL1271_FLAG_IDLE flag per-vif
>   wl12xx: flush packets before stopping dev role
>   wl12xx: fix checking of started dev role
>   wl12xx: stop device role on remove_interface
>   wl12xx: check the actual vif operstate in wl1271_dev_notify
> 
>  drivers/net/wireless/wl12xx/cmd.c    |    3 +
>  drivers/net/wireless/wl12xx/main.c   |  100 +++++++++++++++++-----------------
>  drivers/net/wireless/wl12xx/ps.c     |   10 +++-
>  drivers/net/wireless/wl12xx/scan.c   |    2 +-
>  drivers/net/wireless/wl12xx/wl12xx.h |    2 +-
>  5 files changed, 63 insertions(+), 54 deletions(-)

Applied, the series, with v2 of 1/7.  Thanks!

-- 
Cheers,
Luca.