diff mbox series

[net-next,v5,5/6] ptp: add debugfs interface to see applied channel masks

Message ID 72ae11ff23793730a64cc1a037f9a6d59dbfbeea.1696804243.git.reibax@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ptp: Support for multiple filtered timestamp event queue readers | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1362 this patch: 1362
netdev/cc_maintainers warning 1 maintainers not CCed: linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 1387 this patch: 1387
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest fail Script ptpchmaskfmt.sh not found in tools/testing/selftests/ptp/Makefile
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1387 this patch: 1387
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Xabier Marquiegui Oct. 8, 2023, 10:49 p.m. UTC
Use debugfs to be able to view channel mask applied to every timestamp
event queue.

Every time the device is opened, a new entry is created in
`$DEBUGFS_MOUNTPOINT/ptpN/$INSTANCE_ADDRESS/mask`.

The mask value can be viewed grouped in 32bit decimal values using cat,
or converted to hexadecimal with the included `ptpchmaskfmt.sh` script.
32 bit values are listed from least significant to most significant.

Signed-off-by: Xabier Marquiegui <reibax@gmail.com>
Suggested-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
v1: https://lore.kernel.org/netdev/27d47d720dfa7a0b5a59b32626ed6d02745b6ee0.1696511486.git.reibax@gmail.com/
  - First version
---
 drivers/ptp/ptp_chardev.c                   | 14 ++++++++++++++
 drivers/ptp/ptp_clock.c                     |  7 +++++++
 drivers/ptp/ptp_private.h                   |  4 ++++
 tools/testing/selftests/ptp/ptpchmaskfmt.sh | 14 ++++++++++++++
 4 files changed, 39 insertions(+)
 create mode 100644 tools/testing/selftests/ptp/ptpchmaskfmt.sh

Comments

Jakub Kicinski Oct. 10, 2023, 12:54 a.m. UTC | #1
On Mon,  9 Oct 2023 00:49:20 +0200 Xabier Marquiegui wrote:
> The mask value can be viewed grouped in 32bit decimal values using cat,
> or converted to hexadecimal with the included `ptpchmaskfmt.sh` script.
> 32 bit values are listed from least significant to most significant.

If it's a self-test it should probably be included in the Makefile 
so that bots run it.
Xabier Marquiegui Oct. 11, 2023, 10:36 p.m. UTC | #2
Jakub Kicinski said:
> If it's a self-test it should probably be included in the Makefile 
> so that bots run it.
> -- 
> pw-bot: cr

Thank you for your input Jakub. It's actually designed as a debug tool for
humans. I wasn't thinking about self-tests, and I can't really think of how
that could be pulled of in this specific case. I hope that's ok. If not we
can try to throw a few ideas around and see if we find a way.

Cheers.
Jakub Kicinski Oct. 12, 2023, 11:37 p.m. UTC | #3
On Thu, 12 Oct 2023 00:36:04 +0200 Xabier Marquiegui wrote:
> Jakub Kicinski said:
> > If it's a self-test it should probably be included in the Makefile 
> > so that bots run it.
> 
> Thank you for your input Jakub. It's actually designed as a debug tool for
> humans. I wasn't thinking about self-tests, and I can't really think of how
> that could be pulled of in this specific case. I hope that's ok. If not we
> can try to throw a few ideas around and see if we find a way.

Let's not throw random non-test scripts into selftests. It adds
confusion to our pitiful kernel testing story :(

The netdevsim driver which is supposed to be used for uAPI selftests
now supports PHCs. Maybe we can extend it and build a proper-er test?

Whether we'd then want to move the debugfs entries onto netdevsim
or leave them where you have then now is another question..
Xabier Marquiegui Oct. 16, 2023, 8:14 a.m. UTC | #4
Jakub Kicinski said:
> The netdevsim driver which is supposed to be used for uAPI selftests
> now supports PHCs. Maybe we can extend it and build a proper-er test?
> 
> Whether we'd then want to move the debugfs entries onto netdevsim
> or leave them where you have then now is another question..

That is an interesting idea. Thank you Jakub. I will start looking onto it
at whatever pace my other duties allow me to give it some thought.

One challenge I anticipate encountering is that even if netdevsim has PHC
support via the PTP mock implementation, we will probably have to think
about how to simulate external timestamp events.
diff mbox series

Patch

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index ac2f2b5ea0b7..282cd7d24077 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -10,6 +10,7 @@ 
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/timekeeping.h>
+#include <linux/debugfs.h>
 
 #include <linux/nospec.h>
 
@@ -106,6 +107,7 @@  int ptp_open(struct posix_clock_context *pccontext, fmode_t fmode)
 	struct ptp_clock *ptp =
 		container_of(pccontext->clk, struct ptp_clock, clock);
 	struct timestamp_event_queue *queue;
+	char debugfsname[32];
 
 	queue = kzalloc(sizeof(*queue), GFP_KERNEL);
 	if (!queue)
@@ -119,6 +121,17 @@  int ptp_open(struct posix_clock_context *pccontext, fmode_t fmode)
 	spin_lock_init(&queue->lock);
 	list_add_tail(&queue->qlist, &ptp->tsevqs);
 	pccontext->private_clkdata = queue;
+
+	/* Debugfs contents */
+	sprintf(debugfsname, "0x%p", queue);
+	queue->debugfs_instance =
+		debugfs_create_dir(debugfsname, ptp->debugfs_root);
+	queue->dfs_bitmap.array = (u32 *)queue->mask;
+	queue->dfs_bitmap.n_elements =
+		DIV_ROUND_UP(PTP_MAX_CHANNELS, BITS_PER_BYTE * sizeof(u32));
+	debugfs_create_u32_array("mask", 0444, queue->debugfs_instance,
+				 &queue->dfs_bitmap);
+
 	return 0;
 }
 
