[PATCH 0/2] Display regmap values in debugfs for write only registers

[PATCH 0/2] Display regmap values in debugfs for write only registers

[Cristian Birsan]

Hi,

This patch series adds support for dumping write only device registers in
regmap debugfs.This is useful for debuging devices like audio codecs (such 
as WM8731) and maybe other devices. It addresses the problem described in
the following thread: https://lkml.org/lkml/2015/6/13/98

The logic that decides if the values can be printed is moved to
regmap_printable() function to allow for easier future updates.

Please let me know if you have other sugestions.

Kind regards,
Cristian

Cristian Birsan (2):
  regmap: Add a function to check if a regmap register is cached
  regmap: debugfs: Add support for dumping write only device registers

 drivers/base/regmap/internal.h       |  1 +
 drivers/base/regmap/regmap-debugfs.c | 22 ++++++++++++++++++----
 drivers/base/regmap/regmap.c         | 14 ++++++++++++++
 3 files changed, 33 insertions(+), 4 deletions(-)

-- 
1.9.1

[PATCH 1/2] regmap: Add a function to check if a regmap register is cached

[Cristian Birsan]

Add a function to check if a regmap register is cached. This will be used
in debugfs to dump the cached values of write only registers.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
 drivers/base/regmap/internal.h |  1 +
 drivers/base/regmap/regmap.c   | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 3df9770..cae04f4 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -171,6 +171,7 @@ struct regcache_ops {
 	int (*drop)(struct regmap *map, unsigned int min, unsigned int max);
 };
 
+bool regmap_cached(struct regmap *map, unsigned int reg);
 bool regmap_writeable(struct regmap *map, unsigned int reg);
 bool regmap_readable(struct regmap *map, unsigned int reg);
 bool regmap_volatile(struct regmap *map, unsigned int reg);
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 4ac63c0..e07f3a9 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -92,6 +92,20 @@ bool regmap_writeable(struct regmap *map, unsigned int reg)
 	return true;
 }
 
