[PATCH v5 0/3] dmaengine: fsl_edma: add trace and debugfs support

[PATCH v5 0/3] dmaengine: fsl_edma: add trace and debugfs support

[Frank Li]

Change from v3 to v5
- There are still some discussion about 64bit register access.
  Drop 64 register support and use sperate patch to enable 64bit register
support in future.

Change from v3 to v4
- Fix build warning

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309210500.owiirl4c-lkp@intel.com/

Change from v2 to v3
- Fixed sparse build warning
- improve debugfs_create_regset32 and use debugfs_create_regset() to dump
  all registers

Change from v1 to v2
- Fixed tcd trace issue, data need be saved firstly.

=== Trace ===

echo 1 >/sys/kernel/debug/tracing/tracing_on
echo 1 >/sys/kernel/debug/tracing/events/fsl_edma/enable

Run any dma test
...

cat /sys/kernel/debug/tracing/trace

 uart_testapp_11-448     [000] d..1.    69.185019: edma_fill_tcd:
==== TCD =====
  saddr:  0x831ee020
  soff:       0x8000
  attr:       0xffff
  nbytes: 0xfba40000
  slast:  0x00000000
  daddr:  0x8aaa4800
  doff:       0x0001
  citer:      0x0800
  dlast:  0xfba40020
  csr:        0x0052
  biter:      0x0800

 uart_testapp_11-448     [000] d..2.    69.185022: edma_writew: offset 0001803c: value 00000000
 uart_testapp_11-448     [000] d..2.    69.185023: edma_writel: offset 00018020: value 4259001c
 uart_testapp_11-448     [000] d..2.    69.185024: edma_writel: offset 00018030: value 8aaa4000

=== DebugFS ===

cat /sys/kernel/debug/dmaengine/42000000.dma-controller/42000000.dma-controller-CH00/ch_sbr
0x00208003

Frank Li (3):
  debugfs_create_regset32() support 8/16 bit width registers
  dmaengine: fsl-emda: add debugfs support
  dmaengine: fsl-edma: add trace event support

 drivers/dma/Makefile           |   7 +-
 drivers/dma/fsl-edma-common.c  |   2 +
 drivers/dma/fsl-edma-common.h  |  37 +++++-
 drivers/dma/fsl-edma-debugfs.c | 200 +++++++++++++++++++++++++++++++++
 drivers/dma/fsl-edma-main.c    |   2 +
 drivers/dma/fsl-edma-trace.c   |   4 +
 drivers/dma/fsl-edma-trace.h   | 134 ++++++++++++++++++++++
 fs/debugfs/file.c              |  53 ++++++---
 include/linux/debugfs.h        |  17 ++-
 9 files changed, 428 insertions(+), 28 deletions(-)
 create mode 100644 drivers/dma/fsl-edma-debugfs.c
 create mode 100644 drivers/dma/fsl-edma-trace.c
 create mode 100644 drivers/dma/fsl-edma-trace.h

-- 
2.34.1

[PATCH v5 1/3] debugfs_create_regset32() support 8/16 bit width registers

[Frank Li]

Enhance the flexibility of `debugfs_create_regset32()` to support registers
of various bit widths. The key changes are as follows:

1. Renamed '*reg32' and '*regset32' to '*reg' and '*regset' in relevant
   code to reflect that the register width is not limited to 32 bits.

2. Added 'size' and 'bigendian' fields to the `struct debugfs_reg` to allow
   for specifying the size and endianness of registers. These additions
   enable `debugfs_create_regset()` to support a wider range of register
   types.

3. When 'size' is set to 0, it signifies a 32-bit register. This change
   maintains compatibility with existing code that assumes 32-bit
   registers.

Improve the versatility of `debugfs_create_regset()` and enable it to
handle registers of different sizes and endianness, offering greater
flexibility for debugging and monitoring.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 fs/debugfs/file.c       | 53 +++++++++++++++++++++++++++--------------
 include/linux/debugfs.h | 17 +++++++++----
 2 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 87b3753aa4b1..62cc96bb6d72 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -1137,15 +1137,15 @@ EXPORT_SYMBOL_GPL(debugfs_create_u32_array);
 #ifdef CONFIG_HAS_IOMEM
 
 /*
- * The regset32 stuff is used to print 32-bit registers using the
+ * The regset stuff is used to print 32-bit registers using the
  * seq_file utilities. We offer printing a register set in an already-opened
- * sequential file or create a debugfs file that only prints a regset32.
+ * sequential file or create a debugfs file that only prints a regset.
  */
 
 /**
- * debugfs_print_regs32 - use seq_print to describe a set of registers
+ * debugfs_print_regs - use seq_print to describe a set of registers
  * @s: the seq_file structure being used to generate output
- * @regs: an array if struct debugfs_reg32 structures
+ * @regs: an array if struct debugfs_reg structures
  * @nregs: the length of the above array
  * @base: the base address to be used in reading the registers
  * @prefix: a string to be prefixed to every output line
@@ -1157,30 +1157,47 @@ EXPORT_SYMBOL_GPL(debugfs_create_u32_array);
  * because some peripherals have several blocks of identical registers,
  * for example configuration of dma channels
  */
