[PATCH 1/4] regmap: cache: Factor out reg_present support from rbtree cache

[PATCH 1/4] regmap: cache: Factor out reg_present support from rbtree cache

[Mark Brown]

The idea of maintaining a bitmap of present registers is something that
can usefully be used by other cache types that maintain blocks of cached
registers so move the code out of the rbtree cache and into the generic
regcache code.

Refactor the interface slightly as we go to wrap the set bit and enlarge
bitmap operations (since we never do one without the other) and make it
more robust for reads of uncached registers by bounds checking before we
look at the bitmap.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/internal.h        |   13 +++++++
 drivers/base/regmap/regcache-rbtree.c |   60 ++-------------------------------
 drivers/base/regmap/regcache.c        |   39 +++++++++++++++++++++
 3 files changed, 54 insertions(+), 58 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 663abf0..bd9e164 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -127,6 +127,9 @@ struct regmap {
 	void *cache;
 	u32 cache_dirty;
 
+	unsigned long *cache_present;
+	unsigned int cache_present_nbits;
+
 	struct reg_default *patch;
 	int patch_regs;
 
@@ -202,6 +205,16 @@ unsigned int regcache_get_val(struct regmap *map, const void *base,
 bool regcache_set_val(struct regmap *map, void *base, unsigned int idx,
 		      unsigned int val);
 int regcache_lookup_reg(struct regmap *map, unsigned int reg);
+int regcache_set_reg_present(struct regmap *map, unsigned int reg);
+
+static inline bool regcache_reg_present(struct regmap *map, unsigned int reg)
+{
+	if (!map->cache_present)
+		return true;
+	if (reg > map->cache_present_nbits)
+		return false;
+	return map->cache_present[BIT_WORD(reg)] & BIT_MASK(reg);
+}
 
 int _regmap_raw_write(struct regmap *map, unsigned int reg,
 		      const void *val, size_t val_len, bool async);
diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
index 2635a8f..545153a 100644
--- a/drivers/base/regmap/regcache-rbtree.c
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -36,8 +36,6 @@ struct regcache_rbtree_node {
 struct regcache_rbtree_ctx {
 	struct rb_root root;
 	struct regcache_rbtree_node *cached_rbnode;
-	unsigned long *reg_present;
-	unsigned int reg_present_nbits;
 };
 
 static inline void regcache_rbtree_get_base_top_reg(
@@ -154,7 +152,7 @@ static int rbtree_show(struct seq_file *s, void *ignored)
 	map->lock(map);
 
 	mem_size = sizeof(*rbtree_ctx);
-	mem_size += BITS_TO_LONGS(rbtree_ctx->reg_present_nbits) * sizeof(long);
+	mem_size += BITS_TO_LONGS(map->cache_present_nbits) * sizeof(long);
 
 	for (node = rb_first(&rbtree_ctx->root); node != NULL;
 	     node = rb_next(node)) {
@@ -205,44 +203,6 @@ static void rbtree_debugfs_init(struct regmap *map)
 }
 #endif
 
-static int enlarge_reg_present_bitmap(struct regmap *map, unsigned int reg)
-{
-	struct regcache_rbtree_ctx *rbtree_ctx;
-	unsigned long *reg_present;
-	unsigned int reg_present_size;
-	unsigned int nregs;
-	int i;
-
-	rbtree_ctx = map->cache;
-	nregs = reg + 1;
-	reg_present_size = BITS_TO_LONGS(nregs);
-	reg_present_size *= sizeof(long);
-
-	if (!rbtree_ctx->reg_present) {
-		reg_present = kmalloc(reg_present_size, GFP_KERNEL);
-		if (!reg_present)
-			return -ENOMEM;
-		bitmap_zero(reg_present, nregs);
-		rbtree_ctx->reg_present = reg_present;
-		rbtree_ctx->reg_present_nbits = nregs;
-		return 0;
-	}
-
-	if (nregs > rbtree_ctx->reg_present_nbits) {
-		reg_present = krealloc(rbtree_ctx->reg_present,
-				       reg_present_size, GFP_KERNEL);
-		if (!reg_present)
-			return -ENOMEM;
-		for (i = 0; i < nregs; i++)
-			if (i >= rbtree_ctx->reg_present_nbits)
-				clear_bit(i, reg_present);
-		rbtree_ctx->reg_present = reg_present;
-		rbtree_ctx->reg_present_nbits = nregs;
-	}
-
-	return 0;
-}
-
 static int regcache_rbtree_init(struct regmap *map)
 {
 	struct regcache_rbtree_ctx *rbtree_ctx;
@@ -256,8 +216,6 @@ static int regcache_rbtree_init(struct regmap *map)
 	rbtree_ctx = map->cache;
 	rbtree_ctx->root = RB_ROOT;
 	rbtree_ctx->cached_rbnode = NULL;
-	rbtree_ctx->reg_present = NULL;
-	rbtree_ctx->reg_present_nbits = 0;
 
 	for (i = 0; i < map->num_reg_defaults; i++) {
 		ret = regcache_rbtree_write(map,
@@ -287,8 +245,6 @@ static int regcache_rbtree_exit(struct regmap *map)
 	if (!rbtree_ctx)
 		return 0;
 
-	kfree(rbtree_ctx->reg_present);
-
 	/* free up the rbtree */
 	next = rb_first(&rbtree_ctx->root);
 	while (next) {
@@ -306,17 +262,6 @@ static int regcache_rbtree_exit(struct regmap *map)
 	return 0;
 }
 
-static int regcache_reg_present(struct regmap *map, unsigned int reg)
-{
-	struct regcache_rbtree_ctx *rbtree_ctx;
-
-	rbtree_ctx = map->cache;
-	if (!(rbtree_ctx->reg_present[BIT_WORD(reg)] & BIT_MASK(reg)))
-		return 0;
-	return 1;
-
-}
-
 static int regcache_rbtree_read(struct regmap *map,
 				unsigned int reg, unsigned int *value)
 {
@@ -378,10 +323,9 @@ static int regcache_rbtree_write(struct regmap *map, unsigned int reg,
 
 	rbtree_ctx = map->cache;
 	/* update the reg_present bitmap, make space if necessary */
-	ret = enlarge_reg_present_bitmap(map, reg);
+	ret = regcache_set_reg_present(map, reg);
 	if (ret < 0)
 		return ret;
-	set_bit(reg, rbtree_ctx->reg_present);
 
 	/* if we can't locate it in the cached rbnode we'll have
 	 * to traverse the rbtree looking for it.
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 229c804..0fedf4f 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -121,6 +121,8 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
 	map->reg_defaults_raw = config->reg_defaults_raw;
 	map->cache_word_size = DIV_ROUND_UP(config->val_bits, 8);
 	map->cache_size_raw = map->cache_word_size * config->num_reg_defaults_raw;
+	map->cache_present = NULL;
+	map->cache_present_nbits = 0;
 
 	map->cache = NULL;
 	map->cache_ops = cache_types[i];
@@ -179,6 +181,7 @@ void regcache_exit(struct regmap *map)
 
 	BUG_ON(!map->cache_ops);
 
+	kfree(map->cache_present);
 	kfree(map->reg_defaults);
 	if (map->cache_free)
 		kfree(map->reg_defaults_raw);
@@ -415,6 +418,42 @@ void regcache_cache_bypass(struct regmap *map, bool enable)
 }
 EXPORT_SYMBOL_GPL(regcache_cache_bypass);
 
+int regcache_set_reg_present(struct regmap *map, unsigned int reg)
+{
+	unsigned long *cache_present;
+	unsigned int cache_present_size;
+	unsigned int nregs;
+	int i;
+
+	nregs = reg + 1;
+	cache_present_size = BITS_TO_LONGS(nregs);
+	cache_present_size *= sizeof(long);
+
+	if (!map->cache_present) {
+		cache_present = kmalloc(cache_present_size, GFP_KERNEL);
+		if (!cache_present)
+			return -ENOMEM;
+		bitmap_zero(cache_present, nregs);
+		map->cache_present = cache_present;
+		map->cache_present_nbits = nregs;
+	}
+
+	if (nregs > map->cache_present_nbits) {
+		cache_present = krealloc(map->cache_present,
+					 cache_present_size, GFP_KERNEL);
+		if (!cache_present)
+			return -ENOMEM;
+		for (i = 0; i < nregs; i++)
+			if (i >= map->cache_present_nbits)
+				clear_bit(i, cache_present);
+		map->cache_present = cache_present;
+		map->cache_present_nbits = nregs;
+	}
+
+	set_bit(reg, map->cache_present);
+	return 0;
+}
+
 bool regcache_set_val(struct regmap *map, void *base, unsigned int idx,
 		      unsigned int val)
 {
-- 
1.7.10.4

[PATCH 2/4] regmap: cache: Factor out block sync

[Mark Brown]

The idea of holding blocks of registers in device format is shared between
at least rbtree and lzo cache formats so split out the loop that does the
sync from the rbtree code so optimisations on it can be reused.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/internal.h        |    3 +++
 drivers/base/regmap/regcache-rbtree.c |   48 +++++----------------------------
 drivers/base/regmap/regcache.c        |   42 +++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 42 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index bd9e164..c130536 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -192,6 +192,9 @@ int regcache_read(struct regmap *map,
 int regcache_write(struct regmap *map,
 			unsigned int reg, unsigned int value);
 int regcache_sync(struct regmap *map);
+int regcache_sync_block(struct regmap *map, void *block,
+			unsigned int block_base, unsigned int start,
+			unsigned int end);
 
 static inline const void *regcache_get_val_addr(struct regmap *map,
 						const void *base,
diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
index 545153a..aa0875f 100644
--- a/drivers/base/regmap/regcache-rbtree.c
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -53,12 +53,6 @@ static unsigned int regcache_rbtree_get_register(struct regmap *map,
 	return regcache_get_val(map, rbnode->block, idx);
 }
 
-static const void *regcache_rbtree_get_reg_addr(struct regmap *map,
-	struct regcache_rbtree_node *rbnode, unsigned int idx)
-{
-	return regcache_get_val_addr(map, rbnode->block, idx);
-}
-
 static void regcache_rbtree_set_register(struct regmap *map,
 					 struct regcache_rbtree_node *rbnode,
 					 unsigned int idx, unsigned int val)
@@ -390,11 +384,8 @@ static int regcache_rbtree_sync(struct regmap *map, unsigned int min,
 	struct regcache_rbtree_ctx *rbtree_ctx;
 	struct rb_node *node;
 	struct regcache_rbtree_node *rbnode;
-	unsigned int regtmp;
-	unsigned int val;
-	const void *addr;
 	int ret;
-	int i, base, end;
+	int base, end;
 
 	rbtree_ctx = map->cache;
 	for (node = rb_first(&rbtree_ctx->root); node; node = rb_next(node)) {
@@ -417,40 +408,13 @@ static int regcache_rbtree_sync(struct regmap *map, unsigned int min,
 		else
 			end = rbnode->blklen;
 
-		for (i = base; i < end; i++) {
-			regtmp = rbnode->base_reg + (i * map->reg_stride);
-
-			if (!regcache_reg_present(map, regtmp))
-				continue;
-
-			val = regcache_rbtree_get_register(map, rbnode, i);
-
-			/* Is this the hardware default?  If so skip. */
-			ret = regcache_lookup_reg(map, regtmp);
-			if (ret >= 0 && val == map->reg_defaults[ret].def)
-				continue;
-
-			map->cache_bypass = 1;
-
-			if (regmap_can_raw_write(map)) {
-				addr = regcache_rbtree_get_reg_addr(map,
-								    rbnode, i);
-				ret = _regmap_raw_write(map, regtmp, addr,
-							map->format.val_bytes,
-							false);
-			} else {
-				ret = _regmap_write(map, regtmp, val);
-			}
-
-			map->cache_bypass = 0;
-			if (ret)
-				return ret;
-			dev_dbg(map->dev, "Synced register %#x, value %#x\n",
-				regtmp, val);
-		}
+		ret = regcache_sync_block(map, rbnode->block, rbnode->base_reg,
+					  base, end);
+		if (ret != 0)
+			return ret;
 	}
 
-	return 0;
+	return regmap_async_complete(map);
 }
 
 struct regcache_ops regcache_rbtree_ops = {
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 0fedf4f..bb317db 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -544,3 +544,45 @@ int regcache_lookup_reg(struct regmap *map, unsigned int reg)
 	else
 		return -ENOENT;
 }
+
+int regcache_sync_block(struct regmap *map, void *block,
+			unsigned int block_base, unsigned int start,
+			unsigned int end)
+{
+	unsigned int i, regtmp, val;
+	const void *addr;
+	int ret;
+
+	for (i = start; i < end; i++) {
+		regtmp = block_base + (i * map->reg_stride);
+
+		if (!regcache_reg_present(map, regtmp))
+			continue;
+
+		val = regcache_get_val(map, block, i);
+
+		/* Is this the hardware default?  If so skip. */
+		ret = regcache_lookup_reg(map, regtmp);
+		if (ret >= 0 && val == map->reg_defaults[ret].def)
+			continue;
+
+		map->cache_bypass = 1;
+
+		if (regmap_can_raw_write(map)) {
+			addr = regcache_get_val_addr(map, block, i);
+			ret = _regmap_raw_write(map, regtmp, addr,
+						map->format.val_bytes,
+						false);
+		} else {
+			ret = _regmap_write(map, regtmp, val);
+		}
+
+		map->cache_bypass = 0;
+		if (ret != 0)
+			return ret;
+		dev_dbg(map->dev, "Synced register %#x, value %#x\n",
+			regtmp, val);
+	}
+
+	return 0;
+}
-- 
1.7.10.4

[PATCH 3/4] regmap: cache: Split raw and non-raw syncs

[Mark Brown]

For code clarity after implementing block writes split out the raw and
non-raw I/O sync implementations.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regcache.c |   64 +++++++++++++++++++++++++++++++++-------
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index bb317db..da4d984 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -545,9 +545,43 @@ int regcache_lookup_reg(struct regmap *map, unsigned int reg)
 		return -ENOENT;
 }
 
-int regcache_sync_block(struct regmap *map, void *block,
-			unsigned int block_base, unsigned int start,
-			unsigned int end)
+static int regcache_sync_block_single(struct regmap *map, void *block,
+				      unsigned int block_base,
+				      unsigned int start, unsigned int end)
+{
+	unsigned int i, regtmp, val;
+	int ret;
+
+	for (i = start; i < end; i++) {
+		regtmp = block_base + (i * map->reg_stride);
+
+		if (!regcache_reg_present(map, regtmp))
+			continue;
+
+		val = regcache_get_val(map, block, i);
+
+		/* Is this the hardware default?  If so skip. */
+		ret = regcache_lookup_reg(map, regtmp);
+		if (ret >= 0 && val == map->reg_defaults[ret].def)
+			continue;
+
+		map->cache_bypass = 1;
+
+		ret = _regmap_write(map, regtmp, val);
+
+		map->cache_bypass = 0;
+		if (ret != 0)
+			return ret;
+		dev_dbg(map->dev, "Synced register %#x, value %#x\n",
+			regtmp, val);
+	}
+
+	return 0;
+}
+
+int regcache_sync_block_raw(struct regmap *map, void *block,
+			    unsigned int block_base, unsigned int start,
+			    unsigned int end)
 {
 	unsigned int i, regtmp, val;
 	const void *addr;
@@ -568,14 +602,10 @@ int regcache_sync_block(struct regmap *map, void *block,
 
 		map->cache_bypass = 1;
 
-		if (regmap_can_raw_write(map)) {
-			addr = regcache_get_val_addr(map, block, i);
-			ret = _regmap_raw_write(map, regtmp, addr,
-						map->format.val_bytes,
-						false);
-		} else {
-			ret = _regmap_write(map, regtmp, val);
-		}
+		addr = regcache_get_val_addr(map, block, i);
+		ret = _regmap_raw_write(map, regtmp, addr,
+					map->format.val_bytes,
+					false);
 
 		map->cache_bypass = 0;
 		if (ret != 0)
@@ -586,3 +616,15 @@ int regcache_sync_block(struct regmap *map, void *block,
 
 	return 0;
 }
+
+int regcache_sync_block(struct regmap *map, void *block,
+			unsigned int block_base, unsigned int start,
+			unsigned int end)
+{
+	if (regmap_can_raw_write(map))
+		return regcache_sync_block_raw(map, block, block_base,
+					       start, end);
+	else
+		return regcache_sync_block_single(map, block, block_base,
+						  start, end);
+}
-- 
1.7.10.4

[PATCH 4/4] regmap: cache: Write consecutive registers in a single block write

[Mark Brown]

When syncing blocks of data using raw writes combine the writes into a
single block write, saving us bus overhead for setup, addressing and
teardown.

Currently the block write is done unconditionally as it is expected that
hardware which has a register format which can support raw writes will
support auto incrementing writes, this decision may need to be revised in
future.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regcache.c |   64 +++++++++++++++++++++++++++++-----------
 1 file changed, 47 insertions(+), 17 deletions(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index da4d984..d81f605 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -579,42 +579,72 @@ static int regcache_sync_block_single(struct regmap *map, void *block,
 	return 0;
 }
 
+static int regcache_sync_block_raw_flush(struct regmap *map, const void **data,
+					 unsigned int base, unsigned int cur)
+{
+	size_t val_bytes = map->format.val_bytes;
+	int ret, count;
+
+	if (*data == NULL)
+		return 0;
+
+	count = cur - base;
+
+	dev_dbg(map->dev, "Writing %d bytes for %d registers from 0x%x-0x%x\n",
+		count * val_bytes, count, base, cur - 1);
+
+	map->cache_bypass = 1;
+
+	ret = _regmap_raw_write(map, base, *data, count * val_bytes,
+				false);
+
+	map->cache_bypass = 0;
+
+	*data = NULL;
+
+	return ret;
+}
+
 int regcache_sync_block_raw(struct regmap *map, void *block,
 			    unsigned int block_base, unsigned int start,
 			    unsigned int end)
 {
-	unsigned int i, regtmp, val;
-	const void *addr;
+	unsigned int i, val;
+	unsigned int regtmp = 0;
+	unsigned int base = 0;
+	const void *data = NULL;
 	int ret;
 
 	for (i = start; i < end; i++) {
 		regtmp = block_base + (i * map->reg_stride);
 
-		if (!regcache_reg_present(map, regtmp))
+		if (!regcache_reg_present(map, regtmp)) {
+			ret = regcache_sync_block_raw_flush(map, &data,
+							    base, regtmp);
+			if (ret != 0)
+				return ret;
 			continue;
+		}
 
 		val = regcache_get_val(map, block, i);
 
 		/* Is this the hardware default?  If so skip. */
 		ret = regcache_lookup_reg(map, regtmp);
-		if (ret >= 0 && val == map->reg_defaults[ret].def)
+		if (ret >= 0 && val == map->reg_defaults[ret].def) {
+			ret = regcache_sync_block_raw_flush(map, &data,
+							    base, regtmp);
+			if (ret != 0)
+				return ret;
 			continue;
+		}
 
-		map->cache_bypass = 1;
-
-		addr = regcache_get_val_addr(map, block, i);
-		ret = _regmap_raw_write(map, regtmp, addr,
-					map->format.val_bytes,
-					false);
-
-		map->cache_bypass = 0;
-		if (ret != 0)
-			return ret;
-		dev_dbg(map->dev, "Synced register %#x, value %#x\n",
-			regtmp, val);
+		if (!data) {
+			data = regcache_get_val_addr(map, block, i);
+			base = regtmp;
+		}
 	}
 
-	return 0;
+	return regcache_sync_block_raw_flush(map, &data, base, regtmp);
 }
 
 int regcache_sync_block(struct regmap *map, void *block,
-- 
1.7.10.4

Re: [PATCH 1/4] regmap: cache: Factor out reg_present support from rbtree cache

[Dimitris Papastamos]

On Fri, Mar 29, 2013 at 09:03:42PM +0000, Mark Brown wrote:
> The idea of maintaining a bitmap of present registers is something that
> can usefully be used by other cache types that maintain blocks of cached
> registers so move the code out of the rbtree cache and into the generic
> regcache code.
> 
> Refactor the interface slightly as we go to wrap the set bit and enlarge
> bitmap operations (since we never do one without the other) and make it
> more robust for reads of uncached registers by bounds checking before we
> look at the bitmap.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Looks good.

Reviewed-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

Re: [PATCH 2/4] regmap: cache: Factor out block sync

[Dimitris Papastamos]

On Fri, Mar 29, 2013 at 09:03:43PM +0000, Mark Brown wrote:
> The idea of holding blocks of registers in device format is shared between
> at least rbtree and lzo cache formats so split out the loop that does the
> sync from the rbtree code so optimisations on it can be reused.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Reviewed-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

Re: [PATCH 3/4] regmap: cache: Split raw and non-raw syncs

[Dimitris Papastamos]

On Fri, Mar 29, 2013 at 09:03:44PM +0000, Mark Brown wrote:
> For code clarity after implementing block writes split out the raw and
> non-raw I/O sync implementations.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Reviewed-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

Re: [PATCH 4/4] regmap: cache: Write consecutive registers in a single block write

[Dimitris Papastamos]

On Fri, Mar 29, 2013 at 09:03:45PM +0000, Mark Brown wrote:
> When syncing blocks of data using raw writes combine the writes into a
> single block write, saving us bus overhead for setup, addressing and
> teardown.
> 
> Currently the block write is done unconditionally as it is expected that
> hardware which has a register format which can support raw writes will
> support auto incrementing writes, this decision may need to be revised in
> future.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Reviewed-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>