[PATCH V2 0/2] debugfs: a tool to print 32-bit registers

[PATCH V2 0/2] debugfs: a tool to print 32-bit registers

[Alessandro Rubini]

This is V2 of what I posted a few days ago. I also updates
the file in Documentation.  I Cc:d people who gave feedback.

Patch 2 uses the new tool for an existing file I didn't initially
notice (I had a victim file, but it's seriously failing compilation).
Code removal there is almost zero because Felipe Balbi already did
stuff in the way I do.

Mark Brown suggested to look at drivers/base/regmap, and actually this
may be duplicating what is already being worked on in a more general
way. My code base is earlier than regmap so I didn't notice.
Unfortunately I have no resources left to study regmap as it
deserves.

Mark, if you say this is not useful at this point, I'm fine with it; I
may return with some regbase-aware code when time permits if it's not
already there.

/alessandro

Alessandro Rubini (2):
  debugfs: add tools to printk 32-bit registers
  usb: dwc3: use debugfs_print_regs32()

 Documentation/filesystems/debugfs.txt |   32 +++++++++++-
 drivers/usb/dwc3/debugfs.c            |   15 +----
 fs/debugfs/file.c                     |   90 +++++++++++++++++++++++++++++++++
 include/linux/debugfs.h               |   26 +++++++++
 4 files changed, 150 insertions(+), 13 deletions(-)

Re: [PATCH V2 0/2] debugfs: a tool to print 32-bit registers

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

On Fri, Nov 18, 2011 at 01:20:49AM +0100, Alessandro Rubini wrote:
> This is V2 of what I posted a few days ago. I also updates
> the file in Documentation.  I Cc:d people who gave feedback.
> 
> Patch 2 uses the new tool for an existing file I didn't initially
> notice (I had a victim file, but it's seriously failing compilation).
> Code removal there is almost zero because Felipe Balbi already did
> stuff in the way I do.
> 
> Mark Brown suggested to look at drivers/base/regmap, and actually this
> may be duplicating what is already being worked on in a more general
> way. My code base is earlier than regmap so I didn't notice.
> Unfortunately I have no resources left to study regmap as it
> deserves.
> 
> Mark, if you say this is not useful at this point, I'm fine with it; I
> may return with some regbase-aware code when time permits if it's not
> already there.

-ENOPATCH ?? I didn't receive any replies to this mail with the patch.
Is my mailer screwed up ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V2 0/2] debugfs: a tool to print 32-bit registers

[Alessandro Rubini]

> -ENOPATCH ?? I didn't receive any replies to this mail with the patch.

You are right. Thanks and apologies. I mistyped Giancarlo Asnaghi's alias
in all three, and the sending was interrupted. I resent now.

thanks
/alessandro

[PATCH V2 1/2] debugfs: add tools to printk 32-bit registers

[Alessandro Rubini]

Some debugfs file I deal with are mostly blocks of registers,
i.e. lines of the form "<name> = 0x<value>". Some files are only
registers, some include registers blocks among other material.  This
patch introduces data structures and functions to deal with both
cases.  I expect more users of this over time.

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
---
 Documentation/filesystems/debugfs.txt |   32 +++++++++++-
 fs/debugfs/file.c                     |   90 +++++++++++++++++++++++++++++++++
 include/linux/debugfs.h               |   26 +++++++++
 3 files changed, 147 insertions(+), 1 deletions(-)

diff --git a/Documentation/filesystems/debugfs.txt b/Documentation/filesystems/debugfs.txt
index 742cc06..f04066a 100644
--- a/Documentation/filesystems/debugfs.txt
+++ b/Documentation/filesystems/debugfs.txt
@@ -97,7 +97,8 @@ A read on the resulting file will yield either Y (for non-zero values) or
 N, followed by a newline.  If written to, it will accept either upper- or
 lower-case values, or 1 or 0.  Any other input will be silently ignored.
 
-Finally, a block of arbitrary binary data can be exported with:
+Another option is exporting a block of arbitrary binary data, with
+this structure and function:
 
     struct debugfs_blob_wrapper {
 	void *data;
@@ -115,6 +116,35 @@ can be used to export binary information, but there does not appear to be
 any code which does so in the mainline.  Note that all files created with
 debugfs_create_blob() are read-only.
 
+If you want to dump a block of registers (something that happens quite
+often during development, even if little such code reaches mainline.
+Debugfs offers two functions: one to make a registers-only file, and
+another to insert a register block in the middle of another sequential
+file.
+
+    struct debugfs_reg32 {
+	char *name;
+	unsigned long offset;
+    };
+
+    struct debugfs_regset32 {
+	struct debugfs_reg32 *regs;
+	int nregs;
+	void __iomem *base;
+    };
+
+    struct dentry *debugfs_create_regset32(const char *name, mode_t mode,
+				     struct dentry *parent,
+				     struct debugfs_regset32 *regset);
+
+    int debugfs_print_regs32(struct seq_file *s, struct debugfs_reg32 *regs,
+			 int nregs, void __iomem *base, char *prefix);
+
+The "base" argument may be 0, but you may want to build the reg32 array
+using __stringify, and a number of register names (macros) are actually
+byte offsets over a base for the register block.
+
+
 There are a couple of other directory-oriented helper functions:
 
     struct dentry *debugfs_rename(struct dentry *old_dir, 
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 90f7657..f31a27c 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -15,6 +15,7 @@
 
 #include <linux/module.h>
 #include <linux/fs.h>
+#include <linux/seq_file.h>
 #include <linux/pagemap.h>
 #include <linux/namei.h>
 #include <linux/debugfs.h>
@@ -525,3 +526,92 @@ struct dentry *debugfs_create_blob(const char *name, mode_t mode,
 	return debugfs_create_file(name, mode, parent, blob, &fops_blob);
 }
 EXPORT_SYMBOL_GPL(debugfs_create_blob);
+
+/*
+ * The regset32 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.
+ */
+
+/**
+ * debugfs_print_regs32 - 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
+ * @mregs: 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
+ *
+ * This function outputs a text block describing the current values of
+ * some 32-bit hardware registers. It is meant to be used within debugfs
+ * files based on seq_file that need to show registers, intermixed with other
+ * information. The prefix argument may be used to specify a leading string,
+ * because some peripherals have several blocks of identical registers,
+ * for example configuration of dma channels
+ */
+int debugfs_print_regs32(struct seq_file *s, struct debugfs_reg32 *regs,
+			   int nregs, void __iomem *base, char *prefix)
+{
+	int i, ret = 0;
+
+	for (i = 0; i < nregs; i++, regs++) {
+		if (prefix)
+			ret += seq_printf(s, "%s", prefix);
+		ret += seq_printf(s, "%s = 0x%08x\n", regs->name,
+				  readl((void *)(base + regs->offset)));
+	}
+	return ret;
+}
+EXPORT_SYMBOL_GPL(debugfs_print_regs32);
+
+static int debugfs_show_regset32(struct seq_file *s, void *data)
+{
+	struct debugfs_regset32 *regset = s->private;
+
+	debugfs_print_regs32(s, regset->regs, regset->nregs, regset->base, "");
+	return 0;
+}
+
+static int debugfs_open_regset32(struct inode *inode, struct file *file)
+{
+	return single_open(file, debugfs_show_regset32, inode->i_private);
+}
+
+static const struct file_operations fops_regset32 = {
+	.open =		debugfs_open_regset32,
+	.read =		seq_read,
+	.llseek =	seq_lseek,
+	.release =	single_release,
+};
+
+/**
+ * debugfs_create_regset32 - 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
+ *          to an array of register definitions, the array size and the base
+ *          address where the register bank is to be found.
+ *
+ * This function creates a file in debugfs with the given name that reports
+ * 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.
+ *
+ * This function will return a pointer to a dentry if it succeeds.  This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.)  If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned.  It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_regset32(const char *name, mode_t mode,
+				       struct dentry *parent,
+				       struct debugfs_regset32 *regset)
+{
+	return debugfs_create_file(name, mode, parent, regset, &fops_regset32);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_regset32);
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index e7d9b20..5e6b01f 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -16,6 +16,7 @@
 #define _DEBUGFS_H_
 
 #include <linux/fs.h>
+#include <linux/seq_file.h>
 
 #include <linux/types.h>
 
@@ -26,6 +27,17 @@ struct debugfs_blob_wrapper {
 	unsigned long size;
 };
 
+struct debugfs_reg32 {
+	char *name;
+	unsigned long offset;
+};
+
+struct debugfs_regset32 {
+	struct debugfs_reg32 *regs;
+	int nregs;
+	void __iomem *base;
+};
+
 extern struct dentry *arch_debugfs_dir;
 
 #if defined(CONFIG_DEBUG_FS)
@@ -74,6 +86,13 @@ struct dentry *debugfs_create_blob(const char *name, mode_t mode,
 				  struct dentry *parent,
 				  struct debugfs_blob_wrapper *blob);
 
+struct dentry *debugfs_create_regset32(const char *name, mode_t mode,
+				     struct dentry *parent,
+				     struct debugfs_regset32 *regset);
+
+int debugfs_print_regs32(struct seq_file *s, struct debugfs_reg32 *regs,
+			 int nregs, void __iomem *base, char *prefix);
+
 bool debugfs_initialized(void);
 
 #else
@@ -188,6 +207,13 @@ static inline struct dentry *debugfs_create_blob(const char *name, mode_t mode,
 	return ERR_PTR(-ENODEV);
 }
 
+static inline struct dentry *debugfs_create_regset32(const char *name,
+				   mode_t mode, struct dentry *parent,
+				   struct debugfs_regset32 *regset)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline bool debugfs_initialized(void)
 {
 	return false;
-- 
1.6.0.2

[PATCH V2 2/2] usb: dwc3: use debugfs_print_regs32()

[Alessandro Rubini]

This a use example of the regs32 utilities in debugfs, although
this fuse use ":" as separator between name and value, and debugs
uses "=" (as it looked to me a more common practice).

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Cc: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/dwc3/debugfs.c |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index da1ad77..d9e7a26 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -51,18 +51,13 @@
 #include "gadget.h"
 #include "io.h"
 
-struct dwc3_register {
-	const char	*name;
-	u32		offset;
-};
-
 #define dump_register(nm)				\
 {							\
 	.name	= __stringify(nm),			\
 	.offset	= DWC3_ ##nm,				\
 }
 
-static const struct dwc3_register dwc3_regs[] = {
+static const struct debugfs_reg32 dwc3_regs[] = {
 	dump_register(GSBUSCFG0),
 	dump_register(GSBUSCFG1),
 	dump_register(GTXTHRCFG),
@@ -385,12 +380,8 @@ static int dwc3_regdump_show(struct seq_file *s, void *unused)
 	int			i;
 
 	seq_printf(s, "DesignWare USB3 Core Register Dump\n");
-
-	for (i = 0; i < ARRAY_SIZE(dwc3_regs); i++) {
-		seq_printf(s, "%-20s :    %08x\n", dwc3_regs[i].name,
-				dwc3_readl(dwc->regs, dwc3_regs[i].offset));
-	}
-
+	debugfs_print_regs32(s, dwc3_regs, ARRAY_SIZE(dwc3_regs),
+			     dwc->regs, "");
 	return 0;
 }
 
-- 
1.6.0.2

Re: [PATCH V2 2/2] usb: dwc3: use debugfs_print_regs32()

[Greg KH]

On Fri, Nov 18, 2011 at 02:51:43PM +0100, Alessandro Rubini wrote:
> This a use example of the regs32 utilities in debugfs, although
> this fuse use ":" as separator between name and value, and debugs
> uses "=" (as it looked to me a more common practice).
> 
> Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
> Cc: Felipe Balbi <balbi@ti.com>

Felipe, any objection to me applying this patch?

thanks,

greg k-h

Re: [PATCH V2 2/2] usb: dwc3: use debugfs_print_regs32()

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]

On Fri, Nov 18, 2011 at 10:31:09AM -0800, Greg KH wrote:
> On Fri, Nov 18, 2011 at 02:51:43PM +0100, Alessandro Rubini wrote:
> > This a use example of the regs32 utilities in debugfs, although
> > this fuse use ":" as separator between name and value, and debugs
> > uses "=" (as it looked to me a more common practice).
> > 
> > Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
> > Cc: Felipe Balbi <balbi@ti.com>
> 
> Felipe, any objection to me applying this patch?

none whatsoever:

Acked-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V2 2/2] usb: dwc3: use debugfs_print_regs32()

[Greg KH]

On Fri, Nov 18, 2011 at 02:51:43PM +0100, Alessandro Rubini wrote:
> This a use example of the regs32 utilities in debugfs, although
> this fuse use ":" as separator between name and value, and debugs
> uses "=" (as it looked to me a more common practice).
> 
> Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
> Acked-by: Felipe Balbi <balbi@ti.com>

I've applied this, but it causes the following complier warnings to be
spit out:

drivers/usb/dwc3/debugfs.c: In function ‘dwc3_regdump_show’:
drivers/usb/dwc3/debugfs.c:384:9: warning: passing argument 2 of ‘debugfs_print_regs32’ discards ‘const’ qualifier from pointer target type [enabled by default]
include/linux/debugfs.h:93:5: note: expected ‘struct debugfs_reg32 *’ but argument is of type ‘const struct debugfs_reg32 *’
drivers/usb/dwc3/debugfs.c:380:8: warning: unused variable ‘i’ [-Wunused-variable]

Alessandro, care to send me a follow-on patch to fix this up?

thanks,

greg k-h

[PATCH] usb: dwc3: fix a warning

[Alessandro Rubini]

The previous patch left an unused variable, I apologize.

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
---
 drivers/usb/dwc3/debugfs.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index d9e7a26..fcfa915 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -377,7 +377,6 @@ static const struct debugfs_reg32 dwc3_regs[] = {
 static int dwc3_regdump_show(struct seq_file *s, void *unused)
 {
 	struct dwc3		*dwc = s->private;
-	int			i;
 
 	seq_printf(s, "DesignWare USB3 Core Register Dump\n");
 	debugfs_print_regs32(s, dwc3_regs, ARRAY_SIZE(dwc3_regs),
-- 
1.5.6.5

[PATCH] debugfs: print_regs32: make regs array a const pointer

[Alessandro Rubini]

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
---
 fs/debugfs/file.c       |    2 +-
 include/linux/debugfs.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index f31a27c..fc98ec9 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -548,7 +548,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_blob);
  * because some peripherals have several blocks of identical registers,
  * for example configuration of dma channels
  */
-int debugfs_print_regs32(struct seq_file *s, struct debugfs_reg32 *regs,
+int debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
 			   int nregs, void __iomem *base, char *prefix)
 {
 	int i, ret = 0;
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 5e6b01f..e8c3abc 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -90,7 +90,7 @@ struct dentry *debugfs_create_regset32(const char *name, mode_t mode,
 				     struct dentry *parent,
 				     struct debugfs_regset32 *regset);
 
-int debugfs_print_regs32(struct seq_file *s, struct debugfs_reg32 *regs,
+int debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
 			 int nregs, void __iomem *base, char *prefix);
 
 bool debugfs_initialized(void);
-- 
1.5.6.5

Re: [PATCH V2 0/2] debugfs: a tool to print 32-bit registers

[Mark Brown]

On Fri, Nov 18, 2011 at 01:20:49AM +0100, Alessandro Rubini wrote:

> Mark, if you say this is not useful at this point, I'm fine with it; I
> may return with some regbase-aware code when time permits if it's not
> already there.

We definitely can't just use this directly with regmap as regmap is for
non memory mapped devices and makes some assumptions in that regard.  I
was mostly just suggesting that we try to keep the file formats similar
to make life easier for userspace.