2015-03-20 13:56:47

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 0/9] ath10k: cleanups 2015-03-20

Hi,

Here's a bunch of cleanups and fixes associated
with tx/rx rate code.

Feel free to squash some of the patches if you
think it makes sense or let me know which ones you
want me to squash.

There are a few checkpatch warnings for going over
80 chars. I decided to leave them be to avoid code
wrapping which would make the code less readable
than it is now in my opinion.

I have 2 more patches for .set_bitrate_mask()
callback which depend on this patchset. Once you
apply this patchset to your `pending` branch let
me know, please, so I can push the rest with git
indexes your git repo will recognize.


Michal Kazior (9):
ath10k: move rate definitions to file start
ath10k: derive rate from bitrate
ath10k: drop RATETAB_ENT macro
ath10k: add hw rate definitions
ath10k: use hw rate definitions for fixed rate
ath10k: simplify fixed rate selection
ath10k: rework legacy rx rate decoding
ath10k: deduplicate bitrate to rate idx conversion
ath10k: document ofdm/5ghz rate offset with a macro

drivers/net/wireless/ath/ath10k/htt.h | 1 +
drivers/net/wireless/ath/ath10k/htt_rx.c | 64 ++----------
drivers/net/wireless/ath/ath10k/hw.h | 21 ++++
drivers/net/wireless/ath/ath10k/mac.c | 161 ++++++++++++++++++------------
drivers/net/wireless/ath/ath10k/mac.h | 5 +
drivers/net/wireless/ath/ath10k/rx_desc.h | 22 ++++
drivers/net/wireless/ath/ath10k/wmi.c | 62 +-----------
7 files changed, 163 insertions(+), 173 deletions(-)

--
2.1.4



2015-03-20 13:56:49

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 1/9] ath10k: move rate definitions to file start

Prepare the code for future changes so that new
code can refer to rate-related stuff without
forward declarations.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 63 ++++++++++++++++++-----------------
1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 1214a3b..456d671 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -30,6 +30,38 @@
#include "wmi.h"
#include "wmi-ops.h"

+/*********/
+/* Rates */
+/*********/
+
+#define RATETAB_ENT(_rate, _rateid, _flags) { \
+ .bitrate = (_rate), \
+ .flags = (_flags), \
+ .hw_value = (_rateid), \
+}
+
+static struct ieee80211_rate ath10k_rates[] = {
+ /* CCK */
+ RATETAB_ENT(10, 0x82, 0),
+ RATETAB_ENT(20, 0x84, 0),
+ RATETAB_ENT(55, 0x8b, 0),
+ RATETAB_ENT(110, 0x96, 0),
+ /* OFDM */
+ RATETAB_ENT(60, 0x0c, 0),
+ RATETAB_ENT(90, 0x12, 0),
+ RATETAB_ENT(120, 0x18, 0),
+ RATETAB_ENT(180, 0x24, 0),
+ RATETAB_ENT(240, 0x30, 0),
+ RATETAB_ENT(360, 0x48, 0),
+ RATETAB_ENT(480, 0x60, 0),
+ RATETAB_ENT(540, 0x6c, 0),
+};
+
+#define ath10k_a_rates (ath10k_rates + 4)
+#define ath10k_a_rates_size (ARRAY_SIZE(ath10k_rates) - 4)
+#define ath10k_g_rates (ath10k_rates + 0)
+#define ath10k_g_rates_size (ARRAY_SIZE(ath10k_rates))
+
/**********/
/* Crypto */
/**********/
@@ -5420,12 +5452,6 @@ static const struct ieee80211_ops ath10k_ops = {
#endif
};

-#define RATETAB_ENT(_rate, _rateid, _flags) { \
- .bitrate = (_rate), \
- .flags = (_flags), \
- .hw_value = (_rateid), \
-}
-
#define CHAN2G(_channel, _freq, _flags) { \
.band = IEEE80211_BAND_2GHZ, \
.hw_value = (_channel), \
@@ -5488,31 +5514,6 @@ static const struct ieee80211_channel ath10k_5ghz_channels[] = {
CHAN5G(165, 5825, 0),
};

