From: Lingbo Kong <quic_lingbok@quicinc.com>
To: Kalle Valo <kvalo@kernel.org>
Cc: <ath12k@lists.infradead.org>, <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH v4 1/3] wifi: ath12k: report station mode transmit rate
Date: Tue, 7 May 2024 19:06:36 +0800 [thread overview]
Message-ID: <5912d64a-3f69-44ad-9b06-9e02019141e6@quicinc.com> (raw)
In-Reply-To: <87r0esqsd6.fsf@kernel.org>
On 2024/4/26 19:24, Kalle Valo wrote:
> Lingbo Kong <quic_lingbok@quicinc.com> writes:
>
>> On 2024/4/25 18:37, Kalle Valo wrote:
>>> 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>
>>> [...]
>>>
>>>> +static void ath12k_dp_tx_update(struct ath12k *ar, struct hal_tx_status *ts)
>>>> +{
>>>> + if (ar->last_ppdu_id != 0) {
>>>> + if (ar->last_ppdu_id == ts->ppdu_id ||
>>>> + ar->cached_ppdu_id == ar->last_ppdu_id)
>>>> + ar->cached_ppdu_id = ar->last_ppdu_id;
>>>> +
>>>> + ath12k_dp_tx_update_txcompl(ar, ts);
>>>> + }
>>>> +
>>>> + ar->last_ppdu_id = ts->ppdu_id;
>>>> +}
>>> A code comment would help a lot. Why is ar->cached_ppdu_id needed
>>> here?
>>> And if 'ar->cached_ppdu_id == ar->last_ppdu_id' is true why do then
>>> do
>>> 'ar->cached_ppdu_id = ar->last_ppdu_id'? The value of ar->cached_ppdu_id
>>> is not changing here (unless I'm missing something).
>>> Also I'm worried about locking. How is access to ar->last_ppdu_id
>>> and
>>> ar->cached_ppdu_id protected?
>>>
>>
>> Thanks for pointing to this.
>> you're right, the ar->cached_ppdu_id haven't used in here, so need to
>> delete it.
>> i missed something in here.
>>
>> So, change the ath12k_dp_tx_update(struct ath12k *ar, struct
>> hal_tx_status *ts) to
>> static void ath12k_dp_tx_update(struct ath12k *ar, struct hal_tx_status *ts)
>> {
>> if (ts->flags & HAL_TX_STATUS_FLAGS_FIRST_MSDU) {
>> if (ar->last_ppdu_id != 0)
>> ath12k_dp_tx_update_txcompl(ar, ts);
>> ar->last_ppdu_id = ts->ppdu_id;
>> }
>> }
>
> Access to ar->last_ppdu_id still looks racy to me.
>
> And why do we need to track last_ppdu_id? I don't have time to start
> investigating that right now, a code comment explaining that would help
> a lot.
yes, you are right, kalle, thanks for pointing of this.
There really isn't a need to add a judgement of last_ppdu_id to this place.
The ath12k_dp_tx_update_txcompl() function should be called directly and
no need to define ath12k_dp_tx_update() function.
Best regards
Lingbo Kong
next prev parent reply other threads:[~2024-05-07 11:06 UTC|newest]
Thread overview: 16+ 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 [this message]
2024-04-25 16:54 ` Kalle Valo
2024-04-26 6:41 ` Lingbo Kong
2024-04-26 11:21 ` Kalle Valo
2024-04-30 11:41 ` Lingbo Kong
2024-06-05 6:31 ` 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=5912d64a-3f69-44ad-9b06-9e02019141e6@quicinc.com \
--to=quic_lingbok@quicinc.com \
--cc=ath12k@lists.infradead.org \
--cc=kvalo@kernel.org \
--cc=linux-wireless@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).