On Wed, Aug 26, 2020 at 02:07:36PM +0300, Mika Westerberg wrote:
> From: Gil Fine <gil.fine@xxxxxxxxx>
>
> This adds debugfs interface that can be used for debugging possible
> issues in hardware/software. It exposes router and adapter config spaces
> through files like this:
>
> /sys/kernel/debug/thunderbolt/<DEVICE>/regs
> /sys/kernel/debug/thunderbolt/<DEVICE>/<PORT1>/regs
> /sys/kernel/debug/thunderbolt/<DEVICE>/<PORT1>/path
> /sys/kernel/debug/thunderbolt/<DEVICE>/<PORT1>/counters
> /sys/kernel/debug/thunderbolt/<DEVICE>/<PORT2>/regs
> /sys/kernel/debug/thunderbolt/<DEVICE>/<PORT2>/path
> /sys/kernel/debug/thunderbolt/<DEVICE>/<PORT2>/counters
> ...
>
> The "regs" is either the router or port configuration space register
> dump. The "path" is the port path configuration space and "counters" is
> the optional counters configuration space.
>
> These files contains one register per line so it should be easy to use
> normal filtering tools to find the registers of interest if needed.
>
> The router and adapter regs file becomes writable when
> CONFIG_USB4_DEBUGFS_WRITE is enabled (which is not supposed to be done
> in production systems) and in this case the developer can write "offset
> value" lines there to modify the hardware directly. For convenience this
> also supports the long format the read side produces (but ignores the
> additional fields). The counters file can be written even when
> CONFIG_USB4_DEBUGFS_WRITE is not enabled and it is only used to clear
> the counter values.
>
> Signed-off-by: Gil Fine <gil.fine@xxxxxxxxx>
> Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> ---
> drivers/thunderbolt/Kconfig | 10 +
> drivers/thunderbolt/Makefile | 1 +
> drivers/thunderbolt/debugfs.c | 700 ++++++++++++++++++++++++++++++++++
> drivers/thunderbolt/domain.c | 13 +-
> drivers/thunderbolt/switch.c | 3 +
> drivers/thunderbolt/tb.h | 14 +
> drivers/thunderbolt/tb_regs.h | 4 +-
> 7 files changed, 742 insertions(+), 3 deletions(-)
> create mode 100644 drivers/thunderbolt/debugfs.c
>
> diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
> index 354e61c0f2e5..2257c22f8ab3 100644
> --- a/drivers/thunderbolt/Kconfig
> +++ b/drivers/thunderbolt/Kconfig
> @@ -16,6 +16,16 @@ menuconfig USB4
> To compile this driver a module, choose M here. The module will be
> called thunderbolt.
>
> +config USB4_DEBUGFS_WRITE
> + bool "Enable write by debugfs to configuration spaces (DANGEROUS)"
> + depends on USB4
> + help
> + Enables writing to device configuration registers through
> + debugfs interface.
> +
> + Only enable this if you know what you are doing! Never enable
> + this for production systems or distro kernels.
> +
> config USB4_KUNIT_TEST
> bool "KUnit tests"
> depends on KUNIT=y
> diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
> index 754a529aa132..61d5dff445b6 100644
> --- a/drivers/thunderbolt/Makefile
> +++ b/drivers/thunderbolt/Makefile
> @@ -5,5 +5,6 @@ thunderbolt-objs += domain.o dma_port.o icm.o property.o xdomain.o lc.o tmu.o us
> thunderbolt-objs += nvm.o retimer.o quirks.o
>
> thunderbolt-${CONFIG_ACPI} += acpi.o
> +thunderbolt-$(CONFIG_DEBUG_FS) += debugfs.o
>
> obj-${CONFIG_USB4_KUNIT_TEST} += test.o
> diff --git a/drivers/thunderbolt/debugfs.c b/drivers/thunderbolt/debugfs.c
> new file mode 100644
> index 000000000000..fdfe6e4afbfe
> --- /dev/null
> +++ b/drivers/thunderbolt/debugfs.c
> @@ -0,0 +1,700 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Debugfs interface
> + *
> + * Copyright (C) 2020, Intel Corporation
> + * Authors: Gil Fine <gil.fine@xxxxxxxxx>
> + * Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> + */
> +
> +#include <linux/debugfs.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "tb.h"
> +
> +#define PORT_CAP_PCIE_LEN 1
> +#define PORT_CAP_POWER_LEN 2
> +#define PORT_CAP_LANE_LEN 3
> +#define PORT_CAP_USB3_LEN 5
> +#define PORT_CAP_DP_LEN 8
> +#define PORT_CAP_TMU_LEN 8
> +#define PORT_CAP_BASIC_LEN 9
> +#define PORT_CAP_USB4_LEN 20
> +
> +#define SWITCH_CAP_TMU_LEN 26
> +#define SWITCH_CAP_BASIC_LEN 27
> +
> +#define PATH_LEN 2
> +
> +#define COUNTER_SET_LEN 3
> +
> +#define DEBUGFS_ATTR(__space, __write) \
> +static int __space ## _open(struct inode *inode, struct file *file) \
> +{ \
> + return single_open(file, __space ## _show, inode->i_private); \
> +} \
> + \
> +static const struct file_operations __space ## _fops = { \
> + .owner = THIS_MODULE, \
> + .open = __space ## _open, \
> + .release = single_release, \
> + .read = seq_read, \
> + .write = __write, \
> + .llseek = seq_lseek, \
> +}
> +
> +#define DEBUGFS_ATTR_RO(__space) \
> + DEBUGFS_ATTR(__space, NULL)
> +
> +#define DEBUGFS_ATTR_RW(__space) \
> + DEBUGFS_ATTR(__space, __space ## _write)
We do have DEFINE_SIMPLE_ATTRIBUTE() and DEFINE_DEBUGFS_ATTRIBUTE, do
those work here instead of your custom macro?
Other than that, this series looks fine to me:
Reviewed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
