[PATCH v2 1/2] ath10k: introduce rx_status htt template

[PATCH v2 1/2] ath10k: introduce rx_status htt template

[Janusz Dziedzic]

Introduce rx_status htt template instead of
stack version, as a preparation for fix
rssi and rates reporting.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt.h    |    3 +++
 drivers/net/wireless/ath/ath10k/htt_rx.c |   36 +++++++++++++++---------------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 2d669c0..645a563 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1273,6 +1273,9 @@ struct ath10k_htt {
 	struct tasklet_struct txrx_compl_task;
 	struct sk_buff_head tx_compl_q;
 	struct sk_buff_head rx_compl_q;
+
+	/* rx_status template */
+	struct ieee80211_rx_status rx_status;
 };
 
 #define RX_HTT_HDR_STATUS_LEN 64
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 17f152c..be25e16 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1139,7 +1139,7 @@ static bool ath10k_htt_rx_amsdu_allowed(struct ath10k_htt *htt,
 static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 				  struct htt_rx_indication *rx)
 {
-	struct ieee80211_rx_status rx_status;
+	struct ieee80211_rx_status *rx_status = &htt->rx_status;
 	struct htt_rx_indication_mpdu_range *mpdu_ranges;
 	struct htt_rx_desc *rxd;
 	enum htt_rx_mpdu_status status;
@@ -1154,7 +1154,7 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 
 	lockdep_assert_held(&htt->rx_ring.lock);
 
-	memset(&rx_status, 0, sizeof(rx_status));
+	memset(rx_status, 0, sizeof(*rx_status));
 
 	fw_desc_len = __le16_to_cpu(rx->prefix.fw_rx_desc_bytes);
 	fw_desc = (u8 *)&rx->fw_desc;
@@ -1164,23 +1164,23 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
 
 	/* Fill this once, while this is per-ppdu */
-	rx_status.signal  = ATH10K_DEFAULT_NOISE_FLOOR;
-	rx_status.signal += rx->ppdu.combined_rssi;
+	rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR;
+	rx_status->signal += rx->ppdu.combined_rssi;
 
 	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
 		/* TSF available only in 32-bit */
-		rx_status.mactime = __le32_to_cpu(rx->ppdu.tsf) & 0xffffffff;
-		rx_status.flag |= RX_FLAG_MACTIME_END;
+		rx_status->mactime = __le32_to_cpu(rx->ppdu.tsf) & 0xffffffff;
+		rx_status->flag |= RX_FLAG_MACTIME_END;
 	}
 
-	channel_set = ath10k_htt_rx_h_channel(htt->ar, &rx_status);
+	channel_set = ath10k_htt_rx_h_channel(htt->ar, rx_status);
 
 	if (channel_set) {
-		ath10k_htt_rx_h_rates(htt->ar, rx_status.band,
+		ath10k_htt_rx_h_rates(htt->ar, rx_status->band,
 				      rx->ppdu.info0,
 				      __le32_to_cpu(rx->ppdu.info1),
 				      __le32_to_cpu(rx->ppdu.info2),
-				      &rx_status);
+				      rx_status);
 	}
 
 	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "htt rx ind: ",
@@ -1229,21 +1229,21 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 			}
 
 			if (attention & RX_ATTENTION_FLAGS_FCS_ERR)
-				rx_status.flag |= RX_FLAG_FAILED_FCS_CRC;
+				rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
 			else
-				rx_status.flag &= ~RX_FLAG_FAILED_FCS_CRC;
+				rx_status->flag &= ~RX_FLAG_FAILED_FCS_CRC;
 
 			if (attention & RX_ATTENTION_FLAGS_TKIP_MIC_ERR)
-				rx_status.flag |= RX_FLAG_MMIC_ERROR;
+				rx_status->flag |= RX_FLAG_MMIC_ERROR;
 			else
-				rx_status.flag &= ~RX_FLAG_MMIC_ERROR;
+				rx_status->flag &= ~RX_FLAG_MMIC_ERROR;
 
 			hdr = ath10k_htt_rx_skb_get_hdr(msdu_head);
 
 			if (ath10k_htt_rx_hdr_is_amsdu(hdr))
-				ath10k_htt_rx_amsdu(htt, &rx_status, msdu_head);
+				ath10k_htt_rx_amsdu(htt, rx_status, msdu_head);
 			else
-				ath10k_htt_rx_msdu(htt, &rx_status, msdu_head);
+				ath10k_htt_rx_msdu(htt, rx_status, msdu_head);
 		}
 	}
 
