diff mbox series

wifi: ath12k: fix the issue that calculation of ack signal strength

Message ID 20240511030917.65811-1-quic_lingbok@quicinc.com
State New
Headers show
Series wifi: ath12k: fix the issue that calculation of ack signal strength | expand

Commit Message

Lingbo Kong May 11, 2024, 3:09 a.m. UTC 
Currently, the calculation of ack signal strength is incorrect.

This is because before calculating the ack signal strength, ath12k need
to determine whether the hardware and firmware support db2dbm, if the
hardware and firmware support db2dbm, do not need to add noise floor,
otherwise, need to add noise floor.

Besides, the value of ack_rssi passed by firmware to ath12k should be a
signed number, so change its type to s8.

This patch affects QCN9274 and WCN7850 because they have the same issue.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1

Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/dp_tx.c  | 14 ++++++++++----
 drivers/net/wireless/ath/ath12k/dp_tx.h  |  2 +-
 drivers/net/wireless/ath/ath12k/hal_tx.h |  4 ++--
 3 files changed, 13 insertions(+), 7 deletions(-)


base-commit: 1025c616ee13372f3803b158abb1d87ef368ae3d

Comments

Jeff Johnson May 14, 2024, 12:01 a.m. UTC | #1 
On 5/10/2024 8:09 PM, Lingbo Kong wrote:

Your subject doesn't quite make sense.

Perhaps something like: fix ack signal strength calculation

> Currently, the calculation of ack signal strength is incorrect.
> 
> This is because before calculating the ack signal strength, ath12k need
> to determine whether the hardware and firmware support db2dbm, if the

please split this run-on sentence, s/, if/. If/

> hardware and firmware support db2dbm, do not need to add noise floor,
> otherwise, need to add noise floor.
> 
> Besides, the value of ack_rssi passed by firmware to ath12k should be a
> signed number, so change its type to s8.
> 
> This patch affects QCN9274 and WCN7850 because they have the same issue.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>

Code LGTM & Kalle can fix my comments in pending so don't respin unless there
are additional comments.

Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
Kalle Valo May 16, 2024, 3:58 p.m. UTC | #2 
Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> On 5/10/2024 8:09 PM, Lingbo Kong wrote:
>
> Your subject doesn't quite make sense.
>
> Perhaps something like: fix ack signal strength calculation
>
>> Currently, the calculation of ack signal strength is incorrect.
>> 
>> This is because before calculating the ack signal strength, ath12k need
>> to determine whether the hardware and firmware support db2dbm, if the
>
> please split this run-on sentence, s/, if/. If/
>
>> hardware and firmware support db2dbm, do not need to add noise floor,
>> otherwise, need to add noise floor.
>> 
>> Besides, the value of ack_rssi passed by firmware to ath12k should be a
>> signed number, so change its type to s8.
>> 
>> This patch affects QCN9274 and WCN7850 because they have the same issue.
>> 
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> 
>> Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
>
> Code LGTM & Kalle can fix my comments in pending so don't respin unless there
> are additional comments.

Fixed Jeff's comments. I also removed both db2dbm variables, I felt that
they were not necessary. Please check:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=c7b6d4df1ac8427ac758f35b7c1aa641fa8df75f
Jeff Johnson May 16, 2024, 4:57 p.m. UTC | #3 
On 5/16/2024 8:58 AM, Kalle Valo wrote:
> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
> 
>> On 5/10/2024 8:09 PM, Lingbo Kong wrote:
>>
>> Your subject doesn't quite make sense.
>>
>> Perhaps something like: fix ack signal strength calculation
>>
>>> Currently, the calculation of ack signal strength is incorrect.
>>>
>>> This is because before calculating the ack signal strength, ath12k need
>>> to determine whether the hardware and firmware support db2dbm, if the
>>
>> please split this run-on sentence, s/, if/. If/
>>
>>> hardware and firmware support db2dbm, do not need to add noise floor,
>>> otherwise, need to add noise floor.
>>>
>>> Besides, the value of ack_rssi passed by firmware to ath12k should be a
>>> signed number, so change its type to s8.
>>>
>>> This patch affects QCN9274 and WCN7850 because they have the same issue.
>>>
>>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>>>
>>> Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
>>
>> Code LGTM & Kalle can fix my comments in pending so don't respin unless there
>> are additional comments.
> 
> Fixed Jeff's comments. I also removed both db2dbm variables, I felt that
> they were not necessary. Please check:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=c7b6d4df1ac8427ac758f35b7c1aa641fa8df75f
> 
LGTM, thanks!
Lingbo Kong May 17, 2024, 2:14 a.m. UTC | #4 
On 2024/5/16 23:58, Kalle Valo wrote:
> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
> 
>> On 5/10/2024 8:09 PM, Lingbo Kong wrote:
>>
>> Your subject doesn't quite make sense.
>>
>> Perhaps something like: fix ack signal strength calculation
>>
>>> Currently, the calculation of ack signal strength is incorrect.
>>>
>>> This is because before calculating the ack signal strength, ath12k need
>>> to determine whether the hardware and firmware support db2dbm, if the
>>
>> please split this run-on sentence, s/, if/. If/
>>
>>> hardware and firmware support db2dbm, do not need to add noise floor,
>>> otherwise, need to add noise floor.
>>>
>>> Besides, the value of ack_rssi passed by firmware to ath12k should be a
>>> signed number, so change its type to s8.
>>>
>>> This patch affects QCN9274 and WCN7850 because they have the same issue.
>>>
>>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>>>
>>> Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
>>
>> Code LGTM & Kalle can fix my comments in pending so don't respin unless there
>> are additional comments.
> 
> Fixed Jeff's comments. I also removed both db2dbm variables, I felt that
> they were not necessary. Please check:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=c7b6d4df1ac8427ac758f35b7c1aa641fa8df75f
> 