-void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
+void debugfs_print_regs(struct seq_file *s, const struct debugfs_reg *regs,
 			  int nregs, void __iomem *base, char *prefix)
 {
+	void __iomem *reg;
+	bool b;
 	int i;
 
 	for (i = 0; i < nregs; i++, regs++) {
 		if (prefix)
 			seq_printf(s, "%s", prefix);
-		seq_printf(s, "%s = 0x%08x\n", regs->name,
-			   readl(base + regs->offset));
+
+		b = regs->bigendian;
+		reg = base + regs->offset;
+
+		switch (regs->size) {
+		case sizeof(u8):
+			seq_printf(s, "%s = 0x%02x\n", regs->name, ioread8(reg));
+			break;
+		case sizeof(u16):
+			seq_printf(s, "%s = 0x%04x\n", regs->name,
+				  b ? ioread16be(reg) : ioread16(reg));
+			break;
+		default:
+			seq_printf(s, "%s = 0x%08x\n", regs->name,
+				   b ? ioread32be(reg) : ioread32(reg));
+		}
+
 		if (seq_has_overflowed(s))
 			break;
 	}
 }
-EXPORT_SYMBOL_GPL(debugfs_print_regs32);
+EXPORT_SYMBOL_GPL(debugfs_print_regs);
 
-static int debugfs_regset32_show(struct seq_file *s, void *data)
+static int debugfs_regset_show(struct seq_file *s, void *data)
 {
-	struct debugfs_regset32 *regset = s->private;
+	struct debugfs_regset *regset = s->private;
 
 	if (regset->dev)
 		pm_runtime_get_sync(regset->dev);
 
-	debugfs_print_regs32(s, regset->regs, regset->nregs, regset->base, "");
+	debugfs_print_regs(s, regset->regs, regset->nregs, regset->base, "");
 
 	if (regset->dev)
 		pm_runtime_put(regset->dev);
@@ -1188,16 +1205,16 @@ static int debugfs_regset32_show(struct seq_file *s, void *data)
 	return 0;
 }
 
-DEFINE_SHOW_ATTRIBUTE(debugfs_regset32);
+DEFINE_SHOW_ATTRIBUTE(debugfs_regset);
 
 /**
- * debugfs_create_regset32 - create a debugfs file that returns register values
+ * debugfs_create_regset - create a debugfs file that returns register values
  * @name: a pointer to a string containing the name of the file to create.
  * @mode: the permission that the file should have
  * @parent: a pointer to the parent dentry for this file.  This should be a
  *          directory dentry if set.  If this parameter is %NULL, then the
  *          file will be created in the root of the debugfs filesystem.
- * @regset: a pointer to a struct debugfs_regset32, which contains a pointer
+ * @regset: a pointer to a struct debugfs_regset, which contains a pointer
  *          to an array of register definitions, the array size and the base
  *          address where the register bank is to be found.
  *
@@ -1205,13 +1222,13 @@ DEFINE_SHOW_ATTRIBUTE(debugfs_regset32);
  * the names and values of a set of 32-bit registers. If the @mode variable
  * is so set it can be read from. Writing is not supported.
  */
-void debugfs_create_regset32(const char *name, umode_t mode,
+void debugfs_create_regset(const char *name, umode_t mode,
 			     struct dentry *parent,
-			     struct debugfs_regset32 *regset)
+			     struct debugfs_regset *regset)
 {
-	debugfs_create_file(name, mode, parent, regset, &debugfs_regset32_fops);
+	debugfs_create_file(name, mode, parent, regset, &debugfs_regset_fops);
 }
-EXPORT_SYMBOL_GPL(debugfs_create_regset32);
+EXPORT_SYMBOL_GPL(debugfs_create_regset);
 
 #endif /* CONFIG_HAS_IOMEM */
 
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index ea2d919fd9c7..247ae4217ea5 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -26,18 +26,24 @@ struct debugfs_blob_wrapper {
 	unsigned long size;
 };
 
