[PATCH 0/8] Introduce caching support for regmap

[PATCH 0/8] Introduce caching support for regmap

[Dimitris Papastamos]

This patch series introduces register caching support for regmap.
Some things have been left out, such as support for bulk read operations,
various optimizations, shared caches etc.
Most of this stuff will be implemented incrementally.

Dimitris Papastamos (8):
  regmap: Introduce caching support
  regmap: Add the indexed cache support
  regmap: Add the rbtree cache support
  regmap: Add the LZO cache support
  regmap: Add the regcache_sync trace event
  regmap: Incorporate the regcache core into regmap
  regmap: It is impossible to be given a NULL defaults cache
  regmap: Support NULL cache_defaults_raw

 drivers/base/regmap/Makefile           |    2 +-
 drivers/base/regmap/internal.h         |   56 +++++
 drivers/base/regmap/regcache-indexed.c |   65 +++++
 drivers/base/regmap/regcache-lzo.c     |  361 ++++++++++++++++++++++++++++
 drivers/base/regmap/regcache-rbtree.c  |  400 ++++++++++++++++++++++++++++++++
 drivers/base/regmap/regcache.c         |  287 +++++++++++++++++++++++
 drivers/base/regmap/regmap.c           |   55 +++++
 include/linux/regmap.h                 |   22 ++-
 include/trace/events/regmap.h          |   24 ++
 9 files changed, 1266 insertions(+), 6 deletions(-)
 create mode 100644 drivers/base/regmap/regcache-indexed.c
 create mode 100644 drivers/base/regmap/regcache-lzo.c
 create mode 100644 drivers/base/regmap/regcache-rbtree.c
 create mode 100644 drivers/base/regmap/regcache.c

-- 
1.7.6.1

[PATCH 1/8] regmap: Introduce caching support

[Dimitris Papastamos]

This patch introduces caching support for regmap.  The regcache API
has evolved essentially out of ASoC soc-cache so most of the actual
caching types (except LZO) have been tested in the past.

The purpose of regcache is to optimize in time and space the handling
of register caches.  Time optimization is achieved by not having to go
over a slow bus like I2C to read the value of a register, instead it is
cached locally in memory and can be retrieved faster.  Regarding space
optimization, some of the cache types are better at packing the caches,
for e.g. the rbtree and the LZO caches.  By doing this the sacrifice in
time still wins over doing I2C transactions.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/Makefile   |    2 +-
 drivers/base/regmap/internal.h |   50 ++++++++
 drivers/base/regmap/regcache.c |  251 ++++++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h         |   19 +++-
 4 files changed, 316 insertions(+), 6 deletions(-)
 create mode 100644 drivers/base/regmap/regcache.c

diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 057c13f..2e103ea 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_REGMAP) += regmap.o
+obj-$(CONFIG_REGMAP) += regmap.o regcache.o
 obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o
 obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o
 obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 5ab3fef..b7acbeb 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -17,6 +17,7 @@
 #include <linux/fs.h>
 
 struct regmap;
