[PATCH 1/8] Revert "regmap: Grab the lock in regcache_cache_only()"

[PATCH 1/8] Revert "regmap: Grab the lock in regcache_cache_only()"

[Dimitris Papastamos]

This reverts commit 38f6916976f9e964748f097be3688e334fb60f3d.
---
 drivers/base/regmap/regcache.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 744ed145..f46e247 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -273,9 +273,7 @@ EXPORT_SYMBOL_GPL(regcache_sync);
  */
 void regcache_cache_only(struct regmap *map, bool enable)
 {
-	mutex_lock(&map->sync_lock);
 	map->cache_only = enable;
-	mutex_unlock(&map->sync_lock);
 }
 EXPORT_SYMBOL_GPL(regcache_cache_only);
 
-- 
1.7.6.4

[PATCH 2/8] regmap: Make _regmap_write() global

[Dimitris Papastamos]

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

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 2d51b1b..348ff02 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -91,6 +91,9 @@ bool regmap_readable(struct regmap *map, unsigned int reg);
 bool regmap_volatile(struct regmap *map, unsigned int reg);
 bool regmap_precious(struct regmap *map, unsigned int reg);
 
+int _regmap_write(struct regmap *map, unsigned int reg,
+		  unsigned int val);
+
 #ifdef CONFIG_DEBUG_FS
 extern void regmap_debugfs_initcall(void);
 extern void regmap_debugfs_init(struct regmap *map);
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 3596465..d786ddc 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -296,8 +296,8 @@ static int _regmap_raw_write(struct regmap *map, unsigned int reg,
 	return ret;
 }
 
