Skip to contentSkip to conversation

Introduce caching support for regmap

PATCH Superseded Dimitris Papastamos 3 participants 14 messages Sep 5, 2011 – Sep 15, 2011

Subsystems (3)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v2 cover Introduce caching support for regmap

This patch series introduces register caching support for regmap. I've implemented all the suggestions since v1, except the use of a dedicated init function for each of the compress types. Due to lack of time, I'll submit the updated patches in a couple of days. I'm aware this can't be merged as is due to this outstanding change, but I'm posting it in any case for reviewing.

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 (6):
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

 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  |  399 ++++++++++++++++++++++++++++++++
 drivers/base/regmap/regcache.c         |  311 +++++++++++++++++++++++++
 drivers/base/regmap/regmap.c           |   56 +++++
 include/linux/regmap.h                 |   27 ++-
 include/trace/events/regmap.h          |   24 ++
 9 files changed, 1295 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
Signature · 1 line
1.7.6.1
#1 lore raw

PATCH v2 1/6 regmap: Introduce caching support

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.

---
 drivers/base/regmap/Makefile   |    2 +-
 drivers/base/regmap/internal.h |   52 +++++++
 drivers/base/regmap/regcache.c |  302 ++++++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h         |   24 +++-
 4 files changed, 374 insertions(+), 6 deletions(-)
 create mode 100644 drivers/base/regmap/regcache.c
drivers/base/regmap/Makefile +1 -1
index 057c13f..2e103ea 100644
@@ -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
drivers/base/regmap/internal.h +52 -0

68 lines not on this page · raw message

drivers/base/regmap/regcache.c new file +302 -0

302 lines not on this page · raw message

