From: Kalle Valo <kvalo@kernel.org>
To: Lingbo Kong <quic_lingbok@quicinc.com>
Cc: <ath12k@lists.infradead.org>, <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH v4 1/3] wifi: ath12k: report station mode transmit rate
Date: Thu, 25 Apr 2024 19:54:44 +0300 [thread overview]
Message-ID: <87bk5xs7qj.fsf@kernel.org> (raw)
In-Reply-To: <20240419032122.7009-2-quic_lingbok@quicinc.com> (Lingbo Kong's message of "Fri, 19 Apr 2024 11:21:20 +0800")
Lingbo Kong <quic_lingbok@quicinc.com> writes:
> Currently, the transmit rate of "iw dev xxx station dump" command
> always show an invalid value.
>
> To address this issue, ath12k parse the info of transmit complete
> report from firmware and indicate the transmit rate to mac80211.
>
> This patch affects the station mode of WCN7850 and QCN9274.
>
> After that, "iw dev xxx station dump" show the correct transmit rate.
> Such as:
>
> Station 00:03:7f:12:03:03 (on wlo1)
> inactive time: 872 ms
> rx bytes: 219111
> rx packets: 1133
> tx bytes: 53767
> tx packets: 462
> tx retries: 51
> tx failed: 0
> beacon loss: 0
> beacon rx: 403
> rx drop misc: 74
> signal: -95 dBm
> beacon signal avg: -18 dBm
> tx bitrate: 1441.1 MBit/s 80MHz EHT-MCS 13 EHT-NSS 2 EHT-GI 0
>
> 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.2.1-00201-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
I'm still going throught the patchset, please don't send a new version
yet. Few quick comments:
> +static void ath12k_dp_tx_update_txcompl(struct ath12k *ar, struct hal_tx_status *ts)
> +{
> + struct ath12k_base *ab = ar->ab;
> + struct ath12k_peer *peer;
> + struct ath12k_sta *arsta;
> + struct ieee80211_sta *sta;
> + u16 rate;
> + u8 rate_idx = 0;
> + int ret;
> +
> + spin_lock_bh(&ab->base_lock);
Did you analyse how this function, and especially taking the base_lock,
affects performance?
> +enum nl80211_he_ru_alloc ath12k_mac_he_ru_tones_to_nl80211_he_ru_alloc(u16 ru_tones)
> +{
> + enum nl80211_he_ru_alloc ret;
> +
> + switch (ru_tones) {
> + case 26:
> + ret = NL80211_RATE_INFO_HE_RU_ALLOC_26;
> + break;
> + case 52:
> + ret = NL80211_RATE_INFO_HE_RU_ALLOC_52;
> + break;
> + case 106:
> + ret = NL80211_RATE_INFO_HE_RU_ALLOC_106;
> + break;
> + case 242:
> + ret = NL80211_RATE_INFO_HE_RU_ALLOC_242;
> + break;
> + case 484:
> + ret = NL80211_RATE_INFO_HE_RU_ALLOC_484;
> + break;
> + case 996:
> + ret = NL80211_RATE_INFO_HE_RU_ALLOC_996;
> + break;
> + case (996 * 2):
> + ret = NL80211_RATE_INFO_HE_RU_ALLOC_2x996;
> + break;
> + default:
> + ret = NL80211_RATE_INFO_HE_RU_ALLOC_26;
> + break;
> + }
> +
> + return ret;
> +}
How does this function compare to ath12k_he_ru_tones_to_nl80211_he_ru_alloc()?
> +enum nl80211_eht_gi ath12k_mac_eht_gi_to_nl80211_eht_gi(u8 sgi)
> +{
> + enum nl80211_eht_gi ret;
> +
> + switch (sgi) {
> + case RX_MSDU_START_SGI_0_8_US:
> + ret = NL80211_RATE_INFO_EHT_GI_0_8;
> + break;
> + case RX_MSDU_START_SGI_1_6_US:
> + ret = NL80211_RATE_INFO_EHT_GI_1_6;
> + break;
> + case RX_MSDU_START_SGI_3_2_US:
> + ret = NL80211_RATE_INFO_EHT_GI_3_2;
> + break;
> + default:
> + ret = NL80211_RATE_INFO_EHT_GI_0_8;
> + break;
> + }
> +
> + return ret;
> +}
BTW the ret variable is unnessary, this could be simplified to:
switch (foo) {
case FOO1:
return BAR1;
case FOO2:
return BAR2;
default:
return BAR3;
}
> +enum nl80211_eht_ru_alloc ath12k_mac_eht_ru_tones_to_nl80211_eht_ru_alloc(u16 ru_tones)
> +{
> + enum nl80211_eht_ru_alloc ret;
> +
> + switch (ru_tones) {
> + case 26:
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_26;
> + break;
> + case 52:
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_52;
> + break;
> + case (52 + 26):
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_52P26;
> + break;
> + case 106:
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_106;
> + break;
> + case (106 + 26):
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_106P26;
> + break;
> + case 242:
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_242;
> + break;
> + case 484:
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_484;
> + break;
> + case (484 + 242):
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_484P242;
> + break;
> + case 996:
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_996;
> + break;
> + case (996 + 484):
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_996P484;
> + break;
> + case (996 + 484 + 242):
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_996P484P242;
> + break;
> + case (2 * 996):
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_2x996;
> + break;
> + case (2 * 996 + 484):
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_2x996P484;
> + break;
> + case (3 * 996):
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_3x996;
> + break;
> + case (3 * 996 + 484):
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_3x996P484;
> + break;
> + case (4 * 996):
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_4x996;
> + break;
> + default:
> + ret = NL80211_RATE_INFO_EHT_RU_ALLOC_26;
> + }
> +
> + return ret;
> +}
Same here.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
next prev parent reply other threads:[~2024-04-25 16:54 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-19 3:21 [PATCH v4 0/3] wifi: ath12k: report station mode stats Lingbo Kong
2024-04-19 3:21 ` [PATCH v4 1/3] wifi: ath12k: report station mode transmit rate Lingbo Kong
2024-04-25 10:37 ` Kalle Valo
2024-04-26 8:01 ` Lingbo Kong
2024-04-26 11:24 ` Kalle Valo
2024-05-07 11:06 ` Lingbo Kong
2024-04-25 16:54 ` Kalle Valo [this message]
2024-04-26 6:41 ` Lingbo Kong
2024-04-26 11:21 ` Kalle Valo
2024-04-30 11:41 ` Lingbo Kong
2024-04-29 9:11 ` Karthikeyan Periyasamy
2024-04-29 9:29 ` Lingbo Kong
2024-04-19 3:21 ` [PATCH v4 2/3] wifi: ath12k: report station mode receive rate for IEEE 802.11be Lingbo Kong
2024-04-19 3:21 ` [PATCH v4 3/3] wifi: ath12k: report station mode signal strength Lingbo Kong
2024-04-25 17:03 ` Kalle Valo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87bk5xs7qj.fsf@kernel.org \
--to=kvalo@kernel.org \
--cc=ath12k@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=quic_lingbok@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.