@@ -1257,7 +1257,7 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
 	enum htt_rx_mpdu_encrypt_type enctype;
 	struct htt_rx_desc *rxd;
 	enum rx_msdu_decap_format fmt;
-	struct ieee80211_rx_status rx_status = {};
+	struct ieee80211_rx_status *rx_status = &htt->rx_status;
 	struct ieee80211_hdr *hdr;
 	int ret;
 	bool tkip_mic_err;
@@ -1305,7 +1305,7 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
 
 	enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
 		     RX_MPDU_START_INFO0_ENCRYPT_TYPE);
-	ath10k_htt_rx_h_protected(htt, &rx_status, msdu_head, enctype);
+	ath10k_htt_rx_h_protected(htt, rx_status, msdu_head, enctype);
 	msdu_head->ip_summed = ath10k_htt_rx_get_csum_state(msdu_head);
 
 	if (tkip_mic_err)
@@ -1350,7 +1350,7 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
 
 	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "htt rx frag mpdu: ",
 			msdu_head->data, msdu_head->len);
-	ath10k_process_rx(htt->ar, &rx_status, msdu_head);
+	ath10k_process_rx(htt->ar, rx_status, msdu_head);
 
 end:
 	if (fw_desc_len > 0) {
-- 
1.7.9.5

[PATCH v2 1/2] ath10k: introduce rx_status htt template

[Janusz Dziedzic]

Introduce rx_status htt template instead of
stack version, as a preparation for fix
rssi and rates reporting.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt.h    |    3 +++
 drivers/net/wireless/ath/ath10k/htt_rx.c |   36 +++++++++++++++---------------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 2d669c0..645a563 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1273,6 +1273,9 @@ struct ath10k_htt {
 	struct tasklet_struct txrx_compl_task;
 	struct sk_buff_head tx_compl_q;
 	struct sk_buff_head rx_compl_q;
+
+	/* rx_status template */
+	struct ieee80211_rx_status rx_status;
 };
 
 #define RX_HTT_HDR_STATUS_LEN 64
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 17f152c..be25e16 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1139,7 +1139,7 @@ static bool ath10k_htt_rx_amsdu_allowed(struct ath10k_htt *htt,
 static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 				  struct htt_rx_indication *rx)
 {
-	struct ieee80211_rx_status rx_status;
+	struct ieee80211_rx_status *rx_status = &htt->rx_status;
 	struct htt_rx_indication_mpdu_range *mpdu_ranges;
 	struct htt_rx_desc *rxd;
 	enum htt_rx_mpdu_status status;
@@ -1154,7 +1154,7 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 
 	lockdep_assert_held(&htt->rx_ring.lock);
 
-	memset(&rx_status, 0, sizeof(rx_status));
+	memset(rx_status, 0, sizeof(*rx_status));
 
 	fw_desc_len = __le16_to_cpu(rx->prefix.fw_rx_desc_bytes);
 	fw_desc = (u8 *)&rx->fw_desc;
@@ -1164,23 +1164,23 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
 
 	/* Fill this once, while this is per-ppdu */
-	rx_status.signal  = ATH10K_DEFAULT_NOISE_FLOOR;
-	rx_status.signal += rx->ppdu.combined_rssi;
+	rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR;
+	rx_status->signal += rx->ppdu.combined_rssi;
 
 	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
 		/* TSF available only in 32-bit */
-		rx_status.mactime = __le32_to_cpu(rx->ppdu.tsf) & 0xffffffff;
-		rx_status.flag |= RX_FLAG_MACTIME_END;
+		rx_status->mactime = __le32_to_cpu(rx->ppdu.tsf) & 0xffffffff;
+		rx_status->flag |= RX_FLAG_MACTIME_END;
 	}
 
-	channel_set = ath10k_htt_rx_h_channel(htt->ar, &rx_status);
+	channel_set = ath10k_htt_rx_h_channel(htt->ar, rx_status);
 
 	if (channel_set) {
-		ath10k_htt_rx_h_rates(htt->ar, rx_status.band,
+		ath10k_htt_rx_h_rates(htt->ar, rx_status->band,
 				      rx->ppdu.info0,
 				      __le32_to_cpu(rx->ppdu.info1),
 				      __le32_to_cpu(rx->ppdu.info2),
-				      &rx_status);
+				      rx_status);
 	}
 
 	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "htt rx ind: ",
@@ -1229,21 +1229,21 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 			}
 
 			if (attention & RX_ATTENTION_FLAGS_FCS_ERR)
-				rx_status.flag |= RX_FLAG_FAILED_FCS_CRC;
+				rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
 			else