-/* Note: Be careful if you re-order these. There is code which depends on this
- * ordering.
- */
-static struct ieee80211_rate ath10k_rates[] = {
- /* CCK */
- RATETAB_ENT(10, 0x82, 0),
- RATETAB_ENT(20, 0x84, 0),
- RATETAB_ENT(55, 0x8b, 0),
- RATETAB_ENT(110, 0x96, 0),
- /* OFDM */
- RATETAB_ENT(60, 0x0c, 0),
- RATETAB_ENT(90, 0x12, 0),
- RATETAB_ENT(120, 0x18, 0),
- RATETAB_ENT(180, 0x24, 0),
- RATETAB_ENT(240, 0x30, 0),
- RATETAB_ENT(360, 0x48, 0),
- RATETAB_ENT(480, 0x60, 0),
- RATETAB_ENT(540, 0x6c, 0),
-};
-
-#define ath10k_a_rates (ath10k_rates + 4)
-#define ath10k_a_rates_size (ARRAY_SIZE(ath10k_rates) - 4)
-#define ath10k_g_rates (ath10k_rates + 0)
-#define ath10k_g_rates_size (ARRAY_SIZE(ath10k_rates))
-
struct ath10k *ath10k_mac_create(size_t priv_size)
{
struct ieee80211_hw *hw;
--
2.1.4


2015-03-20 13:56:52

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 3/9] ath10k: drop RATETAB_ENT macro

It was superfluous and confusing. It's better to
define the structure explicitly.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 5721196..f4e76e7 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -34,27 +34,22 @@
/* Rates */
/*********/

-#define RATETAB_ENT(_rate, _rateid, _flags) { \
- .bitrate = (_rate), \
- .flags = (_flags), \
- .hw_value = (_rateid), \
-}
-
static struct ieee80211_rate ath10k_rates[] = {
/* CCK */
- RATETAB_ENT(10, 0, 0),
- RATETAB_ENT(20, 0, 0),
- RATETAB_ENT(55, 0, 0),
- RATETAB_ENT(110, 0, 0),
+ { .bitrate = 10 },
+ { .bitrate = 20 },
+ { .bitrate = 55 },
+ { .bitrate = 110 },
+
/* OFDM */
- RATETAB_ENT(60, 0, 0),
- RATETAB_ENT(90, 0, 0),
- RATETAB_ENT(120, 0, 0),
- RATETAB_ENT(180, 0, 0),
- RATETAB_ENT(240, 0, 0),
- RATETAB_ENT(360, 0, 0),
- RATETAB_ENT(480, 0, 0),
- RATETAB_ENT(540, 0, 0),
+ { .bitrate = 60 },
+ { .bitrate = 90 },
+ { .bitrate = 120 },
+ { .bitrate = 180 },
+ { .bitrate = 240 },
+ { .bitrate = 360 },
+ { .bitrate = 480 },
+ { .bitrate = 540 },
};

#define ath10k_a_rates (ath10k_rates + 4)
--
2.1.4


2015-03-20 13:56:57

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 7/9] ath10k: rework legacy rx rate decoding

Instead of using a hacky table and magic values
use supported band information advertised to
mac80211.

This may impact performance a little when dealing
with legacy rx rates depending on system
architecture. It's probably negligible.

This also fixes a highly theoretical corner case
when HT/VHT rates weren't reported correctly if
channel frequency wasn't known.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 64 ++++++--------------------------
drivers/net/wireless/ath/ath10k/mac.c | 37 ++++++++++++++++--
drivers/net/wireless/ath/ath10k/mac.h | 2 +
3 files changed, 46 insertions(+), 57 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 01a2b38..0cbd538 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -651,44 +651,15 @@ struct amsdu_subframe_hdr {
__be16 len;
} __packed;

