[PATCH v2] regmap: Add regmap dummy driver

[PATCH v2] regmap: Add regmap dummy driver

[Dimitris Papastamos]

Add a pseudo-driver for debugging and stress-testing the
regmap/regcache APIs.  A standard set of tools for working
with this driver (mainly sh scripts) will be put in a repo
at https://github.com/quantumdream/regmap-tools

Some of these tests will require one to build with
REGMAP_ALLOW_WRITE_DEBUGFS defined.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 This is an initial implementation of the regdummy driver for regmap.
 This is mainly useful for debugging/stress-testing regcache as it
 removes the need for real hardware and can be done in an emulated
 environment very easily.

 There'll be incremental patches adding more features such as,
 support for configurable volatile/readable/etc. registers via
 debugfs entries.

 v2 updates:
 - Factored out the access to the MMIO region into a separate
 function.
 - Removed bogus Change-Id

 drivers/base/regmap/Kconfig        |   8 +
 drivers/base/regmap/Makefile       |   1 +
 drivers/base/regmap/regmap-dummy.c | 609 +++++++++++++++++++++++++++++++++++++
 3 files changed, 618 insertions(+)
 create mode 100644 drivers/base/regmap/regmap-dummy.c

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 6be390b..5a1ab02 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -20,3 +20,11 @@ config REGMAP_MMIO
 
 config REGMAP_IRQ
 	bool