@@ -128,6 +141,7 @@  int ptp_release(struct posix_clock_context *pccontext)
 	unsigned long flags;
 
 	if (queue) {
+		debugfs_remove(queue->debugfs_instance);
 		pccontext->private_clkdata = NULL;
 		spin_lock_irqsave(&queue->lock, flags);
 		list_del(&queue->qlist);
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index ed16d9787ce9..2e801cd33220 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -15,6 +15,7 @@ 
 #include <linux/slab.h>
 #include <linux/syscalls.h>
 #include <linux/uaccess.h>
+#include <linux/debugfs.h>
 #include <uapi/linux/sched/types.h>
 
 #include "ptp_private.h"
@@ -185,6 +186,7 @@  static void ptp_clock_release(struct device *dev)
 	spin_unlock_irqrestore(&tsevq->lock, flags);
 	bitmap_free(tsevq->mask);
 	kfree(tsevq);
+	debugfs_remove(ptp->debugfs_root);
 	ida_free(&ptp_clocks_map, ptp->index);
 	kfree(ptp);
 }
@@ -218,6 +220,7 @@  struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
 	struct ptp_clock *ptp;
 	struct timestamp_event_queue *queue = NULL;
 	int err = 0, index, major = MAJOR(ptp_devt);
+	char debugfsname[8];
 	size_t size;
 
 	if (info->n_alarm > PTP_MAX_ALARMS)
@@ -339,6 +342,10 @@  struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
 		return ERR_PTR(err);
 	}
 
+	/* Debugfs initialization */
+	sprintf(debugfsname, "ptp%d", ptp->index);
+	ptp->debugfs_root = debugfs_create_dir(debugfsname, NULL);
+
 	return ptp;
 
 no_pps:
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index ad4ce1b25c86..52f87e394aa6 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -17,6 +17,7 @@ 
 #include <linux/time.h>
 #include <linux/list.h>
 #include <linux/bitmap.h>
+#include <linux/debugfs.h>
 
 #define PTP_MAX_TIMESTAMPS 128
 #define PTP_BUF_TIMESTAMPS 30
@@ -30,6 +31,8 @@  struct timestamp_event_queue {
 	spinlock_t lock;
 	struct list_head qlist;
 	unsigned long *mask;
+	struct dentry *debugfs_instance;
+	struct debugfs_u32_array dfs_bitmap;
 };
 
 struct ptp_clock {
@@ -57,6 +60,7 @@  struct ptp_clock {
 	struct mutex n_vclocks_mux; /* protect concurrent n_vclocks access */
 	bool is_virtual_clock;
 	bool has_cycles;
+	struct dentry *debugfs_root;
 };
 
 #define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
diff --git a/tools/testing/selftests/ptp/ptpchmaskfmt.sh b/tools/testing/selftests/ptp/ptpchmaskfmt.sh
new file mode 100644
index 000000000000..0a06ba8af300
--- /dev/null
+++ b/tools/testing/selftests/ptp/ptpchmaskfmt.sh
@@ -0,0 +1,14 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Simple helper script to transform ptp debugfs timestamp event queue filtering
+# masks from decimal values to hexadecimal values
+
+# Only takes the debugfs mask file path as an argument
+DEBUGFS_MASKFILE="${1}"
+
+#shellcheck disable=SC2013,SC2086
+for int in $(cat "$DEBUGFS_MASKFILE") ; do
+    printf '0x%08X ' "$int"
+done
+echo