-static int _regmap_write(struct regmap *map, unsigned int reg,
-			 unsigned int val)
+int _regmap_write(struct regmap *map, unsigned int reg,
+		  unsigned int val)
 {
 	int ret;
 	BUG_ON(!map->format.format_write && !map->format.format_val);
-- 
1.7.6.4

Re: [PATCH 2/8] regmap: Make _regmap_write() global

[Mark Brown]

On Thu, Sep 29, 2011 at 10:39:07AM +0100, Dimitris Papastamos wrote:
> Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

Before we do this we need to make sync() take the lock, otherwise we end
up with no locks at all being taken during the sync.

Re: [PATCH 2/8] regmap: Make _regmap_write() global

[Mark Brown]

On Thu, Sep 29, 2011 at 10:39:07AM +0100, Dimitris Papastamos wrote:
> Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

Applied, thanks.

[PATCH 3/8] regmap: Use the lockless _regmap_write() for syncing

[Dimitris Papastamos]

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regcache-indexed.c |    4 ++--
 drivers/base/regmap/regcache-lzo.c     |    2 +-
 drivers/base/regmap/regcache-rbtree.c  |    2 +-
 drivers/base/regmap/regcache.c         |    2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/base/regmap/regcache-indexed.c b/drivers/base/regmap/regcache-indexed.c
index 268497a..2e10bb1 100644
--- a/drivers/base/regmap/regcache-indexed.c
+++ b/drivers/base/regmap/regcache-indexed.c
@@ -45,8 +45,8 @@ static int regcache_indexed_sync(struct regmap *map)
 	int ret;
 
 	for (i = 0; i < map->num_reg_defaults; i++) {
-		ret = regmap_write(map, map->reg_defaults[i].reg,
-				   map->reg_defaults[i].def);
+		ret = _regmap_write(map, map->reg_defaults[i].reg,
+				    map->reg_defaults[i].def);
 		if (ret < 0)
 			return ret;
 		dev_dbg(map->dev, "Synced register %#x, value %#x\n",
diff --git a/drivers/base/regmap/regcache-lzo.c b/drivers/base/regmap/regcache-lzo.c
index 9079cb5..ad6af92 100644
--- a/drivers/base/regmap/regcache-lzo.c
+++ b/drivers/base/regmap/regcache-lzo.c
@@ -339,7 +339,7 @@ static int regcache_lzo_sync(struct regmap *map)
 		if (ret)
 			return ret;
 		map->cache_bypass = 1;
-		ret = regmap_write(map, i, val);
+		ret = _regmap_write(map, i, val);
 		map->cache_bypass = 0;
 		if (ret)
 			return ret;
diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
index de32ced..40f23dd 100644
--- a/drivers/base/regmap/regcache-rbtree.c
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -327,7 +327,7 @@ static int regcache_rbtree_sync(struct regmap *map)
 			if (val == def)
 				continue;
 			map->cache_bypass = 1;
-			ret = regmap_write(map, regtmp, val);
+			ret = _regmap_write(map, regtmp, val);
 			map->cache_bypass = 0;
 			if (ret)
 				return ret;
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index f46e247..b6efde7 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -242,7 +242,7 @@ int regcache_sync(struct regmap *map)
 			if (ret < 0)
 				goto out;
 			map->cache_bypass = 1;
-			ret = regmap_write(map, i, val);
+			ret = _regmap_write(map, i, val);
 			map->cache_bypass = 0;
 			if (ret < 0)
 				goto out;
-- 
1.7.6.4

[PATCH 4/8] regmap: Guard regcache_sync() with map->lock

[Dimitris Papastamos]

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

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index b6efde7..674da82 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -230,6 +230,7 @@ int regcache_sync(struct regmap *map)
 
 	BUG_ON(!map->cache_ops);
 
+	mutex_lock(&map->lock);
 	dev_dbg(map->dev, "Syncing %s cache\n",
 		map->cache_ops->name);
 	name = map->cache_ops->name;
@@ -254,6 +255,7 @@ int regcache_sync(struct regmap *map)
 	}
 out:
 	trace_regcache_sync(map->dev, name, "stop");
+	mutex_unlock(&map->lock);
 
 	return ret;
 }
-- 
1.7.6.4

Re: [PATCH 4/8] regmap: Guard regcache_sync() with map->lock

[Mark Brown]

On Thu, Sep 29, 2011 at 10:39:09AM +0100, Dimitris Papastamos wrote:
> Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

Ah, you're doing that now.  This and the previous commit should be a
single commit pushing the lock outwards - right now either commit by
itself will break things.

Re: [PATCH 4/8] regmap: Guard regcache_sync() with map->lock

[Dimitris Papastamos]

On Thu, Sep 29, 2011 at 11:22:56AM +0100, Mark Brown wrote:
> On Thu, Sep 29, 2011 at 10:39:09AM +0100, Dimitris Papastamos wrote:
> > Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
> 
> Ah, you're doing that now.  This and the previous commit should be a
> single commit pushing the lock outwards - right now either commit by
> itself will break things.

Aw yes I've thought about it, just wasn't too sure about squashing the
two commits together.  Will fix and re-send.

[PATCH 5/8] regmap: Save/restore the bypass state upon syncing

[Dimitris Papastamos]

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

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 674da82..5362578 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -227,10 +227,13 @@ int regcache_sync(struct regmap *map)
 	unsigned int val;
 	unsigned int i;
 	const char *name;
+	unsigned int bypass;
 
 	BUG_ON(!map->cache_ops);
 
 	mutex_lock(&map->lock);
+	/* Remember the initial bypass state */
+	bypass = map->cache_bypass;
 	dev_dbg(map->dev, "Syncing %s cache\n",
 		map->cache_ops->name);
 	name = map->cache_ops->name;
@@ -255,6 +258,8 @@ int regcache_sync(struct regmap *map)
 	}
 out:
 	trace_regcache_sync(map->dev, name, "stop");
+	/* Restore the bypass state */
+	map->cache_bypass = bypass;
 	mutex_unlock(&map->lock);
 
 	return ret;
-- 
1.7.6.4

[PATCH 6/8] regmap: Grab the lock in regcache_cache_only()

[Dimitris Papastamos]

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

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 5362578..f91bc7b 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -280,7 +280,9 @@ EXPORT_SYMBOL_GPL(regcache_sync);
  */
 void regcache_cache_only(struct regmap *map, bool enable)
 {
+	mutex_lock(&map->lock);
 	map->cache_only = enable;
+	mutex_unlock(&map->lock);
 }
 EXPORT_SYMBOL_GPL(regcache_cache_only);
 
-- 
1.7.6.4

[PATCH 7/8] regmap: Implement regcache_cache_bypass helper function

[Dimitris Papastamos]