+
+config REGMAP_DUMMY
+	tristate
+	select REGMAP_MMIO
+	help
+	  Say Y or M if you want to add the regdummy driver for regmap.
+	  This is a pseudo-driver used for debugging and stress-testing
+	  the regmap/regcache APIs.
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 5e75d1b..c5d70f1 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -5,3 +5,4 @@ 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
+obj-$(CONFIG_REGMAP_DUMMY) += regmap-dummy.o
diff --git a/drivers/base/regmap/regmap-dummy.c b/drivers/base/regmap/regmap-dummy.c
new file mode 100644
index 0000000..078e090
--- /dev/null
+++ b/drivers/base/regmap/regmap-dummy.c
@@ -0,0 +1,609 @@
+/*
+ * Register map access API - Dummy regmap driver
+ *
+ * Copyright 2012 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/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/uaccess.h>
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/debugfs.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+
+#define DEFAULT_REGS_SIZE 1024
+
+struct regdummy_dev {
+	struct device *dev;
+	struct mutex lock;
+
+	/* Set when regdummy defaults have been modified.
+	 * This is useful to know so we don't reinit the
+	 * cache if there is no reason to do so. */
+	unsigned int dirty:1;
+
+	void *regs;
+	unsigned int regs_size;
+	unsigned int regs_size_new;
+
+	struct regmap *map;
+	struct regmap_config *config;
+	struct reg_default *regdef;
+};
+
+static struct dentry *regdummy_debugfs_root;
+
+/* Default volatile register callback, this should
+ * normally be configured by the user via a debugfs
+ * entry */
+static bool regdummy_volatile_reg(struct device *dev,
+				  unsigned int reg)
+{
+	return false;
+}
+
+/* Default readable register callback, this should
+ * normally be configured by the user via a debugfs
+ * entry */
+static bool regdummy_readable_reg(struct device *dev,
+				  unsigned int reg)
+{
+	return true;
+}
+
+/* Default precious register callback, this should
+ * normally be configured by the user via a debugfs
+ * entry */
+static bool regdummy_precious_reg(struct device *dev,
+				  unsigned int reg)
+{
+	return false;
+}
+
+/* Calculate the length of a fixed format  */
+static size_t regmap_calc_reg_len(int max_val, char *buf, size_t buf_size)
+{
+	snprintf(buf, buf_size, "%x", max_val);
+	return strlen(buf);
+}
+
+static ssize_t regdummy_defaults_read_file(struct file *file, char __user *user_buf,
+					   size_t count, loff_t *ppos)
+{
+	int reg_len, val_len, tot_len;
+	size_t buf_pos = 0;
+	loff_t p = 0;
+	ssize_t ret;
+	int i;
+	struct regdummy_dev *rdevp = file->private_data;
+	struct regmap_config *config;
+	struct reg_default *regdef;
+	unsigned int val;
+	unsigned int j;
+	unsigned int regdef_num;
+	char *buf;
+
+	if (*ppos < 0 || !count)
+		return -EINVAL;
+
+	buf = kmalloc(count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	mutex_lock(&rdevp->lock);
+
+	config = rdevp->config;
+	regdef = rdevp->regdef;
+	regdef_num = rdevp->regs_size / config->reg_stride;
+
+	/* Calculate the length of a fixed format  */
+	reg_len = regmap_calc_reg_len(config->max_register, buf, count);
+	val_len = 2 * DIV_ROUND_UP(config->val_bits, 8);
+	tot_len = reg_len + val_len + 3;      /* : \n */
+
+	for (i = 0; i <= config->max_register; i += config->reg_stride) {
+		if (!regdummy_readable_reg(rdevp->dev, i))
+			continue;
+
+		if (regdummy_precious_reg(rdevp->dev, i))
+			continue;
+
+		/* If we're in the region the user is trying to read */
+		if (p >= *ppos) {
+			/* ...but not beyond it */
+			if (buf_pos >= count - 1 - tot_len)
+				break;
+
+			/* Format the register */
+			snprintf(buf + buf_pos, count - buf_pos, "%.*x: ",
+				 reg_len, i);
+			buf_pos += reg_len + 2;
+
+			/* Format the value, write all X if we can't read */
+			for (j = 0; j < regdef_num; j++) {
+				if (regdef[j].reg == i) {
+					val = regdef[j].def;
+					snprintf(buf + buf_pos, count - buf_pos,
+						 "%.*x", val_len, val);
+					break;
+				}
+			}
+			if (j == regdef_num)
+				memset(buf + buf_pos, 'X', val_len);
+			buf_pos += 2 * DIV_ROUND_UP(config->val_bits, 8);
+
+			buf[buf_pos++] = '\n';
+		}
+		p += tot_len;
+	}
+
+	ret = buf_pos;
+
+	if (copy_to_user(user_buf, buf, buf_pos)) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	*ppos += buf_pos;
+
+out:
+	mutex_unlock(&rdevp->lock);
+
+	kfree(buf);
+	return ret;
+}
+
+static ssize_t regdummy_defaults_write_file(struct file *file,
+					    const char __user *user_buf,
+					    size_t count, loff_t *ppos)
+{
+	char buf[32];
+	size_t buf_size;
+	char *start = buf;
+	unsigned long reg, value;
+	struct regdummy_dev *rdevp = file->private_data;
+	struct regmap_config *config = rdevp->config;
+	struct reg_default *regdef = rdevp->regdef;
+	unsigned int i;
+	unsigned int regdef_num = rdevp->regs_size / config->reg_stride;
+
+	buf_size = min(count, (sizeof(buf)-1));
+	if (copy_from_user(buf, user_buf, buf_size))
+		return -EFAULT;
+	buf[buf_size] = 0;
+
+	while (*start == ' ')
+		start++;
+	reg = simple_strtoul(start, &start, 16);
+	while (*start == ' ')
+		start++;
+	if (strict_strtoul(start, 16, &value))
+		return -EINVAL;
+
+	mutex_lock(&rdevp->lock);
+	for (i = 0; i < regdef_num; i++) {
+		if (regdef[i].reg == reg) {
+			if (regdef[i].def != value)
+				rdevp->dirty = 1;
+			regdef[i].def = value;
+			break;
+		}
+	}
+	mutex_unlock(&rdevp->lock);
+	return buf_size;
+}
+
+static const struct file_operations regdummy_defaults_fops = {
+	.open = simple_open,
+	.read = regdummy_defaults_read_file,
+	.write = regdummy_defaults_write_file,
+	.llseek = default_llseek,
+};
+
+/* Called with lock held */
+static int regdummy_create_debugfs(struct device *dev)
+{
+	struct regdummy_dev *rdevp = dev_get_drvdata(dev);
+
+	regdummy_debugfs_root = debugfs_create_dir("regdummy", NULL);
+	if (IS_ERR(regdummy_debugfs_root))
+		return PTR_ERR(regdummy_debugfs_root);
+
+	debugfs_create_file("defaults", 0400, regdummy_debugfs_root,
+			    rdevp, &regdummy_defaults_fops);
+
+	return 0;
+}
+
+/* Called with lock held */
+static void regdummy_exit_debugfs(struct device *dev)
+{
+	debugfs_remove_recursive(regdummy_debugfs_root);
+}
+
+/* Called with lock held */
+static int regdummy_init_regmap(struct device *dev)
+{
+	struct regmap_config *config;
+	struct reg_default *regdef;
+	int ret;
+	void *regs;
+	unsigned int regdef_num, regdef_num_raw;
+	struct regdummy_dev *rdevp = dev_get_drvdata(dev);
+	unsigned int i;
+
+	config = kzalloc(sizeof(*config), GFP_KERNEL);
+	if (!config)
+		return -ENOMEM;
+	rdevp->config = config;
+
+	/* Fill up a sample regmap_config structure */
+	config->reg_bits = 32;
+	config->val_bits = 32;
+	config->reg_stride = 4;
+
+	/* Allocate our fake __iomem region */
+	rdevp->regs_size = rdevp->regs_size_new = DEFAULT_REGS_SIZE;
+	regs = kzalloc(rdevp->regs_size, GFP_KERNEL);
+	if (!regs) {
+		ret = -ENOMEM;
+		goto err_alloc_config;
+	}
+	rdevp->regs = regs;
+
+	/* Allocate the register defaults */
+	regdef_num = rdevp->regs_size / config->reg_stride;
+	regdef_num_raw = regdef_num * sizeof(*regdef);
+	regdef = kzalloc(regdef_num_raw, GFP_KERNEL);
+	if (!regdef) {
+		ret = -ENOMEM;
+		goto err_alloc_regs;
+	}
+	rdevp->regdef = regdef;
+
+	for (i = 0; i < regdef_num; i++)
+		rdevp->regdef[i].reg = i * config->reg_stride;
+
+	/* Fill up the rest of the regmap_config structure */
+	config->reg_defaults = regdef;
+	config->num_reg_defaults = regdef_num;
+	config->max_register = rdevp->regs_size - config->reg_stride;
+	config->cache_type = REGCACHE_RBTREE;
+	config->volatile_reg = regdummy_volatile_reg;
+	config->readable_reg = regdummy_readable_reg;
+	config->precious_reg = regdummy_precious_reg;
+
+	rdevp->map = regmap_init_mmio(rdevp->dev,
+				      (void __iomem *)rdevp->regs,
+				      config);
+	if (IS_ERR(rdevp->map)) {
+		ret = PTR_ERR(rdevp->map);
+		goto err_alloc_regdef;
+	}
+
+	ret = regdummy_create_debugfs(dev);
+	if (ret < 0) {
+		dev_err(rdevp->dev, "Failed to create debugfs entries: %d\n",
+			ret);
+		goto err_regmap_init;
+	}
+
+	return 0;
+
+err_regmap_init:
+	regmap_exit(rdevp->map);
+err_alloc_regdef:
+	kfree(regdef);
+err_alloc_regs:
+	kfree(regs);
+err_alloc_config:
+	kfree(config);
+	return ret;
+}
+
+/* Called with lock held */
+static void regdummy_free_regmap(struct device *dev)
+{
+	struct regdummy_dev *rdevp = dev_get_drvdata(dev);
+
+	regdummy_exit_debugfs(dev);
+
+	if (rdevp->map)
+		regmap_exit(rdevp->map);
+
+	kfree(rdevp->config);
+	kfree(rdevp->regs);
+	kfree(rdevp->regdef);
+}
+
+/* Called with lock held */
+static void regdummy_write_raw(struct device *dev, void *buf,
+			       unsigned int reg, unsigned int val)
+{
+	struct regdummy_dev *rdevp = dev_get_drvdata(dev);
+	struct regmap_config *config = rdevp->config;
+
+	switch (config->val_bits) {
+	case 8: {
+		u8 *p = buf;
+		p[reg] = val;
+		break;
+	}
+	case 16: {
+		u16 *p = buf;
+		p[reg] = val;
+		break;
+	}
+	case 32: {
+		u32 *p = buf;
+		p[reg] = val;
+		break;
+	}
+	default:
+		BUG();
+		break;
+	}
+}
+
+static int regdummy_reinit_map(struct device *dev)
+{
+	struct regdummy_dev *rdevp = dev_get_drvdata(dev);
+	struct reg_default *regdef_new;
+	struct regmap_config *config = rdevp->config;
+	unsigned int regdef_num_new;
+	unsigned int regdef_num_raw_new;
+	unsigned int regdef_num;
+	void *regs_new;
+	unsigned int i;
+
+	int ret;
+
+	if (!rdevp->dirty)
+		return 0;
+
+	/* Allocate the new fake __iomem region */
+	regs_new = kzalloc(rdevp->regs_size_new, GFP_KERNEL);
+	if (!regs_new)
+		return -ENOMEM;
+
+	/* Allocate the new register defaults */
+	regdef_num_new = rdevp->regs_size_new / config->reg_stride;
+	regdef_num_raw_new = regdef_num_new * sizeof(*regdef_new);
+	regdef_new = kzalloc(regdef_num_raw_new, GFP_KERNEL);
+	if (!regdef_new) {
+		kfree(regs_new);
+		return -ENOMEM;
+	}
+
+	/* Fixup the register defaults and the raw regs region */
+	regdef_num = rdevp->regs_size / config->reg_stride;
+	for (i = 0; i < regdef_num_new; i++) {
+		regdef_new[i].reg = i * config->reg_stride;
+		if (i < regdef_num) {
+			regdef_new[i].def = rdevp->regdef[i].def;
+			regdummy_write_raw(dev, regs_new, i,
+					   rdevp->regdef[i].def);
+		}
+	}
+
+	/* Re-initialize everything */
+	regmap_exit(rdevp->map);
+
+	kfree(rdevp->regs);
+	rdevp->regs = regs_new;
+	rdevp->regs_size = rdevp->regs_size_new;
+
+	kfree(rdevp->regdef);
+	rdevp->regdef = regdef_new;
+
+	config->reg_defaults = regdef_new;
+	config->num_reg_defaults = regdef_num_new;
+	config->max_register = rdevp->regs_size_new - config->reg_stride;
+
+	rdevp->map = regmap_init_mmio(rdevp->dev,
+				      (void __iomem *)rdevp->regs,
+				      config);
+	if (IS_ERR(rdevp->map)) {
+		ret = PTR_ERR(rdevp->map);
+		dev_err(rdevp->dev, "Failed to reinit regmap: %d\n", ret);
+		rdevp->map = NULL;
+		return ret;
+	}
+
+	rdevp->dirty = 0;
+
+	return 0;
+}
+
+static ssize_t reinit_map_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct regdummy_dev *rdevp = dev_get_drvdata(dev);
+	int ret;
+
+	mutex_lock(&rdevp->lock);
+	ret = regdummy_reinit_map(dev);
+	mutex_unlock(&rdevp->lock);
+	if (ret)
+		return ret;
+	return count;
+}
+static DEVICE_ATTR(reinit_map, S_IWUSR, NULL, reinit_map_store);
+
+static ssize_t regs_raw_size_show(struct device *dev, struct device_attribute *attr,
+				  char *buf)
+{
+	struct regdummy_dev *rdevp = dev_get_drvdata(dev);
+	ssize_t ret;
+
+	mutex_lock(&rdevp->lock);
+	ret = scnprintf(buf, PAGE_SIZE, "%u\n", rdevp->regs_size);
+	mutex_unlock(&rdevp->lock);
+	return ret;
+}
+
+static ssize_t regs_raw_size_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	struct regdummy_dev *rdevp = dev_get_drvdata(dev);
+	long val;
+	int ret;
+
+	mutex_lock(&rdevp->lock);
+	ret = strict_strtol(buf, 10, &val);
+	if (ret) {
+		mutex_unlock(&rdevp->lock);
+		return ret;
+	}
+
+	if (!val || (val % rdevp->config->reg_stride)) {
+		mutex_unlock(&rdevp->lock);
+		return -EINVAL;
+	}
+
+	rdevp->regs_size_new = val;
+	rdevp->dirty = 1;
+	regdummy_reinit_map(dev);
+	mutex_unlock(&rdevp->lock);
+	return count;
+}
+static DEVICE_ATTR(regs_raw_size, S_IWUSR | S_IRUGO,
+		   regs_raw_size_show, regs_raw_size_store);
+
+static ssize_t sync_store(struct device *dev,
+			  struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct regdummy_dev *rdevp = dev_get_drvdata(dev);
+	int ret;
+
+	mutex_lock(&rdevp->lock);
+	BUG_ON(!rdevp->map);
+	ret = regcache_sync(rdevp->map);
+	mutex_unlock(&rdevp->lock);
+	if (ret)
+		return ret;
+	return count;
+}
+static DEVICE_ATTR(sync, S_IWUSR, NULL, sync_store);
+
+static int __devinit regdummy_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct regdummy_dev *rdevp;
+
+	rdevp = devm_kzalloc(&pdev->dev, sizeof(*rdevp), GFP_KERNEL);
+	if (!rdevp)
+		return -ENOMEM;
+
+	mutex_init(&rdevp->lock);
+
+	rdevp->dev = &pdev->dev;
+	dev_set_drvdata(rdevp->dev, rdevp);
+
+	mutex_lock(&rdevp->lock);
+	ret = regdummy_init_regmap(rdevp->dev);
+	if (ret < 0) {
+		dev_err(rdevp->dev,
+			"Failed to init regmap: %d\n", ret);
+		mutex_unlock(&rdevp->lock);
+		return ret;
+	}
+
+	ret = device_create_file(rdevp->dev,
+				 &dev_attr_reinit_map);
+	if (ret < 0)
+		goto err_regmap_init;
+
+	ret = device_create_file(rdevp->dev,
+				 &dev_attr_regs_raw_size);
+	if (ret < 0)
+		goto err_create_reinit_map;
+
+	ret = device_create_file(rdevp->dev,
+				 &dev_attr_sync);
+	if (ret < 0)
+		goto err_create_regs_raw_size;
+	mutex_unlock(&rdevp->lock);
+
+	return 0;
+
+err_create_regs_raw_size:
+	device_remove_file(rdevp->dev, &dev_attr_regs_raw_size);
+err_create_reinit_map:
+	device_remove_file(rdevp->dev, &dev_attr_reinit_map);
+err_regmap_init:
+	regdummy_free_regmap(rdevp->dev);
+	mutex_unlock(&rdevp->lock);
+	return ret;
+}
+
+static int regdummy_remove(struct platform_device *pdev)
+{
+	struct regdummy_dev *rdevp = dev_get_drvdata(&pdev->dev);
+
+	mutex_lock(&rdevp->lock);
+	device_remove_file(rdevp->dev, &dev_attr_sync);
+	device_remove_file(rdevp->dev, &dev_attr_regs_raw_size);
+	device_remove_file(rdevp->dev, &dev_attr_reinit_map);
+	regdummy_free_regmap(&pdev->dev);
+	dev_set_drvdata(&pdev->dev, NULL);
+	mutex_unlock(&rdevp->lock);
+
+	return 0;
+}
+
+static struct platform_driver regdummy_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "regdummy",
+	},
+	.probe = regdummy_probe,
+	.remove = regdummy_remove,
+};
+
+static struct platform_device regdummy_device = {
+	.name = "regdummy",
+	.id = 0,
+};
+
+static int __init regdummy_init(void)
+{
+	int ret;
+
+	ret = platform_device_register(&regdummy_device);
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&regdummy_driver);
+	if (ret) {
+		platform_device_unregister(&regdummy_device);
+		return ret;
+	}
+
+	return 0;
+}
+module_init(regdummy_init);
+
+static void __exit regdummy_exit(void)
+{
+	platform_driver_unregister(&regdummy_driver);
+	platform_device_unregister(&regdummy_device);
+}
+module_exit(regdummy_exit);
+
+MODULE_DESCRIPTION("Regmap dummy driver");
+MODULE_AUTHOR("Dimitris Papastamos <dp@opensource.wolfsonmicro.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.11.3

Re: [PATCH v2] regmap: Add regmap dummy driver

[Mark Brown]

On Fri, Jul 27, 2012 at 02:58:20PM +0100, Dimitris Papastamos wrote:
> Add a pseudo-driver for debugging and stress-testing the
> regmap/regcache APIs.  A standard set of tools for working

Overall this looks good, most of the stuff below is fairly small.  As a
very high level comment it'd be really helpful to split this into a
series of commits, for example adding just the dummy device then
building out the functionality.  It'd make review much easier.

> with this driver (mainly sh scripts) will be put in a repo
> at https://github.com/quantumdream/regmap-tools

Any reason not to put this in the tools directory?

> Some of these tests will require one to build with
> REGMAP_ALLOW_WRITE_DEBUGFS defined.

Can we add a write mechanism specifically for this dummy driver?

> +	/* Set when regdummy defaults have been modified.
> +	 * This is useful to know so we don't reinit the
> +	 * cache if there is no reason to do so. */
> +	unsigned int dirty:1;

Should we perhaps just reinit anyway?  It's not like this is performance
critical...

> +/* Default volatile register callback, this should
> + * normally be configured by the user via a debugfs
> + * entry */
> +static bool regdummy_volatile_reg(struct device *dev,
> +				  unsigned int reg)
> +{
> +	return false;
> +}

All these functions just seem to be implementing the default behaviour,
why are they needed?

> +		/* If we're in the region the user is trying to read */
> +		if (p >= *ppos) {
> +			/* ...but not beyond it */
> +			if (buf_pos >= count - 1 - tot_len)
> +				break;

Any potential for code reuse?  This stuff does look awfully familiar!

> +	/* Allocate the new register defaults */
> +	regdef_num_new = rdevp->regs_size_new / config->reg_stride;
> +	regdef_num_raw_new = regdef_num_new * sizeof(*regdef_new);
> +	regdef_new = kzalloc(regdef_num_raw_new, GFP_KERNEL);

Can we factor this stuff out - there's a lot of overlap with the vanilla
init?

> +static struct platform_device regdummy_device = {
> +	.name = "regdummy",
> +	.id = 0,
> +};

Set id to -1 if there's only one of them.

Re: [PATCH v2] regmap: Add regmap dummy driver

[Dimitris Papastamos]

On Sat, Aug 04, 2012 at 11:05:23AM +0100, Mark Brown wrote:
> On Fri, Jul 27, 2012 at 02:58:20PM +0100, Dimitris Papastamos wrote:
> > Add a pseudo-driver for debugging and stress-testing the
> > regmap/regcache APIs.  A standard set of tools for working
> 
> Overall this looks good, most of the stuff below is fairly small.  As a
> very high level comment it'd be really helpful to split this into a
> series of commits, for example adding just the dummy device then
> building out the functionality.  It'd make review much easier.
> 
> > with this driver (mainly sh scripts) will be put in a repo
> > at https://github.com/quantumdream/regmap-tools
> 
> Any reason not to put this in the tools directory?

At the moment the repo is very bare bones.  I was thinking more of
an automated testing framework written in sh or similar.  So it might
grow out into its own repository anyhow.

> > Some of these tests will require one to build with
> > REGMAP_ALLOW_WRITE_DEBUGFS defined.
> 
> Can we add a write mechanism specifically for this dummy driver?

Sure yes.

> > +	/* Set when regdummy defaults have been modified.
> > +	 * This is useful to know so we don't reinit the
> > +	 * cache if there is no reason to do so. */
> > +	unsigned int dirty:1;
> 
> Should we perhaps just reinit anyway?  It's not like this is performance
> critical...

Yea will remove it for simplicity.

> > +/* Default volatile register callback, this should
> > + * normally be configured by the user via a debugfs
> > + * entry */
> > +static bool regdummy_volatile_reg(struct device *dev,
> > +				  unsigned int reg)
> > +{
> > +	return false;
> > +}
> 
> All these functions just seem to be implementing the default behaviour,
> why are they needed?

Hm will remove them for now but it would be useful for these to be set by
the user via debugfs or similar.  These were mainly stubs for that sort
of thing.

> > +		/* If we're in the region the user is trying to read */
> > +		if (p >= *ppos) {
> > +			/* ...but not beyond it */
> > +			if (buf_pos >= count - 1 - tot_len)
> > +				break;
> 
> Any potential for code reuse?  This stuff does look awfully familiar!

Might factor some of these things into a separate regmap-utils.c.

> > +	/* Allocate the new register defaults */
> > +	regdef_num_new = rdevp->regs_size_new / config->reg_stride;
> > +	regdef_num_raw_new = regdef_num_new * sizeof(*regdef_new);
> > +	regdef_new = kzalloc(regdef_num_raw_new, GFP_KERNEL);
> 
> Can we factor this stuff out - there's a lot of overlap with the vanilla
> init?

Sure.

> > +static struct platform_device regdummy_device = {
> > +	.name = "regdummy",
> > +	.id = 0,
> > +};
> 
> Set id to -1 if there's only one of them.

Ok.

Re: [PATCH v2] regmap: Add regmap dummy driver

[Mark Brown]

On Sat, Aug 04, 2012 at 01:43:42PM +0100, Dimitris Papastamos wrote:
> On Sat, Aug 04, 2012 at 11:05:23AM +0100, Mark Brown wrote:

> > Any reason not to put this in the tools directory?

> At the moment the repo is very bare bones.  I was thinking more of
> an automated testing framework written in sh or similar.  So it might
> grow out into its own repository anyhow.

It'd need to be pretty enormous for that I'd expect.

> > All these functions just seem to be implementing the default behaviour,
> > why are they needed?

> Hm will remove them for now but it would be useful for these to be set by
> the user via debugfs or similar.  These were mainly stubs for that sort
> of thing.

I figured that was the idea but until there's code controlling them it's
better to just not have anything.