-static const u8 rx_legacy_rate_idx[] = {
- 3, /* 0x00 - 11Mbps */
- 2, /* 0x01 - 5.5Mbps */
- 1, /* 0x02 - 2Mbps */
- 0, /* 0x03 - 1Mbps */
- 3, /* 0x04 - 11Mbps */
- 2, /* 0x05 - 5.5Mbps */
- 1, /* 0x06 - 2Mbps */
- 0, /* 0x07 - 1Mbps */
- 10, /* 0x08 - 48Mbps */
- 8, /* 0x09 - 24Mbps */
- 6, /* 0x0A - 12Mbps */
- 4, /* 0x0B - 6Mbps */
- 11, /* 0x0C - 54Mbps */
- 9, /* 0x0D - 36Mbps */
- 7, /* 0x0E - 18Mbps */
- 5, /* 0x0F - 9Mbps */
-};
-
static void ath10k_htt_rx_h_rates(struct ath10k *ar,
struct ieee80211_rx_status *status,
struct htt_rx_desc *rxd)
{
- enum ieee80211_band band;
- u8 cck, rate, rate_idx, bw, sgi, mcs, nss;
+ struct ieee80211_supported_band *sband;
+ u8 cck, rate, bw, sgi, mcs, nss;
u8 preamble = 0;
u32 info1, info2, info3;

- /* Band value can't be set as undefined but freq can be 0 - use that to
- * determine whether band is provided.
- *
- * FIXME: Perhaps this can go away if CCK rate reporting is a little
- * reworked?
- */
- if (!status->freq)
- return;
-
- band = status->band;
info1 = __le32_to_cpu(rxd->ppdu_start.info1);
info2 = __le32_to_cpu(rxd->ppdu_start.info2);
info3 = __le32_to_cpu(rxd->ppdu_start.info3);
@@ -697,31 +668,18 @@ static void ath10k_htt_rx_h_rates(struct ath10k *ar,

switch (preamble) {
case HTT_RX_LEGACY:
+ /* To get legacy rate index band is required. Since band can't
+ * be undefined check if freq is non-zero.
+ */
+ if (!status->freq)
+ return;
+
cck = info1 & RX_PPDU_START_INFO1_L_SIG_RATE_SELECT;
rate = MS(info1, RX_PPDU_START_INFO1_L_SIG_RATE);
- rate_idx = 0;
-
- if (rate < 0x08 || rate > 0x0F)
- break;
-
- switch (band) {
- case IEEE80211_BAND_2GHZ:
- if (cck)
- rate &= ~BIT(3);
- rate_idx = rx_legacy_rate_idx[rate];
- break;
- case IEEE80211_BAND_5GHZ:
- rate_idx = rx_legacy_rate_idx[rate];
- /* We are using same rate table registering
- HW - ath10k_rates[]. In case of 5GHz skip
- CCK rates, so -4 here */
- rate_idx -= 4;
- break;
- default:
- break;
- }
+ rate &= ~RX_PPDU_START_RATE_FLAG;

- status->rate_idx = rate_idx;
+ sband = &ar->mac.sbands[status->band];
+ status->rate_idx = ath10k_mac_hw_rate_to_idx(sband, rate);
break;
case HTT_RX_HT:
case HTT_RX_HT_WITH_TXBF:
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 877afde..e1902e3 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -35,10 +35,20 @@
/*********/

static struct ieee80211_rate ath10k_rates[] = {
- { .bitrate = 10, .hw_value = ATH10K_HW_RATE_CCK_LP_1M },
- { .bitrate = 20, .hw_value = ATH10K_HW_RATE_CCK_LP_2M },
- { .bitrate = 55, .hw_value = ATH10K_HW_RATE_CCK_LP_5_5M },
- { .bitrate = 110, .hw_value = ATH10K_HW_RATE_CCK_LP_11M },
+ { .bitrate = 10,
+ .hw_value = ATH10K_HW_RATE_CCK_LP_1M },
+ { .bitrate = 20,
+ .hw_value = ATH10K_HW_RATE_CCK_LP_2M,
+ .hw_value_short = ATH10K_HW_RATE_CCK_SP_2M,
+ .flags = IEEE80211_RATE_SHORT_PREAMBLE },
+ { .bitrate = 55,
+ .hw_value = ATH10K_HW_RATE_CCK_LP_5_5M,
+ .hw_value_short = ATH10K_HW_RATE_CCK_SP_5_5M,
+ .flags = IEEE80211_RATE_SHORT_PREAMBLE },
+ { .bitrate = 110,
+ .hw_value = ATH10K_HW_RATE_CCK_LP_11M,
+ .hw_value_short = ATH10K_HW_RATE_CCK_SP_11M,
+ .flags = IEEE80211_RATE_SHORT_PREAMBLE },

{ .bitrate = 60, .hw_value = ATH10K_HW_RATE_OFDM_6M },
{ .bitrate = 90, .hw_value = ATH10K_HW_RATE_OFDM_9M },
@@ -74,6 +84,25 @@ static u8 ath10k_mac_bitrate_to_rate(int bitrate)
(ath10k_mac_bitrate_is_cck(bitrate) ? BIT(7) : 0);
}

+u8 ath10k_mac_hw_rate_to_idx(const struct ieee80211_supported_band *sband,
+ u8 hw_rate)
+{
+ const struct ieee80211_rate *rate;
+ int i;
+
+ for (i = 0; i < sband->n_bitrates; i++) {
+ rate = &sband->bitrates[i];
+
+ if (rate->hw_value == hw_rate)
+ return i;
+ else if (rate->flags & IEEE80211_RATE_SHORT_PREAMBLE &&
+ rate->hw_value_short == hw_rate)
+ return i;
+ }
+
+ return 0;
+}
+
/**********/
/* Crypto */
/**********/
diff --git a/drivers/net/wireless/ath/ath10k/mac.h b/drivers/net/wireless/ath/ath10k/mac.h
index 3b64d99..e615490 100644
--- a/drivers/net/wireless/ath/ath10k/mac.h
+++ b/drivers/net/wireless/ath/ath10k/mac.h
@@ -47,6 +47,8 @@ bool ath10k_mac_is_peer_wep_key_set(struct ath10k *ar, const u8 *addr,
u8 keyidx);
void ath10k_mac_handle_beacon(struct ath10k *ar, struct sk_buff *skb);
void ath10k_mac_handle_beacon_miss(struct ath10k *ar, u32 vdev_id);
+u8 ath10k_mac_hw_rate_to_idx(const struct ieee80211_supported_band *sband,
+ u8 hw_rate);

static inline struct ath10k_vif *ath10k_vif_to_arvif(struct ieee80211_vif *vif)
{
--
2.1.4


2015-03-20 13:56:55

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 5/9] ath10k: use hw rate definitions for fixed rate

Using raw values is discouraged. Having
defines/enums is preferred.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index f4e76e7..72fbdc2 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4972,20 +4972,18 @@ exit:

/* Helper table for legacy fixed_rate/bitrate_mask */
static const u8 cck_ofdm_rate[] = {
- /* CCK */
- 3, /* 1Mbps */
- 2, /* 2Mbps */
- 1, /* 5.5Mbps */
- 0, /* 11Mbps */
- /* OFDM */
- 3, /* 6Mbps */
- 7, /* 9Mbps */
- 2, /* 12Mbps */
- 6, /* 18Mbps */
- 1, /* 24Mbps */
- 5, /* 36Mbps */
- 0, /* 48Mbps */
- 4, /* 54Mbps */
+ ATH10K_HW_RATE_CCK_LP_1M,
+ ATH10K_HW_RATE_CCK_LP_2M,
+ ATH10K_HW_RATE_CCK_LP_5_5M,
+ ATH10K_HW_RATE_CCK_LP_11M,
+ ATH10K_HW_RATE_OFDM_6M,
+ ATH10K_HW_RATE_OFDM_9M,
+ ATH10K_HW_RATE_OFDM_12M,
+ ATH10K_HW_RATE_OFDM_18M,
+ ATH10K_HW_RATE_OFDM_24M,
+ ATH10K_HW_RATE_OFDM_36M,
+ ATH10K_HW_RATE_OFDM_48M,
+ ATH10K_HW_RATE_OFDM_54M,
};

/* Check if only one bit set */
--
2.1.4


2015-03-20 13:56:58

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 8/9] ath10k: deduplicate bitrate to rate idx conversion

It's possible to derive rate index from bitrate
without any additional mapping structures/logic.

This should have little to none impact on
performance since this is only done for management
frames and the previous approach wasn't
particularly optimized.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 12 +++++++
drivers/net/wireless/ath/ath10k/mac.h | 3 ++
drivers/net/wireless/ath/ath10k/wmi.c | 62 +++--------------------------------
3 files changed, 19 insertions(+), 58 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index e1902e3..c39a300 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -103,6 +103,18 @@ u8 ath10k_mac_hw_rate_to_idx(const struct ieee80211_supported_band *sband,
return 0;
}

+u8 ath10k_mac_bitrate_to_idx(const struct ieee80211_supported_band *sband,
+ u32 bitrate)
+{
+ int i;
+
+ for (i = 0; i < sband->n_bitrates; i++)
+ if (sband->bitrates[i].bitrate == bitrate)
+ return i;
+
+ return 0;
+}
+
/**********/
/* Crypto */
/**********/
diff --git a/drivers/net/wireless/ath/ath10k/mac.h b/drivers/net/wireless/ath/ath10k/mac.h
index e615490..e7df017 100644
--- a/drivers/net/wireless/ath/ath10k/mac.h
+++ b/drivers/net/wireless/ath/ath10k/mac.h
@@ -47,8 +47,11 @@ bool ath10k_mac_is_peer_wep_key_set(struct ath10k *ar, const u8 *addr,
u8 keyidx);
void ath10k_mac_handle_beacon(struct ath10k *ar, struct sk_buff *skb);
void ath10k_mac_handle_beacon_miss(struct ath10k *ar, u32 vdev_id);
+
u8 ath10k_mac_hw_rate_to_idx(const struct ieee80211_supported_band *sband,
u8 hw_rate);
+u8 ath10k_mac_bitrate_to_idx(const struct ieee80211_supported_band *sband,
+ u32 bitrate);

static inline struct ath10k_vif *ath10k_vif_to_arvif(struct ieee80211_vif *vif)
{
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 54430a1..bd7a119 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1352,63 +1352,6 @@ static inline enum ieee80211_band phy_mode_to_band(u32 phy_mode)
return band;
}

-static inline u8 get_rate_idx(u32 rate, enum ieee80211_band band)
-{
- u8 rate_idx = 0;
-
- /* rate in Kbps */
- switch (rate) {
- case 1000:
- rate_idx = 0;
- break;
- case 2000:
- rate_idx = 1;
- break;
- case 5500:
- rate_idx = 2;
- break;
- case 11000:
- rate_idx = 3;
- break;
- case 6000:
- rate_idx = 4;
- break;
- case 9000:
- rate_idx = 5;
- break;
- case 12000:
- rate_idx = 6;
- break;
- case 18000:
- rate_idx = 7;
- break;
- case 24000:
- rate_idx = 8;
- break;
- case 36000:
- rate_idx = 9;
- break;
- case 48000:
- rate_idx = 10;
- break;
- case 54000:
- rate_idx = 11;
- break;
- default:
- break;
- }
-
- if (band == IEEE80211_BAND_5GHZ) {
- if (rate_idx > 3)
- /* Omit CCK rates */
- rate_idx -= 4;
- else
- rate_idx = 0;
- }
-
- return rate_idx;
-}
-
/* If keys are configured, HW decrypts all frames
* with protected bit set. Mark such frames as decrypted.
*/
@@ -1490,6 +1433,7 @@ int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
struct wmi_mgmt_rx_ev_arg arg = {};
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_hdr *hdr;
+ struct ieee80211_supported_band *sband;
u32 rx_status;
u32 channel;
u32 phy_mode;
@@ -1560,9 +1504,11 @@ int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
if (phy_mode == MODE_11B && status->band == IEEE80211_BAND_5GHZ)
ath10k_dbg(ar, ATH10K_DBG_MGMT, "wmi mgmt rx 11b (CCK) on 5GHz\n");

+ sband = &ar->mac.sbands[status->band];
+
status->freq = ieee80211_channel_to_frequency(channel, status->band);
status->signal = snr + ATH10K_DEFAULT_NOISE_FLOOR;
- status->rate_idx = get_rate_idx(rate, status->band);
+ status->rate_idx = ath10k_mac_bitrate_to_idx(sband, rate / 100);

hdr = (struct ieee80211_hdr *)skb->data;
fc = le16_to_cpu(hdr->frame_control);
--
2.1.4


2015-03-20 13:56:59

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 9/9] ath10k: document ofdm/5ghz rate offset with a macro

Don't use literal values for offsets. While at it
rename a function so it is more clear what it
checks for.

This finally takes care of the last magic
5GHz/OFDM offset.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index c39a300..bdd9baa 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -60,8 +60,11 @@ static struct ieee80211_rate ath10k_rates[] = {
{ .bitrate = 540, .hw_value = ATH10K_HW_RATE_OFDM_54M },
};

-#define ath10k_a_rates (ath10k_rates + 4)
-#define ath10k_a_rates_size (ARRAY_SIZE(ath10k_rates) - 4)
+#define ATH10K_MAC_FIRST_OFDM_RATE_IDX 4
+
+#define ath10k_a_rates (ath10k_rates + ATH10K_MAC_FIRST_OFDM_RATE_IDX)
+#define ath10k_a_rates_size (ARRAY_SIZE(ath10k_rates) - \
+ ATH10K_MAC_FIRST_OFDM_RATE_IDX)
#define ath10k_g_rates (ath10k_rates + 0)
#define ath10k_g_rates_size (ARRAY_SIZE(ath10k_rates))