Ensure we've got a function so users can enable/disable the
cache bypass option.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regcache.c |   19 +++++++++++++++++++
 include/linux/regmap.h         |    1 +
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index f91bc7b..eaa2e20 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -286,6 +286,25 @@ void regcache_cache_only(struct regmap *map, bool enable)
 }
 EXPORT_SYMBOL_GPL(regcache_cache_only);
 
+/**
+ * regcache_cache_bypass: Put a register map into cache bypass mode
+ *
+ * @map: map to configure
+ * @cache_only: flag if changes should not be written to the hardware
+ *
+ * When a register map is marked with the cache bypass option, writes
+ * to the register map API will only update the hardware and not the
+ * the cache directly.  This is useful when syncing the cache back to
+ * the hardware.
+ */
+void regcache_cache_bypass(struct regmap *map, bool enable)
+{
+	mutex_lock(&map->lock);
+	map->cache_bypass = enable;
+	mutex_unlock(&map->lock);
+}
+EXPORT_SYMBOL_GPL(regcache_cache_bypass);
+
 bool regcache_set_val(void *base, unsigned int idx,
 		      unsigned int val, unsigned int word_size)
 {
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 76ac255..3daac2d 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -142,5 +142,6 @@ int regmap_update_bits(struct regmap *map, unsigned int reg,
 
 int regcache_sync(struct regmap *map);
 void regcache_cache_only(struct regmap *map, bool enable);
+void regcache_cache_bypass(struct regmap *map, bool enable);
 
 #endif
-- 
1.7.6.4

[PATCH 8/8] regmap: Ensure we scream if we enable cache bypass/only at the same time

[Dimitris Papastamos]

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

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index eaa2e20..82e619e 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -280,6 +280,7 @@ EXPORT_SYMBOL_GPL(regcache_sync);
  */
 void regcache_cache_only(struct regmap *map, bool enable)
 {
+	BUG_ON(map->cache_bypass && enable);
 	mutex_lock(&map->lock);
 	map->cache_only = enable;
 	mutex_unlock(&map->lock);
@@ -299,6 +300,7 @@ EXPORT_SYMBOL_GPL(regcache_cache_only);
  */
 void regcache_cache_bypass(struct regmap *map, bool enable)
 {
+	BUG_ON(map->cache_only && enable);
 	mutex_lock(&map->lock);
 	map->cache_bypass = enable;
 	mutex_unlock(&map->lock);
-- 
1.7.6.4

Re: [PATCH 8/8] regmap: Ensure we scream if we enable cache bypass/only at the same time

[Mark Brown]

On Thu, Sep 29, 2011 at 10:39:13AM +0100, Dimitris Papastamos wrote:

>  void regcache_cache_only(struct regmap *map, bool enable)
>  {
> +	BUG_ON(map->cache_bypass && enable);
>  	mutex_lock(&map->lock);

I'd move these inside the lock otherwise there's an enable/enable race
where two threads check that things are safe simultaneously.  I'd also
change to WARN_ON(), if nothing else it's hard to see BUG_ON()s that
occur prior to console output and PMICs are likely to trigger that sort
of stuff.

Re: [PATCH 8/8] regmap: Ensure we scream if we enable cache bypass/only at the same time

[Dimitris Papastamos]

On Thu, Sep 29, 2011 at 11:25:30AM +0100, Mark Brown wrote:
> On Thu, Sep 29, 2011 at 10:39:13AM +0100, Dimitris Papastamos wrote:
> 
> >  void regcache_cache_only(struct regmap *map, bool enable)
> >  {
> > +	BUG_ON(map->cache_bypass && enable);
> >  	mutex_lock(&map->lock);
> 
> I'd move these inside the lock otherwise there's an enable/enable race
> where two threads check that things are safe simultaneously.  I'd also
> change to WARN_ON(), if nothing else it's hard to see BUG_ON()s that
> occur prior to console output and PMICs are likely to trigger that sort
> of stuff.

Aw yes, of course :)

Re: [PATCH 1/8] Revert "regmap: Grab the lock in regcache_cache_only()"

[Mark Brown]

On Thu, Sep 29, 2011 at 10:39:06AM +0100, Dimitris Papastamos wrote:
> This reverts commit 38f6916976f9e964748f097be3688e334fb60f3d.

When sending things like this you should always explain what you're
doing and why.  As things are this won't apply since I already fixed
this by using the main lock.