-struct debugfs_reg32 {
+struct debugfs_reg {
 	char *name;
+	int size;
+	int bigendian;
 	unsigned long offset;
 };
 
-struct debugfs_regset32 {
+#define debugfs_reg32 debugfs_reg
+
+struct debugfs_regset {
 	const struct debugfs_reg32 *regs;
 	int nregs;
 	void __iomem *base;
 	struct device *dev;	/* Optional device for Runtime PM */
 };
 
+#define debugfs_regset32 debugfs_regset
+
 struct debugfs_u32_array {
 	u32 *array;
 	u32 n_elements;
@@ -145,12 +151,15 @@ struct dentry *debugfs_create_blob(const char *name, umode_t mode,
 				  struct dentry *parent,
 				  struct debugfs_blob_wrapper *blob);
 
-void debugfs_create_regset32(const char *name, umode_t mode,
+void debugfs_create_regset(const char *name, umode_t mode,
 			     struct dentry *parent,
 			     struct debugfs_regset32 *regset);
 
-void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
+#define debugfs_create_regset32 debugfs_create_regset
+
+void debugfs_print_regs(struct seq_file *s, const struct debugfs_reg32 *regs,
 			  int nregs, void __iomem *base, char *prefix);
+#define debugfs_print_regs32 debugfs_print_regs
 
 void debugfs_create_u32_array(const char *name, umode_t mode,
 			      struct dentry *parent,
-- 
2.34.1

[PATCH v5 2/3] dmaengine: fsl-emda: add debugfs support

[Frank Li]

Add debugfs support to fsl-edma to enable dumping of register states.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/dma/Makefile           |   5 +-
 drivers/dma/fsl-edma-common.h  |   8 ++
 drivers/dma/fsl-edma-debugfs.c | 200 +++++++++++++++++++++++++++++++++
 drivers/dma/fsl-edma-main.c    |   2 +
 4 files changed, 213 insertions(+), 2 deletions(-)
 create mode 100644 drivers/dma/fsl-edma-debugfs.c

diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 83553a97a010..a51c6397bcad 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -31,10 +31,11 @@ obj-$(CONFIG_DW_AXI_DMAC) += dw-axi-dmac/
 obj-$(CONFIG_DW_DMAC_CORE) += dw/
 obj-$(CONFIG_DW_EDMA) += dw-edma/
 obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
+fsl-edma-debugfs-$(CONFIG_DEBUG_FS) := fsl-edma-debugfs.o
 obj-$(CONFIG_FSL_DMA) += fsldma.o
-fsl-edma-objs := fsl-edma-main.o fsl-edma-common.o
+fsl-edma-objs := fsl-edma-main.o fsl-edma-common.o $(fsl-edma-debugfs-y)
 obj-$(CONFIG_FSL_EDMA) += fsl-edma.o
-mcf-edma-objs := mcf-edma-main.o fsl-edma-common.o
+mcf-edma-objs := mcf-edma-main.o fsl-edma-common.o $(fsl-edma-debugfs-y)
 obj-$(CONFIG_MCF_EDMA) += mcf-edma.o
 obj-$(CONFIG_FSL_QDMA) += fsl-qdma.o
 obj-$(CONFIG_FSL_RAID) += fsl_raid.o
diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 3cc0cc8fc2d0..029197440bc3 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -336,4 +336,12 @@ void fsl_edma_free_chan_resources(struct dma_chan *chan);
 void fsl_edma_cleanup_vchan(struct dma_device *dmadev);
 void fsl_edma_setup_regs(struct fsl_edma_engine *edma);
 
+#ifdef CONFIG_DEBUG_FS
+void fsl_edma_debugfs_on(struct fsl_edma_engine *edma);
+#else
+static inline void fsl_edma_debugfs_on(struct fsl_edma_engine *edma)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
+
 #endif /* _FSL_EDMA_COMMON_H_ */
diff --git a/drivers/dma/fsl-edma-debugfs.c b/drivers/dma/fsl-edma-debugfs.c
new file mode 100644
index 000000000000..99f34198dae0
--- /dev/null
+++ b/drivers/dma/fsl-edma-debugfs.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/debugfs.h>
+#include <linux/bitfield.h>
+
+#include "fsl-edma-common.h"
+
+#define fsl_edma_debugfs_reg(reg, b, _s, __name)	\
+do {	reg->name = __stringify(__name);		\
+	reg->offset = offsetof(struct _s, __name);	\
+	reg->size = sizeof(((struct _s *)0)->__name);	\
+	reg->bigendian = b;				\
+	reg++;						\
+} while (0)
+
+#define fsl_edma_debugfs_regv1(reg, edma, __name)	\
+do {	reg->name = __stringify(__name);		\
+	reg->offset = edma->regs.__name - edma->membase;\
+	reg->bigendian = edma->big_endian;		\
+	reg++;						\
+} while (0)
+
+static void fsl_edma_debufs_tcdreg(struct fsl_edma_chan *chan, struct dentry *dir)
+{
+	struct debugfs_regset *regset;
+	struct debugfs_reg *reg;
+	struct device *dev;
+	int be;
+
+	be = chan->edma->big_endian;
+
+	dev = &chan->pdev->dev;
+
+	regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
+	if (!regset)
+		return;
+
+	regset->dev = dev;
+	regset->base = chan->tcd;
+
+	/* sizeof(struct fsl_edma_hw_tcd)/sizeof(u16) is enough for hold all registers */
+	reg = devm_kcalloc(dev, sizeof(struct fsl_edma_hw_tcd)/sizeof(u16),
+			   sizeof(*reg), GFP_KERNEL);
+
+	if (!reg)
+		return;
+
+	regset->regs = reg;
+
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, saddr);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, soff);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, attr);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, nbytes);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, slast);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, daddr);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, doff);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, citer);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, dlast_sga);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, csr);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma_hw_tcd, biter);
+
+	regset->nregs = reg - regset->regs;
+
+	debugfs_create_regset("tcd", 0444, dir, regset);
+}
+
+static void fsl_edma3_debufs_chan(struct fsl_edma_chan *chan, struct dentry *entry)
+{
+	struct debugfs_regset *regset;
+	struct debugfs_reg *reg;
+	struct device *dev;
+	int be;
+
+	be = chan->edma->big_endian;
+
+	dev = &chan->pdev->dev;
+
+	regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
+	if (!regset)
+		return;
+
+	regset->dev = dev;
+
+	reg = devm_kcalloc(dev, sizeof(struct fsl_edma3_ch_reg)/sizeof(u32),
+			   sizeof(*reg), GFP_KERNEL);
+
+	if (!reg)
+		return;
+
+	regset->base = chan->tcd;
+	regset->base -= offsetof(struct fsl_edma3_ch_reg, tcd);
+
+	regset->regs = reg;
+
+	fsl_edma_debugfs_reg(reg, be, fsl_edma3_ch_reg, ch_csr);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma3_ch_reg, ch_es);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma3_ch_reg, ch_int);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma3_ch_reg, ch_sbr);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma3_ch_reg, ch_pri);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma3_ch_reg, ch_mux);
+	fsl_edma_debugfs_reg(reg, be, fsl_edma3_ch_reg, ch_mattr);
+
+	regset->nregs = reg - regset->regs;
+	debugfs_create_regset("regs", 0444, entry, regset);
+
+	fsl_edma_debufs_tcdreg(chan, entry);
+}
+
+static void fsl_edma3_debugfs_init(struct fsl_edma_engine *edma)
+{
+	struct fsl_edma_chan *chan;
+	struct dentry *dir;
+	int i;
+
+	for (i = 0; i < edma->n_chans; i++) {
+		if (edma->chan_masked & BIT(i))
+			continue;
+
+		chan = &edma->chans[i];
+		dir = debugfs_create_dir(chan->chan_name, edma->dma_dev.dbg_dev_root);
+
+		fsl_edma3_debufs_chan(chan, dir);
+	}
+
+}
+
+static void fsl_edma_debugfs_init(struct fsl_edma_engine *edma)
+{
+	struct debugfs_regset *regset;
+	struct fsl_edma_chan *chan;
+	struct debugfs_reg *reg;
+	struct dentry *dir;
+	struct device *dev;
+	int i;
+
+	dev = edma->dma_dev.dev;
+
+	regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
+	if (!regset)
+		return;
+
+	regset->dev = dev;
+
+	reg = devm_kcalloc(dev, sizeof(struct edma_regs)/sizeof(void *), sizeof(*reg), GFP_KERNEL);
+
+	if (!reg)
+		return;
+
+	regset->regs = reg;
+	regset->base = edma->membase;
+
+	fsl_edma_debugfs_regv1(reg, edma, cr);
+	fsl_edma_debugfs_regv1(reg, edma, es);
+	fsl_edma_debugfs_regv1(reg, edma, erqh);
+	fsl_edma_debugfs_regv1(reg, edma, erql);
+	fsl_edma_debugfs_regv1(reg, edma, eeih);
+	fsl_edma_debugfs_regv1(reg, edma, eeil);
+	fsl_edma_debugfs_regv1(reg, edma, seei);
+	fsl_edma_debugfs_regv1(reg, edma, ceei);
+	fsl_edma_debugfs_regv1(reg, edma, serq);
+	fsl_edma_debugfs_regv1(reg, edma, cerq);
+	fsl_edma_debugfs_regv1(reg, edma, cint);
+	fsl_edma_debugfs_regv1(reg, edma, cerr);
+	fsl_edma_debugfs_regv1(reg, edma, ssrt);
+	fsl_edma_debugfs_regv1(reg, edma, cdne);
+	fsl_edma_debugfs_regv1(reg, edma, inth);
+	fsl_edma_debugfs_regv1(reg, edma, errh);
+	fsl_edma_debugfs_regv1(reg, edma, errl);
+
+	regset->nregs = reg - regset->regs;
+
+	debugfs_create_regset("regs", 0444, edma->dma_dev.dbg_dev_root, regset);
+
+	for (i = 0; i < edma->n_chans; i++) {
+		if (edma->chan_masked & BIT(i))
+			continue;
+
+		chan = &edma->chans[i];
+		dir = debugfs_create_dir(chan->chan_name, edma->dma_dev.dbg_dev_root);
+
+		fsl_edma_debufs_tcdreg(chan, dir);
+	}
+}
+
+void fsl_edma_debugfs_on(struct fsl_edma_engine *edma)
+{
+	if (!debugfs_initialized())
+		return;
+
+	debugfs_create_bool("big_endian", 0444, edma->dma_dev.dbg_dev_root, &edma->big_endian);
+	debugfs_create_x64("chan_mask", 0444, edma->dma_dev.dbg_dev_root, &edma->chan_masked);
+	debugfs_create_x32("n_chans", 0444, edma->dma_dev.dbg_dev_root, &edma->n_chans);
+
+	if (edma->drvdata->flags & FSL_EDMA_DRV_SPLIT_REG)
+		fsl_edma3_debugfs_init(edma);
+	else
+		fsl_edma_debugfs_init(edma);
+}
+
+
diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
index 63d48d046f04..029a72872821 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -612,6 +612,8 @@ static int fsl_edma_probe(struct platform_device *pdev)
 	if (!(drvdata->flags & FSL_EDMA_DRV_SPLIT_REG))
 		edma_writel(fsl_edma, EDMA_CR_ERGA | EDMA_CR_ERCA, regs->cr);
 
