[PATCH v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics

[PATCH v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics

[Remi Pommarel]

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(-)

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;
-- 
2.40.0

Re: [PATCH v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics

[Aditya Kumar Singh]

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>

-- 
Aditya

Re: [PATCH v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics

[Remi Pommarel]

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 ?

-- 
Remi

Re: [PATCH v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics

[Aditya Kumar Singh]

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.

-- 
Aditya

Re: [PATCH v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics

[Jeff Johnson]

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

Re: [PATCH v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics

[Aditya Kumar Singh]

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.

-- 
Aditya

Re: [PATCH v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics

[Kalle Valo]

Jeff Johnson <jeff.johnson@oss.qualcomm.com> writes:

> 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

Yeah, patchwork only follows linux-wireless.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v2] wifi: ath12k: remove return for empty tx bitrate in mac_op_sta_statistics

[Jeff Johnson]

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,
-- 
Jeff Johnson <jeff.johnson@oss.qualcomm.com>