[PATCH 1/1] regmap: Add support for continously numbered pages across regmap range

[PATCH 1/1] regmap: Add support for continously numbered pages across regmap range

[Mukund Navada]

  Page number in some devices increases across regmap ranges since the
selector register to choose the page remains the same. So in order to
write correct page number, the start_page_num is stored in
regmap_range_cfg.

Signed-off-by: Mukund Navada <navada@ti.com>
---
 drivers/base/regmap/internal.h |    2 ++
 drivers/base/regmap/regmap.c   |    2 ++
 include/linux/regmap.h         |    5 +++++
 3 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 288e135..fd31f5d 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -134,6 +134,8 @@ struct regmap_range_node {
 
 	unsigned int window_start;
 	unsigned int window_len;
+
+	unsigned int start_page_num;
 };
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 42d5cb0..39e37a4 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -645,6 +645,7 @@ struct regmap *regmap_init(struct device *dev,
 		new->selector_shift = range_cfg->selector_shift;
 		new->window_start = range_cfg->window_start;
 		new->window_len = range_cfg->window_len;
+		new->start_page_num = range_cfg->start_page_num;
 
 		if (_regmap_range_add(map, new) == false) {
 			dev_err(map->dev, "Failed to add range %d\n", i);
@@ -833,6 +834,7 @@ static int _regmap_select_page(struct regmap *map, unsigned int *reg,
 
 	win_offset = (*reg - range->range_min) % range->window_len;
 	win_page = (*reg - range->range_min) / range->window_len;
+	win_page += range->start_page_num;
 
 	if (val_num > 1) {
 		/* Bulk write shouldn't cross range boundary */
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index b7e95bf..96f029e 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -217,6 +217,8 @@ struct regmap_config {
  *
  * @window_start: Address of first (lowest) register in data window.
  * @window_len: Number of registers in data window.
+ *
+ * @start_page_num: Start page number for the range
  */
 struct regmap_range_cfg {
 	const char *name;
@@ -233,6 +235,9 @@ struct regmap_range_cfg {
 	/* Data window (per each page) */
 	unsigned int window_start;
 	unsigned int window_len;
+
+	/* Starting page number when it increases across range */
+	unsigned int start_page_num;
 };
 
 typedef int (*regmap_hw_write)(void *context, const void *data,
-- 
1.7.1

Re: [PATCH 1/1] regmap: Add support for continously numbered pages across regmap range

[Mark Brown]

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

On Wed, Nov 28, 2012 at 02:52:57PM +0530, Mukund Navada wrote:
>   Page number in some devices increases across regmap ranges since the
> selector register to choose the page remains the same. So in order to
> write correct page number, the start_page_num is stored in
> regmap_range_cfg.

I'm sorry but I can't understand what this is supposed to do.  The
current paging code expects pages to be numbered continuously with no
gaps.  Can you please clarify?

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

RE: [PATCH 1/1] regmap: Add support for continously numbered pages across regmap range

[Navada Kanyana, Mukund]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Wednesday, November 28, 2012 2:59 PM
> To: Navada Kanyana, Mukund
> Cc: Greg Kroah-Hartman; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/1] regmap: Add support for continously numbered
> pages across regmap range
> 
> On Wed, Nov 28, 2012 at 02:52:57PM +0530, Mukund Navada wrote:
> >   Page number in some devices increases across regmap ranges since the
> > selector register to choose the page remains the same. So in order to
> > write correct page number, the start_page_num is stored in
> > regmap_range_cfg.
> 
> I'm sorry but I can't understand what this is supposed to do.  The
> current paging code expects pages to be numbered continuously with no
> gaps.  Can you please clarify?
But the page number is expected to increase only till end of range. But in devices like TLV320AIC3256, the page number increases beyond range.

Example:

Pages 0 through Page 1 --> first register range for device configuration. Page 8 through Page 16 --> second range for DSP1 coefficient memory
Page 26 through Page 34 --> third range for DSP2 coefficient memory

Window length of each page is 128. So if we need to correctly write the page number in selector register for second range, start_page_num would be 8. If we did not have start_page_num, it would write 0 to selector register. 

Re: [PATCH 1/1] regmap: Add support for continously numbered pages across regmap range

[Mark Brown]

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

On Wed, Nov 28, 2012 at 10:11:05AM +0000, Navada Kanyana, Mukund wrote:

Fix your mailer to word wrap within paragraphs.

> > I'm sorry but I can't understand what this is supposed to do.  The
> > current paging code expects pages to be numbered continuously with no
> > gaps.  Can you please clarify?

> But the page number is expected to increase only till end of range.
> But in devices like TLV320AIC3256, the page number increases beyond
> range.

What does this mean?  How can we have a page beyond the end of the
range?

> Pages 0 through Page 1 --> first register range for device
> configuration. Page 8 through Page 16 --> second range for DSP1
> coefficient memory Page 26 through Page 34 --> third range for DSP2
> coefficient memory

> Window length of each page is 128. So if we need to correctly write
> the page number in selector register for second range, start_page_num
> would be 8. If we did not have start_page_num, it would write 0 to
> selector register. 

I'm sorry but I don't understand how that would happen?  What breaks
accessing page 8?  This sounds like you're doing something wrong when
you're trying ton use the paging.

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

RE: [PATCH 1/1] regmap: Add support for continously numbered pages across regmap range

[Navada Kanyana, Mukund]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Wednesday, November 28, 2012 5:05 PM
> To: Navada Kanyana, Mukund
> Cc: Greg Kroah-Hartman; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/1] regmap: Add support for continously numbered
> pages across regmap range
> 
> On Wed, Nov 28, 2012 at 10:11:05AM +0000, Navada Kanyana, Mukund wrote:
> 
> Fix your mailer to word wrap within paragraphs.
> 
> > > I'm sorry but I can't understand what this is supposed to do.  The
> > > current paging code expects pages to be numbered continuously with no
> > > gaps.  Can you please clarify?
> 
> > But the page number is expected to increase only till end of range.
> > But in devices like TLV320AIC3256, the page number increases beyond
> > range.
> 
> What does this mean?  How can we have a page beyond the end of the
> range?
> 
> > Pages 0 through Page 1 --> first register range for device
> > configuration. Page 8 through Page 16 --> second range for DSP1
> > coefficient memory Page 26 through Page 34 --> third range for DSP2
> > coefficient memory
> 
> > Window length of each page is 128. So if we need to correctly write
> > the page number in selector register for second range, start_page_num
> > would be 8. If we did not have start_page_num, it would write 0 to
> > selector register.
> 
> I'm sorry but I don't understand how that would happen?  What breaks
> accessing page 8?  This sounds like you're doing something wrong when
> you're trying ton use the paging.

I have my regmap_range_cfg as below

#define AIC325X_DSP_WINDOW_LEN  128
#define AIC325X_ADSP_CM_A_BASE  (9 * AIC325X_DSP_WINDOW_LEN)
static const struct regmap_range_cfg aic325x_ranges[] = {
        { .name = "CONF", .range_min = 128,
          .range_max = 383,
          .selector_reg = 0x0,
          .selector_mask = 0xff,
          .selector_shift = 0,
          .window_start = 0, .window_len =128,
        },
        { .name = "MINIDSP_A_CM_A", .range_min = AIC325X_ADSP_CM_A_BASE,
          .range_max = AIC325X_ADSP_CM_A_BASE + 1152,
          .selector_reg = 0x0,
          .selector_mask = 0xff,
          .selector_shift = 0,
          .window_start = 0, .window_len =128,
        },
};

In _regmap_select_page function that calculates win_page 

win_page = (*reg - range->range_min) / range->window_len;

So when my *reg is within MINIDSP_A_CM_A (that corresponds to Page 8), the above win_page calculation would be 0 instead of 8 for *reg value of AIC325X_ADSP_CM_A_BASE.

Re: [PATCH 1/1] regmap: Add support for continously numbered pages across regmap range

[Mark Brown]

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

On Wed, Nov 28, 2012 at 11:52:22AM +0000, Navada Kanyana, Mukund wrote:

> static const struct regmap_range_cfg aic325x_ranges[] = {
>         { .name = "CONF", .range_min = 128,
>           .range_max = 383,
>           .selector_reg = 0x0,
>           .selector_mask = 0xff,
>           .selector_shift = 0,
>           .window_start = 0, .window_len =128,
>         },
>         { .name = "MINIDSP_A_CM_A", .range_min = AIC325X_ADSP_CM_A_BASE,
>           .range_max = AIC325X_ADSP_CM_A_BASE + 1152,
>           .selector_reg = 0x0,
>           .selector_mask = 0xff,
>           .selector_shift = 0,
>           .window_start = 0, .window_len =128,
>         },
> };

You appear to have two different ranges with the same selector and
window.  I'm really not sure why you expect this to work...  what
happens when someone reads from the two different ranges simultaneously?

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

RE: [PATCH 1/1] regmap: Add support for continously numbered pages across regmap range

[Navada Kanyana, Mukund]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Wednesday, November 28, 2012 5:27 PM
> To: Navada Kanyana, Mukund
> Cc: Greg Kroah-Hartman; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/1] regmap: Add support for continously numbered
> pages across regmap range
> 
> On Wed, Nov 28, 2012 at 11:52:22AM +0000, Navada Kanyana, Mukund wrote:
> 
> > static const struct regmap_range_cfg aic325x_ranges[] = {
> >         { .name = "CONF", .range_min = 128,
> >           .range_max = 383,
> >           .selector_reg = 0x0,
> >           .selector_mask = 0xff,
> >           .selector_shift = 0,
> >           .window_start = 0, .window_len =128,
> >         },
> >         { .name = "MINIDSP_A_CM_A", .range_min = AIC325X_ADSP_CM_A_BASE,
> >           .range_max = AIC325X_ADSP_CM_A_BASE + 1152,
> >           .selector_reg = 0x0,
> >           .selector_mask = 0xff,
> >           .selector_shift = 0,
> >           .window_start = 0, .window_len =128,
> >         },
> > };
> 
> You appear to have two different ranges with the same selector and
> window.  I'm really not sure why you expect this to work...  what
> happens when someone reads from the two different ranges simultaneously?
The selector register has to written appropriate page number. Two different ranges cannot be accessed simultaneously without changing the page number in the selector register.

Re: [PATCH 1/1] regmap: Add support for continously numbered pages across regmap range

[Mark Brown]

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

On Wed, Nov 28, 2012 at 12:10:21PM +0000, Navada Kanyana, Mukund wrote:

To repeat, fix your mailer to word wrap within paragraphs.

> > You appear to have two different ranges with the same selector and
> > window.  I'm really not sure why you expect this to work...  what
> > happens when someone reads from the two different ranges simultaneously?

> The selector register has to written appropriate page number. Two
> different ranges cannot be accessed simultaneously without changing
> the page number in the selector register.

This would be exactly the problem, yes.  In any case, this looks like
user error.  You shouldn't be using the same window twice.

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