All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] regmap: introduce fast_io busses, and use a spinlock for them
@ 2012-04-04 21:48 Stephen Warren
  2012-04-04 21:48 ` [PATCH 2/6] regmap: allow regmap instances to be named Stephen Warren
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Stephen Warren @ 2012-04-04 21:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Stephen Warren

From: Stephen Warren <swarren@nvidia.com>

Some bus types have very fast IO. For these, acquiring a mutex for every
IO operation is a significant overhead. Allow busses to indicate their IO
is fast, and enhance regmap to use a spinlock for those busses.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Mark, FYI, this whole series will be a dependency for the Tegra ASoC
series I'll post shortly.

 drivers/base/regmap/internal.h        |    8 ++++-
 drivers/base/regmap/regcache-rbtree.c |    4 +-
 drivers/base/regmap/regcache.c        |   20 +++++-----
 drivers/base/regmap/regmap.c          |   62 ++++++++++++++++++++++++---------
 include/linux/regmap.h                |    3 ++
 5 files changed, 67 insertions(+), 30 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 606b83d..6ee4dc5 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -31,8 +31,14 @@ struct regmap_format {
 	unsigned int (*parse_val)(void *buf);
 };
 
+typedef void (*regmap_lock)(struct regmap *map);
+typedef void (*regmap_unlock)(struct regmap *map);
+
 struct regmap {
-	struct mutex lock;
+	struct mutex mutex;
+	spinlock_t spinlock;
+	regmap_lock lock;
+	regmap_unlock unlock;
 
 	struct device *dev; /* Device we do I/O on */
 	void *work_buf;     /* Scratch buffer used to format I/O */
diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
index fb14a63..9eea758 100644
--- a/drivers/base/regmap/regcache-rbtree.c
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -139,7 +139,7 @@ static int rbtree_show(struct seq_file *s, void *ignored)
 	int nodes = 0;
 	int registers = 0;
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 
 	for (node = rb_first(&rbtree_ctx->root); node != NULL;
 	     node = rb_next(node)) {
@@ -155,7 +155,7 @@ static int rbtree_show(struct seq_file *s, void *ignored)
 	seq_printf(s, "%d nodes, %d registers, average %d registers\n",
 		   nodes, registers, registers / nodes);
 
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 
 	return 0;
 }
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 74b6909..d4368e8 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -264,7 +264,7 @@ int regcache_sync(struct regmap *map)
 
 	BUG_ON(!map->cache_ops || !map->cache_ops->sync);
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 	/* Remember the initial bypass state */
 	bypass = map->cache_bypass;
 	dev_dbg(map->dev, "Syncing %s cache\n",
@@ -296,7 +296,7 @@ out:
 	trace_regcache_sync(map->dev, name, "stop");
 	/* Restore the bypass state */
 	map->cache_bypass = bypass;
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 
 	return ret;
 }
@@ -323,7 +323,7 @@ int regcache_sync_region(struct regmap *map, unsigned int min,
 
 	BUG_ON(!map->cache_ops || !map->cache_ops->sync);
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 
 	/* Remember the initial bypass state */
 	bypass = map->cache_bypass;
@@ -342,7 +342,7 @@ out:
 	trace_regcache_sync(map->dev, name, "stop region");
 	/* Restore the bypass state */
 	map->cache_bypass = bypass;
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 
 	return ret;
 }
@@ -362,11 +362,11 @@ EXPORT_SYMBOL_GPL(regcache_sync_region);
  */
 void regcache_cache_only(struct regmap *map, bool enable)
 {
-	mutex_lock(&map->lock);
+	map->lock(map);
 	WARN_ON(map->cache_bypass && enable);
 	map->cache_only = enable;
 	trace_regmap_cache_only(map->dev, enable);
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 }
 EXPORT_SYMBOL_GPL(regcache_cache_only);
 
@@ -381,9 +381,9 @@ EXPORT_SYMBOL_GPL(regcache_cache_only);
  */
 void regcache_mark_dirty(struct regmap *map)
 {
-	mutex_lock(&map->lock);
+	map->lock(map);
 	map->cache_dirty = true;
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 }
 EXPORT_SYMBOL_GPL(regcache_mark_dirty);
 
@@ -400,11 +400,11 @@ EXPORT_SYMBOL_GPL(regcache_mark_dirty);
  */
 void regcache_cache_bypass(struct regmap *map, bool enable)
 {
-	mutex_lock(&map->lock);
+	map->lock(map);
 	WARN_ON(map->cache_only && enable);
 	map->cache_bypass = enable;
 	trace_regmap_cache_bypass(map->dev, enable);
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 }
 EXPORT_SYMBOL_GPL(regcache_cache_bypass);
 
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 178989a..831ec42 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -179,6 +179,26 @@ static unsigned int regmap_parse_32(void *buf)
 	return b[0];
 }
 
+static void regmap_lock_mutex(struct regmap *map)
+{
+	mutex_lock(&map->mutex);
+}
+
+static void regmap_unlock_mutex(struct regmap *map)
+{
+	mutex_unlock(&map->mutex);
+}
+
+static void regmap_lock_spinlock(struct regmap *map)
+{
+	spin_lock(&map->spinlock);
+}
+
+static void regmap_unlock_spinlock(struct regmap *map)
+{
+	spin_unlock(&map->spinlock);
+}
+
 /**
  * regmap_init(): Initialise register map
  *
@@ -206,7 +226,15 @@ struct regmap *regmap_init(struct device *dev,
 		goto err;
 	}
 
-	mutex_init(&map->lock);
+	if (bus->fast_io) {
+		spin_lock_init(&map->spinlock);
+		map->lock = regmap_lock_spinlock;
+		map->unlock = regmap_unlock_spinlock;
+	} else {
+		mutex_init(&map->mutex);
+		map->lock = regmap_lock_mutex;
+		map->unlock = regmap_unlock_mutex;
+	}
 	map->format.buf_size = (config->reg_bits + config->val_bits) / 8;
 	map->format.reg_bytes = DIV_ROUND_UP(config->reg_bits, 8);
 	map->format.pad_bytes = config->pad_bits / 8;
@@ -386,7 +414,7 @@ int regmap_reinit_cache(struct regmap *map, const struct regmap_config *config)
 {
 	int ret;
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 
 	regcache_exit(map);
 	regmap_debugfs_exit(map);
@@ -405,7 +433,7 @@ int regmap_reinit_cache(struct regmap *map, const struct regmap_config *config)
 
 	ret = regcache_init(map, config);
 
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 
 	return ret;
 }
@@ -555,11 +583,11 @@ int regmap_write(struct regmap *map, unsigned int reg, unsigned int val)
 {
 	int ret;
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 
 	ret = _regmap_write(map, reg, val);
 
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 
 	return ret;
 }
@@ -586,11 +614,11 @@ int regmap_raw_write(struct regmap *map, unsigned int reg,
 {
 	int ret;
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 
 	ret = _regmap_raw_write(map, reg, val, val_len);
 
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 
 	return ret;
 }
@@ -620,7 +648,7 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 	if (!map->format.parse_val)
 		return -EINVAL;
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 
 	/* No formatting is require if val_byte is 1 */
 	if (val_bytes == 1) {
@@ -641,7 +669,7 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		kfree(wval);
 
 out:
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_bulk_write);
@@ -715,11 +743,11 @@ int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val)
 {
 	int ret;
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 
 	ret = _regmap_read(map, reg, val);
 
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 
 	return ret;
 }
@@ -744,7 +772,7 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 	unsigned int v;
 	int ret, i;
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 
 	if (regmap_volatile_range(map, reg, val_count) || map->cache_bypass ||
 	    map->cache_type == REGCACHE_NONE) {
@@ -765,7 +793,7 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 	}
 
  out:
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 
 	return ret;
 }
@@ -818,7 +846,7 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
 	int ret;
 	unsigned int tmp, orig;
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 
 	ret = _regmap_read(map, reg, &orig);
 	if (ret != 0)
@@ -835,7 +863,7 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
 	}
 
 out:
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 
 	return ret;
 }
@@ -902,7 +930,7 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs,
 	if (map->patch)
 		return -EBUSY;
 
-	mutex_lock(&map->lock);
+	map->lock(map);
 
 	bypass = map->cache_bypass;
 
@@ -930,7 +958,7 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs,
 out:
 	map->cache_bypass = bypass;
 
-	mutex_unlock(&map->lock);
+	map->unlock(map);
 
 	return ret;
 }
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index a90abb6..48e9b86 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -109,6 +109,8 @@ typedef int (*regmap_hw_read)(struct device *dev,
 /**
  * Description of a hardware bus for the register map infrastructure.
  *
+ * @fast_io: Register IO is fast. Use a spinlock instead of a mutex
+ *           to perform locking.
  * @write: Write operation.
  * @gather_write: Write operation with split register/value, return -ENOTSUPP
  *                if not implemented  on a given device.
@@ -118,6 +120,7 @@ typedef int (*regmap_hw_read)(struct device *dev,
  *                  a read.
  */
 struct regmap_bus {
+	bool fast_io;
 	regmap_hw_write write;
 	regmap_hw_gather_write gather_write;
 	regmap_hw_read read;
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/6] regmap: allow regmap instances to be named
  2012-04-04 21:48 [PATCH 1/6] regmap: introduce fast_io busses, and use a spinlock for them Stephen Warren
@ 2012-04-04 21:48 ` Stephen Warren
  2012-04-06  3:44   ` Paul Gortmaker
  2012-04-04 21:48 ` [PATCH 3/6] regmap: introduce explicit bus_context for bus callbacks Stephen Warren
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Stephen Warren @ 2012-04-04 21:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Stephen Warren

