diff mbox

[2/2] ath10k: Fix peerid configuration in htt tx desc for htt version < 3.4

Message ID 1446453229-22982-2-git-send-email-vthiagar@qti.qualcomm.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Vasanthakumar Thiagarajan Nov. 2, 2015, 8:33 a.m. UTC
Of a word in struct htt_data_tx_desc htt version >= 3.4 firmware uses LSB 16-bit
for frequency configuration which is used for offchannel tx and MSB 16-bit
is for peerid. But other firmwares using version 2.X (10.1, 10.2.2, 10.2.4
and 10.4) are using 32-bit for peerid in htt tx desc. So far no issue is found
with the existing code setting peerid and freq for HTT version 2.X, this could
be mainly because of 0 as frequecy (home channel) is being always passed with
those firmwares. There may be issues when non-zero freq is passed with firmware
using < 3.4 htt version. To be safe use target_version_major and target_version_minor
along with htt-op-version before configuring peer id and freq in htt tx desc.

Fixes: 8d6d36243610 ("ath10k: fix offchan reliability")
Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qti.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/htt.h    |  9 +++++++--
 drivers/net/wireless/ath/ath10k/htt_tx.c | 13 +++++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

Comments

Michal Kazior Nov. 2, 2015, 10:27 a.m. UTC | #1
On 2 November 2015 at 09:33, Vasanthakumar Thiagarajan
<vthiagar@qti.qualcomm.com> wrote:
> Of a word in struct htt_data_tx_desc htt version >= 3.4 firmware uses LSB 16-bit
> for frequency configuration which is used for offchannel tx and MSB 16-bit
> is for peerid. But other firmwares using version 2.X (10.1, 10.2.2, 10.2.4
> and 10.4) are using 32-bit for peerid in htt tx desc. So far no issue is found
> with the existing code setting peerid and freq for HTT version 2.X, this could
> be mainly because of 0 as frequecy (home channel) is being always passed with
> those firmwares. There may be issues when non-zero freq is passed with firmware
> using < 3.4 htt version. To be safe use target_version_major and target_version_minor
> along with htt-op-version before configuring peer id and freq in htt tx desc.
>
> Fixes: 8d6d36243610 ("ath10k: fix offchan reliability")
> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qti.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath10k/htt.h    |  9 +++++++--
>  drivers/net/wireless/ath/ath10k/htt_tx.c | 13 +++++++++++--
>  2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
> index 2bad50e..44fb4f2 100644
> --- a/drivers/net/wireless/ath/ath10k/htt.h
> +++ b/drivers/net/wireless/ath/ath10k/htt.h
> @@ -166,8 +166,13 @@ struct htt_data_tx_desc {
>         __le16 len;
>         __le16 id;
>         __le32 frags_paddr;
> -       __le16 peerid;
> -       __le16 freq;
> +       union {
> +               __le32 peerid;
> +               struct {
> +                       __le16 peerid;
> +                       __le16 freq;
> +               } __packed offchan_tx;
> +       } __packed;
>         u8 prefetch[0]; /* start of frame, for FW classification engine */
>  } __packed;
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
> index 1682397..bf19247 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> @@ -681,8 +681,17 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
>         skb_cb->htt.txbuf->cmd_tx.len = __cpu_to_le16(msdu->len);
>         skb_cb->htt.txbuf->cmd_tx.id = __cpu_to_le16(msdu_id);
>         skb_cb->htt.txbuf->cmd_tx.frags_paddr = __cpu_to_le32(frags_paddr);
> -       skb_cb->htt.txbuf->cmd_tx.peerid = __cpu_to_le16(HTT_INVALID_PEERID);
> -       skb_cb->htt.txbuf->cmd_tx.freq = __cpu_to_le16(skb_cb->htt.freq);
> +       if (ar->htt.target_version_major >= 3 &&
> +           ar->htt.target_version_minor >= 4 &&
> +           ar->htt.op_version == ATH10K_FW_HTT_OP_VERSION_TLV) {

Hmm.. I think it'd be better to rename
ath10k_mac_need_offchan_tx_work() to, e.g.
ath10k_htt_tx_frm_has_freq() and add the htt.op_version check to it.


Micha?
Vasanthakumar Thiagarajan Nov. 2, 2015, 10:54 a.m. UTC | #2
On Monday 02 November 2015 03:57 PM, Michal Kazior wrote:
> On 2 November 2015 at 09:33, Vasanthakumar Thiagarajan
> <vthiagar@qti.qualcomm.com> wrote:
>> Of a word in struct htt_data_tx_desc htt version >= 3.4 firmware uses LSB 16-bit
>> for frequency configuration which is used for offchannel tx and MSB 16-bit
>> is for peerid. But other firmwares using version 2.X (10.1, 10.2.2, 10.2.4
>> and 10.4) are using 32-bit for peerid in htt tx desc. So far no issue is found
>> with the existing code setting peerid and freq for HTT version 2.X, this could
>> be mainly because of 0 as frequecy (home channel) is being always passed with
>> those firmwares. There may be issues when non-zero freq is passed with firmware
>> using < 3.4 htt version. To be safe use target_version_major and target_version_minor
>> along with htt-op-version before configuring peer id and freq in htt tx desc.
>>
>> Fixes: 8d6d36243610 ("ath10k: fix offchan reliability")
>> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qti.qualcomm.com>
>> ---
>>   drivers/net/wireless/ath/ath10k/htt.h    |  9 +++++++--
>>   drivers/net/wireless/ath/ath10k/htt_tx.c | 13 +++++++++++--
>>   2 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
>> index 2bad50e..44fb4f2 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt.h
>> +++ b/drivers/net/wireless/ath/ath10k/htt.h
>> @@ -166,8 +166,13 @@ struct htt_data_tx_desc {
>>          __le16 len;
>>          __le16 id;
>>          __le32 frags_paddr;
>> -       __le16 peerid;
>> -       __le16 freq;
>> +       union {
>> +               __le32 peerid;
>> +               struct {
>> +                       __le16 peerid;
>> +                       __le16 freq;
>> +               } __packed offchan_tx;
>> +       } __packed;
>>          u8 prefetch[0]; /* start of frame, for FW classification engine */
>>   } __packed;
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
>> index 1682397..bf19247 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
>> @@ -681,8 +681,17 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
>>          skb_cb->htt.txbuf->cmd_tx.len = __cpu_to_le16(msdu->len);
>>          skb_cb->htt.txbuf->cmd_tx.id = __cpu_to_le16(msdu_id);
>>          skb_cb->htt.txbuf->cmd_tx.frags_paddr = __cpu_to_le32(frags_paddr);
>> -       skb_cb->htt.txbuf->cmd_tx.peerid = __cpu_to_le16(HTT_INVALID_PEERID);
>> -       skb_cb->htt.txbuf->cmd_tx.freq = __cpu_to_le16(skb_cb->htt.freq);
>> +       if (ar->htt.target_version_major >= 3 &&
>> +           ar->htt.target_version_minor >= 4 &&
>> +           ar->htt.op_version == ATH10K_FW_HTT_OP_VERSION_TLV) {
>
> Hmm.. I think it'd be better to rename
> ath10k_mac_need_offchan_tx_work() to, e.g.
> ath10k_htt_tx_frm_has_freq() and add the htt.op_version check to it.


Sure, thanks.

Vasanth
diff mbox

Patch

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 2bad50e..44fb4f2 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -166,8 +166,13 @@  struct htt_data_tx_desc {
 	__le16 len;
 	__le16 id;
 	__le32 frags_paddr;
-	__le16 peerid;
-	__le16 freq;
+	union {
+		__le32 peerid;
+		struct {
+			__le16 peerid;
+			__le16 freq;
+		} __packed offchan_tx;
+	} __packed;
 	u8 prefetch[0]; /* start of frame, for FW classification engine */
 } __packed;
 
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 1682397..bf19247 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -681,8 +681,17 @@  int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
 	skb_cb->htt.txbuf->cmd_tx.len = __cpu_to_le16(msdu->len);
 	skb_cb->htt.txbuf->cmd_tx.id = __cpu_to_le16(msdu_id);
 	skb_cb->htt.txbuf->cmd_tx.frags_paddr = __cpu_to_le32(frags_paddr);
-	skb_cb->htt.txbuf->cmd_tx.peerid = __cpu_to_le16(HTT_INVALID_PEERID);
-	skb_cb->htt.txbuf->cmd_tx.freq = __cpu_to_le16(skb_cb->htt.freq);
+	if (ar->htt.target_version_major >= 3 &&
+	    ar->htt.target_version_minor >= 4 &&
+	    ar->htt.op_version == ATH10K_FW_HTT_OP_VERSION_TLV) {
+		skb_cb->htt.txbuf->cmd_tx.offchan_tx.peerid =
+				__cpu_to_le16(HTT_INVALID_PEERID);
+		skb_cb->htt.txbuf->cmd_tx.offchan_tx.freq =
+				__cpu_to_le16(skb_cb->htt.freq);
+	} else {
+		skb_cb->htt.txbuf->cmd_tx.peerid =
+				__cpu_to_le32(HTT_INVALID_PEERID);
+	}
 
 	trace_ath10k_htt_tx(ar, msdu_id, msdu->len, vdev_id, tid);
 	ath10k_dbg(ar, ATH10K_DBG_HTT,