+bool regmap_cached(struct regmap *map, unsigned int reg)
+{
+	if (map->cache == REGCACHE_NONE)
+		return false;
+
+	if (!map->cache_ops)
+		return false;
+
+	if (map->max_register && reg > map->max_register)
+		return false;
+
+	return true;
+}
+
 bool regmap_readable(struct regmap *map, unsigned int reg)
 {
 	if (!map->reg_read)
-- 
1.9.1

[PATCH 1/2] regmap: Add a function to check if a regmap register is cached

[Lars-Peter Clausen]

On 08/04/2016 04:55 PM, Cristian Birsan wrote:
> Add a function to check if a regmap register is cached. This will be used
> in debugfs to dump the cached values of write only registers.
> 
> Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
> ---
>  drivers/base/regmap/internal.h |  1 +
>  drivers/base/regmap/regmap.c   | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
> index 3df9770..cae04f4 100644
> --- a/drivers/base/regmap/internal.h
> +++ b/drivers/base/regmap/internal.h
> @@ -171,6 +171,7 @@ struct regcache_ops {
>  	int (*drop)(struct regmap *map, unsigned int min, unsigned int max);
>  };
>  
> +bool regmap_cached(struct regmap *map, unsigned int reg);
>  bool regmap_writeable(struct regmap *map, unsigned int reg);
>  bool regmap_readable(struct regmap *map, unsigned int reg);
>  bool regmap_volatile(struct regmap *map, unsigned int reg);
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index 4ac63c0..e07f3a9 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -92,6 +92,20 @@ bool regmap_writeable(struct regmap *map, unsigned int reg)
>  	return true;
>  }
>  
> +bool regmap_cached(struct regmap *map, unsigned int reg)
> +{
> +	if (map->cache == REGCACHE_NONE)
> +		return false;
> +
> +	if (!map->cache_ops)
> +		return false;
> +
> +	if (map->max_register && reg > map->max_register)
> +		return false;	

There is a problem with this approach. It does not check if the register is
cached it only checks if the register is cacheable.

This works very poorly for devices with sparse register maps. Sparse
register maps do not assign a register to each register number. There are
often even large gaps in the register map and some devices use up their full
16-bit register space.

Now this change combined with the next patch will cause the register file to
contain an entry for every possible register number, even if the register
number is not assigned. Since unassigned registers are not cached
regcache_read() will return an error and the registers file will print XX
for the register value. This means for sparse register maps the registers
file will be full of thousands of such entries.

[PATCH 1/2] regmap: Add a function to check if a regmap register is cached

[Mark Brown]

On Thu, Aug 04, 2016 at 05:55:57PM +0300, Cristian Birsan wrote:

> +bool regmap_cached(struct regmap *map, unsigned int reg)
> +{
> +	if (map->cache == REGCACHE_NONE)
> +		return false;
> +
> +	if (!map->cache_ops)
> +		return false;
> +
> +	if (map->max_register && reg > map->max_register)
> +		return false;
> +
> +	return true;
> +}

As Lars said this isn't actually checking if the register is cached.  To
do this you need to modify the output code to check if there's a value
cached and try a read if there is one instead.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160804/c7df4d0f/attachment.sig>

[PATCH v2 0/2] Display regmap values in debugfs for write only registers

[Cristian Birsan]

Hi,

Thank you for the review and for the very detailed comments. I updated the 
patch series to fix the problems identified. To make sure a register is 
really cached a regcache_read() is performed on cacheable registers.

The patch was tested using a wm8731 device.

Changes since v1 (https://lkml.org/lkml/2016/8/4/327)
	+ Fix regmap_cached() to return if a register is really cached
	  instead of cacheable. 
	+ Remove unnecesary regcache_read() in regmap_read_debugfs()  

Cristian Birsan (2):
  regmap: Add a function to check if a regmap register is cached
  regmap: debugfs: Add support for dumping write only device registers

 drivers/base/regmap/internal.h       |  1 +
 drivers/base/regmap/regmap-debugfs.c | 16 +++++++++++++---
 drivers/base/regmap/regmap.c         | 23 +++++++++++++++++++++++
 3 files changed, 37 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH v2 1/2] regmap: Add a function to check if a regmap register is cached

[Cristian Birsan]

Add a function to check if a regmap register is cached. This will be used
in debugfs to dump the cached values of write only registers.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
 drivers/base/regmap/internal.h |  1 +
 drivers/base/regmap/regmap.c   | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index a038033..f4be4c1 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -173,6 +173,7 @@ struct regcache_ops {
 	int (*drop)(struct regmap *map, unsigned int min, unsigned int max);
 };
 
+bool regmap_cached(struct regmap *map, unsigned int reg);
 bool regmap_writeable(struct regmap *map, unsigned int reg);
 bool regmap_readable(struct regmap *map, unsigned int reg);
 bool regmap_volatile(struct regmap *map, unsigned int reg);
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 51fa7d6..1f011f9 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -93,6 +93,29 @@ bool regmap_writeable(struct regmap *map, unsigned int reg)
 	return true;
 }
 
+bool regmap_cached(struct regmap *map, unsigned int reg)
+{
+	int ret;
+	unsigned int val;
+
+	if (map->cache == REGCACHE_NONE)
+		return false;
+
+	if (!map->cache_ops)
+		return false;
+
+	if (map->max_register && reg > map->max_register)
+		return false;
+
+	map->lock(map->lock_arg);
+	ret = regcache_read(map, reg, &val);
+	map->unlock(map->lock_arg);
+	if (ret)
+		return false;
+
+	return true;
+}
+
 bool regmap_readable(struct regmap *map, unsigned int reg)
 {
 	if (!map->reg_read)
-- 
1.9.1

Applied "regmap: Add a function to check if a regmap register is cached" to the regmap tree

[Mark Brown]

The patch

   regmap: Add a function to check if a regmap register is cached

has been applied to the regmap tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 1ea975cf1ef57b1e44c0aec4820f60bb3b60904b Mon Sep 17 00:00:00 2001
From: Cristian Birsan <cristian.birsan@microchip.com>
Date: Mon, 8 Aug 2016 18:44:21 +0300
Subject: [PATCH] regmap: Add a function to check if a regmap register is
 cached

Add a function to check if a regmap register is cached. This will be used
in debugfs to dump the cached values of write only registers.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/internal.h |  1 +
 drivers/base/regmap/regmap.c   | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index a0380338946a..f4be4c19bb17 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -173,6 +173,7 @@ struct regcache_ops {
 	int (*drop)(struct regmap *map, unsigned int min, unsigned int max);
 };
 
+bool regmap_cached(struct regmap *map, unsigned int reg);
 bool regmap_writeable(struct regmap *map, unsigned int reg);
 bool regmap_readable(struct regmap *map, unsigned int reg);
 bool regmap_volatile(struct regmap *map, unsigned int reg);
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 51fa7d66a393..1f011f9d6dcb 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -93,6 +93,29 @@ bool regmap_writeable(struct regmap *map, unsigned int reg)
 	return true;
 }
 