+	fsl_edma_debugfs_on(fsl_edma);
+
 	return 0;
 }
 
-- 
2.34.1

[PATCH v5 3/3] dmaengine: fsl-edma: add trace event support

[Frank Li]

Implement trace event support to enhance logging functionality for
register access and the transfer control descriptor (TCD) context.
This will enable more comprehensive monitoring and analysis of system
activities

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/dma/Makefile          |   6 +-
 drivers/dma/fsl-edma-common.c |   2 +
 drivers/dma/fsl-edma-common.h |  29 +++++++-
 drivers/dma/fsl-edma-trace.c  |   4 +
 drivers/dma/fsl-edma-trace.h  | 134 ++++++++++++++++++++++++++++++++++
 5 files changed, 169 insertions(+), 6 deletions(-)
 create mode 100644 drivers/dma/fsl-edma-trace.c
 create mode 100644 drivers/dma/fsl-edma-trace.h

diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index a51c6397bcad..40b2dd554e5d 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -32,10 +32,12 @@ obj-$(CONFIG_DW_DMAC_CORE) += dw/
 obj-$(CONFIG_DW_EDMA) += dw-edma/
 obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
 fsl-edma-debugfs-$(CONFIG_DEBUG_FS) := fsl-edma-debugfs.o
+CFLAGS_fsl-edma-trace.o := -I$(src)
+fsl-edma-trace-$(CONFIG_TRACING) := fsl-edma-trace.o
 obj-$(CONFIG_FSL_DMA) += fsldma.o
