[PATCH v4 2/2] wifi: ath12k: Support pdev Puncture Stats

[Roopni Devanathan]

On 12/17/2024 8:56 PM, Kalle Valo wrote:
> Roopni Devanathan <quic_rdevanat at quicinc.com> writes:
> 
>> From: Rajat Soni <quic_rajson at quicinc.com>
>>
>> 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 at quicinc.com>
>> Signed-off-by: Roopni Devanathan <quic_rdevanat at quicinc.com>
> 
> [...]
> 
>> +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.