+bool regmap_cached(struct regmap *map, unsigned int reg)
+{
+	int ret;
+	unsigned int val;
+
+	if (map->cache == REGCACHE_NONE)
+		return false;
+
+	if (!map->cache_ops)
+		return false;
+
+	if (map->max_register && reg > map->max_register)
+		return false;
+
+	map->lock(map->lock_arg);
+	ret = regcache_read(map, reg, &val);
+	map->unlock(map->lock_arg);
+	if (ret)
+		return false;
+
+	return true;
+}
+
 bool regmap_readable(struct regmap *map, unsigned int reg)
 {
 	if (!map->reg_read)
-- 
2.8.1

[PATCH v2 2/2] regmap: debugfs: Add support for dumping write only device registers

[Cristian Birsan]

Add support for dumping write only device registers in debugfs. This is
useful for audio codecs that have write only registers (like WM8731).
The logic that decides if a value can be printed is moved to
regmap_printable() function to allow for easier future updates.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
 drivers/base/regmap/regmap-debugfs.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 1ee3d40..36ce351 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -77,6 +77,17 @@ static void regmap_debugfs_free_dump_cache(struct regmap *map)
 	}
 }
 
+static bool regmap_printable(struct regmap *map, unsigned int reg)
+{
+	if (regmap_precious(map, reg))
+		return false;
+
+	if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
+		return false;
+
+	return true;
+}
+
 /*
  * Work out where the start offset maps into register numbers, bearing
  * in mind that we suppress hidden registers.
@@ -105,8 +116,7 @@ static unsigned int regmap_debugfs_get_dump_start(struct regmap *map,
 	if (list_empty(&map->debugfs_off_cache)) {
 		for (; i <= map->max_register; i += map->reg_stride) {
 			/* Skip unprinted registers, closing off cache entry */
