Received-SPF: pass (google.com: domain of linux-wireless+bounces-8351-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Kalle Valo <kvalo@kernel.org>
To: Ramya Gnanasekar <quic_rgnanase@quicinc.com>
Cc: <ath12k@lists.infradead.org>,  <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH v2 5/5] wifi: ath12k: Dump additional Tx PDEV HTT stats
References: <20240510050806.514126-1-quic_rgnanase@quicinc.com>
	<20240510050806.514126-6-quic_rgnanase@quicinc.com>
	<87le43d26u.fsf@kernel.org>
	<024e2594-309a-4549-9c69-d527717c5dd4@quicinc.com>
	<87r0dm6vrn.fsf@kernel.org>
	<1be3531d-b550-4b19-93bb-d41e93362e19@quicinc.com>
Date: Fri, 31 May 2024 21:00:22 +0300
In-Reply-To: <1be3531d-b550-4b19-93bb-d41e93362e19@quicinc.com> (Ramya
	Gnanasekar's message of "Tue, 28 May 2024 22:18:18 +0530")
Message-ID: <87plt1vp3d.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Ramya Gnanasekar <quic_rgnanase@quicinc.com> writes:

> On 5/28/2024 4:36 PM, Kalle Valo wrote:
>> Ramya Gnanasekar <quic_rgnanase@quicinc.com> writes:
>> 
>>>>> +static inline void
>>>>> +ath12k_htt_print_tx_pdev_mu_ppdu_dist_stats_tlv(const void *tag_buf,
>>>>> +						struct debug_htt_stats_req *stats_req)
>>>>> +{
>>>>> + const struct ath12k_htt_tx_pdev_mu_ppdu_dist_stats_tlv
>>>>> *htt_stats_buf = tag_buf;
>>>>> +	char *mode;
>>>>> +	u8 j, hw_mode, i, str_buf_len;
>>>>> +	u8 *buf = stats_req->buf;
>>>>> +	u32 len = stats_req->buf_len;
>>>>> +	u32 buf_len = ATH12K_HTT_STATS_BUF_SIZE;
>>>>> +	u32 stats_value;
>>>>> +	u8 max_ppdu = ATH12K_HTT_STATS_MAX_NUM_MU_PPDU_PER_BURST;
>>>>> +	u8 max_sched = ATH12K_HTT_STATS_MAX_NUM_SCHED_STATUS;
>>>>> +	char str_buf[ATH12K_HTT_MAX_STRING_LEN];
>>>>> +
>>>>> +	hw_mode = le32_to_cpu(htt_stats_buf->hw_mode);
>>>>> +
>>>>> +	switch (hw_mode) {
>>>>> +	case ATH12K_HTT_STATS_HWMODE_AC:
>>>>> +		len += scnprintf(buf + len, buf_len - len,
>>>>> +				 "HTT_TX_PDEV_MU_PPDU_DISTRIBUTION_STATS:\n");
>>>>> +		mode = "ac";
>>>>> +		break;
>>>>> +	case ATH12K_HTT_STATS_HWMODE_AX:
>>>>> +		mode = "ax";
>>>>> +		break;
>>>>> +	case ATH12K_HTT_STATS_HWMODE_BE:
>>>>> +		mode = "be";
>>>>> +		break;
>>>>> +	default:
>>>>> +		return;
>>>>> +	}
>>>>
>>>> Why are we not adding "HTT_TX_PDEV_MU_PPDU_DISTRIBUTION_STATS:\n" with
>>>> ax and be modes?
>>>>
>>> Sorry for the delayed response. I was on OoO for a week.
>> 
>> No worries.
>> 
>>> We will receive this TLV for each hw modes. Since
>>> "HTT_TX_PDEV_MU_PPDU_DISTRIBUTION_STATS:\n" is header and it would be
>>> suffice to print it once, hence added it inside hw mode ac which will be
>>> the first hw mode integrated inside the TLV.
>> 
>> I would have expected that we print that outside of
>> ath12k_htt_print_tx_pdev_mu_ppdu_dist_stats_tlv(), before the function
>> is called at all.
>> 
>
> Function itself will be called more than once.

Sure, I got that. But I still think it is not really good design to
print it like that.

Maybe the output could be something like below? Or print the mode
separate in the first line?

HTT_TX_PDEV_MU_PPDU_DISTRIBUTION_STATS:

ac_mu_mimo_num_seq_posted_nr4 = 0
ac_mu_mimo_num_ppdu_posted_per_burst_nr4 =  0:0, 1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0, 9:0,
ac_mu_mimo_num_ppdu_completed_per_burst_nr4 =  0:0, 1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0, 9:0,
ac_mu_mimo_num_seq_term_status_nr4 =  0:0, 1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0,

....

HTT_TX_PDEV_MU_PPDU_DISTRIBUTION_STATS:

ax_mu_mimo_num_seq_posted_nr4 = 0
ax_mu_mimo_num_ppdu_posted_per_burst_nr4 =  0:0, 1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0, 9:0,
ax_mu_mimo_num_ppdu_completed_per_burst_nr4 =  0:0, 1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0, 9:0,
ax_mu_mimo_num_seq_term_status_nr4 =  0:0, 1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0,

ax_mu_mimo_num_seq_posted_nr8 = 0
ax_mu_mimo_num_ppdu_posted_per_burst_nr8 =  0:0, 1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0, 9:0,
ax_mu_mimo_num_ppdu_completed_per_burst_nr8 =  0:0, 1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0, 9:0,
ax_mu_mimo_num_seq_term_status_nr8 =  0:0, 1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0,

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

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