Re: [PATCH ath-next v2 4/8] wifi: ath12k: add device DP stats reset support via debugfs
Pardeep Kaur <[email protected]>
| Newsgroups | org.kernel.vger.linux-wireless,org.infradead.lists.ath12k |
|---|---|
| Message-ID | <[email protected]> |
On 07-08-2026 07:44, Jeff Johnson wrote: > On 8/6/2026 6:24 AM, Pardeep Kaur wrote: >> From: Hariharan Ramanathan <[email protected]> >> >> 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 <[email protected]> >> Co-developed-by: Pardeep Kaur <[email protected]> >> Signed-off-by: Pardeep Kaur <[email protected]> >> --- >> 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); >> } >>