On 12/17/2024 8:56 PM, Kalle Valo wrote:
> Roopni Devanathan <quic_rdevanat@xxxxxxxxxxx> writes:
>
>> From: Rajat Soni <quic_rajson@xxxxxxxxxxx>
>>
>> Add support to request pdev puncture stats from firmware through
>> HTT stats type 46. These stats give the count of number of
>> subbands used in different wifi standards.
>>
>> Sample output:
>> -------------
>> echo 46 > /sys/kernel/debug/ath12k/pci-0000\:06\:00.0/mac0/htt_stats_type
>> cat /sys/kernel/debug/ath12k/pci-0000\:06\:00.0/mac0/htt_stats
>> HTT_PDEV_PUNCTURE_STATS_TLV:
>> mac_id = 0
>> tx_ofdm_su_last_used_pattern_mask = 0x00000001
>> tx_ofdm_su_num_subbands_used_cnt_01 = 217
>> tx_ofdm_su_num_subbands_used_cnt_02 = 0
>> tx_ofdm_su_num_subbands_used_cnt_03 = 0
>> .....
>>
>> HTT_PDEV_PUNCTURE_STATS_TLV:
>> mac_id = 0
>> tx_ax_dl_mu_ofdma_last_used_pattern_mask = 0x00000000
>> tx_ax_dl_mu_ofdma_num_subbands_used_cnt_01 = 0
>> tx_ax_dl_mu_ofdma_num_subbands_used_cnt_02 = 0
>> tx_ax_dl_mu_ofdma_num_subbands_used_cnt_03 = 0
>> .....
>>
>> HTT_PDEV_PUNCTURE_STATS_TLV:
>> mac_id = 0
>> tx_be_dl_mu_ofdma_last_used_pattern_mask = 0x00000000
>> tx_be_dl_mu_ofdma_num_subbands_used_cnt_01 = 0
>> tx_be_dl_mu_ofdma_num_subbands_used_cnt_02 = 0
>> tx_be_dl_mu_ofdma_num_subbands_used_cnt_03 = 0
>> .....
>>
>> HTT_PDEV_PUNCTURE_STATS_TLV:
>> mac_id = 0
>> rx_ax_ul_mu_ofdma_last_used_pattern_mask = 0x00000000
>> rx_ax_ul_mu_ofdma_num_subbands_used_cnt_01 = 0
>> rx_ax_ul_mu_ofdma_num_subbands_used_cnt_02 = 0
>> rx_ax_ul_mu_ofdma_num_subbands_used_cnt_03 = 0
>> .....
>>
>> HTT_PDEV_PUNCTURE_STATS_TLV:
>> mac_id = 0
>> rx_be_ul_mu_ofdma_last_used_pattern_mask = 0x00000000
>> rx_be_ul_mu_ofdma_num_subbands_used_cnt_01 = 0
>> rx_be_ul_mu_ofdma_num_subbands_used_cnt_02 = 0
>> rx_be_ul_mu_ofdma_num_subbands_used_cnt_03 = 0
>> .....
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>>
>> Signed-off-by: Rajat Soni <quic_rajson@xxxxxxxxxxx>
>> Signed-off-by: Roopni Devanathan <quic_rdevanat@xxxxxxxxxxx>
>
> [...]
>
>> +static const char*
>> +ath12k_htt_get_punct_ppdu_type_str(enum ath12k_htt_stats_ppdu_type ppdu_type,
>> + struct debug_htt_stats_req *stats_req)
>> +{
>> + const char *ppdu_type_str = "unknown";
>> + u32 len = stats_req->buf_len;
>> +
>> + switch (ppdu_type) {
>> + case ATH12K_HTT_STATS_PPDU_TYPE_MODE_SU:
>> + ppdu_type_str = "su";
>> + break;
>> + case ATH12K_HTT_STATS_PPDU_TYPE_DL_MU_MIMO:
>> + ppdu_type_str = "dl_mu_mimo";
>> + break;
>> + case ATH12K_HTT_STATS_PPDU_TYPE_UL_MU_MIMO:
>> + ppdu_type_str = "ul_mu_mimo";
>> + break;
>> + case ATH12K_HTT_STATS_PPDU_TYPE_DL_MU_OFDMA:
>> + ppdu_type_str = "dl_mu_ofdma";
>> + break;
>> + case ATH12K_HTT_STATS_PPDU_TYPE_UL_MU_OFDMA:
>> + ppdu_type_str = "ul_mu_ofdma";
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + stats_req->buf_len = len;
>> + return ppdu_type_str;
>> +}
>
> Also here I don't get why the len variable is needed. I would just
> simplify this and the previous function to:
>
> static const char*
> ath12k_htt_get_punct_ppdu_type_str(enum ath12k_htt_stats_ppdu_type ppdu_type,
> struct debug_htt_stats_req *stats_req)
> {
> switch (ppdu_type) {
> case ATH12K_HTT_STATS_PPDU_TYPE_MODE_SU:
> return "su";
> case ATH12K_HTT_STATS_PPDU_TYPE_DL_MU_MIMO:
> return "dl_mu_mimo";
> case ATH12K_HTT_STATS_PPDU_TYPE_UL_MU_MIMO:
> return "ul_mu_mimo";
> case ATH12K_HTT_STATS_PPDU_TYPE_DL_MU_OFDMA:
> return "dl_mu_ofdma";
> case ATH12K_HTT_STATS_PPDU_TYPE_UL_MU_OFDMA:
> return "ul_mu_ofdma";
> default:
> return "unknown";
> }
>
Sure, Kalle. I'll simplify the function definitions and re-spin the next version.
