| Message ID | 38c2a7c4f7eaf57b9306bb95a9e6c42b7d987e05.1738169458.git.repk@triplefau.lt (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Jeff Johnson |
| Headers | show |
| Series | [v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics | expand |
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? :) > 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(-) > > 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; Reviewed-by: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
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 ?
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. Anyways, v1 comment was regarding this blank line and one suggestion that in future submissions, please use base commit tag.
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
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.
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(-)