2010-11-29 13:42:00

by Shahar Levi

[permalink] [raw]
Subject: [PATCH v4 ] wl12xx: BA Initiator support

Add 80211n BA initiator session support wl1271 driver.
BA initiator session management included in FW independently.

Signed-off-by: Shahar Levi <[email protected]>
---
Limitation:
- For now only one BA per direction is supported
- BA receiver session not supported in that patch

Changes from v1:
- Remove wl1271_op_ampdu_action()
- set CONF_BA_INACTIVITY_TIMEOUT as configurable value
- Clean to Linux code style.

Changes from v2:
- Two new ACX commands ACX_BA_SESSION_POLICY_CFG & ACX_BA_SESSION_RX_SETUP
- New struct wl1271_acx_ba_session_policy
- Calling wl1271_set_ba_policies() once in init
- Use ieee80211_back_parties
- Rebase to latest

Changes from v3:
- New FW file name due to new FW API

drivers/net/wireless/wl12xx/acx.c | 50 ++++++++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/acx.h | 41 ++++++++++++++++++++++++++-
drivers/net/wireless/wl12xx/conf.h | 7 +++++
drivers/net/wireless/wl12xx/init.c | 25 +++++++++++++++++
drivers/net/wireless/wl12xx/main.c | 8 ++++-
drivers/net/wireless/wl12xx/wl12xx.h | 6 +++-
6 files changed, 132 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/acx.c b/drivers/net/wireless/wl12xx/acx.c
index 7cbaeb6..6c433b7 100644
--- a/drivers/net/wireless/wl12xx/acx.c
+++ b/drivers/net/wireless/wl12xx/acx.c
@@ -1309,6 +1309,56 @@ out:
return ret;
}

