[RFC 1/2] regmap: if format_write is used, declare all registers as "unreadable"

[RFC 1/2] regmap: if format_write is used, declare all registers as "unreadable"

[Wolfram Sang]

Using .format_write means, we have a custom function to write to the
chip, but not to read back. Also, mark registers as "not precious" and
"not volatile" which is implicit because we cannot read them. Make those
functions use 'regmap_readable' to reuse the checks done there.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/base/regmap/regmap.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index fb3c132..c7b95c3 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -36,6 +36,9 @@ bool regmap_readable(struct regmap *map, unsigned int reg)
 	if (map->max_register && reg > map->max_register)
 		return false;
 
+	if (map->format.format_write)
+		return false;
+
 	if (map->readable_reg)
 		return map->readable_reg(map->dev, reg);
 
@@ -44,7 +47,7 @@ bool regmap_readable(struct regmap *map, unsigned int reg)
 
 bool regmap_volatile(struct regmap *map, unsigned int reg)
 {
-	if (map->max_register && reg > map->max_register)
+	if (!regmap_readable(map, reg))
 		return false;
 
 	if (map->volatile_reg)
@@ -55,7 +58,7 @@ bool regmap_volatile(struct regmap *map, unsigned int reg)
 
 bool regmap_precious(struct regmap *map, unsigned int reg)
 {
-	if (map->max_register && reg > map->max_register)
+	if (!regmap_readable(map, reg))
 		return false;
 
 	if (map->precious_reg)
-- 
1.7.8.3

[RFC 2/2] regmap: make use of cached entries in debugfs

[Wolfram Sang]

Currently, registers not marked as readable won't be displayed via
debugfs. So, a chip with write-only registers shows nothing because of
no readable registers, although regcache knows the current values. Fix
debugfs to only require the readable-flag when no cache is active,
otherwise try reading the cache.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/base/regmap/regmap-debugfs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 6f39747..c5a57f3 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -58,7 +58,7 @@ static ssize_t regmap_map_read_file(struct file *file, char __user *user_buf,
 	tot_len = reg_len + val_len + 3;      /* : \n */
 
 	for (i = 0; i < map->max_register + 1; i++) {
-		if (!regmap_readable(map, i))
+		if (map->cache_type == REGCACHE_NONE && !regmap_readable(map, i))
 			continue;
 
 		if (regmap_precious(map, i))
-- 
1.7.8.3

Re: [RFC 2/2] regmap: make use of cached entries in debugfs

[Mark Brown]

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

On Mon, Jan 30, 2012 at 03:08:17PM +0100, Wolfram Sang wrote:

> -		if (!regmap_readable(map, i))
> +		if (map->cache_type == REGCACHE_NONE && !regmap_readable(map, i))

This isn't quite what you said in the changelog and isn't going to play
nicely with sparse register maps - it should be

	if (regmap_cached(map, i) || regmap_readable(map, i))

or so (regmap_cached() doesn't exist at present but it seems reasonable
that it should).

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

Re: [RFC 2/2] regmap: make use of cached entries in debugfs

[Wolfram Sang]

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

On Mon, Jan 30, 2012 at 02:56:28PM +0000, Mark Brown wrote:
> On Mon, Jan 30, 2012 at 03:08:17PM +0100, Wolfram Sang wrote:
> 
> > -		if (!regmap_readable(map, i))
> > +		if (map->cache_type == REGCACHE_NONE && !regmap_readable(map, i))
> 
> This isn't quite what you said in the changelog

Sorry, I reread and can't find the diff?

> and isn't going to play nicely with sparse register maps

sparse == RBTREE? What does "play nicely" mean here? Too slow?

> - it should be
> 
> 	if (regmap_cached(map, i) || regmap_readable(map, i))
> 
> or so (regmap_cached() doesn't exist at present but it seems reasonable
> that it should).

Sigh, I have to give up for now. Sadly, I am already way beyond my
schedule regarding the amplifier driver and need to work on other things
now.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [RFC 2/2] regmap: make use of cached entries in debugfs

[Mark Brown]

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

On Mon, Jan 30, 2012 at 04:34:27PM +0100, Wolfram Sang wrote:
> On Mon, Jan 30, 2012 at 02:56:28PM +0000, Mark Brown wrote:

> > > -		if (!regmap_readable(map, i))
> > > +		if (map->cache_type == REGCACHE_NONE && !regmap_readable(map, i))

> > This isn't quite what you said in the changelog

> Sorry, I reread and can't find the diff?

Oh, right, you're not actually saying you'll only give cached values
back - my mistake.

> > and isn't going to play nicely with sparse register maps

> sparse == RBTREE? What does "play nicely" mean here? Too slow?

A sparse register map is one that doesn't contain all possible register
addresses, it's orthogonal to the cache - you can have an uncached
device where we know some addresses don't exist or for a cached device
the cache could be any type.

The check for regmap_readable() is there because it causes the debugfs
code to not display registers that don't exist, otherwise we could
easily DoS the output to the point of illegibility by including large
numbers (potentially thousands) of nonexistant registers in the
output making it hard for humans to find the ones that actually exist.

> > - it should be

> > 	if (regmap_cached(map, i) || regmap_readable(map, i))

> > or so (regmap_cached() doesn't exist at present but it seems reasonable
> > that it should).

> Sigh, I have to give up for now. Sadly, I am already way beyond my
> schedule regarding the amplifier driver and need to work on other things
> now.

Oh well.  I might get round to it sometime but essentially all the chips
I work with have read/write support so the issue never comes up.

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

Re: [RFC 1/2] regmap: if format_write is used, declare all registers as "unreadable"

[Mark Brown]

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

On Mon, Jan 30, 2012 at 03:08:16PM +0100, Wolfram Sang wrote:
> Using .format_write means, we have a custom function to write to the
> chip, but not to read back. Also, mark registers as "not precious" and
> "not volatile" which is implicit because we cannot read them. Make those
> functions use 'regmap_readable' to reuse the checks done there.

Applied, thanks.

There are actually some chips that manage to implement read support in
conjunction with odd register formats but none of these are using regmap
right now and it's not altogether clear to me that it's a good idea for
them to do so as they tend to be doing entertaining things that would at
least need some framework additions.  Anyone doing that work can worry
about it then.

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