include/linux/regmap.h +19 -5
index 449e264..7591e01 100644
@@ -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,14 @@ 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: Power on reset values for registers (for use with
+ *                  register cache support).
+ * @num_cache_defaults: Number of elements in cache_defaults.
+ * @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 +74,12 @@ 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;
+	struct reg_default *cache_defaults;
+	unsigned int num_cache_defaults;
+	const void *cache_defaults_raw;
+	unsigned int num_cache_defaults_raw;
 };
 
 typedef int (*regmap_hw_write)(struct device *dev, const void *data,
Signature · 1 line
1.7.6.1
#2 lore raw

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

Quoted 12 lines · 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.

Quoted 1 line

I gave it a try with the rbtree caching on real hardware and noticed some smaller issues, but with those fixed it seems to work fine.

Quoted 152 lines
---
 drivers/base/regmap/Makefile   |    2 +-
 drivers/base/regmap/internal.h |   52 +++++++
 drivers/base/regmap/regcache.c |  302 ++++++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h         |   24 +++-
 4 files changed, 374 insertions(+), 6 deletions(-)
 create mode 100644 drivers/base/regmap/regcache.c
drivers/base/regmap/Makefile +1 -1
index 057c13f..2e103ea 100644
@@ -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
drivers/base/regmap/internal.h +52 -0
index 5ab3fef..dbeccd2 100644
@@ -17,6 +17,7 @@
 #include <linux/fs.h>
 
 struct regmap;
+struct regcache_ops;
 
 struct regmap_format {
 	size_t buf_size;
@@ -46,6 +47,40 @@ 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;
+	/* if set, remember to free cache_defaults_raw */
+	unsigned int cache_free: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 +98,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
drivers/base/regmap/regcache.c new file +53 -0
new file mode 100644
index 0000000..f26df00
@@ -0,0 +1,302 @@
+/*
+ * Register cache access API
+ *
+ * Copyright 2011 Wolfson Microelectronics plc
+ *
+ * Author: Dimitris Papastamos <[email protected]>
+ *
+ * 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;
+	int ret;
+	unsigned int val;
+	void *tmp_buf;
+
+	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->read ||
+	    !map->cache_ops->write ||
+	    !map->cache_ops->name)
+		return -EINVAL;
+
+	/* We still need to ensure that the cache_defaults
+	 * won't vanish from under us.  We'll need to make
+	 * a copy of it.
+	 */

drivers/base/regmap/regcache.c line 63

Quoted 10 lines
drivers/base/regmap/regcache.c new file +10 -0
+	if (map->cache_defaults) {
+		if (!map->num_cache_defaults)
+			return -EINVAL;
+		tmp_buf = kmemdup(map->cache_defaults, map->num_cache_defaults *
+				  sizeof(struct reg_default), GFP_KERNEL);
+		if (!tmp_buf)
+			return -ENOMEM;
+		map->cache_defaults = tmp_buf;
+		goto cache_init;
+	}

Maybe put the cache initialization from raw cache into a separate function and don't use goto. Should keep things a bit more readable.

Quoted 69 lines
drivers/base/regmap/regcache.c new file +68 -0
+	/* 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 */
+	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++;
+	}
+
+cache_init:
+	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;
+}
+
[...]
+
+/**
+ * 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)

drivers/base/regmap/regcache.c line 159

Quoted 10 lines
drivers/base/regmap/regcache.c new file +10 -0
+{
+	unsigned int max_reg;
+
+	if (map->cache_type == REGCACHE_NONE)
+		return 0;
+
+	BUG_ON(!map->cache_ops);
+
+	if (!regmap_readable(map, reg))
+		return 0;

Shouldn't we return some kind of error here? Otherwise value we indicate success, but *value contain random garbage.

Quoted 24 lines
drivers/base/regmap/regcache.c new file +24 -0
+
+	if (map->max_register)
+		max_reg = map->max_register;
+	else
+		max_reg = map->num_cache_defaults_raw;
+
+
+	if (reg < max_reg && !regmap_volatile(map, reg))
+		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)

drivers/base/regmap/regcache.c line 193

Quoted 10 lines
drivers/base/regmap/regcache.c new file +10 -0
+{
+	unsigned int max_reg;
+
+	if (map->cache_type == REGCACHE_NONE)
+		return 0;
+
+	BUG_ON(!map->cache_ops);
+
+	if (!regmap_writeable(map, reg))
+		return 0;

Same here.

Quoted 48 lines
drivers/base/regmap/regcache.c new file +12 -0
+
+	if (map->max_register)
+		max_reg = map->max_register;
+	else
+		max_reg = map->num_cache_defaults_raw;
+
+	if (reg < max_reg && !regmap_volatile(map, reg))
+		return map->cache_ops->write(map, reg, value);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(regcache_write);
+
[...]
include/linux/regmap.h +13 -3
index 449e264..7591e01 100644
@@ -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,14 @@ 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: Power on reset values for registers (for use with
+ *                  register cache support).
+ * @num_cache_defaults: Number of elements in cache_defaults.
+ * @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;

include/linux/regmap.h struct regmap_config { · line 79

Quoted 8 lines
include/linux/regmap.h +3 -2
@@ -64,8 +74,12 @@ 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;
+	struct reg_default *cache_defaults;

Should be made const.

Quoted 4 lines
include/linux/regmap.h +3 -0
+	unsigned int num_cache_defaults;
+	const void *cache_defaults_raw;
+	unsigned int num_cache_defaults_raw;
 };
#3 lore raw

PATCH v2 2/6 regmap: Add the indexed cache support

In reply to 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.

---
 drivers/base/regmap/Makefile           |    2 +-
 drivers/base/regmap/internal.h         |    1 +
 drivers/base/regmap/regcache-indexed.c |   65 ++++++++++++++++++++++++++++++++
 drivers/base/regmap/regcache.c         |    1 +
 include/linux/regmap.h                 |    1 +
 5 files changed, 69 insertions(+), 1 deletions(-)
 create mode 100644 drivers/base/regmap/regcache-indexed.c
drivers/base/regmap/Makefile +1 -1
index 2e103ea..418d151 100644
@@ -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
drivers/base/regmap/internal.h +1 -0
index dbeccd2..4e5a1e1 100644
@@ -115,4 +115,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
drivers/base/regmap/regcache-indexed.c new file +65 -0

65 lines not on this page · raw message

drivers/base/regmap/regcache.c +1 -0
index f26df00..594d518 100644
@@ -16,6 +16,7 @@
 #include "internal.h"
 
 static const struct regcache_ops *cache_types[] = {
+	&regcache_indexed_ops,
 };
 
 int regcache_init(struct regmap *map)
include/linux/regmap.h +1 -0
index 7591e01..502aa6c 100644
@@ -23,6 +23,7 @@ struct spi_device;
 /* An enum of all the supported cache types */
 enum regcache_type {
 	REGCACHE_NONE,
+	REGCACHE_INDEXED,
 };
 
 /**
Signature · 1 line
1.7.6.1
#4 lore raw

PATCH v2 3/6 regmap: Add the rbtree cache support

In reply to 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.

---
 drivers/base/regmap/Makefile          |    2 +-
 drivers/base/regmap/internal.h        |    1 +
 drivers/base/regmap/regcache-rbtree.c |  399 +++++++++++++++++++++++++++++++++
 drivers/base/regmap/regcache.c        |    1 +
 include/linux/regmap.h                |    1 +
 5 files changed, 403 insertions(+), 1 deletions(-)
 create mode 100644 drivers/base/regmap/regcache-rbtree.c
drivers/base/regmap/Makefile +1 -1
index 418d151..20cd650 100644
@@ -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
drivers/base/regmap/internal.h +1 -0
index 4e5a1e1..49f5831 100644
@@ -116,4 +116,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
drivers/base/regmap/regcache-rbtree.c new file +399 -0

399 lines not on this page · raw message

drivers/base/regmap/regcache.c +1 -0
index 594d518..0af3715 100644
@@ -17,6 +17,7 @@
 
 static const struct regcache_ops *cache_types[] = {
 	&regcache_indexed_ops,
+	&regcache_rbtree_ops,
 };
 
 int regcache_init(struct regmap *map)
include/linux/regmap.h +1 -0
index 502aa6c..d7394cb 100644
@@ -24,6 +24,7 @@ struct spi_device;
 enum regcache_type {
 	REGCACHE_NONE,
 	REGCACHE_INDEXED,
+	REGCACHE_RBTREE,
 };
 
 /**
Signature · 1 line
1.7.6.1
#5 lore raw

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

Quoted 10 lines · Dimitris Papastamos

[...]

drivers/base/regmap/regcache-rbtree.c new file +9 -0
+
+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)

drivers/base/regmap/regcache-rbtree.c line 165

Quoted 10 lines
drivers/base/regmap/regcache-rbtree.c new file +10 -0
+		return -ENOMEM;
+
+	rbtree_ctx = map->cache;
+	rbtree_ctx->root = RB_ROOT;
+	rbtree_ctx->cached_rbnode = NULL;
+
+	for (i = 0; i < map->num_cache_defaults; i++) {
+		ret = regcache_rbtree_write(map,
+					    map->cache_defaults[ret].reg,
+					    map->cache_defaults[ret].def);

map->cache_defaults[i]

Quoted 11 lines
drivers/base/regmap/regcache-rbtree.c new file +10 -0
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	regcache_exit(map);
+	return ret;
+}
[...]
#6 lore raw

PATCH v2 4/6 regmap: Add the LZO cache support

In reply to 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.

---
 drivers/base/regmap/Makefile       |    2 +-
 drivers/base/regmap/internal.h     |    2 +
 drivers/base/regmap/regcache-lzo.c |  361 ++++++++++++++++++++++++++++++++++++
 drivers/base/regmap/regcache.c     |    1 +
 include/linux/regmap.h             |    1 +
 5 files changed, 366 insertions(+), 1 deletions(-)
 create mode 100644 drivers/base/regmap/regcache-lzo.c
drivers/base/regmap/Makefile +1 -1
index 20cd650..0573c8a 100644
@@ -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
drivers/base/regmap/internal.h +2 -0
index 49f5831..f8b6219 100644
@@ -117,4 +117,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
drivers/base/regmap/regcache-lzo.c new file +361 -0

361 lines not on this page · raw message

drivers/base/regmap/regcache.c +1 -0
index 0af3715..67613de 100644
@@ -18,6 +18,7 @@
 static const struct regcache_ops *cache_types[] = {
 	&regcache_indexed_ops,
 	&regcache_rbtree_ops,
+	&regcache_lzo_ops,
 };
 
 int regcache_init(struct regmap *map)
include/linux/regmap.h +1 -0
index d7394cb..94c6fd7 100644
@@ -25,6 +25,7 @@ enum regcache_type {
 	REGCACHE_NONE,
 	REGCACHE_INDEXED,
 	REGCACHE_RBTREE,
+	REGCACHE_LZO
 };
 
 /**
Signature · 1 line
1.7.6.1
#7 lore raw

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

Quoted 14 lines · 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.

Quoted 9 lines
---
 drivers/base/regmap/Makefile       |    2 +-
 drivers/base/regmap/internal.h     |    2 +
 drivers/base/regmap/regcache-lzo.c |  361 ++++++++++++++++++++++++++++++++++++
 drivers/base/regmap/regcache.c     |    1 +
 include/linux/regmap.h             |    1 +
 5 files changed, 366 insertions(+), 1 deletions(-)
 create mode 100644 drivers/base/regmap/regcache-lzo.c

Fails to compile due to CONFIG_LZO_{COMPRESS,DECOMPRESS} not being selected. Also it will have issues, with caches where we don't use raw cache defaults.

#8 lore raw

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

Quoted 25 lines · Lars-Peter Clausen
Quoted 14 lines · 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.

Quoted 9 lines
---
 drivers/base/regmap/Makefile       |    2 +-
 drivers/base/regmap/internal.h     |    2 +
 drivers/base/regmap/regcache-lzo.c |  361 ++++++++++++++++++++++++++++++++++++
 drivers/base/regmap/regcache.c     |    1 +
 include/linux/regmap.h             |    1 +
 5 files changed, 366 insertions(+), 1 deletions(-)
 create mode 100644 drivers/base/regmap/regcache-lzo.c
Quoted 2 lines

Fails to compile due to CONFIG_LZO_{COMPRESS,DECOMPRESS} not being selected. Also it will have issues, with caches where we don't use raw cache defaults.

Yes, the LZO code will need to use cache_defaults instead of cache_defaults_raw. Since there are no users at the moment and since most of the potential users do actually use cache_defaults_raw, I think I can fix this incrementally.

Thanks,
Dimitris

#9 lore raw

PATCH v2 5/6 regmap: Add the regcache_sync trace event

In reply to Dimitris Papastamos
---
 drivers/base/regmap/regcache.c |    8 +++++++-
 include/trace/events/regmap.h  |   24 ++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletions(-)
drivers/base/regmap/regcache.c +7 -1
index 67613de..abd13ee 100644
@@ -219,12 +219,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;
 }
include/trace/events/regmap.h +24 -0
index e35e37c..1e3193b 100644
@@ -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 */
Signature · 1 line
1.7.6.1
#10 lore raw