-				rx_status.flag &= ~RX_FLAG_FAILED_FCS_CRC;
+				rx_status->flag &= ~RX_FLAG_FAILED_FCS_CRC;
 
 			if (attention & RX_ATTENTION_FLAGS_TKIP_MIC_ERR)
-				rx_status.flag |= RX_FLAG_MMIC_ERROR;
+				rx_status->flag |= RX_FLAG_MMIC_ERROR;
 			else
-				rx_status.flag &= ~RX_FLAG_MMIC_ERROR;
+				rx_status->flag &= ~RX_FLAG_MMIC_ERROR;
 
 			hdr = ath10k_htt_rx_skb_get_hdr(msdu_head);
 
 			if (ath10k_htt_rx_hdr_is_amsdu(hdr))
-				ath10k_htt_rx_amsdu(htt, &rx_status, msdu_head);
+				ath10k_htt_rx_amsdu(htt, rx_status, msdu_head);
 			else
-				ath10k_htt_rx_msdu(htt, &rx_status, msdu_head);
+				ath10k_htt_rx_msdu(htt, rx_status, msdu_head);
 		}
 	}
 
@@ -1257,7 +1257,7 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
 	enum htt_rx_mpdu_encrypt_type enctype;
 	struct htt_rx_desc *rxd;
 	enum rx_msdu_decap_format fmt;
-	struct ieee80211_rx_status rx_status = {};
+	struct ieee80211_rx_status *rx_status = &htt->rx_status;
 	struct ieee80211_hdr *hdr;
 	int ret;
 	bool tkip_mic_err;
@@ -1305,7 +1305,7 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
 
 	enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
 		     RX_MPDU_START_INFO0_ENCRYPT_TYPE);
-	ath10k_htt_rx_h_protected(htt, &rx_status, msdu_head, enctype);
+	ath10k_htt_rx_h_protected(htt, rx_status, msdu_head, enctype);
 	msdu_head->ip_summed = ath10k_htt_rx_get_csum_state(msdu_head);
 
 	if (tkip_mic_err)
@@ -1350,7 +1350,7 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
 
 	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "htt rx frag mpdu: ",
 			msdu_head->data, msdu_head->len);
-	ath10k_process_rx(htt->ar, &rx_status, msdu_head);
+	ath10k_process_rx(htt->ar, rx_status, msdu_head);
 
 end:
 	if (fw_desc_len > 0) {
-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH v2 2/2] ath10k: fix rssi and rate reporting

[Janusz Dziedzic]

RSSI and RATES fields are valid only when START_VALID
bit is set. So, in current implementation we have to
remember/caclulate them when START_VALID and report the same
when only END_VALID is set.
Currently during heavy traffic we could have:
- 10 packets with START_VALID - correct RSSI and RATES
- 10 packets with END_VALID
- 10 packets with START_VALID - correct RSSI and RATES
- 10 packets with END_VALID
...
Next using monitor interface we will see:
- 10 packets with correct rssi/rates
- 10 packets with rssi=-95/rate=6Mbps

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index be25e16..f7ecc10 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1154,8 +1154,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 
 	lockdep_assert_held(&htt->rx_ring.lock);
 
-	memset(rx_status, 0, sizeof(*rx_status));
-
 	fw_desc_len = __le16_to_cpu(rx->prefix.fw_rx_desc_bytes);
 	fw_desc = (u8 *)&rx->fw_desc;
 
@@ -1164,8 +1162,11 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
 
 	/* Fill this once, while this is per-ppdu */
-	rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR;
-	rx_status->signal += rx->ppdu.combined_rssi;
+	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
+		memset(rx_status, 0, sizeof(*rx_status));
+		rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
+				     rx->ppdu.combined_rssi;
+	}
 
 	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
 		/* TSF available only in 32-bit */
-- 
1.7.9.5

[PATCH v2 2/2] ath10k: fix rssi and rate reporting

[Janusz Dziedzic]

RSSI and RATES fields are valid only when START_VALID
bit is set. So, in current implementation we have to
remember/caclulate them when START_VALID and report the same
when only END_VALID is set.
Currently during heavy traffic we could have:
- 10 packets with START_VALID - correct RSSI and RATES
- 10 packets with END_VALID
- 10 packets with START_VALID - correct RSSI and RATES
- 10 packets with END_VALID
...
Next using monitor interface we will see:
- 10 packets with correct rssi/rates
- 10 packets with rssi=-95/rate=6Mbps

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index be25e16..f7ecc10 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1154,8 +1154,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 
 	lockdep_assert_held(&htt->rx_ring.lock);
 
