Series that add 80211n BA initiator session support wl1271 driver. BA initiator
session management included in FW independently.
BA receiver session not supported in that Series.
Dependencies:
- BA Initiator patches have a runtime dependency on
commit "[PATCH v2] wl1271: 11n Support" series (patches is in linux-wireless
mailing list)
Limitation:
- For now only TID0 supported (Beast Effort).
Thanks,
Shahar
Shahar Levi (3):
wl1271: BA Initiator support, Add Definitions
wl1271: BA Initiator support, BA functions
wl1271: BA Initiator support, active BA ability
drivers/net/wireless/wl12xx/wl1271_acx.c | 51 +++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1271_acx.h | 20 +++++++++++
drivers/net/wireless/wl12xx/wl1271_conf.h | 3 ++
drivers/net/wireless/wl12xx/wl1271_main.c | 54 ++++++++++++++++++++++++++++-
4 files changed, 127 insertions(+), 1 deletions(-)
Thanks for the review.
All will be fix on v2.
Two comments inline.
regards,
Shahar
Luciano Coelho wrote:
> On Thu, 2010-10-07 at 15:06 +0200, ext Shahar Levi wrote:
>> New acx cmd wl1271_acx_ba_session_policy to set BA policy to the FW.
>> Macros to use with BA setting.
>>
>> Signed-off-by: Shahar Levi <[email protected]>
>> ---
>
> You can merge this patch with the next patch, no need to make the
> changes first in the header files and then in the c files, since they go
> very much hand in hand.
>
>
>> drivers/net/wireless/wl12xx/wl1271_acx.h | 16 ++++++++++++++++
>> drivers/net/wireless/wl12xx/wl1271_conf.h | 3 +++
>> 2 files changed, 19 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
>> index b8da1bc..6c3c7c0 100644
>> --- a/drivers/net/wireless/wl12xx/wl1271_acx.h
>> +++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
>> @@ -1055,6 +1055,22 @@ struct wl1271_acx_ht_information {
>> u8 padding[3];
>> } __packed;
>>
>> +/*
>> + * BA sessen interface structure
>> + */
>
> BA session. Actually you can get rid of this comment, as it is obvious
> from the name of the structure that this is about BA sessions.
>
>> +struct wl1271_acx_ba_session_policy {
>> + struct acx_header header;
>> + /* Mac address of: SA as receiver / RA as initiator */
>> + u8 mac_address[ETH_ALEN];
>
> Is this really SA for receiver and RA for initiator? Not SA and DA? Or
> TA and RA?
yes, as initiator it is the receiver address, as receiver it is source
address. I change the comment to:
Mac address of the peer: SA as receiver / RA as initiator
>
>
>> + u8 tid; /* TID */
>
> Comment here is unnecessary.
>
>> + u8 policy; /* Enable / Disable */
>
> Could we change policy to enable here? Then the comment can go away too,
> because the name of the element will be clear enough already.
>
>
>> + u16 win_size; /* windows size in num of packet */
>
> "number of packets"
>
>
>> + u16 inactivity_timeout; /* as initiator inactivity timeout
>> + * in time units(TU) of 1024us.
>> + * as receiver reserved
>> + */
>
> The comment style is wrong.
>
>
>> diff --git a/drivers/net/wireless/wl12xx/wl1271_conf.h b/drivers/net/wireless/wl12xx/wl1271_conf.h
>> index b3e608e..63a0a9a 100644
>> --- a/drivers/net/wireless/wl12xx/wl1271_conf.h
>> +++ b/drivers/net/wireless/wl12xx/wl1271_conf.h
>> @@ -94,6 +94,9 @@ enum {
>> #define HW_BG_RATES_MASK 0xffff
>> #define HW_HT_RATES_OFFSET 16
>>
>> +#define BA_RECEIVER_WIN_SIZE 8
move to wl1271_acx.h, not configurable.
>> +#define BA_INACTIVITY_TIMEOUT 10000
>
> This should be added as a real configuration, like the other stuff in
> the wl1271_conf.h file, which is then used in wl1271_main.c
> default_conf. Please define a struct conf_ba with win_size and
> inactivity_timeout elements and set the actual values in the main file.
>
>
New acx cmd wl1271_acx_ba_session_policy to set BA policy to the FW.
Macros to use with BA setting.
Signed-off-by: Shahar Levi <[email protected]>
---
drivers/net/wireless/wl12xx/wl1271_acx.h | 16 ++++++++++++++++
drivers/net/wireless/wl12xx/wl1271_conf.h | 3 +++
2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
index b8da1bc..6c3c7c0 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.h
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
@@ -1055,6 +1055,22 @@ struct wl1271_acx_ht_information {
u8 padding[3];
} __packed;
+/*
+ * BA sessen interface structure
+ */
+struct wl1271_acx_ba_session_policy {
+ struct acx_header header;
+ /* Mac address of: SA as receiver / RA as initiator */
+ u8 mac_address[ETH_ALEN];
+ u8 tid; /* TID */
+ u8 policy; /* Enable / Disable */
+ u16 win_size; /* windows size in num of packet */
+ u16 inactivity_timeout; /* as initiator inactivity timeout
+ * in time units(TU) of 1024us.
+ * as receiver reserved
+ */
+} __packed;
+
struct wl1271_acx_fw_tsf_information {
struct acx_header header;
diff --git a/drivers/net/wireless/wl12xx/wl1271_conf.h b/drivers/net/wireless/wl12xx/wl1271_conf.h
index b3e608e..63a0a9a 100644
--- a/drivers/net/wireless/wl12xx/wl1271_conf.h
+++ b/drivers/net/wireless/wl12xx/wl1271_conf.h
@@ -94,6 +94,9 @@ enum {
#define HW_BG_RATES_MASK 0xffff
#define HW_HT_RATES_OFFSET 16
+#define BA_RECEIVER_WIN_SIZE 8
+#define BA_INACTIVITY_TIMEOUT 10000
+
enum {
CONF_SG_DISABLE = 0,
CONF_SG_PROTECTIVE,
--
1.6.0.4
Thanks for the review. most will be fix on v2.
Two comments inline.
regards,
Shahar
Luciano Coelho wrote:
> On Thu, 2010-10-07 at 15:06 +0200, ext Shahar Levi wrote:
>> New ops support ampdu_action as initiator returns EINVAL due to
>> the fact that BA initiator session manage in the FW independently.
>> acx function set BA policies.
>
> Please rephrase this as it is hard to understand what the patch really
> is about.
>
>
>> Signed-off-by: Shahar Levi <[email protected]>
>> ---
>
> Some more, mostly style comments. Having mostly style-related comments
> means that your implementation is really good and I appreciate it! It
> just need a bit of polishing and parfumerie ;)
>
> This patch can also be merged with the next one. This doesn't add any
> real functionality and there are things added here (for example the
> mac_addr = FFFFFFFFFFFF is not used at all) that are changed in the next
> patch. This just confuses things.
>
>
>> diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.c b/drivers/net/wireless/wl12xx/wl1271_acx.c
>> index cc6b7d8..f82656e 100644
>> --- a/drivers/net/wireless/wl12xx/wl1271_acx.c
>> +++ b/drivers/net/wireless/wl12xx/wl1271_acx.c
>> @@ -1317,6 +1317,61 @@ out:
>> return ret;
>> }
>>
>> +/*
>> + * wl1271_acx_set_ba_session
>
> Remove this line. And then the comment can be in one line.
>
>
>> + * configure BA session initiator\receiver parameters setting in the FW.
>> + */
>> +int wl1271_acx_set_ba_session(struct wl1271 *wl,
>> + u16 id,
>> + u8 tid_index,
>> + u8 policy)
>
> Fix the indentation and maybe it all fits in one line?
one character is go beyond.
>
>
>> +{
>> + struct wl1271_acx_ba_session_policy *acx;
>> + int ret = 0;
>> + /*
>> + * Note, currently this value will be set to FFFFFFFFFFFF to indicate
>> + * it is relevant for all peers.
>> + */
>> + u8 mac_address[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>
> No need to do this here, since this code is not called and, when you
> call it (ie. in patch 03/03), you remove this FF'ing anyway. ;)
>
>
>> + if (ACX_BA_SESSION_INITIATOR_POLICY == id)
>
> Please use natural order (ie. id == ACX_BA_SESSION_INITIATOR_POLICY).
>
>
>> + acx->inactivity_timeout = BA_INACTIVITY_TIMEOUT;
>> + else
>> + if (ACX_BA_SESSION_RESPONDER_POLICY == id)
>> + acx->inactivity_timeout = 0;
>> + else {
>> + wl1271_error("Incorrect acx command id=%x\n", id);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>
> Use this structure for the if else statements:
>
> if (...) {
> ...
> } else if (...) {
> ...
> } else {
> ...
> }
>
> Or a switch-case.
>
>
>> +
>> + ret = wl1271_cmd_configure(wl,
>> + id,
>> + acx,
>> + sizeof(*acx));
>
> Fits in one line.
>
>
>> diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
>> index 6c3c7c0..f417624 100644
>> --- a/drivers/net/wireless/wl12xx/wl1271_acx.h
>> +++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
>> @@ -1205,6 +1205,10 @@ int wl1271_acx_set_ht_capabilities(struct wl1271 *wl,
>> bool allow_ht_operation);
>> int wl1271_acx_set_ht_information(struct wl1271 *wl,
>> u16 ht_operation_mode);
>> +int wl1271_acx_set_ba_session(struct wl1271 *wl,
>> + u16 id,
>> + u8 tid_index,
>> + u8 policy);
>
> Indentation.
>
>
>> diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
>> index af092f4..53c03e5 100644
>> --- a/drivers/net/wireless/wl12xx/wl1271_main.c
>> +++ b/drivers/net/wireless/wl12xx/wl1271_main.c
>> @@ -1128,6 +1128,22 @@ static void wl1271_configure_filters(struct wl1271 *wl, unsigned int filters)
>> }
>> }
>>
>> +/*
>> + * wl1271_set_ba_policies
>
> Same as earlier: remove this line. And then the comment can be in one
> line.
>
>
>> + * Set the BA session policies to the FW.
>> + */
>> +static void wl1271_set_ba_policies(struct wl1271 *wl)
>> +{
>> + u8 tid_index;
>
> No need for the TAB between u8 and tid_index.
>
>
>> @@ -2056,6 +2072,32 @@ 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)
>> +{
>> + int ret;
>> +
>> + switch (action) {
>> + case IEEE80211_AMPDU_RX_START:
>> + case IEEE80211_AMPDU_RX_STOP:
>> +
>> + /* The BA initiator session management in FW independently */
>> + 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;
>> + }
>> +
>> + return ret;
>> +}
>
> This function doesn't do anything, really. Except for returning -EINVAL
> in TX cases. Is it really needed?
you right, for TX we didn't need it. i will remove it.
On Thu, 2010-10-07 at 15:06 +0200, ext Shahar Levi wrote:
> New ops support ampdu_action as initiator returns EINVAL due to
> the fact that BA initiator session manage in the FW independently.
> acx function set BA policies.
Please rephrase this as it is hard to understand what the patch really
is about.
> Signed-off-by: Shahar Levi <[email protected]>
> ---
Some more, mostly style comments. Having mostly style-related comments
means that your implementation is really good and I appreciate it! It
just need a bit of polishing and parfumerie ;)
This patch can also be merged with the next one. This doesn't add any
real functionality and there are things added here (for example the
mac_addr = FFFFFFFFFFFF is not used at all) that are changed in the next
patch. This just confuses things.
> diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.c b/drivers/net/wireless/wl12xx/wl1271_acx.c
> index cc6b7d8..f82656e 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_acx.c
> +++ b/drivers/net/wireless/wl12xx/wl1271_acx.c
> @@ -1317,6 +1317,61 @@ out:
> return ret;
> }
>
> +/*
> + * wl1271_acx_set_ba_session
Remove this line. And then the comment can be in one line.
> + * configure BA session initiator\receiver parameters setting in the FW.
> + */
> +int wl1271_acx_set_ba_session(struct wl1271 *wl,
> + u16 id,
> + u8 tid_index,
> + u8 policy)
Fix the indentation and maybe it all fits in one line?
> +{
> + struct wl1271_acx_ba_session_policy *acx;
> + int ret = 0;
> + /*
> + * Note, currently this value will be set to FFFFFFFFFFFF to indicate
> + * it is relevant for all peers.
> + */
> + u8 mac_address[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
No need to do this here, since this code is not called and, when you
call it (ie. in patch 03/03), you remove this FF'ing anyway. ;)
> + if (ACX_BA_SESSION_INITIATOR_POLICY == id)
Please use natural order (ie. id == ACX_BA_SESSION_INITIATOR_POLICY).
> + acx->inactivity_timeout = BA_INACTIVITY_TIMEOUT;
> + else
> + if (ACX_BA_SESSION_RESPONDER_POLICY == id)
> + acx->inactivity_timeout = 0;
> + else {
> + wl1271_error("Incorrect acx command id=%x\n", id);
> + ret = -EINVAL;
> + goto out;
> + }
Use this structure for the if else statements:
if (...) {
...
} else if (...) {
...
} else {
...
}
Or a switch-case.
> +
> + ret = wl1271_cmd_configure(wl,
> + id,
> + acx,
> + sizeof(*acx));
Fits in one line.
> diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
> index 6c3c7c0..f417624 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_acx.h
> +++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
> @@ -1205,6 +1205,10 @@ int wl1271_acx_set_ht_capabilities(struct wl1271 *wl,
> bool allow_ht_operation);
> int wl1271_acx_set_ht_information(struct wl1271 *wl,
> u16 ht_operation_mode);
> +int wl1271_acx_set_ba_session(struct wl1271 *wl,
> + u16 id,
> + u8 tid_index,
> + u8 policy);
Indentation.
> diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
> index af092f4..53c03e5 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_main.c
> +++ b/drivers/net/wireless/wl12xx/wl1271_main.c
> @@ -1128,6 +1128,22 @@ static void wl1271_configure_filters(struct wl1271 *wl, unsigned int filters)
> }
> }
>
> +/*
> + * wl1271_set_ba_policies
Same as earlier: remove this line. And then the comment can be in one
line.
> + * Set the BA session policies to the FW.
> + */
> +static void wl1271_set_ba_policies(struct wl1271 *wl)
> +{
> + u8 tid_index;
No need for the TAB between u8 and tid_index.
> @@ -2056,6 +2072,32 @@ 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)
> +{
> + int ret;
> +
> + switch (action) {
> + case IEEE80211_AMPDU_RX_START:
> + case IEEE80211_AMPDU_RX_STOP:
> +
> + /* The BA initiator session management in FW independently */
> + 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;
> + }
> +
> + return ret;
> +}
This function doesn't do anything, really. Except for returning -EINVAL
in TX cases. Is it really needed?
--
Cheers,
Luca.
New ops support ampdu_action as initiator returns EINVAL due to
the fact that BA initiator session manage in the FW independently.
acx function set BA policies.
Signed-off-by: Shahar Levi <[email protected]>
---
drivers/net/wireless/wl12xx/wl1271_acx.c | 55 +++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1271_acx.h | 4 ++
drivers/net/wireless/wl12xx/wl1271_main.c | 42 ++++++++++++++++++++++
3 files changed, 101 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.c b/drivers/net/wireless/wl12xx/wl1271_acx.c
index cc6b7d8..f82656e 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.c
@@ -1317,6 +1317,61 @@ out:
return ret;
}
+/*
+ * wl1271_acx_set_ba_session
+ * configure BA session initiator\receiver parameters setting in the FW.
+ */
+int wl1271_acx_set_ba_session(struct wl1271 *wl,
+ u16 id,
+ u8 tid_index,
+ u8 policy)
+{
+ struct wl1271_acx_ba_session_policy *acx;
+ int ret = 0;
+ /*
+ * Note, currently this value will be set to FFFFFFFFFFFF to indicate
+ * it is relevant for all peers.
+ */
+ u8 mac_address[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+
+ wl1271_debug(DEBUG_ACX, "acx ba session setting");
+
+ acx = kzalloc(sizeof(*acx), GFP_KERNEL);
+ if (!acx) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ memcpy(acx->mac_address, mac_address, ETH_ALEN);
+ acx->tid = tid_index;
+ acx->policy = policy;
+ acx->win_size = BA_RECEIVER_WIN_SIZE;
+
+ if (ACX_BA_SESSION_INITIATOR_POLICY == id)
+ acx->inactivity_timeout = BA_INACTIVITY_TIMEOUT;
+ else
+ if (ACX_BA_SESSION_RESPONDER_POLICY == id)
+ acx->inactivity_timeout = 0;
+ else {
+ wl1271_error("Incorrect acx command id=%x\n", id);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = wl1271_cmd_configure(wl,
+ id,
+ acx,
+ sizeof(*acx));
+ if (ret < 0) {
+ wl1271_warning("acx ba session setting 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/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
index 6c3c7c0..f417624 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.h
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
@@ -1205,6 +1205,10 @@ int wl1271_acx_set_ht_capabilities(struct wl1271 *wl,
bool allow_ht_operation);
int wl1271_acx_set_ht_information(struct wl1271 *wl,
u16 ht_operation_mode);
+int wl1271_acx_set_ba_session(struct wl1271 *wl,
+ u16 id,
+ u8 tid_index,
+ u8 policy);
int wl1271_acx_tsf_info(struct wl1271 *wl, u64 *mactime);
#endif /* __WL1271_ACX_H__ */
diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index af092f4..53c03e5 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -1128,6 +1128,22 @@ static void wl1271_configure_filters(struct wl1271 *wl, unsigned int filters)
}
}
+/*
+ * wl1271_set_ba_policies
+ * Set the BA session policies to the FW.
+ */
+static void wl1271_set_ba_policies(struct wl1271 *wl)
+{
+ u8 tid_index;
+
+ /* 802.11n BA session setting */
+ for (tid_index = 0; tid_index < CONF_TX_MAX_TID_COUNT; ++tid_index)
+ wl1271_acx_set_ba_session(wl,
+ ACX_BA_SESSION_INITIATOR_POLICY,
+ tid_index,
+ true);
+}
+
static int wl1271_dummy_join(struct wl1271 *wl)
{
int ret = 0;
@@ -2056,6 +2072,32 @@ 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)
+{
+ int ret;
+
+ switch (action) {
+ case IEEE80211_AMPDU_RX_START:
+ case IEEE80211_AMPDU_RX_STOP:
+
+ /* The BA initiator session management in FW independently */
+ 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;
+ }
+
+ return ret;
+}
+
/* can't be const, mac80211 writes to this */
static struct ieee80211_rate wl1271_rates[] = {
{ .bitrate = 10,
--
1.6.0.4
On Thu, 2010-10-07 at 15:06 +0200, ext Shahar Levi wrote:
> Add calling to BA policies after join and connect new ops.
>
> Signed-off-by: Shahar Levi <[email protected]>
> ---
I'll wait until you merge these three patches before reviewing this one,
as it seems to be mostly fixing stuff from the previous patch.
--
Cheers,
Luca.
On Thu, 2010-10-07 at 15:06 +0200, ext Shahar Levi wrote:
> New acx cmd wl1271_acx_ba_session_policy to set BA policy to the FW.
> Macros to use with BA setting.
>
> Signed-off-by: Shahar Levi <[email protected]>
> ---
You can merge this patch with the next patch, no need to make the
changes first in the header files and then in the c files, since they go
very much hand in hand.
> drivers/net/wireless/wl12xx/wl1271_acx.h | 16 ++++++++++++++++
> drivers/net/wireless/wl12xx/wl1271_conf.h | 3 +++
> 2 files changed, 19 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
> index b8da1bc..6c3c7c0 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_acx.h
> +++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
> @@ -1055,6 +1055,22 @@ struct wl1271_acx_ht_information {
> u8 padding[3];
> } __packed;
>
> +/*
> + * BA sessen interface structure
> + */
BA session. Actually you can get rid of this comment, as it is obvious
from the name of the structure that this is about BA sessions.
> +struct wl1271_acx_ba_session_policy {
> + struct acx_header header;
> + /* Mac address of: SA as receiver / RA as initiator */
> + u8 mac_address[ETH_ALEN];
Is this really SA for receiver and RA for initiator? Not SA and DA? Or
TA and RA?
> + u8 tid; /* TID */
Comment here is unnecessary.
> + u8 policy; /* Enable / Disable */
Could we change policy to enable here? Then the comment can go away too,
because the name of the element will be clear enough already.
> + u16 win_size; /* windows size in num of packet */
"number of packets"
> + u16 inactivity_timeout; /* as initiator inactivity timeout
> + * in time units(TU) of 1024us.
> + * as receiver reserved
> + */
The comment style is wrong.
> diff --git a/drivers/net/wireless/wl12xx/wl1271_conf.h b/drivers/net/wireless/wl12xx/wl1271_conf.h
> index b3e608e..63a0a9a 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_conf.h
> +++ b/drivers/net/wireless/wl12xx/wl1271_conf.h
> @@ -94,6 +94,9 @@ enum {
> #define HW_BG_RATES_MASK 0xffff
> #define HW_HT_RATES_OFFSET 16
>
> +#define BA_RECEIVER_WIN_SIZE 8
> +#define BA_INACTIVITY_TIMEOUT 10000
This should be added as a real configuration, like the other stuff in
the wl1271_conf.h file, which is then used in wl1271_main.c
default_conf. Please define a struct conf_ba with win_size and
inactivity_timeout elements and set the actual values in the main file.
--
Cheers,
Luca.
Add calling to BA policies after join and connect new ops.
Signed-off-by: Shahar Levi <[email protected]>
---
drivers/net/wireless/wl12xx/wl1271_acx.c | 10 +++-------
drivers/net/wireless/wl12xx/wl1271_main.c | 14 ++++++++++++--
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.c b/drivers/net/wireless/wl12xx/wl1271_acx.c
index f82656e..18ac48f 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.c
@@ -1328,11 +1328,6 @@ int wl1271_acx_set_ba_session(struct wl1271 *wl,
{
struct wl1271_acx_ba_session_policy *acx;
int ret = 0;
- /*
- * Note, currently this value will be set to FFFFFFFFFFFF to indicate
- * it is relevant for all peers.
- */
- u8 mac_address[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
wl1271_debug(DEBUG_ACX, "acx ba session setting");
@@ -1342,14 +1337,14 @@ int wl1271_acx_set_ba_session(struct wl1271 *wl,
goto out;
}
- memcpy(acx->mac_address, mac_address, ETH_ALEN);
+ memcpy(acx->mac_address, wl->bssid, ETH_ALEN);
acx->tid = tid_index;
acx->policy = policy;
acx->win_size = BA_RECEIVER_WIN_SIZE;
if (ACX_BA_SESSION_INITIATOR_POLICY == id)
acx->inactivity_timeout = BA_INACTIVITY_TIMEOUT;
- else
+ else {
if (ACX_BA_SESSION_RESPONDER_POLICY == id)
acx->inactivity_timeout = 0;
else {
@@ -1357,6 +1352,7 @@ int wl1271_acx_set_ba_session(struct wl1271 *wl,
ret = -EINVAL;
goto out;
}
+ }
ret = wl1271_cmd_configure(wl,
id,
diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index 53c03e5..b4e3b02 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -1725,6 +1725,7 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
enum wl1271_cmd_ps_mode mode;
struct wl1271 *wl = hw->priv;
struct ieee80211_sta *sta = ieee80211_find_sta(vif, bss_conf->bssid);
+ bool set_ba = false;
bool do_join = false;
bool set_assoc = false;
int ret;
@@ -1950,7 +1951,9 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
true);
ret = wl1271_acx_set_ht_information(wl,
bss_conf->ht_operation_mode);
- }
+
+ set_ba = true;
+ }
else
if (changed & BSS_CHANGED_ASSOC)
ret = wl1271_acx_set_ht_capabilities(wl,
@@ -1977,6 +1980,9 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
wl1271_warning("cmd join failed %d", ret);
goto out_sleep;
}
+
+ if (set_ba)
+ wl1271_set_ba_policies(wl);
}
out_sleep:
@@ -2080,10 +2086,13 @@ int wl1271_op_ampdu_action(struct ieee80211_hw *hw,
int ret;
switch (action) {
+ /* Falling break here on purpose until we add BA receiver support */
case IEEE80211_AMPDU_RX_START:
case IEEE80211_AMPDU_RX_STOP:
- /* The BA initiator session management in FW independently */
+ /* 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:
@@ -2351,6 +2360,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.6.0.4