PATCH v2 6/6 regmap: Incorporate the regcache core into regmap

In reply to 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.

---
 drivers/base/regmap/regmap.c |   56 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 56 insertions(+), 0 deletions(-)
drivers/base/regmap/regmap.c +56 -0
index fa2bd89..df4e00a 100644
@@ -146,6 +146,13 @@ 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->cache_defaults = config->cache_defaults;
+	map->num_cache_defaults = config->num_cache_defaults;
+	map->num_cache_defaults_raw = config->num_cache_defaults_raw;
+	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 +208,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 +232,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 +337,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 +452,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 +513,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 +554,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:
Signature · 1 line
1.7.6.1
#11 lore raw

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

Quoted 30 lines · 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.

---
 drivers/base/regmap/regmap.c |   56 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 56 insertions(+), 0 deletions(-)
drivers/base/regmap/regmap.c +7 -0
index fa2bd89..df4e00a 100644
@@ -146,6 +146,13 @@ 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->cache_defaults = config->cache_defaults;
+	map->num_cache_defaults = config->num_cache_defaults;
+	map->num_cache_defaults_raw = config->num_cache_defaults_raw;
+	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:

drivers/base/regmap/regmap.c struct regmap *regmap_init(struct device *dev, · line 211

Quoted 4 lines
drivers/base/regmap/regmap.c +1 -0
@@ -201,6 +208,12 @@ struct regmap *regmap_init(struct device *dev,
 		goto err_bus;
 	}
 
+#ifdef CONFIG_REGCACHE

CONFIG_REGCACHE is not defined, so we'll end up with no regcache support.

Quoted 41 lines
drivers/base/regmap/regmap.c +22 -0
+	ret = regcache_init(map);
+	if (ret < 0)
+		goto err_bus;
+#endif
+
 	regmap_debugfs_init(map);
 
 	return map;
@@ -219,6 +232,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 +337,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 +452,16 @@ int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val)
 

