Received-SPF: pass (google.com: domain of linux-wireless+bounces-6237-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <70f73d7c-4ed1-4678-9eb4-7d4594af1df9@quicinc.com>
Date: Fri, 12 Apr 2024 17:45:54 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/3] wifi: ath12k: report station mode signal strength
To: Jeff Johnson <quic_jjohnson@quicinc.com>, <ath12k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>
References: <20240411025322.33888-1-quic_lingbok@quicinc.com>
 <20240411025322.33888-4-quic_lingbok@quicinc.com>
 <b2a14919-6478-4a6a-bdca-e018393eac34@quicinc.com>
Content-Language: en-US
From: Lingbo Kong <quic_lingbok@quicinc.com>
In-Reply-To: <b2a14919-6478-4a6a-bdca-e018393eac34@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

On 2024/4/12 5:51, Jeff Johnson wrote:
> On 4/10/2024 7:53 PM, Lingbo Kong wrote:
>> Currently, the signal strength of "iw dev xxx station dump" always show an
>> invalid value.
>>
>> This is because signal strength is only set in ath12k_mgmt_rx_event()
>> function, and not set for received data packet. So, change to get signal
>> from firmware and report to mac80211.
>>
>> After that, "iw dev xxx station dump" show the correct signal strength.
>> Such as:
>>
>> Station 00:03:7f:12:03:03 (on wlo1)
>>          inactive time:  36 ms
>>          rx bytes:       61571
>>          rx packets:     336
>>          tx bytes:       28204
>>          tx packets:     205
>>          tx retries:     49
>>          tx failed:      0
>>          beacon loss:    0
>>          beacon rx:      83
>>          rx drop misc:   66
>>          signal:         -24 dBm
>>          beacon signal avg:      -22 dBm
>>
>> For WCN7850, the firmware supports db2dbm, so not need to add noise floor.
>> For QCN9274, the firmware not support db2dbm, so need to add noise floor.
>>
>> This patch affects the station mode of WCN7850 and QCN9274.
>>
>> 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>
>> ---
>>   drivers/net/wireless/ath/ath12k/core.h |   2 +
>>   drivers/net/wireless/ath/ath12k/mac.c  |  56 ++++++++++-
>>   drivers/net/wireless/ath/ath12k/wmi.c  | 130 +++++++++++++++++++++++++
>>   drivers/net/wireless/ath/ath12k/wmi.h  |  48 +++++++++
>>   4 files changed, 234 insertions(+), 2 deletions(-)
> [...]
>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.h b/drivers/net/wireless/ath/ath12k/wmi.h
>> index 78afc94a815d..9016eda03d57 100644
>> --- a/drivers/net/wireless/ath/ath12k/wmi.h
>> +++ b/drivers/net/wireless/ath/ath12k/wmi.h
>> @@ -4811,6 +4811,52 @@ struct wmi_twt_disable_event {
>>   	__le32 status;
>>   } __packed;
>>   
>> +struct wmi_stats_event {
>> +	__le32 stats_id;
>> +	__le32 num_pdev_stats;
>> +	__le32 num_vdev_stats;
>> +	__le32 num_peer_stats;
>> +	__le32 num_bcnflt_stats;
>> +	__le32 num_chan_stats;
>> +	__le32 num_mib_stats;
>> +	__le32 pdev_id;
>> +	__le32 num_bcn_stats;
>> +	__le32 num_peer_extd_stats;
>> +	__le32 num_peer_extd2_stats;
>> +} __packed;
>> +
>> +enum wmi_stats_id {
>> +	WMI_REQUEST_VDEV_STAT	= BIT(3),
>> +};
>> +
>> +struct wmi_request_stats_cmd {
>> +	__le32 tlv_header;
>> +	__le32 stats_id;
>> +	__le32 vdev_id;
>> +	struct ath12k_wmi_mac_addr_params peer_macaddr;
>> +	__le32 pdev_id;
>> +} __packed;
>> +
>> +#define WLAN_MAX_AC 4
>> +#define MAX_TX_RATE_VALUES 10
>> +
>> +struct ath12k_wmi_vdev_stats {
> 
> this name does not conform to the "Naming conventions for structures" outlined
> at the beginning of the file:
>   * _params is a structure which is embedded either into _cmd or _event (or
>   * both), it is not sent individually.
> 

Oh, thanks for pointing out.
I get what you mean, but the ath12k_wmi_vdev_stats belongs to event, so, 
i can changes it to wmi_vdev_stats_event, i will apply it in next version.

Best regards
Lingbo Kong


>> +	__le32 vdev_id;
>> +	__le32 beacon_snr;
>> +	__le32 data_snr;
>> +	__le32 num_tx_frames[WLAN_MAX_AC];
>> +	__le32 num_rx_frames;
>> +	__le32 num_tx_frames_retries[WLAN_MAX_AC];
>> +	__le32 num_tx_frames_failures[WLAN_MAX_AC];
>> +	__le32 num_rts_fail;
>> +	__le32 num_rts_success;
>> +	__le32 num_rx_err;
>> +	__le32 num_rx_discard;
>> +	__le32 num_tx_not_acked;
>> +	__le32 tx_rate_history[MAX_TX_RATE_VALUES];
>> +	__le32 beacon_rssi_history[MAX_TX_RATE_VALUES];
>> +} __packed;
>