* [PATCH 1/6 v3] regmap: Introduce caching support
2011-09-15 10:34 [PATCH 0/6 v3] Introduce caching support for regmap Dimitris Papastamos
@ 2011-09-15 10:34 ` Dimitris Papastamos
2011-09-15 15:11 ` Lars-Peter Clausen
2011-09-15 10:34 ` [PATCH 2/6 v3] regmap: Add the indexed cache support Dimitris Papastamos
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-15 10:34 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Brown, Liam Girdwood, Graeme Gregory, Samuel Oritz,
Lars-Peter Clausen
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 | 52 +++++++
drivers/base/regmap/regcache.c | 313 ++++++++++++++++++++++++++++++++++++++++
include/linux/regmap.h | 24 +++-
4 files changed, 385 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..dbeccd2 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,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
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
new file mode 100644
index 0000000..ff79f9f
--- /dev/null
+++ b/drivers/base/regmap/regcache.c
@@ -0,0 +1,313 @@
+/*
+ * 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[] = {
+};
+
+static int regcache_hw_init(struct regmap *map)
+{
+ int i, j;
+ int ret;
+ int count;
+ unsigned int val;
+ void *tmp_buf;
+
+ 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++;
+ }
+
+ return 0;
+}
+
+int regcache_init(struct regmap *map)
+{
+ int ret;
+ int i;
+ 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.
+ */
+ 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;
+ } else {
+ /* 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.
+ */
+ ret = regcache_hw_init(map);
+ if (ret < 0)
+ return ret;
+ }
+
+ 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_free)
+ kfree(map->cache_defaults_raw);
+
+ 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)
+{
+ unsigned int max_reg;
+
+ if (map->cache_type == REGCACHE_NONE)
+ return -ENOSYS;
+
+ BUG_ON(!map->cache_ops);
+
+ if (!regmap_readable(map, reg))
+ return -EIO;
+
+ 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)
+{
+ unsigned int max_reg;
+
+ if (map->cache_type == REGCACHE_NONE)
+ return 0;
+
+ BUG_ON(!map->cache_ops);
+
+ if (!regmap_writeable(map, reg))
+ return -EIO;
+
+ 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);
+
+/**
+ * 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)
+{
+ unsigned 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..7591e01 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,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,
--
1.7.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/6 v3] regmap: Introduce caching support
2011-09-15 10:34 ` [PATCH 1/6 v3] regmap: Introduce caching support Dimitris Papastamos
@ 2011-09-15 15:11 ` Lars-Peter Clausen
2011-09-15 15:32 ` Dimitris Papastamos
0 siblings, 1 reply; 14+ messages in thread
From: Lars-Peter Clausen @ 2011-09-15 15:11 UTC (permalink / raw)
To: Dimitris Papastamos
Cc: linux-kernel, Mark Brown, Liam Girdwood, Graeme Gregory,
Samuel Oritz
Tested-by: Lars-Peter Clausen <lars@metafoo.de> (for REGCACHE_NONE and
REGCACHE_RBTREE)
Some minor comments though:
> -
> +/**
> + * 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)
> +{
> + unsigned int max_reg;
> +
> + if (map->cache_type == REGCACHE_NONE)
> + return -ENOSYS;
> +
> + BUG_ON(!map->cache_ops);
> +
> + if (!regmap_readable(map, reg))
> + return -EIO;
> +
> + if (map->max_register)
> + max_reg = map->max_register;
> + else
> + max_reg = map->num_cache_defaults_raw;
In my opinion we should initialize max_register to num_cache_defaults_raw if it
is not set during initialization instead of doing this check each time. Also
regmap_readable already checks whether the register is in the register range,
so there is no need to repeat the check here.
> +
> +
> + if (reg < max_reg && !regmap_volatile(map, reg))
> + return map->cache_ops->read(map, reg, value);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(regcache_read);
> +
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/6 v3] regmap: Introduce caching support
2011-09-15 15:11 ` Lars-Peter Clausen
@ 2011-09-15 15:32 ` Dimitris Papastamos
0 siblings, 0 replies; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-15 15:32 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: linux-kernel, Mark Brown, Liam Girdwood, Graeme Gregory,
Samuel Oritz
On Thu, Sep 15, 2011 at 05:11:17PM +0200, Lars-Peter Clausen wrote:
> Tested-by: Lars-Peter Clausen <lars@metafoo.de> (for REGCACHE_NONE and
> REGCACHE_RBTREE)
>
> Some minor comments though:
>
> > -
> > +/**
> > + * 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)
> > +{
> > + unsigned int max_reg;
> > +
> > + if (map->cache_type == REGCACHE_NONE)
> > + return -ENOSYS;
> > +
> > + BUG_ON(!map->cache_ops);
> > +
> > + if (!regmap_readable(map, reg))
> > + return -EIO;
> > +
> > + if (map->max_register)
> > + max_reg = map->max_register;
> > + else
> > + max_reg = map->num_cache_defaults_raw;
>
> In my opinion we should initialize max_register to num_cache_defaults_raw if it
> is not set during initialization instead of doing this check each time. Also
> regmap_readable already checks whether the register is in the register range,
> so there is no need to repeat the check here.
Yes, that makes sense.
> > +
> > +
> > + if (reg < max_reg && !regmap_volatile(map, reg))
> > + return map->cache_ops->read(map, reg, value);
> > + return -EINVAL;
> > +}
> > +EXPORT_SYMBOL_GPL(regcache_read);
> > +
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/6 v3] regmap: Add the indexed cache support
2011-09-15 10:34 [PATCH 0/6 v3] Introduce caching support for regmap Dimitris Papastamos
2011-09-15 10:34 ` [PATCH 1/6 v3] regmap: Introduce caching support Dimitris Papastamos
@ 2011-09-15 10:34 ` Dimitris Papastamos
2011-09-15 10:34 ` [PATCH 3/6 v3] regmap: Add the rbtree " Dimitris Papastamos
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-15 10:34 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Brown, Liam Girdwood, Graeme Gregory, Samuel Oritz,
Lars-Peter Clausen
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 | 1 +
include/linux/regmap.h | 1 +
5 files changed, 69 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 dbeccd2..4e5a1e1 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -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
diff --git a/drivers/base/regmap/regcache-indexed.c b/drivers/base/regmap/regcache-indexed.c
new file mode 100644
index 0000000..fab43d3
--- /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 ff79f9f..e4dda94 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -16,6 +16,7 @@
#include "internal.h"
static const struct regcache_ops *cache_types[] = {
+ ®cache_indexed_ops,
};
static int regcache_hw_init(struct regmap *map)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 7591e01..502aa6c 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/6 v3] regmap: Add the rbtree cache support
2011-09-15 10:34 [PATCH 0/6 v3] Introduce caching support for regmap Dimitris Papastamos
2011-09-15 10:34 ` [PATCH 1/6 v3] regmap: Introduce caching support Dimitris Papastamos
2011-09-15 10:34 ` [PATCH 2/6 v3] regmap: Add the indexed cache support Dimitris Papastamos
@ 2011-09-15 10:34 ` Dimitris Papastamos
2011-09-15 10:34 ` [PATCH 4/6 v3] regmap: Add the LZO " Dimitris Papastamos
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-15 10:34 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Brown, Liam Girdwood, Graeme Gregory, Samuel Oritz,
Lars-Peter Clausen
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 | 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
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 4e5a1e1..49f5831 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -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
diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
new file mode 100644
index 0000000..3fee01c
--- /dev/null
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -0,0 +1,399 @@
+/*
+ * 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 {
+ /* the actual rbtree node holding this block */
+ struct rb_node node;
+ /* base register handled by this block */
+ unsigned int base_reg;
+ /* number of bytes needed to represent the register index */
+ unsigned int word_size;
+ /* block of adjacent registers */
+ void *block;
+ /* number of registers available in the block */
+ unsigned int blklen;
+} __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;
+
+ for (i = 0; i < map->num_cache_defaults; i++) {
+ ret = regcache_rbtree_write(map,
+ map->cache_defaults[i].reg,
+ map->cache_defaults[i].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 e4dda94..171ecc28 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -17,6 +17,7 @@
static const struct regcache_ops *cache_types[] = {
®cache_indexed_ops,
+ ®cache_rbtree_ops,
};
static int regcache_hw_init(struct regmap *map)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 502aa6c..d7394cb 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 4/6 v3] regmap: Add the LZO cache support
2011-09-15 10:34 [PATCH 0/6 v3] Introduce caching support for regmap Dimitris Papastamos
` (2 preceding siblings ...)
2011-09-15 10:34 ` [PATCH 3/6 v3] regmap: Add the rbtree " Dimitris Papastamos
@ 2011-09-15 10:34 ` Dimitris Papastamos
2011-09-15 10:34 ` [PATCH 5/6 v3] regmap: Add the regcache_sync trace event Dimitris Papastamos
2011-09-15 10:34 ` [PATCH 6/6 v3] regmap: Incorporate the regcache core into regmap Dimitris Papastamos
5 siblings, 0 replies; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-15 10:34 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Brown, Liam Girdwood, Graeme Gregory, Samuel Oritz,
Lars-Peter Clausen
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/Kconfig | 2 +
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 +
6 files changed, 368 insertions(+), 1 deletions(-)
create mode 100644 drivers/base/regmap/regcache-lzo.c
diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index fabbf6c..2fc6a66 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -4,6 +4,8 @@
config REGMAP
default y if (REGMAP_I2C || REGMAP_SPI)
+ select LZO_COMPRESS
+ select LZO_DECOMPRESS
bool
config REGMAP_I2C
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 49f5831..f8b6219 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -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
diff --git a/drivers/base/regmap/regcache-lzo.c b/drivers/base/regmap/regcache-lzo.c
new file mode 100644
index 0000000..fd29893
--- /dev/null
+++ b/drivers/base/regmap/regcache-lzo.c
@@ -0,0 +1,361 @@
+/*
+ * 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, i, blksize, blkcount;
+ const char *p, *end;
+ unsigned long *sync_bmp;
+
+ ret = 0;
+
+ blkcount = regcache_lzo_block_count();
+ map->cache = kzalloc(blkcount * sizeof *lzo_blocks,
+ GFP_KERNEL);
+ if (!map->cache)
+ return -ENOMEM;
+ 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;
+ }
+
+ return 0;
+err:
+ regcache_exit(map);
+ 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 171ecc28..2565904 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -18,6 +18,7 @@
static const struct regcache_ops *cache_types[] = {
®cache_indexed_ops,
®cache_rbtree_ops,
+ ®cache_lzo_ops,
};
static int regcache_hw_init(struct regmap *map)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index d7394cb..94c6fd7 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/6 v3] regmap: Add the regcache_sync trace event
2011-09-15 10:34 [PATCH 0/6 v3] Introduce caching support for regmap Dimitris Papastamos
` (3 preceding siblings ...)
2011-09-15 10:34 ` [PATCH 4/6 v3] regmap: Add the LZO " Dimitris Papastamos
@ 2011-09-15 10:34 ` Dimitris Papastamos
2011-09-15 10:34 ` [PATCH 6/6 v3] regmap: Incorporate the regcache core into regmap Dimitris Papastamos
5 siblings, 0 replies; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-15 10:34 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Brown, Liam Girdwood, Graeme Gregory, Samuel Oritz,
Lars-Peter Clausen
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 2565904..4440de7 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -230,12 +230,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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 6/6 v3] regmap: Incorporate the regcache core into regmap
2011-09-15 10:34 [PATCH 0/6 v3] Introduce caching support for regmap Dimitris Papastamos
` (4 preceding siblings ...)
2011-09-15 10:34 ` [PATCH 5/6 v3] regmap: Add the regcache_sync trace event Dimitris Papastamos
@ 2011-09-15 10:34 ` Dimitris Papastamos
2011-09-15 15:19 ` Lars-Peter Clausen
5 siblings, 1 reply; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-15 10:34 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Brown, Liam Girdwood, Graeme Gregory, Samuel Oritz,
Lars-Peter Clausen
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 | 46 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index fa2bd89..6858932 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -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,10 @@ struct regmap *regmap_init(struct device *dev,
goto err_bus;
}
+ ret = regcache_init(map);
+ if (ret < 0)
+ goto err_bus;
+
regmap_debugfs_init(map);
return map;
@@ -219,6 +230,7 @@ EXPORT_SYMBOL_GPL(regmap_init);
*/
void regmap_exit(struct regmap *map)
{
+ regcache_exit(map);
regmap_debugfs_exit(map);
kfree(map->work_buf);
module_put(map->bus->owner);
@@ -321,6 +333,18 @@ int regmap_write(struct regmap *map, unsigned int reg, unsigned int val)
mutex_lock(&map->lock);
+ 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;
+ }
+ }
+
ret = _regmap_write(map, reg, val);
mutex_unlock(&map->lock);
@@ -422,6 +446,14 @@ int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val)
mutex_lock(&map->lock);
+ if (!map->cache_bypass) {
+ ret = regcache_read(map, reg, val);
+ if (!ret) {
+ mutex_unlock(&map->lock);
+ return 0;
+ }
+ }
+
ret = _regmap_read(map, reg, val);
mutex_unlock(&map->lock);
@@ -473,6 +505,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 +546,18 @@ int regmap_update_bits(struct regmap *map, unsigned int reg,
tmp &= ~mask;
tmp |= val & mask;
+ 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;
+ }
+ }
+
ret = _regmap_write(map, reg, tmp);
out:
--
1.7.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 6/6 v3] regmap: Incorporate the regcache core into regmap
2011-09-15 10:34 ` [PATCH 6/6 v3] regmap: Incorporate the regcache core into regmap Dimitris Papastamos
@ 2011-09-15 15:19 ` Lars-Peter Clausen
2011-09-15 15:37 ` Dimitris Papastamos
2011-09-15 22:57 ` Mark Brown
0 siblings, 2 replies; 14+ messages in thread
From: Lars-Peter Clausen @ 2011-09-15 15:19 UTC (permalink / raw)
To: Dimitris Papastamos
Cc: linux-kernel, Mark Brown, Liam Girdwood, Graeme Gregory,
Samuel Oritz
> [...]
> @@ -321,6 +333,18 @@ int regmap_write(struct regmap *map, unsigned int reg, unsigned int val)
>
> mutex_lock(&map->lock);
>
> + 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;
> + }
> + }
> +
Would it make sense to move this into _regmap_write ? In that case the code
wouldn't have to be duplicated in regmap_update_bits and as a bonus it wouldn't
have to deal with the mutex either.
> ret = _regmap_write(map, reg, val);
>
> mutex_unlock(&map->lock);
> @@ -422,6 +446,14 @@ int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val)
>
> mutex_lock(&map->lock);
>
> + if (!map->cache_bypass) {
> + ret = regcache_read(map, reg, val);
> + if (!ret) {
> + mutex_unlock(&map->lock);
> + return 0;
> + }
So in case regmap_readable is not true for this register regcache_read will
return -EIO and we'll fallback to an uncached read. This doesn't make sense in
my opinion. Or what are the except semantics regmap_readable supposed to be?
> + }
> +
> ret = _regmap_read(map, reg, val);
>
> mutex_unlock(&map->lock);
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 6/6 v3] regmap: Incorporate the regcache core into regmap
2011-09-15 15:19 ` Lars-Peter Clausen
@ 2011-09-15 15:37 ` Dimitris Papastamos
2011-09-15 22:57 ` Mark Brown
1 sibling, 0 replies; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-15 15:37 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: linux-kernel, Mark Brown, Liam Girdwood, Graeme Gregory,
Samuel Oritz
On Thu, Sep 15, 2011 at 05:19:32PM +0200, Lars-Peter Clausen wrote:
> > [...]
> > @@ -321,6 +333,18 @@ int regmap_write(struct regmap *map, unsigned int reg, unsigned int val)
> >
> > mutex_lock(&map->lock);
> >
> > + 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;
> > + }
> > + }
> > +
>
> Would it make sense to move this into _regmap_write ? In that case the code
> wouldn't have to be duplicated in regmap_update_bits and as a bonus it wouldn't
> have to deal with the mutex either.
Hmm, yes from the point of view of simplifying and avoiding code
duplication.
> > ret = _regmap_write(map, reg, val);
> >
> > mutex_unlock(&map->lock);
> > @@ -422,6 +446,14 @@ int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val)
> >
> > mutex_lock(&map->lock);
> >
> > + if (!map->cache_bypass) {
> > + ret = regcache_read(map, reg, val);
> > + if (!ret) {
> > + mutex_unlock(&map->lock);
> > + return 0;
> > + }
>
> So in case regmap_readable is not true for this register regcache_read will
> return -EIO and we'll fallback to an uncached read. This doesn't make sense in
> my opinion. Or what are the except semantics regmap_readable supposed to be?
I agree the semantics are fuzzy here.
At the moment, we basically have one means of signaling that the cache
can't handle the read request, that is, by returning an error. We
should probably extend that or simply use fixed error codes with a
specially meaning or something similar. I guess there is a more
elegant means of doing this though.
> > + }
> > +
> > ret = _regmap_read(map, reg, val);
> >
> > mutex_unlock(&map->lock);
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 6/6 v3] regmap: Incorporate the regcache core into regmap
2011-09-15 15:19 ` Lars-Peter Clausen
2011-09-15 15:37 ` Dimitris Papastamos
@ 2011-09-15 22:57 ` Mark Brown
2011-09-15 23:20 ` Lars-Peter Clausen
1 sibling, 1 reply; 14+ messages in thread
From: Mark Brown @ 2011-09-15 22:57 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Dimitris Papastamos, linux-kernel, Liam Girdwood, Graeme Gregory,
Samuel Oritz
On Thu, Sep 15, 2011 at 05:19:32PM +0200, Lars-Peter Clausen wrote:
> > + 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;
> > + }
> > + }
> Would it make sense to move this into _regmap_write ? In that case the code
> wouldn't have to be duplicated in regmap_update_bits and as a bonus it wouldn't
> have to deal with the mutex either.
That does seem sensible, yes.
> > + if (!map->cache_bypass) {
> > + ret = regcache_read(map, reg, val);
> > + if (!ret) {
> > + mutex_unlock(&map->lock);
> > + return 0;
> > + }
> So in case regmap_readable is not true for this register regcache_read will
> return -EIO and we'll fallback to an uncached read. This doesn't make sense in
> my opinion. Or what are the except semantics regmap_readable supposed to be?
It's a bit interesting but falling back to asking the hardware is
probably the right thing to do for cases where you're doing things like
using unspecified registers to activate undocumented features - the
register isn't really documented as being there and may do odd things
but if the device manufacturer gives you an approved sequence then you
can probably use it safely.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 6/6 v3] regmap: Incorporate the regcache core into regmap
2011-09-15 22:57 ` Mark Brown
@ 2011-09-15 23:20 ` Lars-Peter Clausen
2011-09-15 23:53 ` Mark Brown
0 siblings, 1 reply; 14+ messages in thread
From: Lars-Peter Clausen @ 2011-09-15 23:20 UTC (permalink / raw)
To: Mark Brown
Cc: Dimitris Papastamos, linux-kernel, Liam Girdwood, Graeme Gregory,
Samuel Oritz
On 09/16/2011 12:57 AM, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 05:19:32PM +0200, Lars-Peter Clausen wrote:
>
>>> + if (!map->cache_bypass) {
>>> + ret = regcache_read(map, reg, val);
>>> + if (!ret) {
>>> + mutex_unlock(&map->lock);
>>> + return 0;
>>> + }
>
>> So in case regmap_readable is not true for this register regcache_read will
>> return -EIO and we'll fallback to an uncached read. This doesn't make sense in
>> my opinion. Or what are the except semantics regmap_readable supposed to be?
>
> It's a bit interesting but falling back to asking the hardware is
> probably the right thing to do for cases where you're doing things like
> using unspecified registers to activate undocumented features - the
> register isn't really documented as being there and may do odd things
> but if the device manufacturer gives you an approved sequence then you
> can probably use it safely.
But for writes the error is propagated to the caller, instead of falling back
to a hw write. There should be at least consistency between the two.
And I think we should document that regmap_readable and regmap_writeable don't
prevent actuall hw access, but are merely meant as hints for stuff like caching.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 6/6 v3] regmap: Incorporate the regcache core into regmap
2011-09-15 23:20 ` Lars-Peter Clausen
@ 2011-09-15 23:53 ` Mark Brown
0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2011-09-15 23:53 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Dimitris Papastamos, linux-kernel, Liam Girdwood, Graeme Gregory,
Samuel Oritz
On Fri, Sep 16, 2011 at 01:20:32AM +0200, Lars-Peter Clausen wrote:
> But for writes the error is propagated to the caller, instead of falling back
> to a hw write. There should be at least consistency between the two.
Hrm, yes we should do that (I haven't actually had time to read this
round yet).
> And I think we should document that regmap_readable and regmap_writeable don't
> prevent actuall hw access, but are merely meant as hints for stuff like caching.
Well, right now we don't enforce anything at all. I wouldn't object to
a patch for this but once we go out of spec I think we're fine so long
as we're senseible.
^ permalink raw reply [flat|nested] 14+ messages in thread