+struct regcache_ops;
 
 struct regmap_format {
 	size_t buf_size;
@@ -46,6 +47,38 @@ struct regmap {
 	bool (*readable_reg)(struct device *dev, unsigned int reg);
 	bool (*volatile_reg)(struct device *dev, unsigned int reg);
 	bool (*precious_reg)(struct device *dev, unsigned int reg);
+
+	/* regcache specific members */
+	const struct regcache_ops *cache_ops;
+	enum regcache_type cache_type;
+
+	/* number of bytes in cache_defaults_raw */
+	unsigned int cache_size_raw;
+	/* number of bytes per word in cache_defaults_raw */
+	unsigned int cache_word_size;
+	/* number of entries in cache_defaults */
+	unsigned int num_cache_defaults;
+	/* number of entries in cache_defaults_raw */
+	unsigned int num_cache_defaults_raw;
+
+	/* if set, only the cache is modified not the HW */
+	unsigned int cache_only:1;
+	/* if set, only the HW is modified not the cache */
+	unsigned int cache_bypass:1;
+
+	struct reg_default *cache_defaults;
+	const void *cache_defaults_raw;
+	void *cache;
+};
+
+struct regcache_ops {
+	const char *name;
+	enum regcache_type type;
+	int (*init)(struct regmap *map);
+	int (*exit)(struct regmap *map);
+	int (*read)(struct regmap *map, unsigned int reg, unsigned int *value);
+	int (*write)(struct regmap *map, unsigned int reg, unsigned int value);
+	int (*sync)(struct regmap *map);
 };
 
 bool regmap_writeable(struct regmap *map, unsigned int reg);
@@ -63,4 +96,21 @@ void regmap_debugfs_init(struct regmap *map) { }
 void regmap_debugfs_exit(struct regmap *map) { }
 #endif
 
+/* regcache core declarations */
+int regcache_init(struct regmap *map);
+void regcache_exit(struct regmap *map);
+int regcache_read(struct regmap *map,
+		       unsigned int reg, unsigned int *value);
+int regcache_write(struct regmap *map,
+			unsigned int reg, unsigned int value);
+int regcache_sync(struct regmap *map);
+
+unsigned int regcache_get_val(const void *base, unsigned int idx,
+			      unsigned int word_size);
+bool regcache_set_val(void *base, unsigned int idx,
+		      unsigned int val, unsigned int word_size);
+int regcache_lookup_reg(struct regmap *map, unsigned int reg);
+int regcache_insert_reg(struct regmap *map, unsigned int reg,
+			unsigned int val);
+
 #endif
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
new file mode 100644
index 0000000..90b7e1f
--- /dev/null
+++ b/drivers/base/regmap/regcache.c
@@ -0,0 +1,251 @@
+/*
+ * Register cache access API
+ *
+ * Copyright 2011 Wolfson Microelectronics plc
+ *
+ * Author: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/slab.h>
+#include <trace/events/regmap.h>
+
+#include "internal.h"
+
+static const struct regcache_ops *cache_types[] = {
+};
+
+int regcache_init(struct regmap *map)
+{
+	int i, j;
+	int count;
+	unsigned int val;
+
+	if (map->cache_type == REGCACHE_NONE)
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(cache_types); ++i)
+		if (cache_types[i]->type == map->cache_type)
+			break;
+
+	if (i == ARRAY_SIZE(cache_types)) {
+		dev_err(map->dev, "Could not match compress type: %d\n",
+			map->cache_type);
+		return -EINVAL;
+	}
+
+	map->cache = NULL;
+	map->cache_ops = cache_types[i];
+	if (!map->cache_ops->name)
+		return -EINVAL;
+
+	if (!map->cache_defaults_raw || !map->num_cache_defaults_raw) {
+		dev_err(map->dev, "Client has not provided a defaults cache\n");
+		return -EINVAL;
+	}
+
+	/* calculate the size of cache_defaults */
+	for (count = 0, i = 0; i < map->num_cache_defaults_raw; i++) {
+		val = regcache_get_val(map->cache_defaults_raw,
+				       i, map->cache_word_size);
+		if (!val)
+			continue;
+		count++;
+	}
+
+	map->cache_defaults = kmalloc(count * sizeof(struct reg_default),
+				      GFP_KERNEL);
+	if (!map->cache_defaults)
+		return -ENOMEM;
+
+	/* fill the cache_defaults */
+	map->num_cache_defaults = count;
+	for (i = 0, j = 0; i < map->num_cache_defaults_raw; i++) {
+		val = regcache_get_val(map->cache_defaults_raw,
+				       i, map->cache_word_size);
+		if (!val)
+			continue;
+		map->cache_defaults[j].reg = i;
+		map->cache_defaults[j].def = val;
+		j++;
+	}
+
+	if (map->cache_ops->init) {
+		dev_dbg(map->dev, "Initializing %s cache\n",
+			map->cache_ops->name);
+		return map->cache_ops->init(map);
+	}
+	return 0;
+}
+
+void regcache_exit(struct regmap *map)
+{
+	if (map->cache_type == REGCACHE_NONE)
+		return;
+
+	BUG_ON(!map->cache_ops);
+
+	kfree(map->cache_defaults);
+	if (map->cache_ops->exit) {
+		dev_dbg(map->dev, "Destroying %s cache\n",
+			map->cache_ops->name);
+		map->cache_ops->exit(map);
+	}
+}
+
+/**
+ * regcache_read: Fetch the value of a given register from the cache.
+ *
+ * @map: map to configure.
+ * @reg: The register index.
+ * @value: The value to be returned.
+ *
+ * Return a negative value on failure, 0 on success.
+ */
+int regcache_read(struct regmap *map,
+		  unsigned int reg, unsigned int *value)
+{
+	if (map->cache_type == REGCACHE_NONE)
+		return 0;
+
+	BUG_ON(!map->cache_ops);
+
+	if (reg < map->num_cache_defaults_raw) {
+		if (!map->volatile_reg ||
+		    (map->volatile_reg && !map->volatile_reg(map->dev, reg))) {
+			if (value && map->cache_ops->read)
+				return map->cache_ops->read(map, reg, value);
+		}
+	}
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(regcache_read);
+
+/**
+ * regcache_write: Set the value of a given register in the cache.
+ *
+ * @map: map to configure.
+ * @reg: The register index.
+ * @value: The new register value.
+ *
+ * Return a negative value on failure, 0 on success.
+ */
+int regcache_write(struct regmap *map,
+		   unsigned int reg, unsigned int value)
+{
+	if (map->cache_type == REGCACHE_NONE)
+		return 0;
+
+	BUG_ON(!map->cache_ops);
+
+	if (reg < map->num_cache_defaults_raw) {
+		if (!map->volatile_reg ||
+		    (map->volatile_reg && !map->volatile_reg(map->dev, reg))) {
+			if (map->cache_ops->write)
+				return map->cache_ops->write(map, reg, value);
+		}
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(regcache_write);
+
+/**
+ * regcache_sync: Sync the register cache with the hardware.
+ *
+ * @map: map to configure.
+ *
+ * Any registers that should not be synced should be marked as
+ * volatile.  In general drivers can choose not to use the provided
+ * syncing functionality if they so require.
+ *
+ * Return a negative value on failure, 0 on success.
+ */
+int regcache_sync(struct regmap *map)
+{
+	BUG_ON(!map->cache_ops);
+
+	if (map->cache_ops->sync) {
+		dev_dbg(map->dev, "Syncing %s cache\n",
+			map->cache_ops->name);
+		return map->cache_ops->sync(map);
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(regcache_sync);
+
+bool regcache_set_val(void *base, unsigned int idx,
+		      unsigned int val, unsigned int word_size)
+{
+	switch (word_size) {
+	case 1: {
+		u8 *cache = base;
+		if (cache[idx] == val)
+			return true;
+		cache[idx] = val;
+		break;
+	}
+	case 2: {
+		u16 *cache = base;
+		if (cache[idx] == val)
+			return true;
+		cache[idx] = val;
+		break;
+	}
+	default:
+		BUG();
+	}
+	/* unreachable */
+	return false;
+}
+
+unsigned int regcache_get_val(const void *base, unsigned int idx,
+			      unsigned int word_size)
+{
+	if (!base)
+		return -EINVAL;
+
+	switch (word_size) {
+	case 1: {
+		const u8 *cache = base;
+		return cache[idx];
+	}
+	case 2: {
+		const u16 *cache = base;
+		return cache[idx];
+	}
+	default:
+		BUG();
+	}
+	/* unreachable */
+	return -1;
+}
+
+int regcache_lookup_reg(struct regmap *map, unsigned int reg)
+{
+	int i;
+
+	for (i = 0; i < map->num_cache_defaults; ++i)
+		if (map->cache_defaults[i].reg == reg)
+			return i;
+	return -1;
+}
+
+int regcache_insert_reg(struct regmap *map, unsigned int reg,
+			unsigned int val)
+{
+	void *tmp;
+
+	tmp = krealloc(map->cache_defaults,
+		       (map->num_cache_defaults + 1) * sizeof(struct reg_default),
+		       GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+	map->cache_defaults = tmp;
+	++map->num_cache_defaults;
+	map->cache_defaults[map->num_cache_defaults - 1].reg = reg;
+	map->cache_defaults[map->num_cache_defaults - 1].def = val;
+	return 0;
+}
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 449e264..b81e86a 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -20,6 +20,11 @@
 struct i2c_client;
 struct spi_device;
 
+/* An enum of all the supported cache types */
+enum regcache_type {
+	REGCACHE_NONE,
+};
+
 /**
  * Default value for a register.  We use an array of structs rather
  * than a simple array as many modern devices have very sparse
@@ -50,9 +55,11 @@ struct reg_default {
  *                (eg, a clear on read interrupt status register).
  *
  * @max_register: Optional, specifies the maximum valid register index.
- * @reg_defaults: Power on reset values for registers (for use with
- *                register cache support).
- * @num_reg_defaults: Number of elements in reg_defaults.
+ *
+ * @cache_type: The actual cache type.
+ * @cache_defaults_raw: Power on reset values for registers (for use with
+ *                 register cache support).
+ * @num_cache_defaults_raw: Number of elements in cache_defaults_raw.
  */
 struct regmap_config {
 	int reg_bits;
@@ -64,8 +71,10 @@ struct regmap_config {
 	bool (*precious_reg)(struct device *dev, unsigned int reg);
 
 	unsigned int max_register;
-	struct reg_default *reg_defaults;
-	int num_reg_defaults;
+
+	enum regcache_type cache_type;
+	const void *cache_defaults_raw;
+	unsigned num_cache_defaults_raw;
 };
 
 typedef int (*regmap_hw_write)(struct device *dev, const void *data,
-- 
1.7.6.1

Re: [PATCH 1/8] regmap: Introduce caching support

[Lars-Peter Clausen]

On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:
> This patch introduces caching support for regmap.  The regcache API
> has evolved essentially out of ASoC soc-cache so most of the actual
> caching types (except LZO) have been tested in the past.
> 
> The purpose of regcache is to optimize in time and space the handling
> of register caches.  Time optimization is achieved by not having to go
> over a slow bus like I2C to read the value of a register, instead it is
> cached locally in memory and can be retrieved faster.  Regarding space
> optimization, some of the cache types are better at packing the caches,
> for e.g. the rbtree and the LZO caches.  By doing this the sacrifice in
> time still wins over doing I2C transactions.
> 
> Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
> ---
>  drivers/base/regmap/Makefile   |    2 +-
>  drivers/base/regmap/internal.h |   50 ++++++++
>  drivers/base/regmap/regcache.c |  251 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/regmap.h         |   19 +++-
>  4 files changed, 316 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/base/regmap/regcache.c
> 
> diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
> index 057c13f..2e103ea 100644
> --- a/drivers/base/regmap/Makefile
> +++ b/drivers/base/regmap/Makefile
> @@ -1,4 +1,4 @@
> -obj-$(CONFIG_REGMAP) += regmap.o
> +obj-$(CONFIG_REGMAP) += regmap.o regcache.o
>  obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o
>  obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o
>  obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
> diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
> index 5ab3fef..b7acbeb 100644
> [...]
> diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
> new file mode 100644
> index 0000000..90b7e1f
> --- /dev/null
> +++ b/drivers/base/regmap/regcache.c
> @@ -0,0 +1,251 @@
> +/*
> + * Register cache access API
> + *
> + * Copyright 2011 Wolfson Microelectronics plc
> + *
> + * Author: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/slab.h>
> +#include <trace/events/regmap.h>
> +
> +#include "internal.h"
> +
> +static const struct regcache_ops *cache_types[] = {
> +};

I wonder if it makes sense to keep a list of regcache_ops this way, or whether
it is not better to just pass the ops we want to use to regcache_init directly.

> +
> +int regcache_init(struct regmap *map)
> +{
> +	int i, j;
> +	int count;
> +	unsigned int val;
> +
> +	if (map->cache_type == REGCACHE_NONE)
> +		return 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(cache_types); ++i)
> +		if (cache_types[i]->type == map->cache_type)
> +			break;
> +
> +	if (i == ARRAY_SIZE(cache_types)) {
> +		dev_err(map->dev, "Could not match compress type: %d\n",
> +			map->cache_type);
> +		return -EINVAL;
> +	}
> +
> +	map->cache = NULL;
> +	map->cache_ops = cache_types[i];
> +	if (!map->cache_ops->name)
> +		return -EINVAL;
> +
> +	if (!map->cache_defaults_raw || !map->num_cache_defaults_raw) {
> +		dev_err(map->dev, "Client has not provided a defaults cache\n");
> +		return -EINVAL;
> +	}

It should be OK to provide no default register values, in this case regmap
should assume that the default for all registers is 0.

> +
> +	/* calculate the size of cache_defaults */
> +	for (count = 0, i = 0; i < map->num_cache_defaults_raw; i++) {
> +		val = regcache_get_val(map->cache_defaults_raw,
> +				       i, map->cache_word_size);
> +		if (!val)
> +			continue;
> +		count++;
> +	}
> +
> +	map->cache_defaults = kmalloc(count * sizeof(struct reg_default),
> +				      GFP_KERNEL);
> +	if (!map->cache_defaults)
> +		return -ENOMEM;
> +
> +	/* fill the cache_defaults */
> +	map->num_cache_defaults = count;
> +	for (i = 0, j = 0; i < map->num_cache_defaults_raw; i++) {
> +		val = regcache_get_val(map->cache_defaults_raw,
> +				       i, map->cache_word_size);
> +		if (!val)
> +			continue;
> +		map->cache_defaults[j].reg = i;
> +		map->cache_defaults[j].def = val;
> +		j++;
> +	}
> +
> +	if (map->cache_ops->init) {
> +		dev_dbg(map->dev, "Initializing %s cache\n",
> +			map->cache_ops->name);
> +		return map->cache_ops->init(map);
> +	}
> +	return 0;
> +}
> +
> +void regcache_exit(struct regmap *map)
> +{
> +	if (map->cache_type == REGCACHE_NONE)
> +		return;
> +
> +	BUG_ON(!map->cache_ops);
> +
> +	kfree(map->cache_defaults);
> +	if (map->cache_ops->exit) {
> +		dev_dbg(map->dev, "Destroying %s cache\n",
> +			map->cache_ops->name);
> +		map->cache_ops->exit(map);
> +	}
> +}
> +
> +/**
> + * regcache_read: Fetch the value of a given register from the cache.
> + *
> + * @map: map to configure.
> + * @reg: The register index.
> + * @value: The value to be returned.
> + *
> + * Return a negative value on failure, 0 on success.
> + */
> +int regcache_read(struct regmap *map,
> +		  unsigned int reg, unsigned int *value)
> +{
> +	if (map->cache_type == REGCACHE_NONE)
> +		return 0;
> +
> +	BUG_ON(!map->cache_ops);
> +
> +	if (reg < map->num_cache_defaults_raw) {
maybe use regmap_readable(map, reg)), but at least check against max_register
and not num_cache_defaults_raw

> +		if (!map->volatile_reg ||
> +		    (map->volatile_reg && !map->volatile_reg(map->dev, reg))) {

if (!regmap_volatile(map, reg))

> +			if (value && map->cache_ops->read)

Will value or cache_ops->read ever be NULL? A register cache that either only
provides read or write would be pretty useless in my opinion, and we should
probably check for this at initialization time and not upon each access.

> +				return map->cache_ops->read(map, reg, value);
> +		}
> +	}
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(regcache_read);
> +
> +/**
> + * regcache_write: Set the value of a given register in the cache.
> + *
> + * @map: map to configure.
> + * @reg: The register index.
> + * @value: The new register value.
> + *
> + * Return a negative value on failure, 0 on success.
> + */
> +int regcache_write(struct regmap *map,
> +		   unsigned int reg, unsigned int value)
> +{
> +	if (map->cache_type == REGCACHE_NONE)
> +		return 0;
> +
> +	BUG_ON(!map->cache_ops);
> +
> +	if (reg < map->num_cache_defaults_raw) {
> +		if (!map->volatile_reg ||
> +		    (map->volatile_reg && !map->volatile_reg(map->dev, reg))) {
> +			if (map->cache_ops->write)
> +				return map->cache_ops->write(map, reg, value);
> +		}
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(regcache_write);

Same comments as for regcache_read

> +
> +/**
> + * regcache_sync: Sync the register cache with the hardware.
> + *
> + * @map: map to configure.
> + *
> + * Any registers that should not be synced should be marked as
> + * volatile.  In general drivers can choose not to use the provided
> + * syncing functionality if they so require.
> + *
> + * Return a negative value on failure, 0 on success.
> + */
> +int regcache_sync(struct regmap *map)
> +{
> +	BUG_ON(!map->cache_ops);
> +
> +	if (map->cache_ops->sync) {
> +		dev_dbg(map->dev, "Syncing %s cache\n",
> +			map->cache_ops->name);
> +		return map->cache_ops->sync(map);
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(regcache_sync);
> +
> +bool regcache_set_val(void *base, unsigned int idx,
> +		      unsigned int val, unsigned int word_size)
> +{
> +	switch (word_size) {
> +	case 1: {
> +		u8 *cache = base;
> +		if (cache[idx] == val)
> +			return true;
> +		cache[idx] = val;
> +		break;
> +	}
> +	case 2: {
> +		u16 *cache = base;
> +		if (cache[idx] == val)
> +			return true;
> +		cache[idx] = val;
> +		break;
> +	}
> +	default:
> +		BUG();
> +	}
> +	/* unreachable */
> +	return false;
> +}
> +
> +unsigned int regcache_get_val(const void *base, unsigned int idx,
> +			      unsigned int word_size)
> +{
> +	if (!base)
> +		return -EINVAL;
> +
> +	switch (word_size) {
> +	case 1: {
> +		const u8 *cache = base;
> +		return cache[idx];
> +	}
> +	case 2: {
> +		const u16 *cache = base;
> +		return cache[idx];
> +	}
> +	default:
> +		BUG();
> +	}
> +	/* unreachable */
> +	return -1;
> +}
> +
> +int regcache_lookup_reg(struct regmap *map, unsigned int reg)
> +{
> +	int i;
unsigned int

> +
> +	for (i = 0; i < map->num_cache_defaults; ++i)
> +		if (map->cache_defaults[i].reg == reg)
> +			return i;
> +	return -1;
> +}
> +
> +int regcache_insert_reg(struct regmap *map, unsigned int reg,
> +			unsigned int val)
> +{
> +	void *tmp;
> +
> +	tmp = krealloc(map->cache_defaults,
> +		       (map->num_cache_defaults + 1) * sizeof(struct reg_default),
> +		       GFP_KERNEL);
> +	if (!tmp)
> +		return -ENOMEM;
> +	map->cache_defaults = tmp;
> +	++map->num_cache_defaults;
> +	map->cache_defaults[map->num_cache_defaults - 1].reg = reg;
> +	map->cache_defaults[map->num_cache_defaults - 1].def = val;
> +	return 0;
> +}
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index 449e264..b81e86a 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -20,6 +20,11 @@
>  struct i2c_client;
>  struct spi_device;
>  
> +/* An enum of all the supported cache types */
> +enum regcache_type {
> +	REGCACHE_NONE,
> +};
> +
>  /**
>   * Default value for a register.  We use an array of structs rather
>   * than a simple array as many modern devices have very sparse
> @@ -50,9 +55,11 @@ struct reg_default {
>   *                (eg, a clear on read interrupt status register).
>   *
>   * @max_register: Optional, specifies the maximum valid register index.
> - * @reg_defaults: Power on reset values for registers (for use with
> - *                register cache support).
> - * @num_reg_defaults: Number of elements in reg_defaults.
> + *
> + * @cache_type: The actual cache type.
> + * @cache_defaults_raw: Power on reset values for registers (for use with
> + *                 register cache support).
> + * @num_cache_defaults_raw: Number of elements in cache_defaults_raw.
>   */
>  struct regmap_config {
>  	int reg_bits;
> @@ -64,8 +71,10 @@ struct regmap_config {
>  	bool (*precious_reg)(struct device *dev, unsigned int reg);
>  
>  	unsigned int max_register;
> -	struct reg_default *reg_defaults;
> -	int num_reg_defaults;

I guess these were removed by accident due to some merge conflict as they were
added just recently. It would be good if you could re-add them and if they are
set initialize cache_defaults using a memdup instead of reading cache_defaults_raw.

> +
> +	enum regcache_type cache_type;
> +	const void *cache_defaults_raw;
> +	unsigned num_cache_defaults_raw;
>  };
>  
>  typedef int (*regmap_hw_write)(struct device *dev, const void *data,

Re: [PATCH 1/8] regmap: Introduce caching support

[Mark Brown]

On Fri, Sep 02, 2011 at 10:02:02PM +0200, Lars-Peter Clausen wrote:
> On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:

> > +static const struct regcache_ops *cache_types[] = {
> > +};

> I wonder if it makes sense to keep a list of regcache_ops this way, or whether
> it is not better to just pass the ops we want to use to regcache_init directly.

Or have a function per cache type.  I'm keen to hide the ops from users
because I don't want to have to worry about them peering inside the
internals.

> > +	if (!map->cache_defaults_raw || !map->num_cache_defaults_raw) {
> > +		dev_err(map->dev, "Client has not provided a defaults cache\n");
> > +		return -EINVAL;
> > +	}

> It should be OK to provide no default register values, in this case regmap
> should assume that the default for all registers is 0.

Yes - Dimitris, as we discussed offline it's pretty much essential for
things like PMICs where the defaults aren't meaningful and may even be
unknowable.

> > +			if (value && map->cache_ops->read)

> Will value or cache_ops->read ever be NULL? A register cache that either only
> provides read or write would be pretty useless in my opinion, and we should
> probably check for this at initialization time and not upon each access.

I agree that it's safe to assume read() if you've got cache_ops.  I
think in the case where we don't cache we should be able to come up with
a suitable noop cache type which we can assign as the ops so like you
say we can just assume a read() op.

Re: [PATCH 1/8] regmap: Introduce caching support

[Dimitris Papastamos]

On Sat, Sep 03, 2011 at 12:48:27AM +0100, Mark Brown wrote:
> On Fri, Sep 02, 2011 at 10:02:02PM +0200, Lars-Peter Clausen wrote:
> > On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:
> 
> > > +static const struct regcache_ops *cache_types[] = {
> > > +};
> 
> > I wonder if it makes sense to keep a list of regcache_ops this way, or whether
> > it is not better to just pass the ops we want to use to regcache_init directly.
> 
> Or have a function per cache type.  I'm keen to hide the ops from users
> because I don't want to have to worry about them peering inside the
> internals.
> 
> > > +	if (!map->cache_defaults_raw || !map->num_cache_defaults_raw) {
> > > +		dev_err(map->dev, "Client has not provided a defaults cache\n");
> > > +		return -EINVAL;
> > > +	}
> 
> > It should be OK to provide no default register values, in this case regmap
> > should assume that the default for all registers is 0.
> 
> Yes - Dimitris, as we discussed offline it's pretty much essential for
> things like PMICs where the defaults aren't meaningful and may even be
> unknowable.

That's implemented in patch 8/8.  I can of course squash that patch into
the other patch.

Thanks,
Dimitris

Re: [PATCH 1/8] regmap: Introduce caching support

[Mark Brown]

On Sat, Sep 03, 2011 at 02:10:32AM +0100, Dimitris Papastamos wrote:
> On Sat, Sep 03, 2011 at 12:48:27AM +0100, Mark Brown wrote:
> > On Fri, Sep 02, 2011 at 10:02:02PM +0200, Lars-Peter Clausen wrote:

> > > It should be OK to provide no default register values, in this case regmap
> > > should assume that the default for all registers is 0.

> > Yes - Dimitris, as we discussed offline it's pretty much essential for
> > things like PMICs where the defaults aren't meaningful and may even be

> That's implemented in patch 8/8.  I can of course squash that patch into
> the other patch.

Ah, good - I was a bit surprised as I did remember discussing this with
you.  Please do move that code into the intial patch, in general it's
usually OK to miss features out of early patches or simplify them but
making them actively buggy isn't great.

Re: [PATCH 1/8] regmap: Introduce caching support

[Dimitris Papastamos]

On Sat, Sep 03, 2011 at 12:48:27AM +0100, Mark Brown wrote:
> On Fri, Sep 02, 2011 at 10:02:02PM +0200, Lars-Peter Clausen wrote:
> > On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:
> 
> > > +static const struct regcache_ops *cache_types[] = {
> > > +};
> 
> > I wonder if it makes sense to keep a list of regcache_ops this way, or whether
> > it is not better to just pass the ops we want to use to regcache_init directly.
> 
> Or have a function per cache type.  I'm keen to hide the ops from users
> because I don't want to have to worry about them peering inside the
> internals.

Yes I guess I'll do that, sounds cleaner.

Thanks,
Dimitris

Re: [PATCH 1/8] regmap: Introduce caching support

[Dimitris Papastamos]

On Fri, Sep 02, 2011 at 10:02:02PM +0200, Lars-Peter Clausen wrote:
> On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:
> > This patch introduces caching support for regmap.  The regcache API
> > has evolved essentially out of ASoC soc-cache so most of the actual
> > caching types (except LZO) have been tested in the past.
> > 
> > The purpose of regcache is to optimize in time and space the handling
> > of register caches.  Time optimization is achieved by not having to go
> > over a slow bus like I2C to read the value of a register, instead it is
> > cached locally in memory and can be retrieved faster.  Regarding space
> > optimization, some of the cache types are better at packing the caches,
> > for e.g. the rbtree and the LZO caches.  By doing this the sacrifice in
> > time still wins over doing I2C transactions.
> > 
> > Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
> > ---
> >  drivers/base/regmap/Makefile   |    2 +-
> >  drivers/base/regmap/internal.h |   50 ++++++++
> >  drivers/base/regmap/regcache.c |  251 ++++++++++++++++++++++++++++++++++++++++
> >  include/linux/regmap.h         |   19 +++-
> >  4 files changed, 316 insertions(+), 6 deletions(-)
> >  create mode 100644 drivers/base/regmap/regcache.c
> > 
> > diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
> > index 057c13f..2e103ea 100644
> > --- a/drivers/base/regmap/Makefile
> > +++ b/drivers/base/regmap/Makefile
> > @@ -1,4 +1,4 @@
> > -obj-$(CONFIG_REGMAP) += regmap.o
> > +obj-$(CONFIG_REGMAP) += regmap.o regcache.o
> >  obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o
> >  obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o
> >  obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
> > diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
> > index 5ab3fef..b7acbeb 100644
> > [...]
> > diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
> > new file mode 100644
> > index 0000000..90b7e1f
> > --- /dev/null
> > +++ b/drivers/base/regmap/regcache.c
> > @@ -0,0 +1,251 @@
> > +/*
> > + * Register cache access API
> > + *
> > + * Copyright 2011 Wolfson Microelectronics plc
> > + *
> > + * Author: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/slab.h>
> > +#include <trace/events/regmap.h>
> > +
> > +#include "internal.h"
> > +
> > +static const struct regcache_ops *cache_types[] = {
> > +};
> 
> I wonder if it makes sense to keep a list of regcache_ops this way, or whether
> it is not better to just pass the ops we want to use to regcache_init directly.

Something like the user actually setting the ops pointer to one of the
exported cache operation types, and then having regcache to use just
that directly?

> > +
> > +int regcache_init(struct regmap *map)
> > +{
> > +	int i, j;
> > +	int count;
> > +	unsigned int val;
> > +
> > +	if (map->cache_type == REGCACHE_NONE)
> > +		return 0;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(cache_types); ++i)
> > +		if (cache_types[i]->type == map->cache_type)
> > +			break;
> > +
> > +	if (i == ARRAY_SIZE(cache_types)) {
> > +		dev_err(map->dev, "Could not match compress type: %d\n",
> > +			map->cache_type);
> > +		return -EINVAL;
> > +	}
> > +
> > +	map->cache = NULL;
> > +	map->cache_ops = cache_types[i];
> > +	if (!map->cache_ops->name)
> > +		return -EINVAL;
> > +
> > +	if (!map->cache_defaults_raw || !map->num_cache_defaults_raw) {
> > +		dev_err(map->dev, "Client has not provided a defaults cache\n");
> > +		return -EINVAL;
> > +	}
> 
> It should be OK to provide no default register values, in this case regmap
> should assume that the default for all registers is 0.

I guess I'll just squash the patch that implementes readback from the HW
right into this patch if that's okay.

> > +/**
> > + * regcache_read: Fetch the value of a given register from the cache.
> > + *
> > + * @map: map to configure.
> > + * @reg: The register index.
> > + * @value: The value to be returned.
> > + *
> > + * Return a negative value on failure, 0 on success.
> > + */
> > +int regcache_read(struct regmap *map,
> > +		  unsigned int reg, unsigned int *value)
> > +{
> > +	if (map->cache_type == REGCACHE_NONE)
> > +		return 0;
> > +
> > +	BUG_ON(!map->cache_ops);
> > +
> > +	if (reg < map->num_cache_defaults_raw) {
> maybe use regmap_readable(map, reg)), but at least check against max_register
> and not num_cache_defaults_raw

Ok.

> > +		if (!map->volatile_reg ||
> > +		    (map->volatile_reg && !map->volatile_reg(map->dev, reg))) {
> 
> if (!regmap_volatile(map, reg))

Yes.

> > +			if (value && map->cache_ops->read)
> 
> Will value or cache_ops->read ever be NULL? A register cache that either only
> provides read or write would be pretty useless in my opinion, and we should
> probably check for this at initialization time and not upon each access.

Yes will check at init time.

> > +int regcache_lookup_reg(struct regmap *map, unsigned int reg)
> > +{
> > +	int i;
> unsigned int

Yes.

> >  /**
> >   * Default value for a register.  We use an array of structs rather
> >   * than a simple array as many modern devices have very sparse
> > @@ -50,9 +55,11 @@ struct reg_default {
> >   *                (eg, a clear on read interrupt status register).
> >   *
> >   * @max_register: Optional, specifies the maximum valid register index.
> > - * @reg_defaults: Power on reset values for registers (for use with
> > - *                register cache support).
> > - * @num_reg_defaults: Number of elements in reg_defaults.
> > + *
> > + * @cache_type: The actual cache type.
> > + * @cache_defaults_raw: Power on reset values for registers (for use with
> > + *                 register cache support).
> > + * @num_cache_defaults_raw: Number of elements in cache_defaults_raw.
> >   */
> >  struct regmap_config {
> >  	int reg_bits;
> > @@ -64,8 +71,10 @@ struct regmap_config {
> >  	bool (*precious_reg)(struct device *dev, unsigned int reg);
> >  
> >  	unsigned int max_register;
> > -	struct reg_default *reg_defaults;
> > -	int num_reg_defaults;
> 
> I guess these were removed by accident due to some merge conflict as they were
> added just recently. It would be good if you could re-add them and if they are
> set initialize cache_defaults using a memdup instead of reading cache_defaults_raw.
> 
> > +
> > +	enum regcache_type cache_type;
> > +	const void *cache_defaults_raw;
> > +	unsigned num_cache_defaults_raw;
> >  };
> >  
> >  typedef int (*regmap_hw_write)(struct device *dev, const void *data,

I was merely renaming these.

Thanks,
Dimitris

Re: [PATCH 1/8] regmap: Introduce caching support

[Lars-Peter Clausen]

On 09/05/2011 11:43 AM, Dimitris Papastamos wrote:
> [...]
>>>  /**
>>>   * Default value for a register.  We use an array of structs rather
>>>   * than a simple array as many modern devices have very sparse
>>> @@ -50,9 +55,11 @@ struct reg_default {
>>>   *                (eg, a clear on read interrupt status register).
>>>   *
>>>   * @max_register: Optional, specifies the maximum valid register index.
>>> - * @reg_defaults: Power on reset values for registers (for use with
>>> - *                register cache support).
>>> - * @num_reg_defaults: Number of elements in reg_defaults.
>>> + *
>>> + * @cache_type: The actual cache type.
>>> + * @cache_defaults_raw: Power on reset values for registers (for use with
>>> + *                 register cache support).
>>> + * @num_cache_defaults_raw: Number of elements in cache_defaults_raw.
>>>   */
>>>  struct regmap_config {
>>>  	int reg_bits;
>>> @@ -64,8 +71,10 @@ struct regmap_config {
>>>  	bool (*precious_reg)(struct device *dev, unsigned int reg);
>>>  
>>>  	unsigned int max_register;
>>> -	struct reg_default *reg_defaults;
>>> -	int num_reg_defaults;
>>
>> I guess these were removed by accident due to some merge conflict as they were
>> added just recently. It would be good if you could re-add them and if they are
>> set initialize cache_defaults using a memdup instead of reading cache_defaults_raw.
>>
>>> +
>>> +	enum regcache_type cache_type;
>>> +	const void *cache_defaults_raw;
>>> +	unsigned num_cache_defaults_raw;
>>>  };
>>>  
>>>  typedef int (*regmap_hw_write)(struct device *dev, const void *data,
> 
> I was merely renaming these.

And you changed the type. We need the reg_default type register defaults for
devices which have a sparse registers set or where the base register address is
at a larger offset.

- Lars

Re: [PATCH 1/8] regmap: Introduce caching support

[Dimitris Papastamos]

On Mon, Sep 05, 2011 at 11:55:13AM +0200, Lars-Peter Clausen wrote:
> On 09/05/2011 11:43 AM, Dimitris Papastamos wrote:
> > [...]
> >>>  /**
> >>>   * Default value for a register.  We use an array of structs rather
> >>>   * than a simple array as many modern devices have very sparse
> >>> @@ -50,9 +55,11 @@ struct reg_default {
> >>>   *                (eg, a clear on read interrupt status register).
> >>>   *
> >>>   * @max_register: Optional, specifies the maximum valid register index.
> >>> - * @reg_defaults: Power on reset values for registers (for use with
> >>> - *                register cache support).
> >>> - * @num_reg_defaults: Number of elements in reg_defaults.
> >>> + *
> >>> + * @cache_type: The actual cache type.
> >>> + * @cache_defaults_raw: Power on reset values for registers (for use with
> >>> + *                 register cache support).
> >>> + * @num_cache_defaults_raw: Number of elements in cache_defaults_raw.
> >>>   */
> >>>  struct regmap_config {
> >>>  	int reg_bits;
> >>> @@ -64,8 +71,10 @@ struct regmap_config {
> >>>  	bool (*precious_reg)(struct device *dev, unsigned int reg);
> >>>  
> >>>  	unsigned int max_register;
> >>> -	struct reg_default *reg_defaults;
> >>> -	int num_reg_defaults;
> >>
> >> I guess these were removed by accident due to some merge conflict as they were
> >> added just recently. It would be good if you could re-add them and if they are
> >> set initialize cache_defaults using a memdup instead of reading cache_defaults_raw.
> >>
> >>> +
> >>> +	enum regcache_type cache_type;
> >>> +	const void *cache_defaults_raw;
> >>> +	unsigned num_cache_defaults_raw;
> >>>  };
> >>>  
> >>>  typedef int (*regmap_hw_write)(struct device *dev, const void *data,
> > 
> > I was merely renaming these.
> 
> And you changed the type. We need the reg_default type register defaults for
> devices which have a sparse registers set or where the base register address is
> at a larger offset.
> 
> - Lars

Aw duh, yes.  Will add support to regcache to be able to use directly
the reg_defaults if they have been provided by the user without having
to derive this from cache_defaults_raw.

Thanks,
Dimitris

[PATCH 2/8] regmap: Add the indexed cache support

[Dimitris Papastamos]

This is the simplest form of a cache available in regcache.  Any
registers whose default value is 0 are ignored.  If any of those
registers are modified in the future, they will be placed in the
cache on demand.  The cache layout is essentially using the provided
register defaults by the regcache core directly and does not re-map
it to another representation.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/Makefile           |    2 +-
 drivers/base/regmap/internal.h         |    1 +
 drivers/base/regmap/regcache-indexed.c |   65 ++++++++++++++++++++++++++++++++
 drivers/base/regmap/regcache.c         |    3 +
 include/linux/regmap.h                 |    1 +
 5 files changed, 71 insertions(+), 1 deletions(-)
 create mode 100644 drivers/base/regmap/regcache-indexed.c

diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 2e103ea..418d151 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_REGMAP) += regmap.o regcache.o
+obj-$(CONFIG_REGMAP) += regmap.o regcache.o regcache-indexed.o
 obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o
 obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o
 obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index b7acbeb..148ab9f 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -113,4 +113,5 @@ int regcache_lookup_reg(struct regmap *map, unsigned int reg);
 int regcache_insert_reg(struct regmap *map, unsigned int reg,
 			unsigned int val);
 
+extern struct regcache_ops regcache_indexed_ops;
 #endif
diff --git a/drivers/base/regmap/regcache-indexed.c b/drivers/base/regmap/regcache-indexed.c
new file mode 100644
index 0000000..1f1af43
--- /dev/null
+++ b/drivers/base/regmap/regcache-indexed.c
@@ -0,0 +1,65 @@
+/*
+ * Register cache access API - indexed caching support
+ *
+ * Copyright 2011 Wolfson Microelectronics plc
+ *
+ * Author: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/slab.h>
+
+#include "internal.h"
+
+static int regcache_indexed_read(struct regmap *map, unsigned int reg,
+				 unsigned int *value)
+{
+	int ret;
+
+	ret = regcache_lookup_reg(map, reg);
+	if (ret < 0)
+		*value = 0;
+	else
+		*value = map->cache_defaults[ret].def;
+	return 0;
+}
+
+static int regcache_indexed_write(struct regmap *map, unsigned int reg,
+				  unsigned int value)
+{
+	int ret;
+
+	ret = regcache_lookup_reg(map, reg);
+	if (ret < 0)
+		return regcache_insert_reg(map, reg, value);
+	map->cache_defaults[ret].def = value;
+	return 0;
+}
+
+static int regcache_indexed_sync(struct regmap *map)
+{
+	int i;
+	int ret;
+
+	for (i = 0; i < map->num_cache_defaults; ++i) {
+		ret = regmap_write(map, map->cache_defaults[i].reg,
+				   map->cache_defaults[i].def);
+		if (ret < 0)
+			return ret;
+		dev_dbg(map->dev, "Synced register %#x, value %#x\n",
+			map->cache_defaults[i].reg,
+			map->cache_defaults[i].def);
+	}
+	return 0;
+}
+
+struct regcache_ops regcache_indexed_ops = {
+	.type = REGCACHE_INDEXED,
+	.name = "indexed",
+	.read = regcache_indexed_read,
+	.write = regcache_indexed_write,
+	.sync = regcache_indexed_sync
+};
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 90b7e1f..dfefe40 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -16,6 +16,9 @@
 #include "internal.h"
 
 static const struct regcache_ops *cache_types[] = {
+#ifdef CONFIG_REGCACHE_INDEXED
+	&regcache_indexed_ops,
+#endif
 };
 
 int regcache_init(struct regmap *map)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index b81e86a..4d1ad09 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -23,6 +23,7 @@ struct spi_device;
 /* An enum of all the supported cache types */
 enum regcache_type {
 	REGCACHE_NONE,
+	REGCACHE_INDEXED,
 };
 
 /**
-- 
1.7.6.1

Re: [PATCH 2/8] regmap: Add the indexed cache support

[Lars-Peter Clausen]

On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:
> This is the simplest form of a cache available in regcache.  Any
> registers whose default value is 0 are ignored.  If any of those
> registers are modified in the future, they will be placed in the
> cache on demand.  The cache layout is essentially using the provided
> register defaults by the regcache core directly and does not re-map
> it to another representation.
> 
> Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

I wonder if we really still need indexed caches, because ...


> ---
>  drivers/base/regmap/Makefile           |    2 +-
>  drivers/base/regmap/internal.h         |    1 +
>  drivers/base/regmap/regcache-indexed.c |   65 ++++++++++++++++++++++++++++++++
>  drivers/base/regmap/regcache.c         |    3 +
>  include/linux/regmap.h                 |    1 +
>  5 files changed, 71 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/base/regmap/regcache-indexed.c
> 
> [...]
> +static int regcache_indexed_read(struct regmap *map, unsigned int reg,
> +				 unsigned int *value)
> +{
> +	int ret;
> +
> +	ret = regcache_lookup_reg(map, reg);

... this lookup will be slower than an rbtree lookup. And given that we use two
integers per register doesn't give it much size advantage either.

I have some patches for the rbtree register cache code, which reduce some of
it's complexity and also implements the adjacent node merging and allows
smaller holes in the rbnodes register block if the block is smaller then an
rbnode. Currently these patches are based on the ASoC register cache code, but
they should be easy to rework onto of the regmap register cache code.

I did some small statistics on the number of rbnodes without and with the
patches applied:

Without patches:
Nodes:
  0: 4000-4000   1
  1: 4009-400a   2
  2: 400b-400c   2
  3: 400d-4010   4
  4: 4015-4015   1
  5: 4017-4017   1
  6: 4019-401b   3
  7: 401c-401d   2
  8: 401e-4020   3
  9: 4021-4022   2
 10: 4023-4026   4
 11: 4029-402a   2
 12: 4031-4031   1
 13: 4036-4036   1
 14: 40eb-40eb   1
 15: 40f2-40f3   2
 16: 40f8-40f9   2
 17: 40fa-40fa   1

Number of nodes: 18 (504 bytes)
Number of registers: 35 (35 bytes)
Total size: 539 bytes

With patches:
Nodes:
  0: 4000-4036  55
  1: 40eb-40fa  16

Number of nodes: 2 (56 bytes)
Number of registers: 71 (71 bytes)
Total size: 127 bytes

In comparison a ASoC flat cache for this driver would have used 250 bytes and
and regmap indexed cache would use 280 bytes. (Not that it really matters at
those sizes anyway)


> +	if (ret < 0)
> +		*value = 0;
> +	else
> +		*value = map->cache_defaults[ret].def;
> +	return 0;
> +}
> +
> [...]
> diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
> index 90b7e1f..dfefe40 100644
> --- a/drivers/base/regmap/regcache.c
> +++ b/drivers/base/regmap/regcache.c
> @@ -16,6 +16,9 @@
>  #include "internal.h"
>  
>  static const struct regcache_ops *cache_types[] = {
> +#ifdef CONFIG_REGCACHE_INDEXED

This symbol doesn't seem to be defined anywhere.

> +	&regcache_indexed_ops,
> +#endif
>  };
>  
>  int regcache_init(struct regmap *map)
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index b81e86a..4d1ad09 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -23,6 +23,7 @@ struct spi_device;
>  /* An enum of all the supported cache types */
>  enum regcache_type {
>  	REGCACHE_NONE,
> +	REGCACHE_INDEXED,
>  };
>  
>  /**

Re: [PATCH 2/8] regmap: Add the indexed cache support

[Dimitris Papastamos]

On Fri, Sep 02, 2011 at 10:16:06PM +0200, Lars-Peter Clausen wrote:
> On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:
> > This is the simplest form of a cache available in regcache.  Any
> > registers whose default value is 0 are ignored.  If any of those
> > registers are modified in the future, they will be placed in the
> > cache on demand.  The cache layout is essentially using the provided
> > register defaults by the regcache core directly and does not re-map
> > it to another representation.
> > 
> > Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
> 
> I wonder if we really still need indexed caches, because ...
> 
> 
> > ---
> >  drivers/base/regmap/Makefile           |    2 +-
> >  drivers/base/regmap/internal.h         |    1 +
> >  drivers/base/regmap/regcache-indexed.c |   65 ++++++++++++++++++++++++++++++++
> >  drivers/base/regmap/regcache.c         |    3 +
> >  include/linux/regmap.h                 |    1 +
> >  5 files changed, 71 insertions(+), 1 deletions(-)
> >  create mode 100644 drivers/base/regmap/regcache-indexed.c
> > 
> > [...]
> > +static int regcache_indexed_read(struct regmap *map, unsigned int reg,
> > +				 unsigned int *value)
> > +{
> > +	int ret;
> > +
> > +	ret = regcache_lookup_reg(map, reg);
> 
> ... this lookup will be slower than an rbtree lookup. And given that we use two
> integers per register doesn't give it much size advantage either.

The indexed cache grew out of the flat cache somehow.  Do you think the
flat cache would be more appropriate than the indexed cache?  One thing
with the flat cache is that we need to make a flat copy of the
cache_defaults (either partial or full).  With the indexed cache we can
use the cache_defaults directly.

I've got a patch that changes the regcache_lookup_reg() to actually use binary
search.  The array is kept sorted after insertions and since insertions
are less common than readbacks it should improve things a bit.

> I have some patches for the rbtree register cache code, which reduce some of
> it's complexity and also implements the adjacent node merging and allows
> smaller holes in the rbnodes register block if the block is smaller then an
> rbnode. Currently these patches are based on the ASoC register cache code, but
> they should be easy to rework onto of the regmap register cache code.
> 
> I did some small statistics on the number of rbnodes without and with the
> patches applied:
> 
> Without patches:
> Nodes:
>   0: 4000-4000   1
>   1: 4009-400a   2
>   2: 400b-400c   2
>   3: 400d-4010   4
>   4: 4015-4015   1
>   5: 4017-4017   1
>   6: 4019-401b   3
>   7: 401c-401d   2
>   8: 401e-4020   3
>   9: 4021-4022   2
>  10: 4023-4026   4
>  11: 4029-402a   2
>  12: 4031-4031   1
>  13: 4036-4036   1
>  14: 40eb-40eb   1
>  15: 40f2-40f3   2
>  16: 40f8-40f9   2
>  17: 40fa-40fa   1
> 
> Number of nodes: 18 (504 bytes)
> Number of registers: 35 (35 bytes)
> Total size: 539 bytes
> 
> With patches:
> Nodes:
>   0: 4000-4036  55
>   1: 40eb-40fa  16
> 
> Number of nodes: 2 (56 bytes)
> Number of registers: 71 (71 bytes)
> Total size: 127 bytes
> 
> In comparison a ASoC flat cache for this driver would have used 250 bytes and
> and regmap indexed cache would use 280 bytes. (Not that it really matters at
> those sizes anyway)

Awesome!!  You could mail me those patches and I'll re-work them to
apply them on top of regcache.

> > +	if (ret < 0)
> > +		*value = 0;
> > +	else
> > +		*value = map->cache_defaults[ret].def;
> > +	return 0;
> > +}
> > +
> > [...]
> > diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
> > index 90b7e1f..dfefe40 100644
> > --- a/drivers/base/regmap/regcache.c
> > +++ b/drivers/base/regmap/regcache.c
> > @@ -16,6 +16,9 @@
> >  #include "internal.h"
> >  
> >  static const struct regcache_ops *cache_types[] = {
> > +#ifdef CONFIG_REGCACHE_INDEXED
> 
> This symbol doesn't seem to be defined anywhere.

Yes I need to remove all of these.

Thanks,
Dimitris

Re: [PATCH 2/8] regmap: Add the indexed cache support

[Lars-Peter Clausen]

On 09/05/2011 11:55 AM, Dimitris Papastamos wrote:
> On Fri, Sep 02, 2011 at 10:16:06PM +0200, Lars-Peter Clausen wrote:
>> On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:
>>> This is the simplest form of a cache available in regcache.  Any
>>> registers whose default value is 0 are ignored.  If any of those
>>> registers are modified in the future, they will be placed in the
>>> cache on demand.  The cache layout is essentially using the provided
>>> register defaults by the regcache core directly and does not re-map
>>> it to another representation.
>>>
>>> Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
>>
>> I wonder if we really still need indexed caches, because ...
>>
>>
>>> ---
>>>  drivers/base/regmap/Makefile           |    2 +-
>>>  drivers/base/regmap/internal.h         |    1 +
>>>  drivers/base/regmap/regcache-indexed.c |   65 ++++++++++++++++++++++++++++++++
>>>  drivers/base/regmap/regcache.c         |    3 +
>>>  include/linux/regmap.h                 |    1 +
>>>  5 files changed, 71 insertions(+), 1 deletions(-)
>>>  create mode 100644 drivers/base/regmap/regcache-indexed.c
>>>
>>> [...]
>>> +static int regcache_indexed_read(struct regmap *map, unsigned int reg,
>>> +				 unsigned int *value)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = regcache_lookup_reg(map, reg);
>>
>> ... this lookup will be slower than an rbtree lookup. And given that we use two
>> integers per register doesn't give it much size advantage either.
> 
> The indexed cache grew out of the flat cache somehow.  Do you think the
> flat cache would be more appropriate than the indexed cache?  One thing
> with the flat cache is that we need to make a flat copy of the
> cache_defaults (either partial or full).  With the indexed cache we can
> use the cache_defaults directly.
> 
> I've got a patch that changes the regcache_lookup_reg() to actually use binary
> search.  The array is kept sorted after insertions and since insertions
> are less common than readbacks it should improve things a bit.
> 

Even with a binary search the lookup will most likely be more expensive then
with an rbtree. Both have an log(n) running time, but the rbtree will most
likely have less nodes than registers, while with the indexed cache we have one
entry per register.

An option would be to switch back to a flat cache, since here the lookup is
basically free. But I don't think it is really worth the maintenance hassle,
the gains are rather marginal. Especially with my patches which add support for
merging adjacent nodes there is almost no overhead compared to the original
ASoC flat cache. You'll end up with one rbnode, so the cached rbnode is always
the one your were looking for and no rbtree lookup is necessary.

In my opinion we should work towards having rbtree as the only cache type, but
with optional lzo compression on the rbtree node blocks.

- Lars

Re: [PATCH 2/8] regmap: Add the indexed cache support

[Mark Brown]

On Mon, Sep 05, 2011 at 12:14:26PM +0200, Lars-Peter Clausen wrote:

> In my opinion we should work towards having rbtree as the only cache type, but
> with optional lzo compression on the rbtree node blocks.

If we are going to do something like that we should probably just go by
block size to decide if we should do the compression and do the thing
with keeping the most recently used LZO block (or perhaps a few)
uncompressed in RAM for speed.  Probably also look at the compression we
get when we decide to compress and not bother doing the compression if
it's a small saving, though we will have burned CPU.

Re: [PATCH 2/8] regmap: Add the indexed cache support

[Mark Brown]

On Fri, Sep 02, 2011 at 10:16:06PM +0200, Lars-Peter Clausen wrote:

> I have some patches for the rbtree register cache code, which reduce some of
> it's complexity and also implements the adjacent node merging and allows
> smaller holes in the rbnodes register block if the block is smaller then an
> rbnode. Currently these patches are based on the ASoC register cache code, but
> they should be easy to rework onto of the regmap register cache code.

Dimitris just reminded me about what you said here - do you still have
those patches floating around?  This would really useful and would
make some other optimistations like not cacheing default values much
easier.

Re: [PATCH 2/8] regmap: Add the indexed cache support

[Lars-Peter Clausen]

On 07/17/2012 01:16 PM, Mark Brown wrote:
> On Fri, Sep 02, 2011 at 10:16:06PM +0200, Lars-Peter Clausen wrote:
> 
>> I have some patches for the rbtree register cache code, which reduce some of
>> it's complexity and also implements the adjacent node merging and allows
>> smaller holes in the rbnodes register block if the block is smaller then an
>> rbnode. Currently these patches are based on the ASoC register cache code, but
>> they should be easy to rework onto of the regmap register cache code.
> 
> Dimitris just reminded me about what you said here - do you still have
> those patches floating around?  This would really useful and would
> make some other optimistations like not cacheing default values much
> easier.

Yes, I still have it, but it's on a machine which is currently broken and I
wont get the replacement part until next week.

- Lars

[PATCH 3/8] regmap: Add the rbtree cache support

[Dimitris Papastamos]

This patch adds support for the rbtree cache compression type.

Each rbnode manages a variable length block of registers.  There can be no
two nodes with overlapping blocks.  Each block has a base register and a
currently top register, all the other registers, if any, lie in between these
two and in ascending order.

The reasoning behind the construction of this rbtree is simple.  In the
snd_soc_rbtree_cache_init() function, we iterate over the register defaults
provided by the regcache core.  For each register value that is non-zero we
insert it in the rbtree.  In order to determine in which rbnode we need
to add the register, we first look if there is another register already
added that is adjacent to the one we are about to add.  If that is the case
we append it in that rbnode block, otherwise we create a new rbnode
with a single register in its block and add it to the tree.

There are various optimizations across the implementation to speed up lookups
by caching the most recently used rbnode.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/Makefile          |    2 +-
 drivers/base/regmap/internal.h        |    1 +
 drivers/base/regmap/regcache-rbtree.c |  400 +++++++++++++++++++++++++++++++++
 drivers/base/regmap/regcache.c        |    3 +
 include/linux/regmap.h                |    1 +
 5 files changed, 406 insertions(+), 1 deletions(-)
 create mode 100644 drivers/base/regmap/regcache-rbtree.c

diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 418d151..20cd650 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_REGMAP) += regmap.o regcache.o regcache-indexed.o
+obj-$(CONFIG_REGMAP) += regmap.o regcache.o regcache-indexed.o regcache-rbtree.o
 obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o
 obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o
 obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 148ab9f..52c09f4 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -114,4 +114,5 @@ int regcache_insert_reg(struct regmap *map, unsigned int reg,
 			unsigned int val);
 
 extern struct regcache_ops regcache_indexed_ops;
+extern struct regcache_ops regcache_rbtree_ops;
 #endif
diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
new file mode 100644
index 0000000..3eeb785
--- /dev/null
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -0,0 +1,400 @@
+/*
+ * Register cache access API - rbtree caching support
+ *
+ * Copyright 2011 Wolfson Microelectronics plc
+ *
+ * Author: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/slab.h>
+#include <linux/rbtree.h>
+
+#include "internal.h"
+
+static int regcache_rbtree_write(struct regmap *map, unsigned int reg,
+				 unsigned int value);
+
+struct regcache_rbtree_node {
+	struct rb_node node; /* the actual rbtree node holding this block */
+	unsigned int base_reg; /* base register handled by this block */
+	unsigned int word_size; /* number of bytes needed to represent the register index */
+	void *block; /* block of adjacent registers */
+	unsigned int blklen; /* number of registers available in the block */
+} __attribute__ ((packed));
+
+struct regcache_rbtree_ctx {
+	struct rb_root root;
+	struct regcache_rbtree_node *cached_rbnode;
+};
+
+static inline void regcache_rbtree_get_base_top_reg(
+	struct regcache_rbtree_node *rbnode,
+	unsigned int *base, unsigned int *top)
+{
+	*base = rbnode->base_reg;
+	*top = rbnode->base_reg + rbnode->blklen - 1;
+}
+
+static unsigned int regcache_rbtree_get_register(
+	struct regcache_rbtree_node *rbnode, unsigned int idx)
+{
+	unsigned int val;
+
+	switch (rbnode->word_size) {
+	case 1: {
+		u8 *p = rbnode->block;
+		val = p[idx];
+		return val;
+	}
+	case 2: {
+		u16 *p = rbnode->block;
+		val = p[idx];
+		return val;
+	}
+	default:
+		BUG();
+		break;
+	}
+	return -1;
+}
+
+static void regcache_rbtree_set_register(struct regcache_rbtree_node *rbnode,
+					 unsigned int idx, unsigned int val)
+{
+	switch (rbnode->word_size) {
+	case 1: {
+		u8 *p = rbnode->block;
+		p[idx] = val;
+		break;
+	}
+	case 2: {
+		u16 *p = rbnode->block;
+		p[idx] = val;
+		break;
+	}
+	default:
+		BUG();
+		break;
+	}
+}
+
+static struct regcache_rbtree_node *regcache_rbtree_lookup(
+	struct rb_root *root, unsigned int reg)
+{
+	struct rb_node *node;
+	struct regcache_rbtree_node *rbnode;
+	unsigned int base_reg, top_reg;
+
+	node = root->rb_node;
+	while (node) {
+		rbnode = container_of(node, struct regcache_rbtree_node, node);
+		regcache_rbtree_get_base_top_reg(rbnode, &base_reg, &top_reg);
+		if (reg >= base_reg && reg <= top_reg)
+			return rbnode;
+		else if (reg > top_reg)
+			node = node->rb_right;
+		else if (reg < base_reg)
+			node = node->rb_left;
+	}
+
+	return NULL;
+}
+
+static int regcache_rbtree_insert(struct rb_root *root,
+				  struct regcache_rbtree_node *rbnode)
+{
+	struct rb_node **new, *parent;
+	struct regcache_rbtree_node *rbnode_tmp;
+	unsigned int base_reg_tmp, top_reg_tmp;
+	unsigned int base_reg;
+
+	parent = NULL;
+	new = &root->rb_node;
+	while (*new) {
+		rbnode_tmp = container_of(*new, struct regcache_rbtree_node,
+					  node);
+		/* base and top registers of the current rbnode */
+		regcache_rbtree_get_base_top_reg(rbnode_tmp, &base_reg_tmp,
+						 &top_reg_tmp);
+		/* base register of the rbnode to be added */
+		base_reg = rbnode->base_reg;
+		parent = *new;
+		/* if this register has already been inserted, just return */
+		if (base_reg >= base_reg_tmp &&
+		    base_reg <= top_reg_tmp)
+			return 0;
+		else if (base_reg > top_reg_tmp)
+			new = &((*new)->rb_right);
+		else if (base_reg < base_reg_tmp)
+			new = &((*new)->rb_left);
+	}
+
+	/* insert the node into the rbtree */
+	rb_link_node(&rbnode->node, parent, new);
+	rb_insert_color(&rbnode->node, root);
+
+	return 1;
+}
+
+static int regcache_rbtree_init(struct regmap *map)
+{
+	struct regcache_rbtree_ctx *rbtree_ctx;
+	int i;
+	int ret;
+
+	map->cache = kmalloc(sizeof *rbtree_ctx, GFP_KERNEL);
+	if (!map->cache)
+		return -ENOMEM;
+
+	rbtree_ctx = map->cache;
+	rbtree_ctx->root = RB_ROOT;
+	rbtree_ctx->cached_rbnode = NULL;
+
+	if (!map->cache_defaults)
+		return 0;
+
+	for (i = 0; i < map->num_cache_defaults_raw; ++i) {
+		ret = regcache_lookup_reg(map, i);
+		if (ret < 0)
+			continue;
+		ret = regcache_rbtree_write(map,
+					    map->cache_defaults[ret].reg,
+					    map->cache_defaults[ret].def);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	regcache_exit(map);
+	return ret;
+}
+
+static int regcache_rbtree_exit(struct regmap *map)
+{
+	struct rb_node *next;
+	struct regcache_rbtree_ctx *rbtree_ctx;
+	struct regcache_rbtree_node *rbtree_node;
+
+	/* if we've already been called then just return */
+	rbtree_ctx = map->cache;
+	if (!rbtree_ctx)
+		return 0;
+
+	/* free up the rbtree */
+	next = rb_first(&rbtree_ctx->root);
+	while (next) {
+		rbtree_node = rb_entry(next, struct regcache_rbtree_node, node);
+		next = rb_next(&rbtree_node->node);
+		rb_erase(&rbtree_node->node, &rbtree_ctx->root);
+		kfree(rbtree_node->block);
+		kfree(rbtree_node);
+	}
+
+	/* release the resources */
+	kfree(map->cache);
+	map->cache = NULL;
+
+	return 0;
+}
+
+static int regcache_rbtree_read(struct regmap *map,
+				unsigned int reg, unsigned int *value)
+{
+	struct regcache_rbtree_ctx *rbtree_ctx;
+	struct regcache_rbtree_node *rbnode;
+	unsigned int base_reg, top_reg;
+	unsigned int reg_tmp;
+
+	rbtree_ctx = map->cache;
+	/* look up the required register in the cached rbnode */
+	rbnode = rbtree_ctx->cached_rbnode;
+	if (rbnode) {
+		regcache_rbtree_get_base_top_reg(rbnode, &base_reg, &top_reg);
+		if (reg >= base_reg && reg <= top_reg) {
+			reg_tmp = reg - base_reg;
+			*value = regcache_rbtree_get_register(rbnode, reg_tmp);
+			return 0;
+		}
+	}
+	/* if we can't locate it in the cached rbnode we'll have
+	 * to traverse the rbtree looking for it.
+	 */
+	rbnode = regcache_rbtree_lookup(&rbtree_ctx->root, reg);
+	if (rbnode) {
+		reg_tmp = reg - rbnode->base_reg;
+		*value = regcache_rbtree_get_register(rbnode, reg_tmp);
+		rbtree_ctx->cached_rbnode = rbnode;
+	} else {
+		/* uninitialized registers default to 0 */
+		*value = 0;
+	}
+
+	return 0;
+}
+
+
+static int regcache_rbtree_insert_to_block(struct regcache_rbtree_node *rbnode,
+					   unsigned int pos, unsigned int reg,
+					   unsigned int value)
+{
+	u8 *blk;
+
+	blk = krealloc(rbnode->block,
+		       (rbnode->blklen + 1) * rbnode->word_size, GFP_KERNEL);
+	if (!blk)
+		return -ENOMEM;
+
+	/* insert the register value in the correct place in the rbnode block */
+	memmove(blk + (pos + 1) * rbnode->word_size,
+		blk + pos * rbnode->word_size,
+		(rbnode->blklen - pos) * rbnode->word_size);
+
+	/* update the rbnode block, its size and the base register */
+	rbnode->block = blk;
+	rbnode->blklen++;
+	if (!pos)
+		rbnode->base_reg = reg;
+
+	regcache_rbtree_set_register(rbnode, pos, value);
+	return 0;
+}
+
+static int regcache_rbtree_write(struct regmap *map, unsigned int reg,
+				 unsigned int value)
+{
+	struct regcache_rbtree_ctx *rbtree_ctx;
+	struct regcache_rbtree_node *rbnode, *rbnode_tmp;
+	struct rb_node *node;
+	unsigned int val;
+	unsigned int reg_tmp;
+	unsigned int base_reg, top_reg;
+	unsigned int pos;
+	int i;
+	int ret;
+
+	rbtree_ctx = map->cache;
+	/* look up the required register in the cached rbnode */
+	rbnode = rbtree_ctx->cached_rbnode;
+	if (rbnode) {
+		regcache_rbtree_get_base_top_reg(rbnode, &base_reg, &top_reg);
+		if (reg >= base_reg && reg <= top_reg) {
+			reg_tmp = reg - base_reg;
+			val = regcache_rbtree_get_register(rbnode, reg_tmp);
+			if (val == value)
+				return 0;
+			regcache_rbtree_set_register(rbnode, reg_tmp, value);
+			return 0;
+		}
+	}
+	/* if we can't locate it in the cached rbnode we'll have
+	 * to traverse the rbtree looking for it.
+	 */
+	rbnode = regcache_rbtree_lookup(&rbtree_ctx->root, reg);
+	if (rbnode) {
+		reg_tmp = reg - rbnode->base_reg;
+		val = regcache_rbtree_get_register(rbnode, reg_tmp);
+		if (val == value)
+			return 0;
+		regcache_rbtree_set_register(rbnode, reg_tmp, value);
+		rbtree_ctx->cached_rbnode = rbnode;
+	} else {
+		/* bail out early, no need to create the rbnode yet */
+		if (!value)
+			return 0;
+		/* look for an adjacent register to the one we are about to add */
+		for (node = rb_first(&rbtree_ctx->root); node;
+		     node = rb_next(node)) {
+			rbnode_tmp = rb_entry(node, struct regcache_rbtree_node, node);
+			for (i = 0; i < rbnode_tmp->blklen; ++i) {
+				reg_tmp = rbnode_tmp->base_reg + i;
+				if (abs(reg_tmp - reg) != 1)
+					continue;
+				/* decide where in the block to place our register */
+				if (reg_tmp + 1 == reg)
+					pos = i + 1;
+				else
+					pos = i;
+				ret = regcache_rbtree_insert_to_block(rbnode_tmp, pos,
+								      reg, value);
+				if (ret)
+					return ret;
+				rbtree_ctx->cached_rbnode = rbnode_tmp;
+				return 0;
+			}
+		}
+		/* we did not manage to find a place to insert it in an existing
+		 * block so create a new rbnode with a single register in its block.
+		 * This block will get populated further if any other adjacent
+		 * registers get modified in the future.
+		 */
+		rbnode = kzalloc(sizeof *rbnode, GFP_KERNEL);
+		if (!rbnode)
+			return -ENOMEM;
+		rbnode->blklen = 1;
+		rbnode->base_reg = reg;
+		rbnode->word_size = map->cache_word_size;
+		rbnode->block = kmalloc(rbnode->blklen * rbnode->word_size,
+					GFP_KERNEL);
+		if (!rbnode->block) {
+			kfree(rbnode);
+			return -ENOMEM;
+		}
+		regcache_rbtree_set_register(rbnode, 0, value);
+		regcache_rbtree_insert(&rbtree_ctx->root, rbnode);
+		rbtree_ctx->cached_rbnode = rbnode;
+	}
+
+	return 0;
+}
+
+static int regcache_rbtree_sync(struct regmap *map)
+{
+	struct regcache_rbtree_ctx *rbtree_ctx;
+	struct rb_node *node;
+	struct regcache_rbtree_node *rbnode;
+	unsigned int regtmp;
+	unsigned int val, def;
+	int ret;
+	int i;
+
+	rbtree_ctx = map->cache;
+	for (node = rb_first(&rbtree_ctx->root); node; node = rb_next(node)) {
+		rbnode = rb_entry(node, struct regcache_rbtree_node, node);
+		for (i = 0; i < rbnode->blklen; ++i) {
+			regtmp = rbnode->base_reg + i;
+			val = regcache_rbtree_get_register(rbnode, i);
+			ret = regcache_lookup_reg(map, i);
+			if (ret < 0)
+				def = 0;
+			else
+				def = map->cache_defaults[ret].def;
+			if (val == def)
+				continue;
+			map->cache_bypass = 1;
+			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);
+		}
+	}
+
+	return 0;
+}
+
+struct regcache_ops regcache_rbtree_ops = {
+	.type = REGCACHE_RBTREE,
+	.name = "rbtree",
+	.init = regcache_rbtree_init,
+	.exit = regcache_rbtree_exit,
+	.read = regcache_rbtree_read,
+	.write = regcache_rbtree_write,
+	.sync = regcache_rbtree_sync
+};
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index dfefe40..2b148d4 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -19,6 +19,9 @@ static const struct regcache_ops *cache_types[] = {
 #ifdef CONFIG_REGCACHE_INDEXED
 	&regcache_indexed_ops,
 #endif
+#ifdef CONFIG_REGCACHE_RBTREE
+	&regcache_rbtree_ops,
+#endif
 };
 
 int regcache_init(struct regmap *map)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 4d1ad09..6d782c0 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -24,6 +24,7 @@ struct spi_device;
 enum regcache_type {
 	REGCACHE_NONE,
 	REGCACHE_INDEXED,
+	REGCACHE_RBTREE,
 };
 
 /**
-- 
1.7.6.1

Re: [PATCH 3/8] regmap: Add the rbtree cache support

[Lars-Peter Clausen]

On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:
> This patch adds support for the rbtree cache compression type.
> 
> Each rbnode manages a variable length block of registers.  There can be no
> two nodes with overlapping blocks.  Each block has a base register and a
> currently top register, all the other registers, if any, lie in between these
> two and in ascending order.
> 
> The reasoning behind the construction of this rbtree is simple.  In the
> snd_soc_rbtree_cache_init() function, we iterate over the register defaults
> provided by the regcache core.  For each register value that is non-zero we
> insert it in the rbtree.  In order to determine in which rbnode we need
> to add the register, we first look if there is another register already
> added that is adjacent to the one we are about to add.  If that is the case
> we append it in that rbnode block, otherwise we create a new rbnode
> with a single register in its block and add it to the tree.
> 
> There are various optimizations across the implementation to speed up lookups
> by caching the most recently used rbnode.
> 
> Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
> [...]
> +
> +static int regcache_rbtree_init(struct regmap *map)
> +{
> +	struct regcache_rbtree_ctx *rbtree_ctx;
> +	int i;
> +	int ret;
> +
> +	map->cache = kmalloc(sizeof *rbtree_ctx, GFP_KERNEL);
> +	if (!map->cache)
> +		return -ENOMEM;
> +
> +	rbtree_ctx = map->cache;
> +	rbtree_ctx->root = RB_ROOT;
> +	rbtree_ctx->cached_rbnode = NULL;
> +
> +	if (!map->cache_defaults)
> +		return 0;
> +
> +	for (i = 0; i < map->num_cache_defaults_raw; ++i) {
> +		ret = regcache_lookup_reg(map, i);
> +		if (ret < 0)
> +			continue;
> +		ret = regcache_rbtree_write(map,
> +					    map->cache_defaults[ret].reg,
> +					    map->cache_defaults[ret].def);
> +		if (ret)
> +			goto err;
> +	}

You can iterate over the caches_defaults elements directly.

[...]
> diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
> index dfefe40..2b148d4 100644
> --- a/drivers/base/regmap/regcache.c
> +++ b/drivers/base/regmap/regcache.c
> @@ -19,6 +19,9 @@ static const struct regcache_ops *cache_types[] = {
>  #ifdef CONFIG_REGCACHE_INDEXED
>  	&regcache_indexed_ops,
>  #endif
> +#ifdef CONFIG_REGCACHE_RBTREE

This symbol is also not defined. It looks as if all the Kconfig changes are not
included in the patchset.

> +	&regcache_rbtree_ops,
> +#endif
>  };

Re: [PATCH 3/8] regmap: Add the rbtree cache support

[Dimitris Papastamos]

On Fri, Sep 02, 2011 at 10:22:58PM +0200, Lars-Peter Clausen wrote:
> On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:
> > This patch adds support for the rbtree cache compression type.
> > 
> > Each rbnode manages a variable length block of registers.  There can be no
> > two nodes with overlapping blocks.  Each block has a base register and a
> > currently top register, all the other registers, if any, lie in between these
> > two and in ascending order.
> > 
> > The reasoning behind the construction of this rbtree is simple.  In the
> > snd_soc_rbtree_cache_init() function, we iterate over the register defaults
> > provided by the regcache core.  For each register value that is non-zero we
> > insert it in the rbtree.  In order to determine in which rbnode we need
> > to add the register, we first look if there is another register already
> > added that is adjacent to the one we are about to add.  If that is the case
> > we append it in that rbnode block, otherwise we create a new rbnode
> > with a single register in its block and add it to the tree.
> > 
> > There are various optimizations across the implementation to speed up lookups
> > by caching the most recently used rbnode.
> > 
> > Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
> > [...]
> > +
> > +static int regcache_rbtree_init(struct regmap *map)
> > +{
> > +	struct regcache_rbtree_ctx *rbtree_ctx;
> > +	int i;
> > +	int ret;
> > +
> > +	map->cache = kmalloc(sizeof *rbtree_ctx, GFP_KERNEL);
> > +	if (!map->cache)
> > +		return -ENOMEM;
> > +
> > +	rbtree_ctx = map->cache;
> > +	rbtree_ctx->root = RB_ROOT;
> > +	rbtree_ctx->cached_rbnode = NULL;
> > +
> > +	if (!map->cache_defaults)
> > +		return 0;
> > +
> > +	for (i = 0; i < map->num_cache_defaults_raw; ++i) {
> > +		ret = regcache_lookup_reg(map, i);
> > +		if (ret < 0)
> > +			continue;
> > +		ret = regcache_rbtree_write(map,
> > +					    map->cache_defaults[ret].reg,
> > +					    map->cache_defaults[ret].def);
> > +		if (ret)
> > +			goto err;
> > +	}
> 
> You can iterate over the caches_defaults elements directly.

Yes.

Thanks,
Dimitris

[PATCH 4/8] regmap: Add the LZO cache support

[Dimitris Papastamos]

This patch adds support for LZO compression when storing the register
cache.

For a typical device whose register map would normally occupy 25kB or 50kB
by using the LZO compression technique, one can get down to ~5-7kB.  There
might be a performance penalty associated with each individual read/write
due to decompressing/compressing the underlying cache, however that should not
be noticeable.  These memory benefits depend on whether the target architecture
can get rid of the memory occupied by the original register defaults cache
which is marked as __devinitconst.  Nevertheless there will be some memory
gain even if the target architecture can't get rid of the original register
map, this should be around ~30-32kB instead of 50kB.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/Makefile       |    2 +-
 drivers/base/regmap/internal.h     |    2 +
 drivers/base/regmap/regcache-lzo.c |  385 ++++++++++++++++++++++++++++++++++++
 drivers/base/regmap/regcache.c     |    3 +
 include/linux/regmap.h             |    1 +
 5 files changed, 392 insertions(+), 1 deletions(-)
 create mode 100644 drivers/base/regmap/regcache-lzo.c

diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 20cd650..0573c8a 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_REGMAP) += regmap.o regcache.o regcache-indexed.o regcache-rbtree.o
+obj-$(CONFIG_REGMAP) += regmap.o regcache.o regcache-indexed.o regcache-rbtree.o regcache-lzo.o
 obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o
 obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o
 obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 52c09f4..9f228c7 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -115,4 +115,6 @@ int regcache_insert_reg(struct regmap *map, unsigned int reg,
 
 extern struct regcache_ops regcache_indexed_ops;
 extern struct regcache_ops regcache_rbtree_ops;
+extern struct regcache_ops regcache_lzo_ops;
+
 #endif
diff --git a/drivers/base/regmap/regcache-lzo.c b/drivers/base/regmap/regcache-lzo.c
new file mode 100644
index 0000000..9e0d7be
--- /dev/null
+++ b/drivers/base/regmap/regcache-lzo.c
@@ -0,0 +1,385 @@
+/*
+ * Register cache access API - LZO caching support
+ *
+ * Copyright 2011 Wolfson Microelectronics plc
+ *
+ * Author: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/slab.h>
+#include <linux/lzo.h>
+
+#include "internal.h"
+
+struct regcache_lzo_ctx {
+	void *wmem;
+	void *dst;
+	const void *src;
+	size_t src_len;
+	size_t dst_len;
+	size_t decompressed_size;
+	unsigned long *sync_bmp;
+	int sync_bmp_nbits;
+};
+
+#define LZO_BLOCK_NUM 8
+static int regcache_lzo_block_count(void)
+{
+	return LZO_BLOCK_NUM;
+}
+
+static int regcache_lzo_prepare(struct regcache_lzo_ctx *lzo_ctx)
+{
+	lzo_ctx->wmem = kmalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL);
+	if (!lzo_ctx->wmem)
+		return -ENOMEM;
+	return 0;
+}
+
+static int regcache_lzo_compress(struct regcache_lzo_ctx *lzo_ctx)
+{
+	size_t compress_size;
+	int ret;
+
+	ret = lzo1x_1_compress(lzo_ctx->src, lzo_ctx->src_len,
+			       lzo_ctx->dst, &compress_size, lzo_ctx->wmem);
+	if (ret != LZO_E_OK || compress_size > lzo_ctx->dst_len)
+		return -EINVAL;
+	lzo_ctx->dst_len = compress_size;
+	return 0;
+}
+
+static int regcache_lzo_decompress(struct regcache_lzo_ctx *lzo_ctx)
+{
+	size_t dst_len;
+	int ret;
+
+	dst_len = lzo_ctx->dst_len;
+	ret = lzo1x_decompress_safe(lzo_ctx->src, lzo_ctx->src_len,
+				    lzo_ctx->dst, &dst_len);
+	if (ret != LZO_E_OK || dst_len != lzo_ctx->dst_len)
+		return -EINVAL;
+	return 0;
+}
+
+static int regcache_lzo_compress_cache_block(struct regmap *map,
+		struct regcache_lzo_ctx *lzo_ctx)
+{
+	int ret;
+
+	lzo_ctx->dst_len = lzo1x_worst_compress(PAGE_SIZE);
+	lzo_ctx->dst = kmalloc(lzo_ctx->dst_len, GFP_KERNEL);
+	if (!lzo_ctx->dst) {
+		lzo_ctx->dst_len = 0;
+		return -ENOMEM;
+	}
+
+	ret = regcache_lzo_compress(lzo_ctx);
+	if (ret < 0)
+		return ret;
+	return 0;
+}
+
+static int regcache_lzo_decompress_cache_block(struct regmap *map,
+		struct regcache_lzo_ctx *lzo_ctx)
+{
+	int ret;
+
+	lzo_ctx->dst_len = lzo_ctx->decompressed_size;
+	lzo_ctx->dst = kmalloc(lzo_ctx->dst_len, GFP_KERNEL);
+	if (!lzo_ctx->dst) {
+		lzo_ctx->dst_len = 0;
+		return -ENOMEM;
+	}
+
+	ret = regcache_lzo_decompress(lzo_ctx);
+	if (ret < 0)
+		return ret;
+	return 0;
+}
+
+static inline int regcache_lzo_get_blkindex(struct regmap *map,
+					    unsigned int reg)
+{
+	return (reg * map->cache_word_size) /
+		DIV_ROUND_UP(map->cache_size_raw, regcache_lzo_block_count());
+}
+
+static inline int regcache_lzo_get_blkpos(struct regmap *map,
+					  unsigned int reg)
+{
+	return reg % (DIV_ROUND_UP(map->cache_size_raw, regcache_lzo_block_count()) /
+		      map->cache_word_size);
+}
+
+static inline int regcache_lzo_get_blksize(struct regmap *map)
+{
+	return DIV_ROUND_UP(map->cache_size_raw, regcache_lzo_block_count());
+}
+
+static int regcache_lzo_init(struct regmap *map)
+{
+	struct regcache_lzo_ctx **lzo_blocks;
+	size_t bmp_size;
+	int ret, tofree, i, blksize, blkcount;
+	const char *p, *end;
+	unsigned long *sync_bmp;
+
+	ret = 0;
+
+	/*
+	 * If we have not been given a default register cache
+	 * then allocate a dummy zero-ed out region, compress it
+	 * and remember to free it afterwards.
+	 */
+	tofree = 0;
+	if (!map->cache_defaults_raw) {
+		map->cache_defaults_raw = kzalloc(map->cache_size_raw, GFP_KERNEL);
+		if (!map->cache_defaults_raw)
+			return -ENOMEM;
+		tofree = 1;
+	}
+
+	blkcount = regcache_lzo_block_count();
+	map->cache = kzalloc(blkcount * sizeof *lzo_blocks,
+			     GFP_KERNEL);
+	if (!map->cache) {
+		ret = -ENOMEM;
+		goto err_tofree;
+	}
+	lzo_blocks = map->cache;
+
+	/*
+	 * allocate a bitmap to be used when syncing the cache with
+	 * the hardware.  Each time a register is modified, the corresponding
+	 * bit is set in the bitmap, so we know that we have to sync
+	 * that register.
+	 */
+	bmp_size = map->num_cache_defaults_raw;
+	sync_bmp = kmalloc(BITS_TO_LONGS(bmp_size) * sizeof(long),
+			   GFP_KERNEL);
+	if (!sync_bmp) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	bitmap_zero(sync_bmp, bmp_size);
+
+	/* allocate the lzo blocks and initialize them */
+	for (i = 0; i < blkcount; ++i) {
+		lzo_blocks[i] = kzalloc(sizeof **lzo_blocks,
+					GFP_KERNEL);
+		if (!lzo_blocks[i]) {
+			kfree(sync_bmp);
+			ret = -ENOMEM;
+			goto err;
+		}
+		lzo_blocks[i]->sync_bmp = sync_bmp;
+		lzo_blocks[i]->sync_bmp_nbits = bmp_size;
+		/* alloc the working space for the compressed block */
+		ret = regcache_lzo_prepare(lzo_blocks[i]);
+		if (ret < 0)
+			goto err;
+	}
+
+	blksize = regcache_lzo_get_blksize(map);
+	p = map->cache_defaults_raw;
+	end = map->cache_defaults_raw + map->cache_size_raw;
+	/* compress the register map and fill the lzo blocks */
+	for (i = 0; i < blkcount; ++i, p += blksize) {
+		lzo_blocks[i]->src = p;
+		if (p + blksize > end)
+			lzo_blocks[i]->src_len = end - p;
+		else
+			lzo_blocks[i]->src_len = blksize;
+		ret = regcache_lzo_compress_cache_block(map,
+						       lzo_blocks[i]);
+		if (ret < 0)
+			goto err;
+		lzo_blocks[i]->decompressed_size =
+			lzo_blocks[i]->src_len;
+	}
+
+	if (tofree) {
+		kfree(map->cache_defaults_raw);
+		map->cache_defaults_raw = NULL;
+	}
+	return 0;
+err:
+	regcache_exit(map);
+err_tofree:
+	if (tofree) {
+		kfree(map->cache_defaults_raw);
+		map->cache_defaults_raw = NULL;
+	}
+	return ret;
+}
+
+static int regcache_lzo_exit(struct regmap *map)
+{
+	struct regcache_lzo_ctx **lzo_blocks;
+	int i, blkcount;
+
+	lzo_blocks = map->cache;
+	if (!lzo_blocks)
+		return 0;
+
+	blkcount = regcache_lzo_block_count();
+	/*
+	 * the pointer to the bitmap used for syncing the cache
+	 * is shared amongst all lzo_blocks.  Ensure it is freed
+	 * only once.
+	 */
+	if (lzo_blocks[0])
+		kfree(lzo_blocks[0]->sync_bmp);
+	for (i = 0; i < blkcount; ++i) {
+		if (lzo_blocks[i]) {
+			kfree(lzo_blocks[i]->wmem);
+			kfree(lzo_blocks[i]->dst);
+		}
+		/* each lzo_block is a pointer returned by kmalloc or NULL */
+		kfree(lzo_blocks[i]);
+	}
+	kfree(lzo_blocks);
+	map->cache = NULL;
+	return 0;
+}
+
+static int regcache_lzo_read(struct regmap *map,
+			     unsigned int reg, unsigned int *value)
+{
+	struct regcache_lzo_ctx *lzo_block, **lzo_blocks;
+	int ret, blkindex, blkpos;
+	size_t blksize, tmp_dst_len;
+	void *tmp_dst;
+
+	*value = 0;
+	/* index of the compressed lzo block */
+	blkindex = regcache_lzo_get_blkindex(map, reg);
+	/* register index within the decompressed block */
+	blkpos = regcache_lzo_get_blkpos(map, reg);
+	/* size of the compressed block */
+	blksize = regcache_lzo_get_blksize(map);
+	lzo_blocks = map->cache;
+	lzo_block = lzo_blocks[blkindex];
+
+	/* save the pointer and length of the compressed block */
+	tmp_dst = lzo_block->dst;
+	tmp_dst_len = lzo_block->dst_len;
+
+	/* prepare the source to be the compressed block */
+	lzo_block->src = lzo_block->dst;
+	lzo_block->src_len = lzo_block->dst_len;
+
+	/* decompress the block */
+	ret = regcache_lzo_decompress_cache_block(map, lzo_block);
+	if (ret >= 0)
+		/* fetch the value from the cache */
+		*value = regcache_get_val(lzo_block->dst, blkpos,
+					  map->cache_word_size);
+
+	kfree(lzo_block->dst);
+	/* restore the pointer and length of the compressed block */
+	lzo_block->dst = tmp_dst;
+	lzo_block->dst_len = tmp_dst_len;
+	return 0;
+}
+
+static int regcache_lzo_write(struct regmap *map,
+			      unsigned int reg, unsigned int value)
+{
+	struct regcache_lzo_ctx *lzo_block, **lzo_blocks;
+	int ret, blkindex, blkpos;
+	size_t blksize, tmp_dst_len;
+	void *tmp_dst;
+
+	/* index of the compressed lzo block */
+	blkindex = regcache_lzo_get_blkindex(map, reg);
+	/* register index within the decompressed block */
+	blkpos = regcache_lzo_get_blkpos(map, reg);
+	/* size of the compressed block */
+	blksize = regcache_lzo_get_blksize(map);
+	lzo_blocks = map->cache;
+	lzo_block = lzo_blocks[blkindex];
+
+	/* save the pointer and length of the compressed block */
+	tmp_dst = lzo_block->dst;
+	tmp_dst_len = lzo_block->dst_len;
+
+	/* prepare the source to be the compressed block */
+	lzo_block->src = lzo_block->dst;
+	lzo_block->src_len = lzo_block->dst_len;
+
+	/* decompress the block */
+	ret = regcache_lzo_decompress_cache_block(map, lzo_block);
+	if (ret < 0) {
+		kfree(lzo_block->dst);
+		goto out;
+	}
+
+	/* write the new value to the cache */
+	if (regcache_set_val(lzo_block->dst, blkpos, value,
+			     map->cache_word_size)) {
+		kfree(lzo_block->dst);
+		goto out;
+	}
+
+	/* prepare the source to be the decompressed block */
+	lzo_block->src = lzo_block->dst;
+	lzo_block->src_len = lzo_block->dst_len;
+
+	/* compress the block */
+	ret = regcache_lzo_compress_cache_block(map, lzo_block);
+	if (ret < 0) {
+		kfree(lzo_block->dst);
+		kfree(lzo_block->src);
+		goto out;
+	}
+
+	/* set the bit so we know we have to sync this register */
+	set_bit(reg, lzo_block->sync_bmp);
+	kfree(tmp_dst);
+	kfree(lzo_block->src);
+	return 0;
+out:
+	lzo_block->dst = tmp_dst;
+	lzo_block->dst_len = tmp_dst_len;
+	return ret;
+}
+
+static int regcache_lzo_sync(struct regmap *map)
+{
+	struct regcache_lzo_ctx **lzo_blocks;
+	unsigned int val;
+	int i;
+	int ret;
+
+	lzo_blocks = map->cache;
+	for_each_set_bit(i, lzo_blocks[0]->sync_bmp, lzo_blocks[0]->sync_bmp_nbits) {
+		ret = regcache_read(map, i, &val);
+		if (ret)
+			return ret;
+		map->cache_bypass = 1;
+		ret = regmap_write(map, i, val);
+		map->cache_bypass = 0;
+		if (ret)
+			return ret;
+		dev_dbg(map->dev, "Synced register %#x, value %#x\n",
+			i, val);
+	}
+
+	return 0;
+}
+
+struct regcache_ops regcache_lzo_ops = {
+	.type = REGCACHE_LZO,
+	.name = "lzo",
+	.init = regcache_lzo_init,
+	.exit = regcache_lzo_exit,
+	.read = regcache_lzo_read,
+	.write = regcache_lzo_write,
+	.sync = regcache_lzo_sync
+};
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 2b148d4..6aece9e 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -22,6 +22,9 @@ static const struct regcache_ops *cache_types[] = {
 #ifdef CONFIG_REGCACHE_RBTREE
 	&regcache_rbtree_ops,
 #endif
+#ifdef CONFIG_REGCACHE_LZO
+	&regcache_lzo_ops,
+#endif
 };
 
 int regcache_init(struct regmap *map)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 6d782c0..0653e56 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -25,6 +25,7 @@ enum regcache_type {
 	REGCACHE_NONE,
 	REGCACHE_INDEXED,
 	REGCACHE_RBTREE,
+	REGCACHE_LZO
 };
 
 /**
-- 
1.7.6.1

Re: [PATCH 4/8] regmap: Add the LZO cache support

[Matthieu CASTET]

Dimitris Papastamos <dp <at> opensource.wolfsonmicro.com> writes:

> 
> This patch adds support for LZO compression when storing the register
> cache.
> 
> For a typical device whose register map would normally occupy 25kB or 50kB
> by using the LZO compression technique, one can get down to ~5-7kB.  
Do you have example of such case ?

Caching 25kB or 50kB looks crazy.

Re: [PATCH 4/8] regmap: Add the LZO cache support

[Mark Brown]

On Mon, Sep 05, 2011 at 09:40:45PM +0000, Matthieu CASTET wrote:
> Dimitris Papastamos <dp <at> opensource.wolfsonmicro.com> writes:

Don't drop CCs from mails if you want people to actually read them.

> > This patch adds support for LZO compression when storing the register
> > cache.

> > For a typical device whose register map would normally occupy 25kB or 50kB
> > by using the LZO compression technique, one can get down to ~5-7kB.  

> Do you have example of such case ?

> Caching 25kB or 50kB looks crazy.

Audio CODECs for smartphones can easily have well over a thousand
registers (excluding things like DSP memories) connected over
rel;atively slow buses like I2C or SPI with lots of read/modify/write
cycles (so cutting out the physical read is a real win) and we want to
power them off when not in use.

[PATCH 5/8] regmap: Add the regcache_sync trace event

[Dimitris Papastamos]

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regcache.c |    8 +++++++-
 include/trace/events/regmap.h  |   24 ++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 6aece9e..6346d77 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -174,12 +174,18 @@ EXPORT_SYMBOL_GPL(regcache_write);
  */
 int regcache_sync(struct regmap *map)
 {
+	int ret;
+	const char *name;
+
 	BUG_ON(!map->cache_ops);
 
 	if (map->cache_ops->sync) {
 		dev_dbg(map->dev, "Syncing %s cache\n",
 			map->cache_ops->name);
-		return map->cache_ops->sync(map);
+		name = map->cache_ops->name;
+		trace_regcache_sync(map->dev, name, "start");
+		ret = map->cache_ops->sync(map);
+		trace_regcache_sync(map->dev, name, "stop");
 	}
 	return 0;
 }
diff --git a/include/trace/events/regmap.h b/include/trace/events/regmap.h
index e35e37c..1e3193b 100644
--- a/include/trace/events/regmap.h
+++ b/include/trace/events/regmap.h
@@ -106,6 +106,30 @@ DEFINE_EVENT(regmap_block, regmap_hw_write_done,
 	TP_ARGS(dev, reg, count)
 );
 
+TRACE_EVENT(regcache_sync,
+
+	TP_PROTO(struct device *dev, const char *type,
+		 const char *status),
+
+	TP_ARGS(dev, type, status),
+
+	TP_STRUCT__entry(
+		__string(       name,           dev_name(dev)   )
+		__string(	status,		status		)
+		__string(	type,		type		)
+		__field(	int,		type		)
+	),
+
+	TP_fast_assign(
+		__assign_str(name, dev_name(dev));
+		__assign_str(status, status);
+		__assign_str(type, type);
+	),
+
+	TP_printk("%s type=%s status=%s", __get_str(name),
+		  __get_str(type), __get_str(status))
+);
+
 #endif /* _TRACE_REGMAP_H */
 
 /* This part must be outside protection */
-- 
1.7.6.1

[PATCH 6/8] regmap: Incorporate the regcache core into regmap

[Dimitris Papastamos]

This patch incorporates the regcache core code into regmap.  All previous
patches have been no-ops essentially up to this point.

The bulk read operation is not supported by regcache at the moment.  This
will be implemented incrementally.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regmap.c |   55 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index fa2bd89..fdc4266 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -146,6 +146,12 @@ struct regmap *regmap_init(struct device *dev,
 	map->readable_reg = config->readable_reg;
 	map->volatile_reg = config->volatile_reg;
 	map->precious_reg = config->precious_reg;
+	map->cache_type = config->cache_type;
+	map->num_cache_defaults_raw = config->num_cache_defaults_raw;
+	map->cache_defaults = NULL;
+	map->cache_defaults_raw = config->cache_defaults_raw;
+	map->cache_size_raw = (config->val_bits / 8) * config->num_cache_defaults_raw;
+	map->cache_word_size = config->val_bits / 8;
 
 	switch (config->reg_bits) {
 	case 4:
@@ -201,6 +207,12 @@ struct regmap *regmap_init(struct device *dev,
 		goto err_bus;
 	}
 
+#ifdef CONFIG_REGCACHE
+	ret = regcache_init(map);
+	if (ret < 0)
+		goto err_bus;
+#endif
+
 	regmap_debugfs_init(map);
 
 	return map;
@@ -219,6 +231,9 @@ EXPORT_SYMBOL_GPL(regmap_init);
  */
 void regmap_exit(struct regmap *map)
 {
+#ifdef CONFIG_REGCACHE
+	regcache_exit(map);
+#endif
 	regmap_debugfs_exit(map);
 	kfree(map->work_buf);
 	module_put(map->bus->owner);
@@ -321,6 +336,20 @@ int regmap_write(struct regmap *map, unsigned int reg, unsigned int val)
 
 	mutex_lock(&map->lock);
 
+#ifdef CONFIG_REGCACHE
+	if (!map->cache_bypass) {
+		ret = regcache_write(map, reg, val);
+		if (ret < 0) {
+			mutex_unlock(&map->lock);
+			return ret;
+		}
+		if (map->cache_only) {
+			mutex_unlock(&map->lock);
+			return 0;
+		}
+	}
+#endif
+
 	ret = _regmap_write(map, reg, val);
 
 	mutex_unlock(&map->lock);
@@ -422,6 +451,16 @@ int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val)
 
 	mutex_lock(&map->lock);
 
+#ifdef CONFIG_REGCACHE
+	if (!map->cache_bypass) {
+		ret = regcache_read(map, reg, val);
+		if (!ret) {
+			mutex_unlock(&map->lock);
+			return 0;
+		}
+	}
+#endif
+
 	ret = _regmap_read(map, reg, val);
 
 	mutex_unlock(&map->lock);
@@ -473,6 +512,8 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 	int ret, i;
 	size_t val_bytes = map->format.val_bytes;
 
+	WARN_ON(map->cache_type != REGCACHE_NONE);
+
 	if (!map->format.parse_val)
 		return -EINVAL;
 
@@ -512,6 +553,20 @@ int regmap_update_bits(struct regmap *map, unsigned int reg,
 	tmp &= ~mask;
 	tmp |= val & mask;
 
+#ifdef CONFIG_REGCACHE
+	if (!map->cache_bypass) {
+		ret = regcache_write(map, reg, tmp);
+		if (ret < 0) {
+			mutex_unlock(&map->lock);
+			return ret;
+		}
+		if (map->cache_only) {
+			mutex_unlock(&map->lock);
+			return 0;
+		}
+	}
+#endif
+
 	ret = _regmap_write(map, reg, tmp);
 
 out:
-- 
1.7.6.1

[PATCH 7/8] regmap: It is impossible to be given a NULL defaults cache

[Dimitris Papastamos]

In the new regcache API, it is impossible to hand a NULL defaults
cache to any of the cache compression types.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regcache-lzo.c |   30 +++---------------------------
 1 files changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/base/regmap/regcache-lzo.c b/drivers/base/regmap/regcache-lzo.c
index 9e0d7be..18e1a8c 100644
--- a/drivers/base/regmap/regcache-lzo.c
+++ b/drivers/base/regmap/regcache-lzo.c
@@ -125,32 +125,17 @@ static int regcache_lzo_init(struct regmap *map)
 {
 	struct regcache_lzo_ctx **lzo_blocks;
 	size_t bmp_size;
-	int ret, tofree, i, blksize, blkcount;
+	int ret, i, blksize, blkcount;
 	const char *p, *end;
 	unsigned long *sync_bmp;
 
 	ret = 0;
 
-	/*
-	 * If we have not been given a default register cache
-	 * then allocate a dummy zero-ed out region, compress it
-	 * and remember to free it afterwards.
-	 */
-	tofree = 0;
-	if (!map->cache_defaults_raw) {
-		map->cache_defaults_raw = kzalloc(map->cache_size_raw, GFP_KERNEL);
-		if (!map->cache_defaults_raw)
-			return -ENOMEM;
-		tofree = 1;
-	}
-
 	blkcount = regcache_lzo_block_count();
 	map->cache = kzalloc(blkcount * sizeof *lzo_blocks,
 			     GFP_KERNEL);
-	if (!map->cache) {
-		ret = -ENOMEM;
-		goto err_tofree;
-	}
+	if (!map->cache)
+		return -ENOMEM;
 	lzo_blocks = map->cache;
 
 	/*
@@ -203,18 +188,9 @@ static int regcache_lzo_init(struct regmap *map)
 			lzo_blocks[i]->src_len;
 	}
 
-	if (tofree) {
-		kfree(map->cache_defaults_raw);
-		map->cache_defaults_raw = NULL;
-	}
 	return 0;
 err:
 	regcache_exit(map);
-err_tofree:
-	if (tofree) {
-		kfree(map->cache_defaults_raw);
-		map->cache_defaults_raw = NULL;
-	}
 	return ret;
 }
 
-- 
1.7.6.1

[PATCH 8/8] regmap: Support NULL cache_defaults_raw

[Dimitris Papastamos]

Some IC's like PMIC's tend not to have cache defaults.  Support this
by reading back from the hardware using the bulk read operation the values
of all registers and constructing the cache defaults manually.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/internal.h |    2 ++
 drivers/base/regmap/regcache.c |   27 ++++++++++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 9f228c7..f8b6219 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -65,6 +65,8 @@ struct regmap {
 	unsigned int cache_only:1;
 	/* if set, only the HW is modified not the cache */
 	unsigned int cache_bypass:1;
+	/* if set, remember to free cache_defaults_raw */
+	unsigned int cache_free:1;
 
 	struct reg_default *cache_defaults;
 	const void *cache_defaults_raw;
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 6346d77..fb35fc4 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -31,7 +31,9 @@ int regcache_init(struct regmap *map)
 {
 	int i, j;
 	int count;
+	int ret;
 	unsigned int val;
+	void *tmp_buf;
 
 	if (map->cache_type == REGCACHE_NONE)
 		return 0;
@@ -51,9 +53,25 @@ int regcache_init(struct regmap *map)
 	if (!map->cache_ops->name)
 		return -EINVAL;
 
-	if (!map->cache_defaults_raw || !map->num_cache_defaults_raw) {
-		dev_err(map->dev, "Client has not provided a defaults cache\n");
-		return -EINVAL;
+	/* Some devices such as PMIC's don't have cache defaults,
+	 * we cope with this by reading back the HW registers and
+	 * crafting the cache defaults by hand.
+	 */
+	if (!map->cache_defaults_raw) {
+		if (!map->num_cache_defaults_raw)
+			return -EINVAL;
+		dev_warn(map->dev, "No cache defaults, reading back from HW\n");
+		tmp_buf = kmalloc(map->cache_size_raw, GFP_KERNEL);
+		if (!tmp_buf)
+			return -EINVAL;
+		ret = regmap_bulk_read(map, 0, tmp_buf,
+				       map->num_cache_defaults_raw);
+		if (ret < 0) {
+			kfree(tmp_buf);
+			return ret;
+		}
+		map->cache_defaults_raw = tmp_buf;
+		map->cache_free = 1;
 	}
 
 	/* calculate the size of cache_defaults */
@@ -98,6 +116,9 @@ void regcache_exit(struct regmap *map)
 	BUG_ON(!map->cache_ops);
 
 	kfree(map->cache_defaults);
+	if (map->cache_free)
+		kfree(map->cache_defaults_raw);
+
 	if (map->cache_ops->exit) {
 		dev_dbg(map->dev, "Destroying %s cache\n",
 			map->cache_ops->name);
-- 
1.7.6.1