-fsl-edma-objs := fsl-edma-main.o fsl-edma-common.o $(fsl-edma-debugfs-y)
+fsl-edma-objs := fsl-edma-main.o fsl-edma-common.o $(fsl-edma-debugfs-y) ${fsl-edma-trace-y}
 obj-$(CONFIG_FSL_EDMA) += fsl-edma.o
-mcf-edma-objs := mcf-edma-main.o fsl-edma-common.o $(fsl-edma-debugfs-y)
+mcf-edma-objs := mcf-edma-main.o fsl-edma-common.o $(fsl-edma-debugfs-y) ${fsl-edma-trace-y}
 obj-$(CONFIG_MCF_EDMA) += mcf-edma.o
 obj-$(CONFIG_FSL_QDMA) += fsl-qdma.o
 obj-$(CONFIG_FSL_RAID) += fsl_raid.o
diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index a0f5741abcc4..0182e2695fdc 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -521,6 +521,8 @@ void fsl_edma_fill_tcd(struct fsl_edma_chan *fsl_chan,
 		csr |= EDMA_TCD_CSR_START;
 
 	tcd->csr = cpu_to_le16(csr);
+
+	trace_edma_fill_tcd(fsl_chan->edma, tcd);
 }
 
 static struct fsl_edma_desc *fsl_edma_alloc_desc(struct fsl_edma_chan *fsl_chan,
diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 029197440bc3..453c997d0119 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -234,6 +234,9 @@ struct fsl_edma_engine {
 	edma_writel(chan->edma, val,				\
 		   (void __iomem *)&(container_of(chan->tcd, struct fsl_edma3_ch_reg, tcd)->__name))
 
+/* Need after struct defination */
+#include "fsl-edma-trace.h"
+
 /*
  * R/W functions for big- or little-endian registers:
  * The eDMA controller's endian is independent of the CPU core's endian.
@@ -242,18 +245,30 @@ struct fsl_edma_engine {
  */
 static inline u32 edma_readl(struct fsl_edma_engine *edma, void __iomem *addr)
 {
+	u32 val;
+
 	if (edma->big_endian)
-		return ioread32be(addr);
+		val = ioread32be(addr);
 	else
-		return ioread32(addr);
+		val = ioread32(addr);
+
+	trace_edma_readl(edma, addr, val);
+
+	return val;
 }
 
 static inline u16 edma_readw(struct fsl_edma_engine *edma, void __iomem *addr)
 {
+	u16 val;
+
 	if (edma->big_endian)
-		return ioread16be(addr);
+		val = ioread16be(addr);
 	else
-		return ioread16(addr);
+		val = ioread16(addr);
+
+	trace_edma_readw(edma, addr, val);
+
+	return val;
 }
 
 static inline void edma_writeb(struct fsl_edma_engine *edma,
@@ -264,6 +279,8 @@ static inline void edma_writeb(struct fsl_edma_engine *edma,
 		iowrite8(val, (void __iomem *)((unsigned long)addr ^ 0x3));
 	else
 		iowrite8(val, addr);
+
+	trace_edma_writeb(edma, addr, val);
 }
 
 static inline void edma_writew(struct fsl_edma_engine *edma,
@@ -274,6 +291,8 @@ static inline void edma_writew(struct fsl_edma_engine *edma,
 		iowrite16be(val, (void __iomem *)((unsigned long)addr ^ 0x2));
 	else
 		iowrite16(val, addr);
+
+	trace_edma_writew(edma, addr, val);
 }
 
 static inline void edma_writel(struct fsl_edma_engine *edma,
@@ -283,6 +302,8 @@ static inline void edma_writel(struct fsl_edma_engine *edma,
 		iowrite32be(val, addr);
 	else
 		iowrite32(val, addr);
+
+	trace_edma_writel(edma, addr, val);
 }
 
 static inline struct fsl_edma_chan *to_fsl_edma_chan(struct dma_chan *chan)
diff --git a/drivers/dma/fsl-edma-trace.c b/drivers/dma/fsl-edma-trace.c
new file mode 100644
index 000000000000..28300ad80bb7
--- /dev/null
+++ b/drivers/dma/fsl-edma-trace.c
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define CREATE_TRACE_POINTS
+#include "fsl-edma-common.h"
diff --git a/drivers/dma/fsl-edma-trace.h b/drivers/dma/fsl-edma-trace.h
new file mode 100644
index 000000000000..9dd08a42ad54
--- /dev/null
+++ b/drivers/dma/fsl-edma-trace.h
@@ -0,0 +1,134 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2023 NXP.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM fsl_edma
+
+#if !defined(__LINUX_FSL_EDMA_TRACE) || defined(TRACE_HEADER_MULTI_READ)
+#define __LINUX_FSL_EDMA_TRACE
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(edma_log_io,
+	TP_PROTO(struct fsl_edma_engine *edma, void __iomem *addr, u32 value),
+	TP_ARGS(edma, addr, value),
+	TP_STRUCT__entry(
+		__field(struct fsl_edma_engine *, edma)
+		__field(void __iomem *, addr)
+		__field(u32, value)
+	),
+	TP_fast_assign(
+		__entry->edma = edma;
+		__entry->addr = addr;
+		__entry->value = value;
+	),
+	TP_printk("offset %08x: value %08x",
+		(u32)(__entry->addr - __entry->edma->membase), __entry->value)
+);
+
+DEFINE_EVENT(edma_log_io, edma_readl,
+	TP_PROTO(struct fsl_edma_engine *edma, void __iomem *addr, u32 value),
+	TP_ARGS(edma, addr, value)
+);
+
+DEFINE_EVENT(edma_log_io, edma_writel,
+	TP_PROTO(struct fsl_edma_engine *edma, void __iomem *addr,  u32 value),
+	TP_ARGS(edma, addr, value)
+);
+
+DEFINE_EVENT(edma_log_io, edma_readw,
+	TP_PROTO(struct fsl_edma_engine *edma, void __iomem *addr, u32 value),
+	TP_ARGS(edma, addr, value)
+);
+
+DEFINE_EVENT(edma_log_io, edma_writew,
+	TP_PROTO(struct fsl_edma_engine *edma, void __iomem *addr,  u32 value),
+	TP_ARGS(edma, addr, value)
+);
+
+DEFINE_EVENT(edma_log_io, edma_readb,
+	TP_PROTO(struct fsl_edma_engine *edma, void __iomem *addr, u32 value),
+	TP_ARGS(edma, addr, value)
+);
+
+DEFINE_EVENT(edma_log_io, edma_writeb,
+	TP_PROTO(struct fsl_edma_engine *edma, void __iomem *addr,  u32 value),
+	TP_ARGS(edma, addr, value)
+);
+
+DECLARE_EVENT_CLASS(edma_log_tcd,
+	TP_PROTO(struct fsl_edma_engine *edma, struct fsl_edma_hw_tcd *tcd),
+	TP_ARGS(edma, tcd),
+	TP_STRUCT__entry(
+		__field(struct fsl_edma_engine *, edma)
+		__field(u32, saddr)
+		__field(u16, soff)
+		__field(u16, attr)
+		__field(u32, nbytes)
+		__field(u32, slast)
+		__field(u32, daddr)
+		__field(u16, doff)
+		__field(u16, citer)
+		__field(u32, dlast_sga)
+		__field(u16, csr)
+		__field(u16, biter)
+
+	),
+	TP_fast_assign(
+		__entry->edma = edma;
+		__entry->saddr = le32_to_cpu(tcd->saddr),
+		__entry->soff = le16_to_cpu(tcd->soff),
+		__entry->attr = le16_to_cpu(tcd->attr),
+		__entry->nbytes = le32_to_cpu(tcd->nbytes),
+		__entry->slast = le32_to_cpu(tcd->slast),
+		__entry->daddr = le32_to_cpu(tcd->daddr),
+		__entry->doff = le16_to_cpu(tcd->doff),
+		__entry->citer = le16_to_cpu(tcd->citer),
+		__entry->dlast_sga = le32_to_cpu(tcd->dlast_sga),
+		__entry->csr = le16_to_cpu(tcd->csr),
+		__entry->biter = le16_to_cpu(tcd->biter);
+	),
+	TP_printk("\n==== TCD =====\n"
+		  "  saddr:  0x%08x\n"
+		  "  soff:       0x%04x\n"
+		  "  attr:       0x%04x\n"
+		  "  nbytes: 0x%08x\n"
+		  "  slast:  0x%08x\n"
+		  "  daddr:  0x%08x\n"
+		  "  doff:       0x%04x\n"
+		  "  citer:      0x%04x\n"
+		  "  dlast:  0x%08x\n"
+		  "  csr:        0x%04x\n"
+		  "  biter:      0x%04x\n",
+		__entry->saddr,
+		__entry->soff,
+		__entry->attr,
+		__entry->nbytes,
+		__entry->slast,
+		__entry->daddr,
+		__entry->doff,
+		__entry->citer,
+		__entry->dlast_sga,
+		__entry->csr,
+		__entry->biter)
+);
+
+DEFINE_EVENT(edma_log_tcd, edma_fill_tcd,
+	TP_PROTO(struct fsl_edma_engine *edma, struct fsl_edma_hw_tcd *tcd),
+	TP_ARGS(edma, tcd)
+);
+
+#endif
+
+/* this part must be outside header guard */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE fsl-edma-trace
+
+#include <trace/define_trace.h>
-- 
2.34.1

Re: [PATCH v5 1/3] debugfs_create_regset32() support 8/16 bit width registers

[Arnd Bergmann]

On Mon, Oct 2, 2023, at 20:37, Frank Li wrote:
> Enhance the flexibility of `debugfs_create_regset32()` to support registers
> of various bit widths. The key changes are as follows:
>
> 1. Renamed '*reg32' and '*regset32' to '*reg' and '*regset' in relevant
>    code to reflect that the register width is not limited to 32 bits.
>
> 2. Added 'size' and 'bigendian' fields to the `struct debugfs_reg` to allow
>    for specifying the size and endianness of registers. These additions
>    enable `debugfs_create_regset()` to support a wider range of register
>    types.
>
> 3. When 'size' is set to 0, it signifies a 32-bit register. This change
>    maintains compatibility with existing code that assumes 32-bit
>    registers.
>
> Improve the versatility of `debugfs_create_regset()` and enable it to
> handle registers of different sizes and endianness, offering greater
> flexibility for debugging and monitoring.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

This version looks correct to me, I see no fundamental problems with it.
In fact, you could list "support for ioport_map() output" to the features
above.

A few other thoughts from my side, all of which could be ignored:

- if the ioport access is not an important feature, we can instead
  support 64-bit readl() as I commented in a previous email. We just
  can't easily have both.

- instead of treating every value of "regs->size" other than 1 and 2
  as meaning '32-bit read', I would explicitly check for 0 and 4
  here

- Another more complicated but also more featureful variant would
  be to use the 'regmap' infrastructure as the abstraction, this would
  also provide access to big-endian, variable register width
  (including 64-bit), and pio, along with additional features and
  other bus types. Not sure it's worth it, but could be interesting
  to try out.

     Arnd

Re: [PATCH v5 1/3] debugfs_create_regset32() support 8/16 bit width registers

[Frank Li]

On Mon, Oct 02, 2023 at 08:55:23PM +0200, Arnd Bergmann wrote:
> On Mon, Oct 2, 2023, at 20:37, Frank Li wrote:
> > Enhance the flexibility of `debugfs_create_regset32()` to support registers
> > of various bit widths. The key changes are as follows:
> >
> > 1. Renamed '*reg32' and '*regset32' to '*reg' and '*regset' in relevant
> >    code to reflect that the register width is not limited to 32 bits.
> >
> > 2. Added 'size' and 'bigendian' fields to the `struct debugfs_reg` to allow
> >    for specifying the size and endianness of registers. These additions
> >    enable `debugfs_create_regset()` to support a wider range of register
> >    types.
> >
> > 3. When 'size' is set to 0, it signifies a 32-bit register. This change
> >    maintains compatibility with existing code that assumes 32-bit
> >    registers.
> >
> > Improve the versatility of `debugfs_create_regset()` and enable it to
> > handle registers of different sizes and endianness, offering greater
> > flexibility for debugging and monitoring.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> 
> This version looks correct to me, I see no fundamental problems with it.
> In fact, you could list "support for ioport_map() output" to the features
> above.
> 
> A few other thoughts from my side, all of which could be ignored:
> 
> - if the ioport access is not an important feature, we can instead
>   support 64-bit readl() as I commented in a previous email. We just
>   can't easily have both.

We will get 64bit dma edma soon. So I can test and upstream it when I get
it.

> 
> - instead of treating every value of "regs->size" other than 1 and 2
>   as meaning '32-bit read', I would explicitly check for 0 and 4
>   here

Yes, I will update next version.

> 
> - Another more complicated but also more featureful variant would
>   be to use the 'regmap' infrastructure as the abstraction, this would
>   also provide access to big-endian, variable register width
>   (including 64-bit), and pio, along with additional features and
>   other bus types. Not sure it's worth it, but could be interesting
>   to try out.

Yes, debugfs_create_regset may need big change or create new version for
regmap. It is out of scope this patches.  I will try later.

> 
>      Arnd

Re: [PATCH v5 1/3] debugfs_create_regset32() support 8/16 bit width registers

[Arnd Bergmann]

On Mon, Oct 2, 2023, at 21:07, Frank Li wrote:
> On Mon, Oct 02, 2023 at 08:55:23PM +0200, Arnd Bergmann wrote:
>
>> A few other thoughts from my side, all of which could be ignored:
>> 
>> - if the ioport access is not an important feature, we can instead
>>   support 64-bit readl() as I commented in a previous email. We just
>>   can't easily have both.
>
> We will get 64bit dma edma soon. So I can test and upstream it when I get
> it.

Ok, so if we already know this is going to be needed, then I would skip
the PIO support and just use read{bwlq}() with the optional swab() instead
of the ioread variants. Otherwise there is a risk that someone starts
relying on the port I/O feature and make it harder to remove.

     Arnd