From: Stephen Warren <swarren@nvidia.com>

Some devices have multiple separate register regions. Logically, one
regmap would be created per region. One issue that prevents this is that
each instance will attempt to create the same debugfs files. Avoid this
by allowing regmaps to be named, and use the name to construct the
debugfs directory name.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/base/regmap/internal.h       |    3 ++-
 drivers/base/regmap/regmap-debugfs.c |   14 +++++++++++---
 drivers/base/regmap/regmap.c         |    4 ++--
 include/linux/regmap.h               |    5 +++++
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 6ee4dc5..d9ea8f5 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -47,6 +47,7 @@ struct regmap {
 
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debugfs;
+	const char *debugfs_name;
 #endif
 
 	unsigned int max_register;
@@ -110,7 +111,7 @@ int _regmap_write(struct regmap *map, unsigned int reg,
 
 #ifdef CONFIG_DEBUG_FS
 extern void regmap_debugfs_initcall(void);
-extern void regmap_debugfs_init(struct regmap *map);
+extern void regmap_debugfs_init(struct regmap *map, const char *name);
 extern void regmap_debugfs_exit(struct regmap *map);
 #else
 static inline void regmap_debugfs_initcall(void) { }
diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 251eb70..df97c93 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -242,10 +242,17 @@ static const struct file_operations regmap_access_fops = {
 	.llseek = default_llseek,
 };
 
-void regmap_debugfs_init(struct regmap *map)
+void regmap_debugfs_init(struct regmap *map, const char *name)
 {
-	map->debugfs = debugfs_create_dir(dev_name(map->dev),
-					  regmap_debugfs_root);
+	if (name) {
+		map->debugfs_name = kasprintf(GFP_KERNEL, "%s-%s",
+					      dev_name(map->dev), name);
+		name = map->debugfs_name;
+	} else {
+		name = dev_name(map->dev);
+	}
+
+	map->debugfs = debugfs_create_dir(name, regmap_debugfs_root);
 	if (!map->debugfs) {
 		dev_warn(map->dev, "Failed to create debugfs directory\n");
 		return;
@@ -274,6 +281,7 @@ void regmap_debugfs_init(struct regmap *map)
 void regmap_debugfs_exit(struct regmap *map)
 {
 	debugfs_remove_recursive(map->debugfs);
+	kfree(map->debugfs_name);
 }
 
 void regmap_debugfs_initcall(void)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 831ec42..339b2c8 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -343,7 +343,7 @@ struct regmap *regmap_init(struct device *dev,
 		goto err_map;
 	}
 
-	regmap_debugfs_init(map);
+	regmap_debugfs_init(map, config->name);
 
 	ret = regcache_init(map, config);
 	if (ret < 0)
@@ -426,7 +426,7 @@ int regmap_reinit_cache(struct regmap *map, const struct regmap_config *config)
 	map->precious_reg = config->precious_reg;
 	map->cache_type = config->cache_type;
 
-	regmap_debugfs_init(map);
+	regmap_debugfs_init(map, config->name);
 
 	map->cache_bypass = false;
 	map->cache_only = false;
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 48e9b86..2fd41e3 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -46,6 +46,9 @@ struct reg_default {
 /**
  * Configuration for the register map of a device.
  *
+ * @name: Optional name of the regmap. Useful when a device has multiple
+ *        register regions.
+ *
  * @reg_bits: Number of bits in a register address, mandatory.
  * @pad_bits: Number of bits of padding between register and value.
  * @val_bits: Number of bits in a register value, mandatory.
@@ -77,6 +80,8 @@ struct reg_default {
  * @num_reg_defaults_raw: Number of elements in reg_defaults_raw.
  */
 struct regmap_config {
+	const char *name;
+
 	int reg_bits;
 	int pad_bits;
 	int val_bits;
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/6] regmap: introduce explicit bus_context for bus callbacks
  2012-04-04 21:48 [PATCH 1/6] regmap: introduce fast_io busses, and use a spinlock for them Stephen Warren
  2012-04-04 21:48 ` [PATCH 2/6] regmap: allow regmap instances to be named Stephen Warren
@ 2012-04-04 21:48 ` Stephen Warren
  2012-04-04 21:48 ` [PATCH 4/6] regmap: add MMIO bus support Stephen Warren
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Stephen Warren @ 2012-04-04 21:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Stephen Warren

From: Stephen Warren <swarren@nvidia.com>

The only context needed by I2C and SPI bus definitions is the device
itself; this can be converted to an i2c_client or spi_device in order
to perform IO on the device. However, other bus types may need more
context in order to perform IO. Enable this by having regmap_init accept
a bus_context parameter, and pass this to all bus callbacks. The
existing callbacks simply pass the struct device here. Future bus types
may pass something else.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/base/regmap/internal.h   |    1 +
 drivers/base/regmap/regmap-i2c.c |   13 ++++++++-----
 drivers/base/regmap/regmap-spi.c |   13 ++++++++-----
 drivers/base/regmap/regmap.c     |   19 +++++++++++++------
 include/linux/regmap.h           |   10 +++++++---
 5 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index d9ea8f5..9bc1d27 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -44,6 +44,7 @@ struct regmap {
 	void *work_buf;     /* Scratch buffer used to format I/O */
 	struct regmap_format format;  /* Buffer format */
 	const struct regmap_bus *bus;
+	void *bus_context;
 
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debugfs;
diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index 9a3a8c5..5f6b247 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -15,8 +15,9 @@
 #include <linux/module.h>
 #include <linux/init.h>
 
-static int regmap_i2c_write(struct device *dev, const void *data, size_t count)
+static int regmap_i2c_write(void *context, const void *data, size_t count)
 {
+	struct device *dev = context;
 	struct i2c_client *i2c = to_i2c_client(dev);
 	int ret;
 
@@ -29,10 +30,11 @@ static int regmap_i2c_write(struct device *dev, const void *data, size_t count)
 		return -EIO;
 }
 
-static int regmap_i2c_gather_write(struct device *dev,
+static int regmap_i2c_gather_write(void *context,
 				   const void *reg, size_t reg_size,
 				   const void *val, size_t val_size)
 {
+	struct device *dev = context;
 	struct i2c_client *i2c = to_i2c_client(dev);
 	struct i2c_msg xfer[2];
 	int ret;
@@ -62,10 +64,11 @@ static int regmap_i2c_gather_write(struct device *dev,
 		return -EIO;
 }
 
-static int regmap_i2c_read(struct device *dev,
+static int regmap_i2c_read(void *context,
 			   const void *reg, size_t reg_size,
 			   void *val, size_t val_size)
 {
+	struct device *dev = context;
 	struct i2c_client *i2c = to_i2c_client(dev);
 	struct i2c_msg xfer[2];
 	int ret;
@@ -107,7 +110,7 @@ static struct regmap_bus regmap_i2c = {
 struct regmap *regmap_init_i2c(struct i2c_client *i2c,
 			       const struct regmap_config *config)
 {
-	return regmap_init(&i2c->dev, &regmap_i2c, config);
+	return regmap_init(&i2c->dev, &regmap_i2c, &i2c->dev, config);
 }
 EXPORT_SYMBOL_GPL(regmap_init_i2c);
 
@@ -124,7 +127,7 @@ EXPORT_SYMBOL_GPL(regmap_init_i2c);
 struct regmap *devm_regmap_init_i2c(struct i2c_client *i2c,
 				    const struct regmap_config *config)
 {
-	return devm_regmap_init(&i2c->dev, &regmap_i2c, config);
+	return devm_regmap_init(&i2c->dev, &regmap_i2c, &i2c->dev, config);
 }
 EXPORT_SYMBOL_GPL(devm_regmap_init_i2c);
 
diff --git a/drivers/base/regmap/regmap-spi.c b/drivers/base/regmap/regmap-spi.c
index 7c0c35a..ffa46a9 100644
--- a/drivers/base/regmap/regmap-spi.c
+++ b/drivers/base/regmap/regmap-spi.c
@@ -15,17 +15,19 @@
 #include <linux/init.h>
 #include <linux/module.h>
 
-static int regmap_spi_write(struct device *dev, const void *data, size_t count)
+static int regmap_spi_write(void *context, const void *data, size_t count)
 {
+	struct device *dev = context;
 	struct spi_device *spi = to_spi_device(dev);
 
 	return spi_write(spi, data, count);
 }
 
-static int regmap_spi_gather_write(struct device *dev,
+static int regmap_spi_gather_write(void *context,
 				   const void *reg, size_t reg_len,
 				   const void *val, size_t val_len)
 {
+	struct device *dev = context;
 	struct spi_device *spi = to_spi_device(dev);
 	struct spi_message m;
 	struct spi_transfer t[2] = { { .tx_buf = reg, .len = reg_len, },
@@ -38,10 +40,11 @@ static int regmap_spi_gather_write(struct device *dev,
 	return spi_sync(spi, &m);
 }
 
-static int regmap_spi_read(struct device *dev,
+static int regmap_spi_read(void *context,
 			   const void *reg, size_t reg_size,
 			   void *val, size_t val_size)
 {
+	struct device *dev = context;
 	struct spi_device *spi = to_spi_device(dev);
 
 	return spi_write_then_read(spi, reg, reg_size, val, val_size);
@@ -66,7 +69,7 @@ static struct regmap_bus regmap_spi = {
 struct regmap *regmap_init_spi(struct spi_device *spi,
 			       const struct regmap_config *config)
 {
-	return regmap_init(&spi->dev, &regmap_spi, config);
+	return regmap_init(&spi->dev, &regmap_spi, &spi->dev, config);
 }
 EXPORT_SYMBOL_GPL(regmap_init_spi);
 
@@ -83,7 +86,7 @@ EXPORT_SYMBOL_GPL(regmap_init_spi);
 struct regmap *devm_regmap_init_spi(struct spi_device *spi,
 				    const struct regmap_config *config)
 {
-	return devm_regmap_init(&spi->dev, &regmap_spi, config);
+	return devm_regmap_init(&spi->dev, &regmap_spi, &spi->dev, config);
 }
 EXPORT_SYMBOL_GPL(devm_regmap_init_spi);
 
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 339b2c8..2b5e7ae 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -204,6 +204,7 @@ static void regmap_unlock_spinlock(struct regmap *map)
  *
  * @dev: Device that will be interacted with
  * @bus: Bus-specific callbacks to use with device
+ * @bus_context: Data passed to bus-specific callbacks
  * @config: Configuration for register map
  *
  * The return value will be an ERR_PTR() on error or a valid pointer to
@@ -212,6 +213,7 @@ static void regmap_unlock_spinlock(struct regmap *map)
  */
 struct regmap *regmap_init(struct device *dev,
 			   const struct regmap_bus *bus,
+			   void *bus_context,
 			   const struct regmap_config *config)
 {
 	struct regmap *map;
@@ -243,6 +245,7 @@ struct regmap *regmap_init(struct device *dev,
 	map->reg_shift = config->pad_bits % 8;
 	map->dev = dev;
 	map->bus = bus;
+	map->bus_context = bus_context;
 	map->max_register = config->max_register;
 	map->writeable_reg = config->writeable_reg;
 	map->readable_reg = config->readable_reg;
@@ -370,6 +373,7 @@ static void devm_regmap_release(struct device *dev, void *res)
  *
  * @dev: Device that will be interacted with
  * @bus: Bus-specific callbacks to use with device
+ * @bus_context: Data passed to bus-specific callbacks
  * @config: Configuration for register map
  *
  * The return value will be an ERR_PTR() on error or a valid pointer
@@ -379,6 +383,7 @@ static void devm_regmap_release(struct device *dev, void *res)
  */
 struct regmap *devm_regmap_init(struct device *dev,
 				const struct regmap_bus *bus,
+				void *bus_context,
 				const struct regmap_config *config)
 {
 	struct regmap **ptr, *regmap;
@@ -387,7 +392,7 @@ struct regmap *devm_regmap_init(struct device *dev,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	regmap = regmap_init(dev, bus, config);
+	regmap = regmap_init(dev, bus, bus_context, config);
 	if (!IS_ERR(regmap)) {
 		*ptr = regmap;
 		devres_add(dev, ptr);
@@ -445,6 +450,8 @@ void regmap_exit(struct regmap *map)
 {
 	regcache_exit(map);
 	regmap_debugfs_exit(map);
+	if (map->bus->free_context)
+		map->bus->free_context(map->bus_context);
 	kfree(map->work_buf);
 	kfree(map);
 }
@@ -498,12 +505,12 @@ static int _regmap_raw_write(struct regmap *map, unsigned int reg,
 	 */
 	if (val == (map->work_buf + map->format.pad_bytes +
 		    map->format.reg_bytes))
-		ret = map->bus->write(map->dev, map->work_buf,
+		ret = map->bus->write(map->bus_context, map->work_buf,
 				      map->format.reg_bytes +
 				      map->format.pad_bytes +
 				      val_len);
 	else if (map->bus->gather_write)
-		ret = map->bus->gather_write(map->dev, map->work_buf,
+		ret = map->bus->gather_write(map->bus_context, map->work_buf,
 					     map->format.reg_bytes +
 					     map->format.pad_bytes,
 					     val, val_len);
@@ -518,7 +525,7 @@ static int _regmap_raw_write(struct regmap *map, unsigned int reg,
 		memcpy(buf, map->work_buf, map->format.reg_bytes);
 		memcpy(buf + map->format.reg_bytes + map->format.pad_bytes,
 		       val, val_len);
-		ret = map->bus->write(map->dev, buf, len);
+		ret = map->bus->write(map->bus_context, buf, len);
 
 		kfree(buf);
 	}
@@ -552,7 +559,7 @@ int _regmap_write(struct regmap *map, unsigned int reg,
 
 		trace_regmap_hw_write_start(map->dev, reg, 1);
 
-		ret = map->bus->write(map->dev, map->work_buf,
+		ret = map->bus->write(map->bus_context, map->work_buf,
 				      map->format.buf_size);
 
 		trace_regmap_hw_write_done(map->dev, reg, 1);
@@ -693,7 +700,7 @@ static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 	trace_regmap_hw_read_start(map->dev, reg,
 				   val_len / map->format.val_bytes);
 
-	ret = map->bus->read(map->dev, map->work_buf,
+	ret = map->bus->read(map->bus_context, map->work_buf,
 			     map->format.reg_bytes + map->format.pad_bytes,
 			     val, val_len);
 
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 2fd41e3..3ff16f0 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -102,14 +102,15 @@ struct regmap_config {
 	u8 write_flag_mask;
 };
 
-typedef int (*regmap_hw_write)(struct device *dev, const void *data,
+typedef int (*regmap_hw_write)(void *context, const void *data,
 			       size_t count);
-typedef int (*regmap_hw_gather_write)(struct device *dev,
+typedef int (*regmap_hw_gather_write)(void *context,
 				      const void *reg, size_t reg_len,
 				      const void *val, size_t val_len);
-typedef int (*regmap_hw_read)(struct device *dev,
+typedef int (*regmap_hw_read)(void *context,
 			      const void *reg_buf, size_t reg_size,
 			      void *val_buf, size_t val_size);
+typedef void (*regmap_hw_free_context)(void *context);
 
 /**
  * Description of a hardware bus for the register map infrastructure.
@@ -129,11 +130,13 @@ struct regmap_bus {
 	regmap_hw_write write;
 	regmap_hw_gather_write gather_write;
 	regmap_hw_read read;
+	regmap_hw_free_context free_context;
 	u8 read_flag_mask;
 };
 
 struct regmap *regmap_init(struct device *dev,
 			   const struct regmap_bus *bus,
+			   void *bus_context,
 			   const struct regmap_config *config);
 struct regmap *regmap_init_i2c(struct i2c_client *i2c,
 			       const struct regmap_config *config);
@@ -142,6 +145,7 @@ struct regmap *regmap_init_spi(struct spi_device *dev,
 
 struct regmap *devm_regmap_init(struct device *dev,
 				const struct regmap_bus *bus,
+				void *bus_context,
 				const struct regmap_config *config);
 struct regmap *devm_regmap_init_i2c(struct i2c_client *i2c,
 				    const struct regmap_config *config);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/6] regmap: add MMIO bus support
  2012-04-04 21:48 [PATCH 1/6] regmap: introduce fast_io busses, and use a spinlock for them Stephen Warren
  2012-04-04 21:48 ` [PATCH 2/6] regmap: allow regmap instances to be named Stephen Warren
  2012-04-04 21:48 ` [PATCH 3/6] regmap: introduce explicit bus_context for bus callbacks Stephen Warren
@ 2012-04-04 21:48 ` Stephen Warren
  2012-04-04 22:59   ` Mark Brown
  2012-04-04 21:48 ` [PATCH 5/6] regmap: add runtime PM calls to debugfs file IO Stephen Warren
  2012-04-04 21:48 ` [PATCH 6/6] regmap: prevent division by zero in rbtree_show Stephen Warren
  4 siblings, 1 reply; 13+ messages in thread
From: Stephen Warren @ 2012-04-04 21:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Stephen Warren

From: Stephen Warren <swarren@nvidia.com>

This is a basic memory-mapped-IO bus for regmap. It has the following
features and limitations:

* Registers themselves may be 8, 16, 32, or 64-bit. 64-bit is only
  supported on 64-bit platforms.
* Register offsets are limited to precisely 32-bit.
* IO is performed using readl/writel, with no provision for using the
  __raw_readl or readl_relaxed variants.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/base/regmap/Kconfig       |    3 +
 drivers/base/regmap/Makefile      |    1 +
 drivers/base/regmap/regmap-mmio.c |  217 +++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h            |    6 +
 4 files changed, 227 insertions(+), 0 deletions(-)
 create mode 100644 drivers/base/regmap/regmap-mmio.c

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 0f6c7fb..9ef0a53 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -14,5 +14,8 @@ config REGMAP_I2C
 config REGMAP_SPI
 	tristate
 
+config REGMAP_MMIO
+	tristate
+
 config REGMAP_IRQ
 	bool
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index defd579..5e75d1b 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -3,4 +3,5 @@ obj-$(CONFIG_REGMAP) += 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
+obj-$(CONFIG_REGMAP_MMIO) += regmap-mmio.o
 obj-$(CONFIG_REGMAP_IRQ) += regmap-irq.o
diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
new file mode 100644
index 0000000..1a7b5ee
--- /dev/null
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -0,0 +1,217 @@
+/*
+ * Register map access API - MMIO support
+ *
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+struct regmap_mmio_context {
+	void __iomem *regs;
+	unsigned val_bytes;
+};
+
+static int regmap_mmio_gather_write(void *context,
+				    const void *reg, size_t reg_size,
+				    const void *val, size_t val_size)
+{
+	struct regmap_mmio_context *ctx = context;
+	u32 offset;
+
+	if (reg_size != 4)
+		return -EIO;
+	if (val_size % ctx->val_bytes)
+		return -EIO;
+
+	offset = be32_to_cpup(reg);
+
+	while (val_size) {
+		switch (ctx->val_bytes) {
+		case 1:
+			writeb(*(u8 *)val, ctx->regs + offset);
+			break;
+		case 2:
+			writew(be16_to_cpup(val), ctx->regs + offset);
+			break;
+		case 4:
+			writel(be32_to_cpup(val), ctx->regs + offset);
+			break;
+#ifdef CONFIG_64BIT
+		case 8:
+			writeq(be64_to_cpup(val), ctx->regs + offset);
+			break;
+#endif
+		default:
+			/* Should be caught by regmap_mmio_check_config */
+			return -EIO;
+		}
+		val_size -= ctx->val_bytes;
+		val += ctx->val_bytes;
+		offset += ctx->val_bytes;
+	}
+
+	return 0;
+}
+
+static int regmap_mmio_write(void *context, const void *data, size_t count)
+{
+	if (count < 4)
+		return -EIO;
+	return regmap_mmio_gather_write(context, data, 4, data + 4, count - 4);
+}
+
+static int regmap_mmio_read(void *context,
+			    const void *reg, size_t reg_size,
+			    void *val, size_t val_size)
+{
+	struct regmap_mmio_context *ctx = context;
+	u32 offset;
+
+	if (reg_size != 4)
+		return -EIO;
+	if (val_size % ctx->val_bytes)
+		return -EIO;
+
+	offset = be32_to_cpup(reg);
+
+	while (val_size) {
+		switch (ctx->val_bytes) {
+		case 1:
+			*(u8 *)val = readb(ctx->regs + offset);
+			break;
+		case 2:
+			*(u16 *)val = cpu_to_be16(readw(ctx->regs + offset));
+			break;
+		case 4:
+			*(u32 *)val = cpu_to_be32(readl(ctx->regs + offset));
+			break;
+#ifdef CONFIG_64BIT
+		case 8:
+			*(u64 *)val = cpu_to_be32(readq(ctx->regs + offset));
+			break;
+#endif
+		default:
+			/* Should be caught by regmap_mmio_check_config */
+			return -EIO;
+		}
+		val_size -= ctx->val_bytes;
+		val += ctx->val_bytes;
+		offset += ctx->val_bytes;
+	}
+
+	return 0;
+}
+
+static void regmap_mmio_free_context(void *context)
+{
+	kfree(context);
+}
+
+static struct regmap_bus regmap_mmio = {
+	.fast_io = true,
+	.write = regmap_mmio_write,
+	.gather_write = regmap_mmio_gather_write,
+	.read = regmap_mmio_read,
+	.free_context = regmap_mmio_free_context,
+};
+
+struct regmap_mmio_context *regmap_mmio_gen_context(void __iomem *regs,
+					const struct regmap_config *config)
+{
+	struct regmap_mmio_context *ctx;
+
+	if (config->reg_bits != 32)
+		return ERR_PTR(-EINVAL);
+
+	if (config->pad_bits)
+		return ERR_PTR(-EINVAL);
+
+	switch (config->val_bits) {
+	case 8:
+	case 16:
+	case 32:
+#ifdef CONFIG_64BIT
+	case 64:
+#endif
+		break;
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+
+	ctx = kzalloc(GFP_KERNEL, sizeof(*ctx));
+	if (!ctx)
+		return ERR_PTR(-ENOMEM);
+
+	ctx->regs = regs;
+	ctx->val_bytes = config->val_bits / 8;
+
+	return ctx;
+}
+
+/**
+ * regmap_init_mmio(): Initialise register map
+ *
+ * @dev: Device that will be interacted with
+ * @regs: Pointer to memory-mapped IO region
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer to
+ * a struct regmap.
+ */
+struct regmap *regmap_init_mmio(struct device *dev,
+				void __iomem *regs,
+				const struct regmap_config *config)
+{
+	struct regmap_mmio_context *ctx;
+
+	ctx = regmap_mmio_gen_context(regs, config);
+	if (IS_ERR(ctx))
+		return ERR_CAST(ctx);
+
+	return regmap_init(dev, &regmap_mmio, ctx, config);
+}
+EXPORT_SYMBOL_GPL(regmap_init_mmio);
+
+/**
+ * devm_regmap_init_mmio(): Initialise managed register map
+ *
+ * @dev: Device that will be interacted with
+ * @regs: Pointer to memory-mapped IO region
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct regmap.  The regmap will be automatically freed by the
+ * device management code.
+ */
+struct regmap *devm_regmap_init_mmio(struct device *dev,
+				     void __iomem *regs,
+				     const struct regmap_config *config)
+{
+	struct regmap_mmio_context *ctx;
+
+	ctx = regmap_mmio_gen_context(regs, config);
+	if (IS_ERR(ctx))
+		return ERR_CAST(ctx);
+
+	return devm_regmap_init(dev, &regmap_mmio, ctx, config);
+}
+EXPORT_SYMBOL_GPL(devm_regmap_init_mmio);
+
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 3ff16f0..680ddd7 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -142,6 +142,9 @@ struct regmap *regmap_init_i2c(struct i2c_client *i2c,
 			       const struct regmap_config *config);
 struct regmap *regmap_init_spi(struct spi_device *dev,
 			       const struct regmap_config *config);
+struct regmap *regmap_init_mmio(struct device *dev,
+				void __iomem *regs,
+				const struct regmap_config *config);
 
 struct regmap *devm_regmap_init(struct device *dev,
 				const struct regmap_bus *bus,
@@ -151,6 +154,9 @@ struct regmap *devm_regmap_init_i2c(struct i2c_client *i2c,
 				    const struct regmap_config *config);
 struct regmap *devm_regmap_init_spi(struct spi_device *dev,
 				    const struct regmap_config *config);
+struct regmap *devm_regmap_init_mmio(struct device *dev,
+				     void __iomem *regs,
+				     const struct regmap_config *config);
 
 void regmap_exit(struct regmap *map);
 int regmap_reinit_cache(struct regmap *map,
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/6] regmap: add runtime PM calls to debugfs file IO
  2012-04-04 21:48 [PATCH 1/6] regmap: introduce fast_io busses, and use a spinlock for them Stephen Warren
                   ` (2 preceding siblings ...)
  2012-04-04 21:48 ` [PATCH 4/6] regmap: add MMIO bus support Stephen Warren
@ 2012-04-04 21:48 ` Stephen Warren
  2012-04-04 22:19   ` Mark Brown
  2012-04-04 21:48 ` [PATCH 6/6] regmap: prevent division by zero in rbtree_show Stephen Warren
  4 siblings, 1 reply; 13+ messages in thread
From: Stephen Warren @ 2012-04-04 21:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Stephen Warren

From: Stephen Warren <swarren@nvidia.com>

It's quite probably that devices need to be active in order for their
registers to be read/written. In normal regmap usage by drivers, it's the
responsibility of the driver to assure this if needed. However, regmap
debugfs file handling is outside the driver's control, so we need to
explicitly ensure this.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/base/regmap/regmap-debugfs.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index df97c93..7a18fdb 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -15,6 +15,7 @@
 #include <linux/debugfs.h>
 #include <linux/uaccess.h>
 #include <linux/device.h>
+#include <linux/pm_runtime.h>
 
 #include "internal.h"
 
@@ -80,6 +81,8 @@ static ssize_t regmap_map_read_file(struct file *file, char __user *user_buf,
 	val_len = 2 * map->format.val_bytes;
 	tot_len = reg_len + val_len + 3;      /* : \n */
 
+	pm_runtime_get(map->dev);
+
 	for (i = 0; i < map->max_register + 1; i++) {
 		if (!regmap_readable(map, i))
 			continue;
@@ -112,6 +115,8 @@ static ssize_t regmap_map_read_file(struct file *file, char __user *user_buf,
 		p += tot_len;
 	}
 
+	pm_runtime_put(map->dev);
+
 	ret = buf_pos;
 
 	if (copy_to_user(user_buf, buf, buf_pos)) {
@@ -160,7 +165,9 @@ static ssize_t regmap_map_write_file(struct file *file,
 	/* Userspace has been fiddling around behind the kernel's back */
 	add_taint(TAINT_USER);
 
+	pm_runtime_get(map->dev);
 	regmap_write(map, reg, value);
+	pm_runtime_put(map->dev);
 	return buf_size;
 }
 #else
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/6] regmap: prevent division by zero in rbtree_show
  2012-04-04 21:48 [PATCH 1/6] regmap: introduce fast_io busses, and use a spinlock for them Stephen Warren
                   ` (3 preceding siblings ...)
  2012-04-04 21:48 ` [PATCH 5/6] regmap: add runtime PM calls to debugfs file IO Stephen Warren
@ 2012-04-04 21:48 ` Stephen Warren
  2012-04-04 22:24   ` Mark Brown
  4 siblings, 1 reply; 13+ messages in thread
From: Stephen Warren @ 2012-04-04 21:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel, Stephen Warren

From: Stephen Warren <swarren@nvidia.com>

If there are no nodes in the cache, nodes will be 0, so calculating
"registers / nodes" will cause division by zero.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/base/regmap/regcache-rbtree.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
index 9eea758..e49e71f 100644
--- a/drivers/base/regmap/regcache-rbtree.c
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -138,6 +138,7 @@ static int rbtree_show(struct seq_file *s, void *ignored)
 	unsigned int base, top;
 	int nodes = 0;
 	int registers = 0;
+	int average;
 
 	map->lock(map);
 
@@ -152,8 +153,13 @@ static int rbtree_show(struct seq_file *s, void *ignored)
 		registers += top - base + 1;
 	}
 
+	if (nodes)
+		average = registers / nodes;
+	else
+		average = 0;
+
 	seq_printf(s, "%d nodes, %d registers, average %d registers\n",
-		   nodes, registers, registers / nodes);
+		   nodes, registers, average);
 
 	map->unlock(map);
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/6] regmap: add runtime PM calls to debugfs file IO
  2012-04-04 21:48 ` [PATCH 5/6] regmap: add runtime PM calls to debugfs file IO Stephen Warren
@ 2012-04-04 22:19   ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2012-04-04 22:19 UTC (permalink / raw)
  To: Stephen Warren; +Cc: linux-kernel, Stephen Warren

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

On Wed, Apr 04, 2012 at 03:48:32PM -0600, Stephen Warren wrote:

> It's quite probably that devices need to be active in order for their
> registers to be read/written. In normal regmap usage by drivers, it's the
> responsibility of the driver to assure this if needed. However, regmap
> debugfs file handling is outside the driver's control, so we need to
> explicitly ensure this.

This feels wrong, it's too invasive on the device.  If we're in cache
only mode (which we should be if the device is off and we have a cache)
then I'd expect us to error out if we can't satisfy the read from cache
rather than wake the device up which might be terribly expensive and
cause register writes which change the device state we were trying to
inspect, and if we have a cache then we might not need to interact with
the hardware at all at which point the wakeup would've been needless.
It's certainly not what I'd expect a diagnostic interface to be doing,
I'd expect it to reflect the current state of the device.

Now, quite how the mmio bus deals with access to a suspended device is
probably platform specific and certainly more fun than at least I2C (SPI
has issues since it's hard to tell if the device is there or not and the
MISO line might be undriven but at least you'll not lock up or anything).

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 6/6] regmap: prevent division by zero in rbtree_show
  2012-04-04 21:48 ` [PATCH 6/6] regmap: prevent division by zero in rbtree_show Stephen Warren
@ 2012-04-04 22:24   ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2012-04-04 22:24 UTC (permalink / raw)
  To: Stephen Warren; +Cc: linux-kernel, Stephen Warren

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

On Wed, Apr 04, 2012 at 03:48:33PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> If there are no nodes in the cache, nodes will be 0, so calculating
> "registers / nodes" will cause division by zero.

Applied, thanks.  I'm not sure what this patch was generated against,
git am can't seem to find the blobs referenced.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/6] regmap: add MMIO bus support
  2012-04-04 21:48 ` [PATCH 4/6] regmap: add MMIO bus support Stephen Warren
@ 2012-04-04 22:59   ` Mark Brown
  2012-04-04 23:15     ` Stephen Warren
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2012-04-04 22:59 UTC (permalink / raw)
  To: Stephen Warren; +Cc: linux-kernel, Stephen Warren

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

On Wed, Apr 04, 2012 at 03:48:31PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> This is a basic memory-mapped-IO bus for regmap. It has the following
> features and limitations:

I applied these up to here but it was painful as patch 2 didn't apply
cleanly to -rc1 and git am couldn't find the blobs to use for
resolution.  Please check things worked out OK but I'm pretty sure they
did.

> * Registers themselves may be 8, 16, 32, or 64-bit. 64-bit is only
>   supported on 64-bit platforms.
> * Register offsets are limited to precisely 32-bit.
> * IO is performed using readl/writel, with no provision for using the
>   __raw_readl or readl_relaxed variants.

Also limited native endian register I/O.  It would have been much better
to fix this in the core - please consider producing followup patches to
push the code there, though it's far from essential.

> +static int regmap_mmio_gather_write(void *context,
> +				    const void *reg, size_t reg_size,
> +				    const void *val, size_t val_size)
> +{
> +	struct regmap_mmio_context *ctx = context;
> +	u32 offset;
> +
> +	if (reg_size != 4)
> +		return -EIO;

Given that you constrain on registration too this should be BUG_ON(),
we're seriously confused if we're specifying a different register size
here and -EIO is going to be a bit obscure.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/6] regmap: add MMIO bus support
  2012-04-04 22:59   ` Mark Brown
@ 2012-04-04 23:15     ` Stephen Warren
  2012-04-05  8:38       ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Stephen Warren @ 2012-04-04 23:15 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel

On 04/04/2012 04:59 PM, Mark Brown wrote:
> On Wed, Apr 04, 2012 at 03:48:31PM -0600, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> This is a basic memory-mapped-IO bus for regmap. It has the following
>> features and limitations:
> 
> I applied these up to here but it was painful as patch 2 didn't apply
> cleanly to -rc1 and git am couldn't find the blobs to use for
> resolution.  Please check things worked out OK but I'm pretty sure they
> did.

The result of merging your topic/mmio and for-next branches is the same
as my local copy of patch 4 I sent, modulo that my local copy still has
regmap_open_file(), but that's not related.

So yes, I think the patches applied fine.

>> * Registers themselves may be 8, 16, 32, or 64-bit. 64-bit is only
>>   supported on 64-bit platforms.
>> * Register offsets are limited to precisely 32-bit.
>> * IO is performed using readl/writel, with no provision for using the
>>   __raw_readl or readl_relaxed variants.
> 
> Also limited native endian register I/O.  It would have been much better
> to fix this in the core - please consider producing followup patches to
> push the code there, though it's far from essential.

I assume the solution here is to:

* Always use __raw_readl/__raw_writel in regmap-mmio.c so that there's
never any endianness conversion, and don't use the endianness conversion
macros when reading/writing the work buffer.
* Implement alternative formatters that format in LE instead of BE
* Add a field to regmap_config indicating which endianness the data
should be in, and use this flag to select the LE/BE formatters.

Does that sound about right?

>> +static int regmap_mmio_gather_write(void *context,
>> +				    const void *reg, size_t reg_size,
>> +				    const void *val, size_t val_size)
>> +{
>> +	struct regmap_mmio_context *ctx = context;
>> +	u32 offset;
>> +
>> +	if (reg_size != 4)
>> +		return -EIO;
> 
> Given that you constrain on registration too this should be BUG_ON(),
> we're seriously confused if we're specifying a different register size
> here and -EIO is going to be a bit obscure.

I wondered about that. I would have been quite happy to repost with that
change. Do you want an incremental patch?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/6] regmap: add MMIO bus support
  2012-04-04 23:15     ` Stephen Warren
@ 2012-04-05  8:38       ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2012-04-05  8:38 UTC (permalink / raw)
  To: Stephen Warren; +Cc: linux-kernel

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

On Wed, Apr 04, 2012 at 05:15:42PM -0600, Stephen Warren wrote:

> I assume the solution here is to:

> * Always use __raw_readl/__raw_writel in regmap-mmio.c so that there's
> never any endianness conversion, and don't use the endianness conversion
> macros when reading/writing the work buffer.
> * Implement alternative formatters that format in LE instead of BE
> * Add a field to regmap_config indicating which endianness the data
> should be in, and use this flag to select the LE/BE formatters.

> Does that sound about right?

Sounds about right, yes - probably worth having a third option for the
formatting which does no translation for on-SoC device.

> > Given that you constrain on registration too this should be BUG_ON(),
> > we're seriously confused if we're specifying a different register size
> > here and -EIO is going to be a bit obscure.

> I wondered about that. I would have been quite happy to repost with that
> change. Do you want an incremental patch?

Yes, please.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] regmap: allow regmap instances to be named
  2012-04-04 21:48 ` [PATCH 2/6] regmap: allow regmap instances to be named Stephen Warren
@ 2012-04-06  3:44   ` Paul Gortmaker
  2012-04-06  5:12     ` Stephen Warren
  0 siblings, 1 reply; 13+ messages in thread
From: Paul Gortmaker @ 2012-04-06  3:44 UTC (permalink / raw)
  To: Stephen Warren; +Cc: Mark Brown, linux-kernel, Stephen Warren, linux-next

On Wed, Apr 4, 2012 at 5:48 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> Some devices have multiple separate register regions. Logically, one
> regmap would be created per region. One issue that prevents this is that
> each instance will attempt to create the same debugfs files. Avoid this
> by allowing regmaps to be named, and use the name to construct the
> debugfs directory name.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  drivers/base/regmap/internal.h       |    3 ++-
>  drivers/base/regmap/regmap-debugfs.c |   14 +++++++++++---
>  drivers/base/regmap/regmap.c         |    4 ++--
>  include/linux/regmap.h               |    5 +++++
>  4 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
> index 6ee4dc5..d9ea8f5 100644
> --- a/drivers/base/regmap/internal.h
> +++ b/drivers/base/regmap/internal.h
> @@ -47,6 +47,7 @@ struct regmap {
>
>  #ifdef CONFIG_DEBUG_FS
>        struct dentry *debugfs;
> +       const char *debugfs_name;
>  #endif
>
>        unsigned int max_register;
> @@ -110,7 +111,7 @@ int _regmap_write(struct regmap *map, unsigned int reg,
>
>  #ifdef CONFIG_DEBUG_FS
>  extern void regmap_debugfs_initcall(void);
> -extern void regmap_debugfs_init(struct regmap *map);
> +extern void regmap_debugfs_init(struct regmap *map, const char *name);

Hi Stephen,

Can you take a look at these linux-next fails?

drivers/base/regmap/regmap.c:349:2: error: too many arguments to
function 'regmap_debugfs_init'
drivers/base/regmap/regmap.c:434:2: error: too many arguments to
function 'regmap_debugfs_init'
make[4]: *** [drivers/base/regmap/regmap.o] Error 1

I'm assuming it is somehow related to the above commit (or family
of commits) -- since it complains about arg #s and you are changing
the arg #.  (I didn't bisect to 100% confirm this.)

http://kisskb.ellerman.id.au/kisskb/buildresult/6039717/
http://kisskb.ellerman.id.au/kisskb/buildresult/6040074/
http://kisskb.ellerman.id.au/kisskb/buildresult/6040070/


Thanks,
Paul.
>  extern void regmap_debugfs_exit(struct regmap *map);
>  #else
>  static inline void regmap_debugfs_initcall(void) { }
> diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
> index 251eb70..df97c93 100644
> --- a/drivers/base/regmap/regmap-debugfs.c
> +++ b/drivers/base/regmap/regmap-debugfs.c
> @@ -242,10 +242,17 @@ static const struct file_operations regmap_access_fops = {
>        .llseek = default_llseek,
>  };
>
> -void regmap_debugfs_init(struct regmap *map)
> +void regmap_debugfs_init(struct regmap *map, const char *name)
>  {
> -       map->debugfs = debugfs_create_dir(dev_name(map->dev),
> -                                         regmap_debugfs_root);
> +       if (name) {
> +               map->debugfs_name = kasprintf(GFP_KERNEL, "%s-%s",
> +                                             dev_name(map->dev), name);
> +               name = map->debugfs_name;
> +       } else {
> +               name = dev_name(map->dev);
> +       }
> +
> +       map->debugfs = debugfs_create_dir(name, regmap_debugfs_root);
>        if (!map->debugfs) {
>                dev_warn(map->dev, "Failed to create debugfs directory\n");
>                return;
> @@ -274,6 +281,7 @@ void regmap_debugfs_init(struct regmap *map)
>  void regmap_debugfs_exit(struct regmap *map)
>  {
>        debugfs_remove_recursive(map->debugfs);
> +       kfree(map->debugfs_name);
>  }
>
>  void regmap_debugfs_initcall(void)
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index 831ec42..339b2c8 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -343,7 +343,7 @@ struct regmap *regmap_init(struct device *dev,
>                goto err_map;
>        }
>
> -       regmap_debugfs_init(map);
> +       regmap_debugfs_init(map, config->name);
>
>        ret = regcache_init(map, config);
>        if (ret < 0)
> @@ -426,7 +426,7 @@ int regmap_reinit_cache(struct regmap *map, const struct regmap_config *config)
>        map->precious_reg = config->precious_reg;
>        map->cache_type = config->cache_type;
>
> -       regmap_debugfs_init(map);
> +       regmap_debugfs_init(map, config->name);
>
>        map->cache_bypass = false;
>        map->cache_only = false;
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index 48e9b86..2fd41e3 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -46,6 +46,9 @@ struct reg_default {
>  /**
>  * Configuration for the register map of a device.
>  *
> + * @name: Optional name of the regmap. Useful when a device has multiple
> + *        register regions.
> + *
>  * @reg_bits: Number of bits in a register address, mandatory.
>  * @pad_bits: Number of bits of padding between register and value.
>  * @val_bits: Number of bits in a register value, mandatory.
> @@ -77,6 +80,8 @@ struct reg_default {
>  * @num_reg_defaults_raw: Number of elements in reg_defaults_raw.
>  */
>  struct regmap_config {
> +       const char *name;
> +
>        int reg_bits;
>        int pad_bits;
>        int val_bits;
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] regmap: allow regmap instances to be named
  2012-04-06  3:44   ` Paul Gortmaker
@ 2012-04-06  5:12     ` Stephen Warren
  0 siblings, 0 replies; 13+ messages in thread
From: Stephen Warren @ 2012-04-06  5:12 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: Mark Brown, linux-kernel, linux-next

On 04/05/2012 09:44 PM, Paul Gortmaker wrote:
> On Wed, Apr 4, 2012 at 5:48 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> Some devices have multiple separate register regions. Logically, one
>> regmap would be created per region. One issue that prevents this is that
>> each instance will attempt to create the same debugfs files. Avoid this
>> by allowing regmaps to be named, and use the name to construct the
>> debugfs directory name.
...
> Hi Stephen,
> 
> Can you take a look at these linux-next fails?
> 
> drivers/base/regmap/regmap.c:349:2: error: too many arguments to
> function 'regmap_debugfs_init'
> drivers/base/regmap/regmap.c:434:2: error: too many arguments to
> function 'regmap_debugfs_init'
> make[4]: *** [drivers/base/regmap/regmap.o] Error 1

Oops. Sorry about that; evidently tegra_defconfig has CONFIG_DEBUG_FS
enabled and I didn't realize it. Fix posted.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-04-06  5:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-04 21:48 [PATCH 1/6] regmap: introduce fast_io busses, and use a spinlock for them Stephen Warren
2012-04-04 21:48 ` [PATCH 2/6] regmap: allow regmap instances to be named Stephen Warren
2012-04-06  3:44   ` Paul Gortmaker
2012-04-06  5:12     ` Stephen Warren
2012-04-04 21:48 ` [PATCH 3/6] regmap: introduce explicit bus_context for bus callbacks Stephen Warren
2012-04-04 21:48 ` [PATCH 4/6] regmap: add MMIO bus support Stephen Warren
2012-04-04 22:59   ` Mark Brown
2012-04-04 23:15     ` Stephen Warren
2012-04-05  8:38       ` Mark Brown
2012-04-04 21:48 ` [PATCH 5/6] regmap: add runtime PM calls to debugfs file IO Stephen Warren
2012-04-04 22:19   ` Mark Brown
2012-04-04 21:48 ` [PATCH 6/6] regmap: prevent division by zero in rbtree_show Stephen Warren
2012-04-04 22:24   ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.