@@ -2001,10 +2004,10 @@ static void ath10k_peer_assoc_h_qos(struct ath10k *ar,
sta->addr, !!(arg->peer_flags & WMI_PEER_QOS));
}

-static bool ath10k_mac_sta_has_11g_rates(struct ieee80211_sta *sta)
+static bool ath10k_mac_sta_has_ofdm_only(struct ieee80211_sta *sta)
{
- /* First 4 rates in ath10k_rates are CCK (11b) rates. */
- return sta->supp_rates[IEEE80211_BAND_2GHZ] >> 4;
+ return sta->supp_rates[IEEE80211_BAND_2GHZ] >>
+ ATH10K_MAC_FIRST_OFDM_RATE_IDX;
}

static void ath10k_peer_assoc_h_phymode(struct ath10k *ar,
@@ -2026,7 +2029,7 @@ static void ath10k_peer_assoc_h_phymode(struct ath10k *ar,
phymode = MODE_11NG_HT40;
else
phymode = MODE_11NG_HT20;
- } else if (ath10k_mac_sta_has_11g_rates(sta)) {
+ } else if (ath10k_mac_sta_has_ofdm_only(sta)) {
phymode = MODE_11G;
} else {
phymode = MODE_11B;
--
2.1.4


2015-03-30 12:19:28

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/9] ath10k: cleanups 2015-03-20

Michal Kazior <[email protected]> writes:

> Here's a bunch of cleanups and fixes associated
> with tx/rx rate code.
>
> Feel free to squash some of the patches if you
> think it makes sense or let me know which ones you
> want me to squash.
>
> There are a few checkpatch warnings for going over
> 80 chars. I decided to leave them be to avoid code
> wrapping which would make the code less readable
> than it is now in my opinion.
>
> I have 2 more patches for .set_bitrate_mask()
> callback which depend on this patchset. Once you
> apply this patchset to your `pending` branch let
> me know, please, so I can push the rest with git
> indexes your git repo will recognize.

Sorry, I had missed this comment and didn't realise to tell you when I
applied the patches to pending branch. The bad part with patchwork is
that it doesn't show the cover letter at all :(

> Michal Kazior (9):
> ath10k: move rate definitions to file start
> ath10k: derive rate from bitrate
> ath10k: drop RATETAB_ENT macro
> ath10k: add hw rate definitions
> ath10k: use hw rate definitions for fixed rate
> ath10k: simplify fixed rate selection
> ath10k: rework legacy rx rate decoding
> ath10k: deduplicate bitrate to rate idx conversion
> ath10k: document ofdm/5ghz rate offset with a macro

Thanks, all patches applied.

--
Kalle Valo

2015-03-20 13:56:54

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 4/9] ath10k: add hw rate definitions