-	memset(rx_status, 0, sizeof(*rx_status));
-
 	fw_desc_len = __le16_to_cpu(rx->prefix.fw_rx_desc_bytes);
 	fw_desc = (u8 *)&rx->fw_desc;
 
@@ -1164,8 +1162,11 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
 
 	/* Fill this once, while this is per-ppdu */
-	rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR;
-	rx_status->signal += rx->ppdu.combined_rssi;
+	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
+		memset(rx_status, 0, sizeof(*rx_status));
+		rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
+				     rx->ppdu.combined_rssi;
+	}
 
 	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
 		/* TSF available only in 32-bit */
-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v2 2/2] ath10k: fix rssi and rate reporting

[Ben Greear]

On 03/24/2014 01:24 PM, Janusz Dziedzic wrote:
> RSSI and RATES fields are valid only when START_VALID
> bit is set. So, in current implementation we have to
> remember/caclulate them when START_VALID and report the same
> when only END_VALID is set.
> Currently during heavy traffic we could have:
> - 10 packets with START_VALID - correct RSSI and RATES
> - 10 packets with END_VALID
> - 10 packets with START_VALID - correct RSSI and RATES
> - 10 packets with END_VALID

I applied all 8 of these patches to my tree, and it does fix the problems I saw with
rx-rate and signal being broken (perhaps because I am using raw rx mode for
my stations).

I'll leave these in my tree and will let you know if I see any
problems that might be related to them.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH v2 2/2] ath10k: fix rssi and rate reporting

[Ben Greear]

On 03/24/2014 01:24 PM, Janusz Dziedzic wrote:
> RSSI and RATES fields are valid only when START_VALID
> bit is set. So, in current implementation we have to
> remember/caclulate them when START_VALID and report the same
> when only END_VALID is set.
> Currently during heavy traffic we could have:
> - 10 packets with START_VALID - correct RSSI and RATES
> - 10 packets with END_VALID
> - 10 packets with START_VALID - correct RSSI and RATES
> - 10 packets with END_VALID

I applied all 8 of these patches to my tree, and it does fix the problems I saw with
rx-rate and signal being broken (perhaps because I am using raw rx mode for
my stations).

I'll leave these in my tree and will let you know if I see any
problems that might be related to them.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v2 1/2] ath10k: introduce rx_status htt template

[Kalle Valo]

Janusz Dziedzic <janusz.dziedzic@tieto.com> writes:

> Introduce rx_status htt template instead of
> stack version, as a preparation for fix
> rssi and rates reporting.
>
> Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>

Thanks, both patches applied.

-- 
Kalle Valo

Re: [PATCH v2 1/2] ath10k: introduce rx_status htt template

[Kalle Valo]

Janusz Dziedzic <janusz.dziedzic@tieto.com> writes:

> Introduce rx_status htt template instead of
> stack version, as a preparation for fix
> rssi and rates reporting.
>
> Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>

Thanks, both patches applied.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v2 2/2] ath10k: fix rssi and rate reporting

[Ben Greear]

I have a few questions on this patch (long since accepted upstream).

First, why do you only memset the rx_status if START_VALID is set,
and why do you not set:

rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL

in an else clause if START_VALID is not set?

The patch definitely helps, but I'm still getting less accuracy than expected
(as compared to ath9k going through a similar rate vs range test).

Thanks,
Ben


> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index be25e16..f7ecc10 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -1154,8 +1154,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>  
>  	lockdep_assert_held(&htt->rx_ring.lock);
>  
> -	memset(rx_status, 0, sizeof(*rx_status));
> -
>  	fw_desc_len = __le16_to_cpu(rx->prefix.fw_rx_desc_bytes);
>  	fw_desc = (u8 *)&rx->fw_desc;
>  
> @@ -1164,8 +1162,11 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>  	mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
>  
>  	/* Fill this once, while this is per-ppdu */
> -	rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR;
> -	rx_status->signal += rx->ppdu.combined_rssi;
> +	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
> +		memset(rx_status, 0, sizeof(*rx_status));
> +		rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
> +				     rx->ppdu.combined_rssi;
> +	}
>  
>  	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
>  		/* TSF available only in 32-bit */
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH v2 2/2] ath10k: fix rssi and rate reporting

[Ben Greear]

I have a few questions on this patch (long since accepted upstream).

First, why do you only memset the rx_status if START_VALID is set,
and why do you not set:

rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL

in an else clause if START_VALID is not set?

