Add new ampdu_action ops to support receiver BA.
The BA initiator session management in FW independently.
Signed-off-by: Shahar Levi <[email protected]>
---
Dependencies:
- BA Initiator patches have a runtime dependency on commit
"[RFC v3] wl1271: BA Initiator support"
drivers/net/wireless/wl12xx/acx.c | 38 ++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/acx.h | 21 ++++++++++++++
drivers/net/wireless/wl12xx/event.c | 3 +++
drivers/net/wireless/wl12xx/event.h | 17 ++++++++++-
drivers/net/wireless/wl12xx/main.c | 50 +++++++++++++++++++++++++++++++++++
5 files changed, 128 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/acx.c b/drivers/net/wireless/wl12xx/acx.c
index 6c433b7..85d87dd 100644
--- a/drivers/net/wireless/wl12xx/acx.c
+++ b/drivers/net/wireless/wl12xx/acx.c
@@ -1359,6 +1359,44 @@ out:
return ret;
}
+/* setup BA session receiver setting in the FW. */
+int wl1271_acx_set_ba_receiver_session(struct wl1271 *wl, u8 tid_index,
+ u16 *ssn, u8 policy)
+{
+ struct wl1271_acx_ba_receiver_setup *acx;
+ int ret;
+
+ wl1271_debug(DEBUG_ACX, "acx ba receiver session setting");
+
+ acx = kzalloc(sizeof(*acx), GFP_KERNEL);
+ if (!acx) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ /* Single link for now */
+ acx->link_id = 1;
+ acx->tid = tid_index;
+ acx->enable = policy;
+ acx->win_size = 0;
+
+ if (policy)
+ acx->ssn = *ssn;
+
+ ret = wl1271_cmd_configure(wl,
+ ACX_BA_SESSION_RX_SETUP,
+ acx,
+ sizeof(*acx));
+ if (ret < 0) {
+ wl1271_warning("acx ba receiver session failed: %d", ret);
+ goto out;
+ }
+
+out:
+ kfree(acx);
+ return ret;
+}
+
int wl1271_acx_tsf_info(struct wl1271 *wl, u64 *mactime)
{
struct wl1271_acx_fw_tsf_information *tsf_info;
diff --git a/drivers/net/wireless/wl12xx/acx.h b/drivers/net/wireless/wl12xx/acx.h
index c511dce..6da8423 100644
--- a/drivers/net/wireless/wl12xx/acx.h
+++ b/drivers/net/wireless/wl12xx/acx.h
@@ -1080,6 +1080,25 @@ struct wl1271_acx_ba_session_policy {
u8 padding[3];
} __packed;
+struct wl1271_acx_ba_receiver_setup {
+ struct acx_header header;
+
+ /* Specifies Link Id, Range 0-31, 0xFF means ANY Link Id */
+ u8 link_id;
+
+ u8 tid;
+
+ u8 enable;
+
+ u8 padding[1];
+
+ /* Windows size in number of packets */
+ u16 win_size;
+
+ /* BA session starting sequence number. RANGE 0-FFF */
+ u16 ssn;
+} __packed;
+
struct wl1271_acx_fw_tsf_information {
struct acx_header header;
@@ -1217,6 +1236,8 @@ int wl1271_acx_set_ht_information(struct wl1271 *wl,
int wl1271_acx_set_ba_session(struct wl1271 *wl,
enum ieee80211_back_parties direction,
u8 tid_index, u8 policy);
+int wl1271_acx_set_ba_receiver_session(struct wl1271 *wl, u8 tid_index,
+ u16 *ssn, u8 policy);
int wl1271_acx_tsf_info(struct wl1271 *wl, u64 *mactime);
#endif /* __WL1271_ACX_H__ */
diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
index f9146f5..4e053ff 100644
--- a/drivers/net/wireless/wl12xx/event.c
+++ b/drivers/net/wireless/wl12xx/event.c
@@ -241,6 +241,9 @@ static int wl1271_event_process(struct wl1271 *wl, struct event_mailbox *mbox)
wl1271_event_rssi_trigger(wl, mbox);
}
+ if (vector & BA_SESSION_RX_CONSTRAINT_EVENT_ID)
+ wl1271_debug(DEBUG_EVENT, "BA_SESSION_RX_CONSTRAINT_EVENT_ID");
+
if (wl->vif && beacon_loss)
ieee80211_connection_loss(wl->vif);
diff --git a/drivers/net/wireless/wl12xx/event.h b/drivers/net/wireless/wl12xx/event.h
index 6cce014..4c697d7 100644
--- a/drivers/net/wireless/wl12xx/event.h
+++ b/drivers/net/wireless/wl12xx/event.h
@@ -67,7 +67,7 @@ enum {
HEALTH_CHECK_REPLY_EVENT_ID = BIT(27),
PERIODIC_SCAN_COMPLETE_EVENT_ID = BIT(28),
PERIODIC_SCAN_REPORT_EVENT_ID = BIT(29),
- BA_SESSION_TEAR_DOWN_EVENT_ID = BIT(30),
+ BA_SESSION_RX_CONSTRAINT_EVENT_ID = BIT(30),
EVENT_MBOX_ALL_EVENT_ID = 0x7fffffff,
};
@@ -115,7 +115,20 @@ struct event_mailbox {
u8 scheduled_scan_status;
u8 ps_status;
- u8 reserved_5[29];
+ /*
+ * Bitmap, Each bit set represents the Role ID for which this constraint
+ * is set. Range: 0 - FF, FF means ANY role
+ */
+ u8 ba_role_id;
+ /*
+ * Bitmap, Each bit set represents the Link ID for which this constraint
+ * is set. Not applicable if ba_role_id is set to ANY role (FF).
+ * Range: 0 - FFFF, FFFF means ANY link in that role
+ */
+ u8 ba_link_id;
+ u8 ba_allowed;
+
+ u8 reserved_5[26];
} __packed;
int wl1271_event_unmask(struct wl1271 *wl);
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index b8c42d7..8ef2fcd 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2204,6 +2204,55 @@ static int wl1271_op_get_survey(struct ieee80211_hw *hw, int idx,
return 0;
}
+int wl1271_op_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+ enum ieee80211_ampdu_mlme_action action,
+ struct ieee80211_sta *sta, u16 tid, u16 *ssn)
+{
+ struct wl1271 *wl = hw->priv;
+ int ret;
+
+ ret = wl1271_ps_elp_wakeup(wl, false);
+ if (ret < 0)
+ goto out;
+
+ switch (action) {
+ case IEEE80211_AMPDU_RX_START:
+ if (wl->ba_allowed) {
+ ret = wl1271_acx_set_ba_receiver_session(wl, tid, ssn,
+ true);
+ if (!ret)
+ wl->ba_rx_bitmap |= (u8)(BIT(0) << tid);
+ } else
+ ret = -EPERM;
+ break;
+
+ case IEEE80211_AMPDU_RX_STOP:
+ ret = wl1271_acx_set_ba_receiver_session(wl, tid, ssn, false);
+ if (!ret)
+ wl->ba_rx_bitmap &= ~(u8)(BIT(0) << tid);
+ break;
+
+ /*
+ * The BA initiator session management in FW independently.
+ * Falling break here on purpose for all TX APDU commands.
+ */
+ case IEEE80211_AMPDU_TX_START:
+ case IEEE80211_AMPDU_TX_STOP:
+ case IEEE80211_AMPDU_TX_OPERATIONAL:
+ ret = -EINVAL;
+ break;
+
+ default:
+ wl1271_error("Incorrect ampdu action id=%x\n", action);
+ ret = -ENODEV;
+ }
+
+ wl1271_ps_elp_sleep(wl);
+
+out:
+ return ret;
+}
+
/* can't be const, mac80211 writes to this */
static struct ieee80211_rate wl1271_rates[] = {
{ .bitrate = 10,
@@ -2458,6 +2507,7 @@ static const struct ieee80211_ops wl1271_ops = {
.conf_tx = wl1271_op_conf_tx,
.get_tsf = wl1271_op_get_tsf,
.get_survey = wl1271_op_get_survey,
+ .ampdu_action = wl1271_op_ampdu_action,
CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
};
--
1.7.0.4
On Wed, 2010-11-24 at 18:33 +0200, ext Shahar Levi wrote:
>
> +int wl1271_op_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> + enum ieee80211_ampdu_mlme_action action,
> + struct ieee80211_sta *sta, u16 tid, u16 *ssn)
> +{
> + struct wl1271 *wl = hw->priv;
> + int ret;
> +
> + ret = wl1271_ps_elp_wakeup(wl, false);
> + if (ret < 0)
> + goto out;
> +
> + switch (action) {
> + case IEEE80211_AMPDU_RX_START:
> + if (wl->ba_allowed) {
> + ret = wl1271_acx_set_ba_receiver_session(wl, tid, ssn,
> + true);
> + if (!ret)
> + wl->ba_rx_bitmap |= (u8)(BIT(0) << tid);
> + } else
> + ret = -EPERM;
> + break;
> +
> + case IEEE80211_AMPDU_RX_STOP:
> + ret = wl1271_acx_set_ba_receiver_session(wl, tid, ssn, false);
> + if (!ret)
> + wl->ba_rx_bitmap &= ~(u8)(BIT(0) << tid);
> + break;
> +
> + /*
These "BIT(0) << tid" thingies look weird. Why not BIT(tid) ?
-Juuso
On 11/29/2010 11:51 AM, Juuso Oikarinen wrote:
> On Wed, 2010-11-24 at 18:33 +0200, ext Shahar Levi wrote:
>> Add new ampdu_action ops to support receiver BA.
>> The BA initiator session management in FW independently.
>>
>> Signed-off-by: Shahar Levi<[email protected]>
>> ---
[...]
>> + /* Single link for now */
>> + acx->link_id = 1;
>> + acx->tid = tid_index;
>> + acx->enable = policy;
>> + acx->win_size = 0;
>> +
>
> Here the window size is configured to 0.
>
> As here the mac80211 does the processing and responding of the add BA
> request from the AP, how does the firmware know what window size
> eventually was taken into use?
>
> Also, as we already discussed on IRC, the mac80211 currently responds to
> the add BA request with the same window size suggested by the AP - in
> case of my test AP, this value is 64 - which is too large for the
> firmware to cope. So the firmware imposed limit to the window size must
> be handled somehow.
>
> -Juuso
you right. as discussed on IRC it should be done on the FW. we still
looking on that.
thanks.
On Wed, 2010-11-24 at 18:33 +0200, ext Shahar Levi wrote:
> Add new ampdu_action ops to support receiver BA.
> The BA initiator session management in FW independently.
>
> Signed-off-by: Shahar Levi <[email protected]>
> ---
>
> +int wl1271_op_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> + enum ieee80211_ampdu_mlme_action action,
> + struct ieee80211_sta *sta, u16 tid, u16 *ssn)
> +{
> + struct wl1271 *wl = hw->priv;
> + int ret;
> +
> + ret = wl1271_ps_elp_wakeup(wl, false);
> + if (ret < 0)
> + goto out;
> +
> + switch (action) {
> + case IEEE80211_AMPDU_RX_START:
> + if (wl->ba_allowed) {
> + ret = wl1271_acx_set_ba_receiver_session(wl, tid, ssn,
> + true);
> + if (!ret)
> + wl->ba_rx_bitmap |= (u8)(BIT(0) << tid);
> + } else
> + ret = -EPERM;
> + break;
> +
> + case IEEE80211_AMPDU_RX_STOP:
> + ret = wl1271_acx_set_ba_receiver_session(wl, tid, ssn, false);
> + if (!ret)
> + wl->ba_rx_bitmap &= ~(u8)(BIT(0) << tid);
> + break;
> +
> + /*
> + * The BA initiator session management in FW independently.
> + * Falling break here on purpose for all TX APDU commands.
> + */
> + case IEEE80211_AMPDU_TX_START:
> + case IEEE80211_AMPDU_TX_STOP:
> + case IEEE80211_AMPDU_TX_OPERATIONAL:
> + ret = -EINVAL;
> + break;
> +
> + default:
> + wl1271_error("Incorrect ampdu action id=%x\n", action);
> + ret = -ENODEV;
> + }
> +
> + wl1271_ps_elp_sleep(wl);
> +
> +out:
> + return ret;
> +}
> +
Seems to me that locking of the wl1271 mutex is missing here.
Also, I think it would be good to add checking for interface status
(wl->state == WL1271_STATE_OFF) here too. In normal cases this would not
be needed, but there are at least some hardware recovery scenarios where
this could be called while the firmware is off, causing all types of odd
behavior.
-Juuso
On 11/29/2010 11:52 AM, Luciano Coelho wrote:
> On Wed, 2010-11-24 at 18:33 +0200, ext Shahar Levi wrote:
>> Add new ampdu_action ops to support receiver BA.
>> The BA initiator session management in FW independently.
>>
>> Signed-off-by: Shahar Levi<[email protected]>
>> ---
>
> [...]
>
>> diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
>> index f9146f5..4e053ff 100644
>> --- a/drivers/net/wireless/wl12xx/event.c
>> +++ b/drivers/net/wireless/wl12xx/event.c
>> @@ -241,6 +241,9 @@ static int wl1271_event_process(struct wl1271 *wl, struct event_mailbox *mbox)
>> wl1271_event_rssi_trigger(wl, mbox);
>> }
>>
>> + if (vector& BA_SESSION_RX_CONSTRAINT_EVENT_ID)
>> + wl1271_debug(DEBUG_EVENT, "BA_SESSION_RX_CONSTRAINT_EVENT_ID");
>> +
>> if (wl->vif&& beacon_loss)
>> ieee80211_connection_loss(wl->vif);
>>
>> diff --git a/drivers/net/wireless/wl12xx/event.h b/drivers/net/wireless/wl12xx/event.h
>> index 6cce014..4c697d7 100644
>> --- a/drivers/net/wireless/wl12xx/event.h
>> +++ b/drivers/net/wireless/wl12xx/event.h
>> @@ -67,7 +67,7 @@ enum {
>> HEALTH_CHECK_REPLY_EVENT_ID = BIT(27),
>> PERIODIC_SCAN_COMPLETE_EVENT_ID = BIT(28),
>> PERIODIC_SCAN_REPORT_EVENT_ID = BIT(29),
>> - BA_SESSION_TEAR_DOWN_EVENT_ID = BIT(30),
>> + BA_SESSION_RX_CONSTRAINT_EVENT_ID = BIT(30),
>> EVENT_MBOX_ALL_EVENT_ID = 0x7fffffff,
>> };
>
> BA_SESSION_RX_CONSTRAING_EVENT_ID is not really used in this patch.
> Please move it to the "wl12xx: Stop BA session event from device", where
> it makes more sense.
>
i will move all event code move to the next patch.
Thanks
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBKdXVzbyBPaWthcmluZW4gW21h
aWx0bzpqdXVzby5vaWthcmluZW5Abm9raWEuY29tXQ0KPiBTZW50OiBGcmlkYXksIE5vdmVtYmVy
IDI2LCAyMDEwIDExOjU3IEFNDQo+IFRvOiBMZXZpLCBTaGFoYXINCj4gQ2M6IGxpbnV4LXdpcmVs
ZXNzQHZnZXIua2VybmVsLm9yZzsgTHVjaWFubyBDb2VsaG8NCj4gU3ViamVjdDogUmU6IFtQQVRD
SCBdIHdsMTJ4eDogQkEgcmVjZWl2ZXIgc3VwcG9ydA0KPiANCj4gT24gV2VkLCAyMDEwLTExLTI0
IGF0IDE4OjMzICswMjAwLCBleHQgU2hhaGFyIExldmkgd3JvdGU6DQo+ID4NCj4gPiAraW50IHds
MTI3MV9vcF9hbXBkdV9hY3Rpb24oc3RydWN0IGllZWU4MDIxMV9odyAqaHcsIHN0cnVjdA0KPiBp
ZWVlODAyMTFfdmlmICp2aWYsDQo+ID4gKwkJCSAgIGVudW0gaWVlZTgwMjExX2FtcGR1X21sbWVf
YWN0aW9uIGFjdGlvbiwNCj4gPiArCQkJICAgc3RydWN0IGllZWU4MDIxMV9zdGEgKnN0YSwgdTE2
IHRpZCwgdTE2ICpzc24pDQo+ID4gK3sNCj4gPiArCXN0cnVjdCB3bDEyNzEgKndsID0gaHctPnBy
aXY7DQo+ID4gKwlpbnQgcmV0Ow0KPiA+ICsNCj4gPiArCXJldCA9IHdsMTI3MV9wc19lbHBfd2Fr
ZXVwKHdsLCBmYWxzZSk7DQo+ID4gKwlpZiAocmV0IDwgMCkNCj4gPiArCQlnb3RvIG91dDsNCj4g
PiArDQo+ID4gKwlzd2l0Y2ggKGFjdGlvbikgew0KPiA+ICsJY2FzZSBJRUVFODAyMTFfQU1QRFVf
UlhfU1RBUlQ6DQo+ID4gKwkJaWYgKHdsLT5iYV9hbGxvd2VkKSB7DQo+ID4gKwkJCXJldCA9IHds
MTI3MV9hY3hfc2V0X2JhX3JlY2VpdmVyX3Nlc3Npb24od2wsIHRpZCwNCj4gc3NuLA0KPiA+ICsJ
CQkJCQkJCSB0cnVlKTsNCj4gPiArCQkJaWYgKCFyZXQpDQo+ID4gKwkJCQl3bC0+YmFfcnhfYml0
bWFwIHw9ICh1OCkoQklUKDApIDw8IHRpZCk7DQo+ID4gKwkJfSBlbHNlDQo+ID4gKwkJCXJldCA9
IC1FUEVSTTsNCj4gPiArCQlicmVhazsNCj4gPiArDQo+ID4gKwljYXNlIElFRUU4MDIxMV9BTVBE
VV9SWF9TVE9QOg0KPiA+ICsJCXJldCA9IHdsMTI3MV9hY3hfc2V0X2JhX3JlY2VpdmVyX3Nlc3Np
b24od2wsIHRpZCwgc3NuLA0KPiBmYWxzZSk7DQo+ID4gKwkJaWYgKCFyZXQpDQo+ID4gKwkJCXds
LT5iYV9yeF9iaXRtYXAgJj0gfih1OCkoQklUKDApIDw8IHRpZCk7DQo+ID4gKwkJYnJlYWs7DQo+
ID4gKw0KPiA+ICsJLyoNCj4gDQo+IFRoZXNlICJCSVQoMCkgPDwgdGlkIiB0aGluZ2llcyBsb29r
IHdlaXJkLiBXaHkgbm90IEJJVCh0aWQpID8NCj4gDQo+IC1KdXVzbw0KR29vZCBjYXRjaC4gV2ls
bCBiZSBmaXggb24gdjIuDQpUaGFua3MsDQpTaGFoYXINCg0K
On Wed, 2010-11-24 at 18:33 +0200, ext Shahar Levi wrote:
> Add new ampdu_action ops to support receiver BA.
> The BA initiator session management in FW independently.
>
> Signed-off-by: Shahar Levi <[email protected]>
> ---
[...]
> diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
> index f9146f5..4e053ff 100644
> --- a/drivers/net/wireless/wl12xx/event.c
> +++ b/drivers/net/wireless/wl12xx/event.c
> @@ -241,6 +241,9 @@ static int wl1271_event_process(struct wl1271 *wl, struct event_mailbox *mbox)
> wl1271_event_rssi_trigger(wl, mbox);
> }
>
> + if (vector & BA_SESSION_RX_CONSTRAINT_EVENT_ID)
> + wl1271_debug(DEBUG_EVENT, "BA_SESSION_RX_CONSTRAINT_EVENT_ID");
> +
> if (wl->vif && beacon_loss)
> ieee80211_connection_loss(wl->vif);
>
> diff --git a/drivers/net/wireless/wl12xx/event.h b/drivers/net/wireless/wl12xx/event.h
> index 6cce014..4c697d7 100644
> --- a/drivers/net/wireless/wl12xx/event.h
> +++ b/drivers/net/wireless/wl12xx/event.h
> @@ -67,7 +67,7 @@ enum {
> HEALTH_CHECK_REPLY_EVENT_ID = BIT(27),
> PERIODIC_SCAN_COMPLETE_EVENT_ID = BIT(28),
> PERIODIC_SCAN_REPORT_EVENT_ID = BIT(29),
> - BA_SESSION_TEAR_DOWN_EVENT_ID = BIT(30),
> + BA_SESSION_RX_CONSTRAINT_EVENT_ID = BIT(30),
> EVENT_MBOX_ALL_EVENT_ID = 0x7fffffff,
> };
BA_SESSION_RX_CONSTRAING_EVENT_ID is not really used in this patch.
Please move it to the "wl12xx: Stop BA session event from device", where
it makes more sense.
--
Cheers,
Luca.
On 11/25/2010 11:52 AM, Juuso Oikarinen wrote:
> On Wed, 2010-11-24 at 18:33 +0200, ext Shahar Levi wrote:
>> Add new ampdu_action ops to support receiver BA.
>> The BA initiator session management in FW independently.
>>
>> Signed-off-by: Shahar Levi<[email protected]>
>> ---
>>
>> +int wl1271_op_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>> + enum ieee80211_ampdu_mlme_action action,
>> + struct ieee80211_sta *sta, u16 tid, u16 *ssn)
>> +{
>> + struct wl1271 *wl = hw->priv;
>> + int ret;
>> +
>> + ret = wl1271_ps_elp_wakeup(wl, false);
>> + if (ret< 0)
>> + goto out;
>> +
>> + switch (action) {
>> + case IEEE80211_AMPDU_RX_START:
>> + if (wl->ba_allowed) {
>> + ret = wl1271_acx_set_ba_receiver_session(wl, tid, ssn,
>> + true);
>> + if (!ret)
>> + wl->ba_rx_bitmap |= (u8)(BIT(0)<< tid);
>> + } else
>> + ret = -EPERM;
>> + break;
>> +
>> + case IEEE80211_AMPDU_RX_STOP:
>> + ret = wl1271_acx_set_ba_receiver_session(wl, tid, ssn, false);
>> + if (!ret)
>> + wl->ba_rx_bitmap&= ~(u8)(BIT(0)<< tid);
>> + break;
>> +
>> + /*
>> + * The BA initiator session management in FW independently.
>> + * Falling break here on purpose for all TX APDU commands.
>> + */
>> + case IEEE80211_AMPDU_TX_START:
>> + case IEEE80211_AMPDU_TX_STOP:
>> + case IEEE80211_AMPDU_TX_OPERATIONAL:
>> + ret = -EINVAL;
>> + break;
>> +
>> + default:
>> + wl1271_error("Incorrect ampdu action id=%x\n", action);
>> + ret = -ENODEV;
>> + }
>> +
>> + wl1271_ps_elp_sleep(wl);
>> +
>> +out:
>> + return ret;
>> +}
>> +
>
> Seems to me that locking of the wl1271 mutex is missing here.
You right. will be fix on v2
>
> Also, I think it would be good to add checking for interface status
> (wl->state == WL1271_STATE_OFF) here too. In normal cases this would not
> be needed, but there are at least some hardware recovery scenarios where
> this could be called while the firmware is off, causing all types of odd
> behavior.
it seems that it is good protraction. the setting of WL1271_STATE_OFF is
in wl1271_alloc_hw() and __wl1271_op_remove_interface(). i was expect
that in static void wl1271_recovery_work() after validate (wl->state !=
WL1271_STATE_ON) we will set it to WL1271_STATE_OFF.
> -Juuso
>