drivers/base/regmap/regmap.c int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val) · line 462

Quoted 10 lines
drivers/base/regmap/regmap.c +8 -0
 	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;
+		}
+	}

I think this will need tighter integration. For example do we want to fallback to a hw read when the cached read fails? And also regcache_read will return for REGCACHE_NONE, which means we neither do a cached read nor a hw read.

Quoted 5 lines
drivers/base/regmap/regmap.c +2 -0
+#endif
+
 	ret = _regmap_read(map, reg, val);
 
 	mutex_unlock(&map->lock);
#12 lore raw

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

Quoted 14 lines · Lars-Peter Clausen

drivers/base/regmap/regmap.c int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val) · line 462

Quoted 8 lines · Dimitris Papastamos
drivers/base/regmap/regmap.c +8 -0
+#ifdef CONFIG_REGCACHE
+	if (!map->cache_bypass) {
+		ret = regcache_read(map, reg, val);
+		if (!ret) {
+			mutex_unlock(&map->lock);
+			return 0;
+		}
+	}

I think this will need tighter integration. For example do we want to fallback to a hw read when the cached read fails? And also regcache_read will return for REGCACHE_NONE, which means we neither do a cached read nor a hw read.

I think falling through to hardware is sensible - if nothing else we've got to do that for uncached registers. Though if we're in cache only mode we probably don't want to do that as the hardware may be powered off. But in general it does make sense to me.

#13 lore raw
In reply to Dimitris Papastamos
Quoted 6 lines · Dimitris Papastamos

This patch series introduces register caching support for regmap. I've implemented all the suggestions since v1, except the use of a dedicated init function for each of the compress types. Due to lack of time, I'll submit the updated patches in a couple of days. I'm aware this can't be merged as is due to this outstanding change, but I'm posting it in any case for reviewing.

This is looking good - the major thing is the Kconfig stuff to make sure it's properly enabled.