Prepare defines for future use.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt.h | 1 +
drivers/net/wireless/ath/ath10k/hw.h | 21 +++++++++++++++++++++
drivers/net/wireless/ath/ath10k/rx_desc.h | 22 ++++++++++++++++++++++
3 files changed, 44 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 874bf44..923d173 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -25,6 +25,7 @@
#include <net/mac80211.h>

#include "htc.h"
+#include "hw.h"
#include "rx_desc.h"

enum htt_dbg_stats_type {
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 7f04645..70c6ed9 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -180,6 +180,27 @@ struct ath10k_pktlog_hdr {
u8 payload[0];
} __packed;

+enum ath10k_hw_rate_ofdm {
+ ATH10K_HW_RATE_OFDM_48M = 0,
+ ATH10K_HW_RATE_OFDM_24M,
+ ATH10K_HW_RATE_OFDM_12M,
+ ATH10K_HW_RATE_OFDM_6M,
+ ATH10K_HW_RATE_OFDM_54M,
+ ATH10K_HW_RATE_OFDM_36M,
+ ATH10K_HW_RATE_OFDM_18M,
+ ATH10K_HW_RATE_OFDM_9M,
+};
+
+enum ath10k_hw_rate_cck {
+ ATH10K_HW_RATE_CCK_LP_11M = 0,
+ ATH10K_HW_RATE_CCK_LP_5_5M,
+ ATH10K_HW_RATE_CCK_LP_2M,
+ ATH10K_HW_RATE_CCK_LP_1M,
+ ATH10K_HW_RATE_CCK_SP_11M,
+ ATH10K_HW_RATE_CCK_SP_5_5M,
+ ATH10K_HW_RATE_CCK_SP_2M,
+};
+
/* Target specific defines for MAIN firmware */
#define TARGET_NUM_VDEVS 8
#define TARGET_NUM_PEER_AST 2
diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h b/drivers/net/wireless/ath/ath10k/rx_desc.h
index e9cc778..492b5a5 100644
--- a/drivers/net/wireless/ath/ath10k/rx_desc.h
+++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
@@ -661,6 +661,28 @@ struct rx_msdu_end {
#define RX_PPDU_START_INFO5_SERVICE_MASK 0x0000ffff
#define RX_PPDU_START_INFO5_SERVICE_LSB 0

+/* No idea what this flag means. It seems to be always set in rate. */
+#define RX_PPDU_START_RATE_FLAG BIT(3)
+
+enum rx_ppdu_start_rate {
+ RX_PPDU_START_RATE_OFDM_48M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_OFDM_48M,
+ RX_PPDU_START_RATE_OFDM_24M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_OFDM_24M,
+ RX_PPDU_START_RATE_OFDM_12M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_OFDM_12M,
+ RX_PPDU_START_RATE_OFDM_6M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_OFDM_6M,
+ RX_PPDU_START_RATE_OFDM_54M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_OFDM_54M,
+ RX_PPDU_START_RATE_OFDM_36M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_OFDM_36M,
+ RX_PPDU_START_RATE_OFDM_18M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_OFDM_18M,
+ RX_PPDU_START_RATE_OFDM_9M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_OFDM_9M,
+
+ RX_PPDU_START_RATE_CCK_LP_11M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_CCK_LP_11M,
+ RX_PPDU_START_RATE_CCK_LP_5_5M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_CCK_LP_5_5M,
+ RX_PPDU_START_RATE_CCK_LP_2M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_CCK_LP_2M,
+ RX_PPDU_START_RATE_CCK_LP_1M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_CCK_LP_1M,
+ RX_PPDU_START_RATE_CCK_SP_11M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_CCK_SP_11M,
+ RX_PPDU_START_RATE_CCK_SP_5_5M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_CCK_SP_5_5M,
+ RX_PPDU_START_RATE_CCK_SP_2M = RX_PPDU_START_RATE_FLAG | ATH10K_HW_RATE_CCK_SP_2M,
+};
+
struct rx_ppdu_start {
struct {
u8 pri20_mhz;
--
2.1.4


2015-03-20 13:56:56

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 6/9] ath10k: simplify fixed rate selection

Use the existing ieee80211_rate array instead of
definining separate one. This gets rid of the ugly
4-index offset when dealing with 5GHz band.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 56 +++++++++++------------------------
1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 72fbdc2..877afde 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -35,21 +35,19 @@
/*********/

static struct ieee80211_rate ath10k_rates[] = {
- /* CCK */
- { .bitrate = 10 },
- { .bitrate = 20 },
- { .bitrate = 55 },
- { .bitrate = 110 },
-
- /* OFDM */
- { .bitrate = 60 },
- { .bitrate = 90 },
- { .bitrate = 120 },
- { .bitrate = 180 },
- { .bitrate = 240 },
- { .bitrate = 360 },
- { .bitrate = 480 },
- { .bitrate = 540 },
+ { .bitrate = 10, .hw_value = ATH10K_HW_RATE_CCK_LP_1M },
+ { .bitrate = 20, .hw_value = ATH10K_HW_RATE_CCK_LP_2M },
+ { .bitrate = 55, .hw_value = ATH10K_HW_RATE_CCK_LP_5_5M },
+ { .bitrate = 110, .hw_value = ATH10K_HW_RATE_CCK_LP_11M },
+
+ { .bitrate = 60, .hw_value = ATH10K_HW_RATE_OFDM_6M },
+ { .bitrate = 90, .hw_value = ATH10K_HW_RATE_OFDM_9M },
+ { .bitrate = 120, .hw_value = ATH10K_HW_RATE_OFDM_12M },
+ { .bitrate = 180, .hw_value = ATH10K_HW_RATE_OFDM_18M },
+ { .bitrate = 240, .hw_value = ATH10K_HW_RATE_OFDM_24M },
+ { .bitrate = 360, .hw_value = ATH10K_HW_RATE_OFDM_36M },
+ { .bitrate = 480, .hw_value = ATH10K_HW_RATE_OFDM_48M },
+ { .bitrate = 540, .hw_value = ATH10K_HW_RATE_OFDM_54M },
};

#define ath10k_a_rates (ath10k_rates + 4)
@@ -4970,22 +4968,6 @@ exit:
return ret;
}

-/* Helper table for legacy fixed_rate/bitrate_mask */
-static const u8 cck_ofdm_rate[] = {
- ATH10K_HW_RATE_CCK_LP_1M,
- ATH10K_HW_RATE_CCK_LP_2M,
- ATH10K_HW_RATE_CCK_LP_5_5M,
- ATH10K_HW_RATE_CCK_LP_11M,
- ATH10K_HW_RATE_OFDM_6M,
- ATH10K_HW_RATE_OFDM_9M,
- ATH10K_HW_RATE_OFDM_12M,
- ATH10K_HW_RATE_OFDM_18M,
- ATH10K_HW_RATE_OFDM_24M,
- ATH10K_HW_RATE_OFDM_36M,
- ATH10K_HW_RATE_OFDM_48M,
- ATH10K_HW_RATE_OFDM_54M,
-};
-
/* Check if only one bit set */
static int ath10k_check_single_mask(u32 mask)
{
@@ -5133,6 +5115,7 @@ ath10k_bitrate_mask_rate(struct ath10k *ar,
u8 *fixed_rate,
u8 *fixed_nss)
{
+ struct ieee80211_supported_band *sband;
u8 rate = 0, pream = 0, nss = 0, i;
enum wmi_rate_preamble preamble;

@@ -5146,17 +5129,12 @@ ath10k_bitrate_mask_rate(struct ath10k *ar,
case WMI_RATE_PREAMBLE_CCK:
case WMI_RATE_PREAMBLE_OFDM:
i = ffs(mask->control[band].legacy) - 1;
+ sband = &ar->mac.sbands[band];

- if (band == IEEE80211_BAND_2GHZ && i < 4)
- pream = WMI_RATE_PREAMBLE_CCK;
-
- if (band == IEEE80211_BAND_5GHZ)
- i += 4;
-
- if (i >= ARRAY_SIZE(cck_ofdm_rate))
+ if (WARN_ON(i >= sband->n_bitrates))
return false;

- rate = cck_ofdm_rate[i];
+ rate = sband->bitrates[i].hw_value;
break;
case WMI_RATE_PREAMBLE_HT:
for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++)
--
2.1.4


2015-03-20 13:56:51

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 2/9] ath10k: derive rate from bitrate

There's no need to store rate values in hw_value.
This frees up the hw_value for better future use.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 47 +++++++++++++++++++++++++----------
1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 456d671..5721196 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -42,19 +42,19 @@

static struct ieee80211_rate ath10k_rates[] = {
/* CCK */
- RATETAB_ENT(10, 0x82, 0),
- RATETAB_ENT(20, 0x84, 0),
- RATETAB_ENT(55, 0x8b, 0),
- RATETAB_ENT(110, 0x96, 0),
+ RATETAB_ENT(10, 0, 0),
+ RATETAB_ENT(20, 0, 0),
+ RATETAB_ENT(55, 0, 0),
+ RATETAB_ENT(110, 0, 0),
/* OFDM */
- RATETAB_ENT(60, 0x0c, 0),
- RATETAB_ENT(90, 0x12, 0),
- RATETAB_ENT(120, 0x18, 0),
- RATETAB_ENT(180, 0x24, 0),
- RATETAB_ENT(240, 0x30, 0),
- RATETAB_ENT(360, 0x48, 0),
- RATETAB_ENT(480, 0x60, 0),
- RATETAB_ENT(540, 0x6c, 0),
+ RATETAB_ENT(60, 0, 0),
+ RATETAB_ENT(90, 0, 0),
+ RATETAB_ENT(120, 0, 0),
+ RATETAB_ENT(180, 0, 0),
+ RATETAB_ENT(240, 0, 0),
+ RATETAB_ENT(360, 0, 0),
+ RATETAB_ENT(480, 0, 0),
+ RATETAB_ENT(540, 0, 0),
};

#define ath10k_a_rates (ath10k_rates + 4)
@@ -62,6 +62,25 @@ static struct ieee80211_rate ath10k_rates[] = {
#define ath10k_g_rates (ath10k_rates + 0)
#define ath10k_g_rates_size (ARRAY_SIZE(ath10k_rates))

+static bool ath10k_mac_bitrate_is_cck(int bitrate)
+{
+ switch (bitrate) {
+ case 10:
+ case 20:
+ case 55:
+ case 110:
+ return true;
+ }
+
+ return false;
+}
+
+static u8 ath10k_mac_bitrate_to_rate(int bitrate)
+{
+ return DIV_ROUND_UP(bitrate, 5) |
+ (ath10k_mac_bitrate_is_cck(bitrate) ? BIT(7) : 0);
+}
+
/**********/
/* Crypto */
/**********/
@@ -1699,6 +1718,7 @@ static void ath10k_peer_assoc_h_rates(struct ath10k *ar,
const struct ieee80211_supported_band *sband;
const struct ieee80211_rate *rates;
u32 ratemask;
+ u8 rate;
int i;

lockdep_assert_held(&ar->conf_mutex);
@@ -1713,7 +1733,8 @@ static void ath10k_peer_assoc_h_rates(struct ath10k *ar,
if (!(ratemask & 1))
continue;

- rateset->rates[rateset->num_rates] = rates->hw_value;
+ rate = ath10k_mac_bitrate_to_rate(rates->bitrate);
+ rateset->rates[rateset->num_rates] = rate;
rateset->num_rates++;
}
}
--
2.1.4