kalle, thank you very much:)
Kalle Valo May 17, 2024, 6:57 a.m. UTC | #5 
Lingbo Kong <quic_lingbok@quicinc.com> wrote:

> Currently, the calculation of ack signal strength is incorrect.
> 
> This is because before calculating the ack signal strength, ath12k need
> to determine whether the hardware and firmware support db2dbm. If the
> hardware and firmware support db2dbm, do not need to add noise floor,
> otherwise, need to add noise floor.
> 
> Besides, the value of ack_rssi passed by firmware to ath12k should be a
> signed number, so change its type to s8.
> 
> This patch affects QCN9274 and WCN7850 because they have the same issue.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

Patch applied to ath-next branch of ath.git, thanks.

ed61863debb2 wifi: ath12k: fix ack signal strength calculation
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c
index a22fa43c87ec..57bfa1b54655 100644
--- a/drivers/net/wireless/ath/ath12k/dp_tx.c
+++ b/drivers/net/wireless/ath/ath12k/dp_tx.c
@@ -374,6 +374,8 @@  ath12k_dp_tx_htt_tx_complete_buf(struct ath12k_base *ab,
 	struct ieee80211_tx_info *info;
 	struct ath12k_skb_cb *skb_cb;
 	struct ath12k *ar;
+	bool db2dbm = test_bit(WMI_TLV_SERVICE_HW_DB2DBM_CONVERSION_SUPPORT,
+			       ab->wmi_ab.svc_map);
 
 	skb_cb = ATH12K_SKB_CB(msdu);
 	info = IEEE80211_SKB_CB(msdu);
@@ -393,8 +395,9 @@  ath12k_dp_tx_htt_tx_complete_buf(struct ath12k_base *ab,
 	if (ts->acked) {
 		if (!(info->flags & IEEE80211_TX_CTL_NO_ACK)) {
 			info->flags |= IEEE80211_TX_STAT_ACK;
-			info->status.ack_signal = ATH12K_DEFAULT_NOISE_FLOOR +
-						  ts->ack_rssi;
+			info->status.ack_signal = ts->ack_rssi;
+			if (!db2dbm)
+				info->status.ack_signal += ATH12K_DEFAULT_NOISE_FLOOR;
 			info->status.flags = IEEE80211_TX_STATUS_ACK_SIGNAL_VALID;
 		} else {
 			info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED;
@@ -451,6 +454,8 @@  static void ath12k_dp_tx_complete_msdu(struct ath12k *ar,
 	struct ath12k_hw *ah = ar->ah;
 	struct ieee80211_tx_info *info;
 	struct ath12k_skb_cb *skb_cb;
+	bool db2dbm = test_bit(WMI_TLV_SERVICE_HW_DB2DBM_CONVERSION_SUPPORT,
+			       ab->wmi_ab.svc_map);
 
 	if (WARN_ON_ONCE(ts->buf_rel_source != HAL_WBM_REL_SRC_MODULE_TQM)) {
 		/* Must not happen */
@@ -486,8 +491,9 @@  static void ath12k_dp_tx_complete_msdu(struct ath12k *ar,
 	case HAL_WBM_TQM_REL_REASON_FRAME_ACKED:
 		if (!(info->flags & IEEE80211_TX_CTL_NO_ACK)) {
 			info->flags |= IEEE80211_TX_STAT_ACK;
-			info->status.ack_signal = ATH12K_DEFAULT_NOISE_FLOOR +
-						  ts->ack_rssi;
+			info->status.ack_signal = ts->ack_rssi;
+			if (!db2dbm)
+				info->status.ack_signal += ATH12K_DEFAULT_NOISE_FLOOR;
 			info->status.flags = IEEE80211_TX_STATUS_ACK_SIGNAL_VALID;
 		}
 		break;
diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.h b/drivers/net/wireless/ath/ath12k/dp_tx.h
index 7faa81faf6a7..55ff8cc721e3 100644
--- a/drivers/net/wireless/ath/ath12k/dp_tx.h
+++ b/drivers/net/wireless/ath/ath12k/dp_tx.h
@@ -12,7 +12,7 @@ 
 
 struct ath12k_dp_htt_wbm_tx_status {
 	bool acked;
-	int ack_rssi;
+	s8 ack_rssi;
 };
 
 int ath12k_dp_tx_htt_h2t_ver_req_msg(struct ath12k_base *ab);
diff --git a/drivers/net/wireless/ath/ath12k/hal_tx.h b/drivers/net/wireless/ath/ath12k/hal_tx.h
index 7c837094a6f7..3cf5973771d7 100644
--- a/drivers/net/wireless/ath/ath12k/hal_tx.h
+++ b/drivers/net/wireless/ath/ath12k/hal_tx.h
@@ -1,7 +1,7 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause-Clear */
 /*
  * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2022, 2024 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #ifndef ATH12K_HAL_TX_H
@@ -57,7 +57,7 @@  struct hal_tx_info {
 struct hal_tx_status {
 	enum hal_wbm_rel_src_module buf_rel_source;
 	enum hal_wbm_tqm_rel_reason status;
-	u8 ack_rssi;
+	s8 ack_rssi;
 	u32 flags; /* %HAL_TX_STATUS_FLAGS_ */
 	u32 ppdu_id;
 	u8 try_cnt;