The patch definitely helps, but I'm still getting less accuracy than expected
(as compared to ath9k going through a similar rate vs range test).

Thanks,
Ben


> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index be25e16..f7ecc10 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -1154,8 +1154,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>  
>  	lockdep_assert_held(&htt->rx_ring.lock);
>  
> -	memset(rx_status, 0, sizeof(*rx_status));
> -
>  	fw_desc_len = __le16_to_cpu(rx->prefix.fw_rx_desc_bytes);
>  	fw_desc = (u8 *)&rx->fw_desc;
>  
> @@ -1164,8 +1162,11 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>  	mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
>  
>  	/* Fill this once, while this is per-ppdu */
> -	rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR;
> -	rx_status->signal += rx->ppdu.combined_rssi;
> +	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
> +		memset(rx_status, 0, sizeof(*rx_status));
> +		rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
> +				     rx->ppdu.combined_rssi;
> +	}
>  
>  	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
>  		/* TSF available only in 32-bit */
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v2 2/2] ath10k: fix rssi and rate reporting

[Janusz Dziedzic]

On 15 May 2014 19:24, Ben Greear <greearb@candelatech.com> wrote:
> I have a few questions on this patch (long since accepted upstream).
>
> First, why do you only memset the rx_status if START_VALID is set,
> and why do you not set:
>
> rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL
>
> in an else clause if START_VALID is not set?
>
> The patch definitely helps, but I'm still getting less accuracy than expected
> (as compared to ath9k going through a similar rate vs range test).
>

>From START_VALID to END_VALID we have same RSSI/RATES like reported
when START_VALID.
We could wait to get all packets from START_VALID to END_VALID and
after that report them to upper
layer. But currently we don't do that and report what we have and have
to remember RSSI/RATES.

BTW there is still a problem with TSF which is valid only when
END_VALID. So in current implementation we don't report this correctly
for packets we report when only START_VALID.

BR
Janusz

>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -1154,8 +1154,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>>
>>       lockdep_assert_held(&htt->rx_ring.lock);
>>
>> -     memset(rx_status, 0, sizeof(*rx_status));
>> -
>>       fw_desc_len = __le16_to_cpu(rx->prefix.fw_rx_desc_bytes);
>>       fw_desc = (u8 *)&rx->fw_desc;
>>
>> @@ -1164,8 +1162,11 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>>       mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
>>
>>       /* Fill this once, while this is per-ppdu */
>> -     rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR;
>> -     rx_status->signal += rx->ppdu.combined_rssi;
>> +     if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
>> +             memset(rx_status, 0, sizeof(*rx_status));
>> +             rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
>> +                                  rx->ppdu.combined_rssi;
>> +     }
>>
>>       if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
>>               /* TSF available only in 32-bit */
>>
>
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>

Re: [PATCH v2 2/2] ath10k: fix rssi and rate reporting

[Janusz Dziedzic]

On 15 May 2014 19:24, Ben Greear <greearb@candelatech.com> wrote:
> I have a few questions on this patch (long since accepted upstream).
>
> First, why do you only memset the rx_status if START_VALID is set,
> and why do you not set:
>
> rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL
>
> in an else clause if START_VALID is not set?
>
> The patch definitely helps, but I'm still getting less accuracy than expected
> (as compared to ath9k going through a similar rate vs range test).
>

From START_VALID to END_VALID we have same RSSI/RATES like reported
when START_VALID.
We could wait to get all packets from START_VALID to END_VALID and
after that report them to upper
layer. But currently we don't do that and report what we have and have
to remember RSSI/RATES.

BTW there is still a problem with TSF which is valid only when
END_VALID. So in current implementation we don't report this correctly
for packets we report when only START_VALID.

BR
Janusz

>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -1154,8 +1154,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>>
>>       lockdep_assert_held(&htt->rx_ring.lock);
>>
>> -     memset(rx_status, 0, sizeof(*rx_status));
>> -
>>       fw_desc_len = __le16_to_cpu(rx->prefix.fw_rx_desc_bytes);
>>       fw_desc = (u8 *)&rx->fw_desc;
>>
>> @@ -1164,8 +1162,11 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>>       mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
>>
>>       /* Fill this once, while this is per-ppdu */
>> -     rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR;
>> -     rx_status->signal += rx->ppdu.combined_rssi;
>> +     if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
>> +             memset(rx_status, 0, sizeof(*rx_status));
>> +             rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
>> +                                  rx->ppdu.combined_rssi;
>> +     }
>>
>>       if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
>>               /* TSF available only in 32-bit */
>>
>
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k