+/* Configure BA session initiator\receiver parameters setting in the FW. */
+int wl1271_acx_set_ba_session(struct wl1271 *wl,
+ enum ieee80211_back_parties direction,
+ u8 tid_index, u8 policy)
+{
+ struct wl1271_acx_ba_session_policy *acx;
+ int ret;
+
+ wl1271_debug(DEBUG_ACX, "acx ba session setting");
+
+ acx = kzalloc(sizeof(*acx), GFP_KERNEL);
+ if (!acx) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ /* ANY role */
+ acx->role_id = 0xff;
+ acx->tid = tid_index;
+ acx->enable = policy;
+ acx->win_size = BA_WIN_SIZE;
+ acx->ba_direction = direction;
+
+ switch (direction) {
+ case WLAN_BACK_INITIATOR:
+ acx->inactivity_timeout = wl->conf.ht.inactivity_timeout;
+ break;
+ case WLAN_BACK_RECIPIENT:
+ acx->inactivity_timeout = 0;
+ break;
+ default:
+ wl1271_error("Incorrect acx command id=%x\n", direction);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = wl1271_cmd_configure(wl,
+ ACX_BA_SESSION_POLICY_CFG,
+ 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/acx.h b/drivers/net/wireless/wl12xx/acx.h
index 75a6306..c511dce 100644
--- a/drivers/net/wireless/wl12xx/acx.h
+++ b/drivers/net/wireless/wl12xx/acx.h
@@ -1046,6 +1046,40 @@ struct wl1271_acx_ht_information {
u8 padding[3];
} __packed;

+#define BA_WIN_SIZE 8
+
+struct wl1271_acx_ba_session_policy {
+ struct acx_header header;
+ /*
+ * Specifies role Id, Range 0-7, 0xFF means ANY role.
+ * Future use. For now this field is irrelevant
+ */
+ u8 role_id;
+ /*
+ * Specifies Link Id, Range 0-31, 0xFF means ANY Link Id.
+ * Not applicable if Role Id is set to ANY.
+ */
+ u8 link_id;
+
+ u8 tid;
+
+ u8 enable;
+
+ /* Windows size in number of packets */
+ u16 win_size;
+
+ /*
+ * As initiator inactivity timeout in time units(TU) of 1024us.
+ * As receiver reserved
+ */
+ u16 inactivity_timeout;
+
+ /* Initiator = 1/Receiver = 0 */
+ u8 ba_direction;
+
+ u8 padding[3];
+} __packed;
+
struct wl1271_acx_fw_tsf_information {
struct acx_header header;

@@ -1108,8 +1142,8 @@ enum {
ACX_RSSI_SNR_WEIGHTS = 0x0052,
ACX_KEEP_ALIVE_MODE = 0x0053,
ACX_SET_KEEP_ALIVE_CONFIG = 0x0054,
- ACX_BA_SESSION_RESPONDER_POLICY = 0x0055,
- ACX_BA_SESSION_INITIATOR_POLICY = 0x0056,
+ ACX_BA_SESSION_POLICY_CFG = 0x0055,
+ ACX_BA_SESSION_RX_SETUP = 0x0056,
ACX_PEER_HT_CAP = 0x0057,
ACX_HT_BSS_OPERATION = 0x0058,
ACX_COEX_ACTIVITY = 0x0059,
@@ -1180,6 +1214,9 @@ 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,
+ enum ieee80211_back_parties direction,
+ 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/conf.h b/drivers/net/wireless/wl12xx/conf.h
index a16b361..41df771 100644
--- a/drivers/net/wireless/wl12xx/conf.h
+++ b/drivers/net/wireless/wl12xx/conf.h
@@ -1090,6 +1090,12 @@ struct conf_rf_settings {
u8 tx_per_channel_power_compensation_5[CONF_TX_PWR_COMPENSATION_LEN_5];
};

+#define CONF_BA_INACTIVITY_TIMEOUT 10000
+
+struct conf_ht_setting {
+ u16 inactivity_timeout;
+};
+
struct conf_drv_settings {
struct conf_sg_settings sg;
struct conf_rx_settings rx;
@@ -1100,6 +1106,7 @@ struct conf_drv_settings {
struct conf_roam_trigger_settings roam_trigger;
struct conf_scan_settings scan;
struct conf_rf_settings rf;
+ struct conf_ht_setting ht;
};

#endif
diff --git a/drivers/net/wireless/wl12xx/init.c b/drivers/net/wireless/wl12xx/init.c
index 7949d34..7419ac5 100644
--- a/drivers/net/wireless/wl12xx/init.c
+++ b/drivers/net/wireless/wl12xx/init.c
@@ -208,6 +208,26 @@ static int wl1271_init_beacon_broadcast(struct wl1271 *wl)
return 0;
}

+static int wl1271_set_ba_policies(struct wl1271 *wl)
+{
+ u8 tid_index;
+ u8 ret;
+
+ /* Reset the BA RX indicators */
+ wl->ba_allowed = true;
+ wl->ba_rx_bitmap = 0;
+
+ /* 802.11n initiator BA session setting */
+ for (tid_index = 0; tid_index < CONF_TX_MAX_TID_COUNT; ++tid_index) {
+ ret = wl1271_acx_set_ba_session(wl, WLAN_BACK_INITIATOR,
+ tid_index, true);
+ if (ret < 0)
+ break;
+ }
+
+ return ret;
+}
+
int wl1271_hw_init(struct wl1271 *wl)
{
struct conf_tx_ac_category *conf_ac;
@@ -359,6 +379,11 @@ int wl1271_hw_init(struct wl1271 *wl)
if (ret < 0)
goto out_free_memmap;

+ /* Configure initiator BA sessions policies */
+ ret = wl1271_set_ba_policies(wl);
+ if (ret < 0)
+ goto out_free_memmap;
+
return 0;

out_free_memmap:
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 7fecefe..c1812c8 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -233,7 +233,7 @@ static struct conf_drv_settings default_conf = {
.avg_weight_rssi_beacon = 20,
.avg_weight_rssi_data = 10,
.avg_weight_snr_beacon = 20,
- .avg_weight_snr_data = 10
+ .avg_weight_snr_data = 10,
},
.scan = {
.min_dwell_time_active = 7500,
@@ -252,6 +252,9 @@ static struct conf_drv_settings default_conf = {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
},
},
+ .ht = {
+ .inactivity_timeout = CONF_BA_INACTIVITY_TIMEOUT,
+ },
};

static void __wl1271_op_remove_interface(struct wl1271 *wl);
@@ -2083,7 +2086,8 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
wl1271_warning("Set ht cap true failed %d", ret);
goto out_sleep;
}
- ret = wl1271_acx_set_ht_information(wl,
+
+ ret = wl1271_acx_set_ht_information(wl,
bss_conf->ht_operation_mode);
if (ret < 0) {
wl1271_warning("Set ht information failed %d", ret);
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index e904c72..0cf07cd 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -111,7 +111,7 @@ enum {
CFG_RX_CTL_EN | CFG_RX_BCN_EN | \
CFG_RX_AUTH_EN | CFG_RX_ASSOC_EN)

-#define WL1271_FW_NAME "wl1271-fw.bin"
+#define WL1271_FW_NAME "wl1271-fw-2.bin"
#define WL1271_NVS_NAME "wl1271-nvs.bin"

#define WL1271_TX_SECURITY_LO16(s) ((u16)((s) & 0xffff))
@@ -398,6 +398,10 @@ struct wl1271 {

/* Most recently reported noise in dBm */
s8 noise;
+
+ /* RX BA constraint value */
+ u8 ba_allowed;
+ u8 ba_rx_bitmap;
};

int wl1271_plt_start(struct wl1271 *wl);
--
1.7.0.4



2010-11-30 07:15:58

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v4 ] wl12xx: BA Initiator support

On Mon, 2010-11-29 at 23:07 -0800, ext Ohad Ben-Cohen wrote:
> On Mon, Nov 29, 2010 at 10:11 PM, Luciano Coelho
> <[email protected]> wrote:
> ...
> > In theory the API *has* changed, not just extended. Check this:
> >
> > - ACX_BA_SESSION_RESPONDER_POLICY = 0x0055,
> > - ACX_BA_SESSION_INITIATOR_POLICY = 0x0056,
> > + ACX_BA_SESSION_POLICY_CFG = 0x0055,
> > + ACX_BA_SESSION_RX_SETUP = 0x0056,
> >
> > But in practice, this doesn't matter, because we were not using the
> > RESPONDER/INITIATOR commands before...
>
> Right. So an old driver will still work with this new firmware.
>
> > It's basically just those two extra commands that were added. And one
> > new event that is part of a future patch.
> >
> > In theory, we could check the firmware revision after boot and bail out
> > if the version doesn't match.
>
> Why not just disable BA sessions in this case (and keep that new event
> masked), and let the driver keep running (just like it does today) ?
>
> This way the new driver will work even with the old firmware (yes,
> with degraded functionality, but most random ppl will just not care),
> and of course, the old driver will keep working with the new firmware.
>
> For us developers who lurk in linux-wireless it seems like a trivial
> change, but if we consider the growing size of the 12xx community, and
> the long period of time for which such a change will be effective (ppl
> upgrading to latest compat, ppl that will one day upgrade to 2.6.38,
> future ppl that will bisect and cross this firmware name change,
> etc...), it's actually a lot of accumulated pain.
>
> To keep our community happy, I vote to eliminate this pain when not necessary.

Hear, hear!

Shahar, can you fix that?

--
Cheers,
Luca.


2010-11-30 07:07:45

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH v4 ] wl12xx: BA Initiator support

On Mon, Nov 29, 2010 at 10:11 PM, Luciano Coelho
<[email protected]> wrote:
...
> In theory the API *has* changed, not just extended. ?Check this:
>
> - ? ? ? ACX_BA_SESSION_RESPONDER_POLICY = 0x0055,
> - ? ? ? ACX_BA_SESSION_INITIATOR_POLICY = 0x0056,
> + ? ? ? ACX_BA_SESSION_POLICY_CFG ? = 0x0055,
> + ? ? ? ACX_BA_SESSION_RX_SETUP ? ? = 0x0056,
>
> But in practice, this doesn't matter, because we were not using the
> RESPONDER/INITIATOR commands before...

Right. So an old driver will still work with this new firmware.

> It's basically just those two extra commands that were added. ?And one
> new event that is part of a future patch.
>
> In theory, we could check the firmware revision after boot and bail out
> if the version doesn't match.

Why not just disable BA sessions in this case (and keep that new event
masked), and let the driver keep running (just like it does today) ?

This way the new driver will work even with the old firmware (yes,
with degraded functionality, but most random ppl will just not care),
and of course, the old driver will keep working with the new firmware.

For us developers who lurk in linux-wireless it seems like a trivial
change, but if we consider the growing size of the 12xx community, and
the long period of time for which such a change will be effective (ppl
upgrading to latest compat, ppl that will one day upgrade to 2.6.38,
future ppl that will bisect and cross this firmware name change,
etc...), it's actually a lot of accumulated pain.

To keep our community happy, I vote to eliminate this pain when not necessary.

>
>
> --
> Cheers,
> Luca.
>
>

2010-11-30 00:44:44

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH v4 ] wl12xx: BA Initiator support

Hi Shahar,

On Mon, Nov 29, 2010 at 5:42 AM, Shahar Levi <[email protected]> wrote:
...
> -#define WL1271_FW_NAME "wl1271-fw.bin"
> +#define WL1271_FW_NAME "wl1271-fw-2.bin"

Changing the firmware's major version involves usability and
maintainability pain to the entire wl12xx community, especially if
done repeatedly, and therefore should be generally avoided.

When the firmware API changes, and the old driver can't work with it
anymore, then you have no choice but changing the name.

But if it's just a small API extension enabling new driver
functionality, you should really avoid bumping the firmware version,
and instead try to dynamically detect it from the driver and act
appropriately.

Please explain what has exactly been changed in the firmware and let's
try to find a clean solution.

Thanks,
Ohad.

2010-11-30 09:05:56

by Shahar Levi

[permalink] [raw]
Subject: Re: [PATCH v4 ] wl12xx: BA Initiator support

On 11/30/2010 09:15 AM, Luciano Coelho wrote:
> On Mon, 2010-11-29 at 23:07 -0800, ext Ohad Ben-Cohen wrote:
>> On Mon, Nov 29, 2010 at 10:11 PM, Luciano Coelho
>> <[email protected]> wrote:
>> ...
>>> In theory the API *has* changed, not just extended. Check this:
>>>
>>> - ACX_BA_SESSION_RESPONDER_POLICY = 0x0055,
>>> - ACX_BA_SESSION_INITIATOR_POLICY = 0x0056,
>>> + ACX_BA_SESSION_POLICY_CFG = 0x0055,
>>> + ACX_BA_SESSION_RX_SETUP = 0x0056,
>>>
>>> But in practice, this doesn't matter, because we were not using the
>>> RESPONDER/INITIATOR commands before...
>>
>> Right. So an old driver will still work with this new firmware.
>>
>>> It's basically just those two extra commands that were added. And one
>>> new event that is part of a future patch.
>>>
>>> In theory, we could check the firmware revision after boot and bail out
>>> if the version doesn't match.
>>
>> Why not just disable BA sessions in this case (and keep that new event
>> masked), and let the driver keep running (just like it does today) ?
>>
>> This way the new driver will work even with the old firmware (yes,
>> with degraded functionality, but most random ppl will just not care),
>> and of course, the old driver will keep working with the new firmware.
>>
>> For us developers who lurk in linux-wireless it seems like a trivial
>> change, but if we consider the growing size of the 12xx community, and
>> the long period of time for which such a change will be effective (ppl
>> upgrading to latest compat, ppl that will one day upgrade to 2.6.38,
>> future ppl that will bisect and cross this firmware name change,
>> etc...), it's actually a lot of accumulated pain.
>>
>> To keep our community happy, I vote to eliminate this pain when not necessary.
>
> Hear, hear!
>
> Shahar, can you fix that?
Yes, will do.
Shahar



2010-11-30 06:11:47

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v4 ] wl12xx: BA Initiator support

On Mon, 2010-11-29 at 16:44 -0800, ext Ohad Ben-Cohen wrote:
> Hi Shahar,
>
> On Mon, Nov 29, 2010 at 5:42 AM, Shahar Levi <[email protected]> wrote:
> ...
> > -#define WL1271_FW_NAME "wl1271-fw.bin"
> > +#define WL1271_FW_NAME "wl1271-fw-2.bin"
>
> Changing the firmware's major version involves usability and
> maintainability pain to the entire wl12xx community, especially if
> done repeatedly, and therefore should be generally avoided.

Yes, I was thinking about the same thing yesterday. Then I had to
leave, so I'm still not sure whether this is really needed.


> When the firmware API changes, and the old driver can't work with it
> anymore, then you have no choice but changing the name.

In theory the API *has* changed, not just extended. Check this:

- ACX_BA_SESSION_RESPONDER_POLICY = 0x0055,
- ACX_BA_SESSION_INITIATOR_POLICY = 0x0056,
+ ACX_BA_SESSION_POLICY_CFG = 0x0055,
+ ACX_BA_SESSION_RX_SETUP = 0x0056,

But in practice, this doesn't matter, because we were not using the
RESPONDER/INITIATOR commands before...


> But if it's just a small API extension enabling new driver
> functionality, you should really avoid bumping the firmware version,
> and instead try to dynamically detect it from the driver and act
> appropriately.
>
> Please explain what has exactly been changed in the firmware and let's
> try to find a clean solution.

It's basically just those two extra commands that were added. And one
new event that is part of a future patch.

In theory, we could check the firmware revision after boot and bail out
if the version doesn't match. Maybe we should add a small
infrastructure for that kind of check.

This new firmware is only required if 11n is enabled too, so at least we
should not have the driver fail whe the new fw version is not there,
unless 11n is enabled.


--
Cheers,
Luca.