[PATCH v2] regmap: debugfs: Fix start_reg calculation (v2)

[PATCH v2] regmap: debugfs: Fix start_reg calculation (v2)

[Srinivas KANDAGATLA]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

If we dump syscon regmap registers via debugfs you will notice that the
dump contains lot of XXXXXXXX values.

An example configuration is:
syscon@fdde0000{
	compatible      = "syscon";
	reg		= <0xfdde0000 0x15c>;
};

example dump:
cat  /sys/kernel/debug/regmap/myregmap/registers
...
154: 001c1dff
158: 00000003
05a: XXXXXXXX
05e: XXXXXXXX
...

regmap_debugfs_get_dump_start should return the offset of the register
it should start reading from, However in the current code it does not
consider stride while calculating this. If we use the return value to
dump the registers we can endup with wrong start address, in the example
case for the second loop the code ends up with start_reg = 0x56. Which
is incorrect. Also this keeps incremeting by stride, which can than
result in unaligned address

This patch fixes this issue by considering the stride value at all
required places in regmap_debugfs_get_dump_start function.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
 drivers/base/regmap/regmap-debugfs.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 81d6f60..6cd1b78a 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -97,7 +97,8 @@ static unsigned int regmap_debugfs_get_dump_start(struct regmap *map,
 					c->max = p - 1;
 					fpos_offset = c->max - c->min;
 					reg_offset = fpos_offset / map->debugfs_tot_len;
-					c->max_reg = c->base_reg + reg_offset;
+					c->max_reg = c->base_reg +
+						(reg_offset * map->reg_stride);
 					list_add_tail(&c->list,
 						      &map->debugfs_off_cache);
 					c = NULL;
@@ -126,7 +127,7 @@ static unsigned int regmap_debugfs_get_dump_start(struct regmap *map,
 		c->max = p - 1;
 		fpos_offset = c->max - c->min;
 		reg_offset = fpos_offset / map->debugfs_tot_len;
-		c->max_reg = c->base_reg + reg_offset;
+		c->max_reg = c->base_reg + (reg_offset * map->reg_stride);
 		list_add_tail(&c->list,
 			      &map->debugfs_off_cache);
 	}
@@ -145,7 +146,7 @@ static unsigned int regmap_debugfs_get_dump_start(struct regmap *map,
 			fpos_offset = from - c->min;
 			reg_offset = fpos_offset / map->debugfs_tot_len;
 			*pos = c->min + (reg_offset * map->debugfs_tot_len);
-			return c->base_reg + reg_offset;
+			return c->base_reg + (reg_offset * map->reg_stride);
 		}
 
 		*pos = c->max;
-- 
1.7.6.5

Re: [PATCH v2] regmap: debugfs: Fix start_reg calculation (v2)

[Mark Brown]

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

On Wed, May 08, 2013 at 11:20:09AM +0100, Srinivas KANDAGATLA wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> 
> If we dump syscon regmap registers via debugfs you will notice that the
> dump contains lot of XXXXXXXX values.

Applied, thanks.

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

Re: [PATCH v2] regmap: debugfs: Fix start_reg calculation (v2)

[Mark Brown]

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

On Wed, May 08, 2013 at 11:20:09AM +0100, Srinivas KANDAGATLA wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> 
> If we dump syscon regmap registers via debugfs you will notice that the
> dump contains lot of XXXXXXXX values.

Sorry, can you please rebase this against v3.10-rc1?  This patch was
against v3.9 and I'm not now convinced that the issue still exists.

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

Re: [PATCH v2] regmap: debugfs: Fix start_reg calculation (v2)

[Srinivas KANDAGATLA]

Hi Mark,
On 12/05/13 15:59, Mark Brown wrote:
> On Wed, May 08, 2013 at 11:20:09AM +0100, Srinivas KANDAGATLA wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>>
>> If we dump syscon regmap registers via debugfs you will notice that the
>> dump contains lot of XXXXXXXX values.
> 
> Sorry, can you please rebase this against v3.10-rc1?  This patch was
> against v3.9 and I'm not now convinced that the issue still exists.
> 
I did try 3.10-rc1, and I can not reproduce the issue.
very similar patch "regmap: debugfs: Simplify calculation of
`c->max_reg'" addressed the issue.

However It looks like one of the return from the
regmap_debugfs_get_dump_start() still returns wrong register offset.

<snip>
if (from >= c->min && from <= c->max) {
       fpos_offset = from - c->min;
       reg_offset = fpos_offset / map->debugfs_tot_len;
       *pos = c->min + (reg_offset * map->debugfs_tot_len);
       mutex_unlock(&map->cache_lock);
       return c->base_reg + reg_offset;
}
</snip>

Thanks,
srini

Re: [PATCH v2] regmap: debugfs: Fix start_reg calculation (v2)

[Mark Brown]

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

On Mon, May 13, 2013 at 07:53:33AM +0100, Srinivas KANDAGATLA wrote:

> I did try 3.10-rc1, and I can not reproduce the issue.
> very similar patch "regmap: debugfs: Simplify calculation of
> `c->max_reg'" addressed the issue.

OK, that's what I thought.

> However It looks like one of the return from the
> regmap_debugfs_get_dump_start() still returns wrong register offset.

Please send a patch for that.

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