-			if (!regmap_readable(map, i) ||
-			    regmap_precious(map, i)) {
+			if (!regmap_printable(map, i)) {
 				if (c) {
 					c->max = p - 1;
 					c->max_reg = i - map->reg_stride;
@@ -204,7 +214,7 @@ static ssize_t regmap_read_debugfs(struct regmap *map, unsigned int from,
 	start_reg = regmap_debugfs_get_dump_start(map, from, *ppos, &p);
 
 	for (i = start_reg; i <= to; i += map->reg_stride) {
-		if (!regmap_readable(map, i))
+		if (!regmap_readable(map, i) && !regmap_cached(map, i))
 			continue;
 
 		if (regmap_precious(map, i))
-- 
1.9.1

Applied "regmap: debugfs: Add support for dumping write only device registers" to the regmap tree

[Mark Brown]

The patch

   regmap: debugfs: Add support for dumping write only device registers

has been applied to the regmap tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 359a2f17604e2c1c8938be2bba32a8cef9cf6303 Mon Sep 17 00:00:00 2001
From: Cristian Birsan <cristian.birsan@microchip.com>
Date: Mon, 8 Aug 2016 18:44:22 +0300
Subject: [PATCH] regmap: debugfs: Add support for dumping write only device
 registers

Add support for dumping write only device registers in debugfs. This is
useful for audio codecs that have write only registers (like WM8731).
The logic that decides if a value can be printed is moved to
regmap_printable() function to allow for easier future updates.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/regmap-debugfs.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 1ee3d40861c7..36ce3511c733 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -77,6 +77,17 @@ static void regmap_debugfs_free_dump_cache(struct regmap *map)
 	}
 }
 
+static bool regmap_printable(struct regmap *map, unsigned int reg)
+{
+	if (regmap_precious(map, reg))
+		return false;
+
+	if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
+		return false;
+
+	return true;
+}
+
 /*
  * Work out where the start offset maps into register numbers, bearing
  * in mind that we suppress hidden registers.
@@ -105,8 +116,7 @@ static unsigned int regmap_debugfs_get_dump_start(struct regmap *map,
 	if (list_empty(&map->debugfs_off_cache)) {
 		for (; i <= map->max_register; i += map->reg_stride) {
 			/* Skip unprinted registers, closing off cache entry */
-			if (!regmap_readable(map, i) ||
-			    regmap_precious(map, i)) {
+			if (!regmap_printable(map, i)) {
 				if (c) {
 					c->max = p - 1;
 					c->max_reg = i - map->reg_stride;
@@ -204,7 +214,7 @@ static ssize_t regmap_read_debugfs(struct regmap *map, unsigned int from,
 	start_reg = regmap_debugfs_get_dump_start(map, from, *ppos, &p);
 
 	for (i = start_reg; i <= to; i += map->reg_stride) {
-		if (!regmap_readable(map, i))
+		if (!regmap_readable(map, i) && !regmap_cached(map, i))
 			continue;
 
 		if (regmap_precious(map, i))
-- 
2.8.1

Applied "regmap: debugfs: Add support for dumping write only device registers" to the regmap tree

[kbuild test robot]

Hi Mark,

[auto build test ERROR on regmap/for-next]
[also build test ERROR on v4.8-rc1 next-20160809]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mark-Brown/Applied-regmap-debugfs-Add-support-for-dumping-write-only-device-registers-to-the-regmap-tree/20160809-205351
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next
config: x86_64-randconfig-x013-201632 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/base/regmap/regmap-debugfs.c: In function 'regmap_printable':
>> drivers/base/regmap/regmap-debugfs.c:85:37: error: implicit declaration of function 'regmap_cached' [-Werror=implicit-function-declaration]
     if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
                                        ^~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/regmap_cached +85 drivers/base/regmap/regmap-debugfs.c

    79	
    80	static bool regmap_printable(struct regmap *map, unsigned int reg)
    81	{
    82		if (regmap_precious(map, reg))
    83			return false;
    84	
  > 85		if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
    86			return false;
    87	
    88		return true;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 25238 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160809/8bfc58e1/attachment-0001.obj>

Applied "regmap: debugfs: Add support for dumping write only device registers" to the regmap tree

[kbuild test robot]

Hi Mark,

[auto build test WARNING on regmap/for-next]
[also build test WARNING on v4.8-rc1 next-20160809]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mark-Brown/Applied-regmap-debugfs-Add-support-for-dumping-write-only-device-registers-to-the-regmap-tree/20160809-205351
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next
config: x86_64-randconfig-x019-201632 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/asm-generic/bug.h:4:0,
                    from arch/x86/include/asm/bug.h:35,
                    from include/linux/bug.h:4,
                    from include/linux/mmdebug.h:4,
                    from include/linux/gfp.h:4,
                    from include/linux/slab.h:14,
                    from drivers/base/regmap/regmap-debugfs.c:13:
   drivers/base/regmap/regmap-debugfs.c: In function 'regmap_printable':
   drivers/base/regmap/regmap-debugfs.c:85:37: error: implicit declaration of function 'regmap_cached' [-Werror=implicit-function-declaration]
     if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
                                        ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/base/regmap/regmap-debugfs.c:85:2: note: in expansion of macro 'if'
     if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
     ^~
   cc1: some warnings being treated as errors

vim +/if +85 drivers/base/regmap/regmap-debugfs.c

     7	 *
     8	 * This program is free software; you can redistribute it and/or modify
     9	 * it under the terms of the GNU General Public License version 2 as
    10	 * published by the Free Software Foundation.
    11	 */
    12	
  > 13	#include <linux/slab.h>
    14	#include <linux/mutex.h>
    15	#include <linux/debugfs.h>
    16	#include <linux/uaccess.h>
    17	#include <linux/device.h>
    18	#include <linux/list.h>
    19	
    20	#include "internal.h"
    21	
    22	struct regmap_debugfs_node {
    23		struct regmap *map;
    24		const char *name;
    25		struct list_head link;
    26	};
    27	
    28	static struct dentry *regmap_debugfs_root;
    29	static LIST_HEAD(regmap_debugfs_early_list);
    30	static DEFINE_MUTEX(regmap_debugfs_early_lock);
    31	
    32	/* Calculate the length of a fixed format  */
    33	static size_t regmap_calc_reg_len(int max_val)
    34	{
    35		return snprintf(NULL, 0, "%x", max_val);
    36	}
    37	
    38	static ssize_t regmap_name_read_file(struct file *file,
    39					     char __user *user_buf, size_t count,
    40					     loff_t *ppos)
    41	{
    42		struct regmap *map = file->private_data;
    43		int ret;
    44		char *buf;
    45	
    46		buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
    47		if (!buf)
    48			return -ENOMEM;
    49	
    50		ret = snprintf(buf, PAGE_SIZE, "%s\n", map->dev->driver->name);
    51		if (ret < 0) {
    52			kfree(buf);
    53			return ret;
    54		}
    55	
    56		ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
    57		kfree(buf);
    58		return ret;
    59	}
    60	
    61	static const struct file_operations regmap_name_fops = {
    62		.open = simple_open,
    63		.read = regmap_name_read_file,
    64		.llseek = default_llseek,
    65	};
    66	
    67	static void regmap_debugfs_free_dump_cache(struct regmap *map)
    68	{
    69		struct regmap_debugfs_off_cache *c;
    70	
    71		while (!list_empty(&map->debugfs_off_cache)) {
    72			c = list_first_entry(&map->debugfs_off_cache,
    73					     struct regmap_debugfs_off_cache,
    74					     list);
    75			list_del(&c->list);
    76			kfree(c);
    77		}
    78	}
    79	
    80	static bool regmap_printable(struct regmap *map, unsigned int reg)
    81	{
    82		if (regmap_precious(map, reg))
    83			return false;
    84	
  > 85		if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
    86			return false;
    87	
    88		return true;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 25983 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160809/88e78350/attachment-0001.obj>

Applied "regmap: debugfs: Add support for dumping write only device registers" to the regmap tree

[Cristian Birsan]

Hi,

It seems like a false alarm. The 0-DAY tree does not contain the same content as regmap/for-next tree.

>From the patch series "regmap: debugfs: Add support for dumping write only device registers the" the PATCH v2 1/2 is missing form 0-DAY tree while the PATCH v2 2/2 is applied. This triggered the warning. Let's wait for a new build ...

Cristi

On 08/09/2016 04:18 PM, kbuild test robot wrote:
> Hi Mark,
> 
> [auto build test WARNING on regmap/for-next]
> [also build test WARNING on v4.8-rc1 next-20160809]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Mark-Brown/Applied-regmap-debugfs-Add-support-for-dumping-write-only-device-registers-to-the-regmap-tree/20160809-205351
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next
> config: x86_64-randconfig-x019-201632 (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from include/asm-generic/bug.h:4:0,
>                     from arch/x86/include/asm/bug.h:35,
>                     from include/linux/bug.h:4,
>                     from include/linux/mmdebug.h:4,
>                     from include/linux/gfp.h:4,
>                     from include/linux/slab.h:14,
>                     from drivers/base/regmap/regmap-debugfs.c:13:
>    drivers/base/regmap/regmap-debugfs.c: In function 'regmap_printable':
>    drivers/base/regmap/regmap-debugfs.c:85:37: error: implicit declaration of function 'regmap_cached' [-Werror=implicit-function-declaration]
>      if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
>                                         ^
>    include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
>      if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
>                                  ^~~~
>>> drivers/base/regmap/regmap-debugfs.c:85:2: note: in expansion of macro 'if'
>      if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
>      ^~
>    cc1: some warnings being treated as errors
> 
> vim +/if +85 drivers/base/regmap/regmap-debugfs.c
> 
>      7	 *
>      8	 * This program is free software; you can redistribute it and/or modify
>      9	 * it under the terms of the GNU General Public License version 2 as
>     10	 * published by the Free Software Foundation.
>     11	 */
>     12	
>   > 13	#include <linux/slab.h>
>     14	#include <linux/mutex.h>
>     15	#include <linux/debugfs.h>
>     16	#include <linux/uaccess.h>
>     17	#include <linux/device.h>
>     18	#include <linux/list.h>
>     19	
>     20	#include "internal.h"
>     21	
>     22	struct regmap_debugfs_node {
>     23		struct regmap *map;
>     24		const char *name;
>     25		struct list_head link;
>     26	};
>     27	
>     28	static struct dentry *regmap_debugfs_root;
>     29	static LIST_HEAD(regmap_debugfs_early_list);
>     30	static DEFINE_MUTEX(regmap_debugfs_early_lock);
>     31	
>     32	/* Calculate the length of a fixed format  */
>     33	static size_t regmap_calc_reg_len(int max_val)
>     34	{
>     35		return snprintf(NULL, 0, "%x", max_val);
>     36	}
>     37	
>     38	static ssize_t regmap_name_read_file(struct file *file,
>     39					     char __user *user_buf, size_t count,
>     40					     loff_t *ppos)
>     41	{
>     42		struct regmap *map = file->private_data;
>     43		int ret;
>     44		char *buf;
>     45	
>     46		buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
>     47		if (!buf)
>     48			return -ENOMEM;
>     49	
>     50		ret = snprintf(buf, PAGE_SIZE, "%s\n", map->dev->driver->name);
>     51		if (ret < 0) {
>     52			kfree(buf);
>     53			return ret;
>     54		}
>     55	
>     56		ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
>     57		kfree(buf);
>     58		return ret;
>     59	}
>     60	
>     61	static const struct file_operations regmap_name_fops = {
>     62		.open = simple_open,
>     63		.read = regmap_name_read_file,
>     64		.llseek = default_llseek,
>     65	};
>     66	
>     67	static void regmap_debugfs_free_dump_cache(struct regmap *map)
>     68	{
>     69		struct regmap_debugfs_off_cache *c;
>     70	
>     71		while (!list_empty(&map->debugfs_off_cache)) {
>     72			c = list_first_entry(&map->debugfs_off_cache,
>     73					     struct regmap_debugfs_off_cache,
>     74					     list);
>     75			list_del(&c->list);
>     76			kfree(c);
>     77		}
>     78	}
>     79	
>     80	static bool regmap_printable(struct regmap *map, unsigned int reg)
>     81	{
>     82		if (regmap_precious(map, reg))
>     83			return false;
>     84	
>   > 85		if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
>     86			return false;
>     87	
>     88		return true;
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 

[PATCH 2/2] regmap: debugfs: Add support for dumping write only device registers

[Cristian Birsan]

Add support for dumping write only device registers in debugfs. This is
useful for audio codecs that have write only registers (like WM8731).
The logic that decides if a value can be printed is moved to
regmap_printable() function to allow for easier future updates.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
 drivers/base/regmap/regmap-debugfs.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 3f0a7e2..8db10e9 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -77,6 +77,17 @@ static void regmap_debugfs_free_dump_cache(struct regmap *map)
 	}
 }
 
+static bool regmap_printable(struct regmap *map, unsigned int reg)
+{
+	if (regmap_precious(map, reg))
+		return false;
+
+	if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
+		return false;
+
+	return true;
+}
+
 /*
  * Work out where the start offset maps into register numbers, bearing
  * in mind that we suppress hidden registers.
@@ -105,8 +116,7 @@ static unsigned int regmap_debugfs_get_dump_start(struct regmap *map,
 	if (list_empty(&map->debugfs_off_cache)) {
 		for (; i <= map->max_register; i += map->reg_stride) {
 			/* Skip unprinted registers, closing off cache entry */
-			if (!regmap_readable(map, i) ||
-			    regmap_precious(map, i)) {
+			if (!regmap_printable(map, i)) {
 				if (c) {
 					c->max = p - 1;
 					c->max_reg = i - map->reg_stride;
@@ -204,7 +214,7 @@ static ssize_t regmap_read_debugfs(struct regmap *map, unsigned int from,
 	start_reg = regmap_debugfs_get_dump_start(map, from, *ppos, &p);
 
 	for (i = start_reg; i <= to; i += map->reg_stride) {
-		if (!regmap_readable(map, i))
+		if (!regmap_readable(map, i) && !regmap_cached(map, i))
 			continue;
 
 		if (regmap_precious(map, i))
@@ -222,7 +232,11 @@ static ssize_t regmap_read_debugfs(struct regmap *map, unsigned int from,
 			buf_pos += map->debugfs_reg_len + 2;
 
 			/* Format the value, write all X if we can't read */
-			ret = regmap_read(map, i, &val);
+			if (regmap_readable(map, i))
+				ret = regmap_read(map, i, &val);
+			else
+				ret = regcache_read(map, i, &val);
+
 			if (ret == 0)
 				snprintf(buf + buf_pos, count - buf_pos,
 					 "%.*x", map->debugfs_val_len, val);
-- 
1.9.1

[PATCH 2/2] regmap: debugfs: Add support for dumping write only device registers

[Mark Brown]

On Thu, Aug 04, 2016 at 05:55:58PM +0300, Cristian Birsan wrote:
> Add support for dumping write only device registers in debugfs. This is
> useful for audio codecs that have write only registers (like WM8731).
> The logic that decides if a value can be printed is moved to
> regmap_printable() function to allow for easier future updates.

Please check your CC list when sending things upstream - try to ensure
that people you're sending patches to are relevant to the patch.
Maintainers often get lots of mail and having to sort out mail that's
not really relevant to them can make it easier for relevant mail to get
missed.

> +static bool regmap_printable(struct regmap *map, unsigned int reg)
> +{
> +	if (regmap_precious(map, reg))
> +		return false;
> +
> +	if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
> +		return false;
> +
> +	return true;
> +}

This only has one user...

>  	for (i = start_reg; i <= to; i += map->reg_stride) {
> -		if (!regmap_readable(map, i))
> +		if (!regmap_readable(map, i) && !regmap_cached(map, i))
>  			continue;
>  

...though it could have more.

> -			ret = regmap_read(map, i, &val);
> +			if (regmap_readable(map, i))
> +				ret = regmap_read(map, i, &val);
> +			else
> +				ret = regcache_read(map, i, &val);
> +

I don't understand this change, a read will go to cache anyway.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160804/d65aa7b6/attachment.sig>

[PATCH 2/2] regmap: debugfs: Add support for dumping write only device registers

[Nicolas Ferre]

Le 04/08/2016 ? 22:26, Mark Brown a ?crit :
> On Thu, Aug 04, 2016 at 05:55:58PM +0300, Cristian Birsan wrote:
>> Add support for dumping write only device registers in debugfs. This is
>> useful for audio codecs that have write only registers (like WM8731).
>> The logic that decides if a value can be printed is moved to
>> regmap_printable() function to allow for easier future updates.
> 
> Please check your CC list when sending things upstream - try to ensure
> that people you're sending patches to are relevant to the patch.
> Maintainers often get lots of mail and having to sort out mail that's
> not really relevant to them can make it easier for relevant mail to get
> missed.

Mark,

Just FYI, I gave Cristian the CC list he could use: In fact the Atmel /
Microchip / Free-Electons people are in the list because we are all
working together on the AT91 platforms now. And these platforms use the
audio codec Cristian is working on.


>> +static bool regmap_printable(struct regmap *map, unsigned int reg)
>> +{
>> +	if (regmap_precious(map, reg))
>> +		return false;
>> +
>> +	if (!regmap_readable(map, reg) && !regmap_cached(map, reg))
>> +		return false;
>> +
>> +	return true;
>> +}
> 
> This only has one user...
> 
>>  	for (i = start_reg; i <= to; i += map->reg_stride) {
>> -		if (!regmap_readable(map, i))
>> +		if (!regmap_readable(map, i) && !regmap_cached(map, i))
>>  			continue;
>>  
> 
> ...though it could have more.
> 
>> -			ret = regmap_read(map, i, &val);
>> +			if (regmap_readable(map, i))
>> +				ret = regmap_read(map, i, &val);
>> +			else
>> +				ret = regcache_read(map, i, &val);
>> +
> 
> I don't understand this change, a read will go to cache anyway.
> 


-- 
Nicolas Ferre

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160805/b0d40700/attachment.sig>