[PATCH 4/6] wifi: ath12k: Add API to print s8 arrays in HTT stats

[Roopni Devanathan]

On 12/19/2024 5:28 PM, Kalle Valo wrote:
> Roopni Devanathan <quic_rdevanat at quicinc.com> writes:
> 
>> Currently, print_array_to_buf() is used to print arrays to HTT
>> stats buffer. This API supports printing only arrays with 32-bit
>> unsigned integers. Add API print_array_to_buf_s8(), which
>> implements print_array_to_buf()'s functionality to all the
>> arrays with 8-bit signed integers.
>>
>> 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: Roopni Devanathan <quic_rdevanat at quicinc.com>
> 
> s/API/function/
> 
> And maybe not even use the term 'function' that, saying foo() should be
> clear anyone that it's a function.
> 
> An example output would be nice.

Sure, Kalle. I'll modify the commit message in next version.

> 
>> ---
>>  .../wireless/ath/ath12k/debugfs_htt_stats.c   | 28 +++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c
>> index 4569ff40e9d8..621676daf971 100644
>> --- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c
>> +++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c
>> @@ -48,6 +48,34 @@ print_array_to_buf(u8 *buf, u32 offset, const char *header,
>>  					footer);
>>  }
>>  
>> +static u32
>> +print_array_to_buf_s8(u8 *buf, u32 offset, const char *header, u32 stats_index,
>> +		      const s8 *array, u32 array_len, const char *footer)
>> +{
> 
> I would rather have a separate buf_len variable instead of silently
> assuming it's ATH12K_HTT_STATS_BUF_SIZE, much safer that way.
> 

Sure. I'll change this accordingly.

>> +	int index = 0;
>> +	u8 i;
>> +
>> +	if (header) {
>> +		index += scnprintf(buf + offset,
>> +				   ATH12K_HTT_STATS_BUF_SIZE - offset,
>> +				   "%s = ", header);
>> +	}
> 
> Empty line here.
> 
>> +	for (i = 0; i < array_len; i++) {
>> +		index += scnprintf(buf + offset + index,
>> +				   (ATH12K_HTT_STATS_BUF_SIZE - offset) - index,
>> +				   " %u:%d,", stats_index++, array[i]);
>> +	}
> 
> Empty line.
> 
>> +	index--;
>> +	*(buf + offset + index) = '\0';
> 
> Avoid pointer arithmetic:
> 
> buf[offset + index] = '\0'
> 
> But first check that 'offset + index < buf_len'.

Sure, I'll make changes in next version.

> 
>> +	if (footer) {
>> +		index += scnprintf(buf + offset + index,
>> +				   (ATH12K_HTT_STATS_BUF_SIZE - offset) - index,
>> +				   "%s", footer);
>> +	}
> 
> Empty line.
> 

I'll add empty line where it is necessary, in the next version.

>> +	return index;
>> +}
>> +
>>  static const char *ath12k_htt_ax_tx_rx_ru_size_to_str(u8 ru_size)
>>  {
>>  	switch (ru_size) {
>