| Message ID | 38c2a7c4f7eaf57b9306bb95a9e6c42b7d987e05.1738169458.git.repk@triplefau.lt |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics | expand |
On 1/31/25 00:27, Jeff Johnson wrote: > On 1/30/2025 1:18 AM, Aditya Kumar Singh wrote: >> On 1/30/25 14:19, Remi Pommarel wrote: >>> On Thu, Jan 30, 2025 at 12:21:54PM +0530, Aditya Kumar Singh wrote: >>>> On 1/29/25 22:25, Remi Pommarel wrote: >>>>> Currently in ath12k_mac_op_sta_statistics() there is the following >>>>> logic: >>>>> >>>>> if (!arsta->txrate.legacy && !arsta->txrate.nss) >>>>> return; >>>>> >>>>> Because ath12k_sta_statistics is used to report many info to iw wlan0 link, >>>>> if it return for empty legacy and nss of arsta->txrate, then the other >>>>> stats after it will not be set. >>>>> >>>>> To address this issue remove the return and instead invert the logic to set >>>>> the txrate logic if (arsta->txrate.legacy || arsta->txrate.nss). >>>>> >>>>> The same was done also in both ath10k with commit 1cd6ba8ae33e ("ath10k: >>>>> remove return for NL80211_STA_INFO_TX_BITRATE") and ath11k as well with >>>>> commit 1d795645e1ee ("ath11k: remove return for empty tx bitrate in >>>>> mac_op_sta_statistics"). >>>>> >>>>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1 >>>> >>>> MISSING_BLANK_LINE >>>> 'Tested-on:' tag missing blank line after it. >>>> >>>> You missed v1 comment? :) >>> >>> Yes sorry I think your mail never reached me, did you CC me ? Do you >>> need a respin ? >>> >> >> No problem. No need of respin just because of this. I think Jeff can fix >> in pending? >> >> It's strange that the v1 reply appears to be delivered from my mail box >> and it is sent to the list. I see even Jeff replying to that. But none >> of those are captured in patchwork. Not sure. > > it was only sent to the ath12k list > perhaps you need to include linux-wireless@vger.kernel.org > >> >> Anyways, v1 comment was regarding this blank line and one suggestion >> that in future submissions, please use base commit tag. >> > > I've fixed the blank line in pending: > https://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git/commit/?h=pending&id=c7b9df20d6a48a279f4b537920049094701da14b Looks good, thanks.
On Wed, 29 Jan 2025 17:55:17 +0100, Remi Pommarel wrote: > Currently in ath12k_mac_op_sta_statistics() there is the following > logic: > > if (!arsta->txrate.legacy && !arsta->txrate.nss) > return; > > Because ath12k_sta_statistics is used to report many info to iw wlan0 link, > if it return for empty legacy and nss of arsta->txrate, then the other > stats after it will not be set. > > [...] Applied, thanks! [1/1] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics commit: 8a5ad319f2e6f0462dbcb1bd3a6ba5097f629a5b Best regards,
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c index 4fb7e235be66..e9663c6ac72c 100644 --- a/drivers/net/wireless/ath/ath12k/mac.c +++ b/drivers/net/wireless/ath/ath12k/mac.c @@ -10170,23 +10170,22 @@ static void ath12k_mac_op_sta_statistics(struct ieee80211_hw *hw, sinfo->tx_duration = arsta->tx_duration; sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_DURATION); - if (!arsta->txrate.legacy && !arsta->txrate.nss) - return; - - if (arsta->txrate.legacy) { - sinfo->txrate.legacy = arsta->txrate.legacy; - } else { - sinfo->txrate.mcs = arsta->txrate.mcs; - sinfo->txrate.nss = arsta->txrate.nss; - sinfo->txrate.bw = arsta->txrate.bw; - sinfo->txrate.he_gi = arsta->txrate.he_gi; - sinfo->txrate.he_dcm = arsta->txrate.he_dcm; - sinfo->txrate.he_ru_alloc = arsta->txrate.he_ru_alloc; - sinfo->txrate.eht_gi = arsta->txrate.eht_gi; - sinfo->txrate.eht_ru_alloc = arsta->txrate.eht_ru_alloc; - } - sinfo->txrate.flags = arsta->txrate.flags; - sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_BITRATE); + if (arsta->txrate.legacy || arsta->txrate.nss) { + if (arsta->txrate.legacy) { + sinfo->txrate.legacy = arsta->txrate.legacy; + } else { + sinfo->txrate.mcs = arsta->txrate.mcs; + sinfo->txrate.nss = arsta->txrate.nss; + sinfo->txrate.bw = arsta->txrate.bw; + sinfo->txrate.he_gi = arsta->txrate.he_gi; + sinfo->txrate.he_dcm = arsta->txrate.he_dcm; + sinfo->txrate.he_ru_alloc = arsta->txrate.he_ru_alloc; + sinfo->txrate.eht_gi = arsta->txrate.eht_gi; + sinfo->txrate.eht_ru_alloc = arsta->txrate.eht_ru_alloc; + } + sinfo->txrate.flags = arsta->txrate.flags; + sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_BITRATE); + } /* TODO: Use real NF instead of default one. */ signal = arsta->rssi_comb;
Currently in ath12k_mac_op_sta_statistics() there is the following logic: if (!arsta->txrate.legacy && !arsta->txrate.nss) return; Because ath12k_sta_statistics is used to report many info to iw wlan0 link, if it return for empty legacy and nss of arsta->txrate, then the other stats after it will not be set. To address this issue remove the return and instead invert the logic to set the txrate logic if (arsta->txrate.legacy || arsta->txrate.nss). The same was done also in both ath10k with commit 1cd6ba8ae33e ("ath10k: remove return for NL80211_STA_INFO_TX_BITRATE") and ath11k as well with commit 1d795645e1ee ("ath11k: remove return for empty tx bitrate in mac_op_sta_statistics"). Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1 Signed-off-by: Remi Pommarel <repk@triplefau.lt> --- Changes in v2: - Rebase on ath-next drivers/net/wireless/ath/ath12k/mac.c | 33 +++++++++++++-------------- 1 file changed, 16 insertions(+), 17 deletions(-)