Lingbo Kong <quic_lingbok@xxxxxxxxxxx> 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@xxxxxxxxxxx>
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
