Re: [PATCH ath-next v2 4/8] wifi: ath12k: add device DP stats reset support via debugfs

[Pardeep Kaur <pardeep.kaur@oss.qualcomm.com>]

On 07-08-2026 07:44, Jeff Johnson wrote:
> On 8/6/2026 6:24 AM, Pardeep Kaur wrote:
>> From: Hariharan Ramanathan <hariharan.ramanathan@oss.qualcomm.com>
>>
>> There is no way to reset device DP stats counters without reloading
>> the driver, making it difficult to isolate issues to a specific time
>> window during debugging.
>>
>> Add a write handler to the device_dp_stats debugfs file so that
>> writing 'reset' clears all device DP stats counters. Change the file
>> mode from 0400 to 0600 to allow write access. Use strcmp() with
>> newline stripping to match the input, consistent with the existing
>> pattern in debugfs.c. Return -EINVAL on unrecognised input.
>>
>> No lock is taken around the memset since the counters are updated
>> locklessly in the datapath; taking dp_lock would be misleading as it
>> does not protect device_stats updates.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.6.r1-00402-QCAHKSWPL_SILICONZ-1
>>
>> Signed-off-by: Hariharan Ramanathan <hariharan.ramanathan@oss.qualcomm.com>
>> Co-developed-by: Pardeep Kaur <pardeep.kaur@oss.qualcomm.com>
>> Signed-off-by: Pardeep Kaur <pardeep.kaur@oss.qualcomm.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/debugfs.c | 35 ++++++++++++++++++++++-
>>   1 file changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/debugfs.c b/drivers/net/wireless/ath/ath12k/debugfs.c
>> index ec49692107a8..0e446a666f4c 100644
>> --- a/drivers/net/wireless/ath/ath12k/debugfs.c
>> +++ b/drivers/net/wireless/ath/ath12k/debugfs.c
>> @@ -1220,8 +1220,41 @@ static ssize_t ath12k_debugfs_dump_device_dp_stats(struct file *file,
>>   	return ret;
>>   }
>>   
>> +static ssize_t
>> +ath12k_debugfs_write_device_dp_stats(struct file *file,
>> +				     const char __user *user_buf,
>> +				     size_t count, loff_t *ppos)
>> +{
>> +	struct ath12k_base *ab = file->private_data;
>> +	struct ath12k_dp *dp = ath12k_ab_to_dp(ab);
>> +	struct ath12k_device_dp_stats *device_stats = &dp->device_stats;
>> +	char buf[20] = {};
>> +	int ret;
>> +
>> +	if (count >= sizeof(buf))
>> +		return -EINVAL;
>> +
>> +	ret = copy_from_user(buf, user_buf, count);
>> +	if (ret)
>> +		return -EFAULT;
>> +
>> +	/* drop the possible '\n' from the end */
>> +	if (count > 0 && buf[count - 1] == '\n')
>> +		buf[count - 1] = '\0';
>> +
> my review agent says:
> The new ath12k_debugfs_write_device_dp_stats uses raw copy_from_user(buf,
> user_buf, count) and never checks or advances *ppos. A write split across two
> write(2) calls (non-zero ppos on the second) will copy the second chunk into
> buf[0] instead of buf[ppos], silently failing to match "reset". The existing
> ath12k_write_simulate_fw_crash in the same file (line 63) uses
> simple_write_to_buffer(), which is the idiomatic kernel pattern here. Replace
> with:
> ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
> if (ret <= 0)
>      return ret;
will follow the existing pattern and update the patch
>
>> +	if (!strcmp(buf, "reset")) {
>> +		memset(device_stats, 0, sizeof(*device_stats));
>> +		return count;
>> +	}
>> +
>> +	ath12k_warn(ab, "unsupported command: %s\n", buf);
>> +
>> +	return -EINVAL;
>> +}
>> +
>>   static const struct file_operations fops_device_dp_stats = {
>>   	.read = ath12k_debugfs_dump_device_dp_stats,
>> +	.write = ath12k_debugfs_write_device_dp_stats,
>>   	.open = simple_open,
>>   	.owner = THIS_MODULE,
>>   	.llseek = default_llseek,
>> @@ -1232,7 +1265,7 @@ void ath12k_debugfs_pdev_create(struct ath12k_base *ab)
>>   	debugfs_create_file("simulate_fw_crash", 0600, ab->debugfs_soc, ab,
>>   			    &fops_simulate_fw_crash);
>>   
>> -	debugfs_create_file("device_dp_stats", 0400, ab->debugfs_soc, ab,
>> +	debugfs_create_file("device_dp_stats", 0600, ab->debugfs_soc, ab,
>>   			    &fops_device_dp_stats);
>>   }
>>