> -----Original Message-----
> From: ath10k <ath10k-bounces@xxxxxxxxxxxxxxxxxxx> On Behalf Of Kalle Valo
> Sent: Thursday, April 9, 2020 10:22 PM
> To: Yu Wang <yyuwang@xxxxxxxxxxxxxx>
> Cc: linux-wireless@xxxxxxxxxxxxxxx; ath10k@xxxxxxxxxxxxxxxxxxx
> Subject: [EXT] Re: [PATCH v2 2/2] ath10k: correct legacy rate in tx stats
>
> Yu Wang <yyuwang@xxxxxxxxxxxxxx> wrote:
>
> > When working in station mode, after connected to a legacy AP, 11g
> > only, for example, the tx bitrate is incorrect in output of command
> > 'iw wlan0 link'.
> >
> > That's because the legacy tx bitrate value reported by firmware is not
> > well handled:
> > For QCA6174, the value represents rate index, but treated as a real
> > rate; For QCA9888, the value is real rate, with unit 'Mbps', but
> > treated as '100kbps'.
> >
> > To fix this issue:
> > 1. Translate the rate index to real rate for QCA6174; 2. Translate the
> > rate from 'Mbps' to 'kbps' for QCA9888.
> >
> > Tested with:
> > QCA6174 PCIe with firmware WLAN.RM.4.4.1.c3-00031.
> > QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00029.
> > QCA9888 PCIe with firmware 10.4-3.9.0.2-00040.
> >
> > Signed-off-by: Yu Wang <yyuwang@xxxxxxxxxxxxxx>
> > Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
>
> My comments don't seem to go to patchwork, so trying again:
>
> What about QCA988X and WCN3990, how do they behave? Does this patch
> break those?
Since HTT_T2H_MSG_TYPE_PPDU_STATS_IND is a newly added message, suppose it's won't break the other functions.
I don’t have environment to verify the patch with QCA988X and WCN3990.
Can anyone help on this?
>
> > +cck = (preamble == WMI_RATE_PREAMBLE_CCK);
> > +hw_rate = ATH10K_HW_LEGACY_RATE(*ratecode);
> > +for (i = 0; i < sband->n_bitrates; i++) {
> > +bitrates = &sband->bitrates[i];
> > +if (ath10k_mac_bitrate_is_cck(bitrates->bitrate) != cck)
> > +continue;
> > +
> > +if (bitrates->hw_value == hw_rate ||
> > + (bitrates->flags & IEEE80211_RATE_SHORT_PREAMBLE &&
> > + bitrates->hw_value_short == hw_rate)) {
> > +bitrate = bitrates->bitrate;
> > +
> > +/* The bitrate will be recovered in
> > + * ath10k_update_per_peer_tx_stats().
> > + */
> > +if (bitrate == 55)
> > +bitrate = 60;
> > +
> > +bitrate = bitrate / 10;
>
> Here you use magic value 60 but in ath10k_update_per_peer_tx_stats() you use
> magic value 50:
>
> > +/* from 1Mbps to 100Kbps */
> > +rate = rate * 10;
> > +if (rate == 50)
> > +rate = 55;
>
> Am I missing something or how is this supposed to work?
In existing code, ath10k_update_per_peer_tx_stats() will check the bitrate and convert 6_CCK to 5(in the comment: FW sends CCK rate 5.5Mbps as 6), and then 5 will be recovered to 55.
That's why we need to convert bitrate 55 to 6 when processing PPDU_STATS.
if (txrate.flags == WMI_RATE_PREAMBLE_CCK ||
txrate.flags == WMI_RATE_PREAMBLE_OFDM) {
rate = ATH10K_HW_LEGACY_RATE(peer_stats->ratecode);
/* This is hacky, FW sends CCK rate 5.5Mbps as 6 */
if (rate == 6 && txrate.flags == WMI_RATE_PREAMBLE_CCK)
rate = 5;
rate_idx = ath10k_get_legacy_rate_idx(ar, rate);
if (rate_idx < 0)
return;
/* from 1Mbps to 100Kbps */
rate = rate * 10;
if (rate == 50)
rate = 55;
>
> --
> https://patchwork.kernel.org/patch/11251001/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatch
> es
>
> _______________________________________________
> ath10k mailing list
> ath10k@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/ath10k
