* [PATCH 2/7] regmap: Modify map->cache_bypass directly
2011-09-28 10:43 [PATCH 1/7] regmap: Fix regcache_sync generic implementation Dimitris Papastamos
@ 2011-09-28 10:43 ` Dimitris Papastamos
2011-09-28 12:56 ` Mark Brown
2011-09-28 10:43 ` [PATCH 3/7] regmap: Add a mutex to guard the sync operation Dimitris Papastamos
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-28 10:43 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Lars-Peter Clausen
In preperation for the upcoming patches, modify map->cache_bypass
directly. The helper functions will grab an exclusive lock. Because
we'll have acquired the same lock we need to avoid a deadlock.
Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
drivers/base/regmap/regcache.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 5364dde..f46e247 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -241,9 +241,9 @@ int regcache_sync(struct regmap *map)
ret = regcache_read(map, i, &val);
if (ret < 0)
goto out;
- regcache_cache_bypass(map, true);
+ map->cache_bypass = 1;
ret = regmap_write(map, i, val);
- regcache_cache_bypass(map, false);
+ map->cache_bypass = 0;
if (ret < 0)
goto out;
dev_dbg(map->dev, "Synced register %#x, value %#x\n",
--
1.7.6.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/7] regmap: Add a mutex to guard the sync operation
2011-09-28 10:43 [PATCH 1/7] regmap: Fix regcache_sync generic implementation Dimitris Papastamos
2011-09-28 10:43 ` [PATCH 2/7] regmap: Modify map->cache_bypass directly Dimitris Papastamos
@ 2011-09-28 10:43 ` Dimitris Papastamos
2011-09-28 12:52 ` Mark Brown
2011-09-28 10:43 ` [PATCH 4/7] regmap: Save/restore the bypass state upon syncing Dimitris Papastamos
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-28 10:43 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Lars-Peter Clausen
Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
drivers/base/regmap/internal.h | 1 +
drivers/base/regmap/regcache.c | 4 ++++
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 2d51b1b..a0b931d 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -32,6 +32,7 @@ struct regmap_format {
struct regmap {
struct mutex lock;
+ struct mutex sync_lock;
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.c b/drivers/base/regmap/regcache.c
index f46e247..b843b5f 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -130,6 +130,8 @@ int regcache_init(struct regmap *map)
if (!map->max_register)
map->max_register = map->num_reg_defaults_raw;
+ mutex_init(&map->sync_lock);
+
if (map->cache_ops->init) {
dev_dbg(map->dev, "Initializing %s cache\n",
map->cache_ops->name);
@@ -230,6 +232,7 @@ int regcache_sync(struct regmap *map)
BUG_ON(!map->cache_ops);
+ mutex_lock(&map->sync_lock);
dev_dbg(map->dev, "Syncing %s cache\n",
map->cache_ops->name);
name = map->cache_ops->name;
@@ -254,6 +257,7 @@ int regcache_sync(struct regmap *map)
}
out:
trace_regcache_sync(map->dev, name, "stop");
+ mutex_unlock(&map->sync_lock);
return ret;
}
--
1.7.6.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/7] regmap: Add a mutex to guard the sync operation
2011-09-28 10:43 ` [PATCH 3/7] regmap: Add a mutex to guard the sync operation Dimitris Papastamos
@ 2011-09-28 12:52 ` Mark Brown
2011-09-28 14:24 ` Dimitris Papastamos
0 siblings, 1 reply; 14+ messages in thread
From: Mark Brown @ 2011-09-28 12:52 UTC (permalink / raw)
To: Dimitris Papastamos; +Cc: linux-kernel, Lars-Peter Clausen
On Wed, Sep 28, 2011 at 11:43:43AM +0100, Dimitris Papastamos wrote:
> + mutex_lock(&map->sync_lock);
> dev_dbg(map->dev, "Syncing %s cache\n",
> map->cache_ops->name);
> name = map->cache_ops->name;
> @@ -254,6 +257,7 @@ int regcache_sync(struct regmap *map)
> }
> out:
> trace_regcache_sync(map->dev, name, "stop");
> + mutex_unlock(&map->sync_lock);
Shouldn't we either be taking the regmap lock when doing the sync or
otherwise guarding against something other than a cache sync?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/7] regmap: Add a mutex to guard the sync operation
2011-09-28 12:52 ` Mark Brown
@ 2011-09-28 14:24 ` Dimitris Papastamos
2011-09-28 15:19 ` Mark Brown
0 siblings, 1 reply; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-28 14:24 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Lars-Peter Clausen
On Wed, Sep 28, 2011 at 01:52:50PM +0100, Mark Brown wrote:
> On Wed, Sep 28, 2011 at 11:43:43AM +0100, Dimitris Papastamos wrote:
>
> > + mutex_lock(&map->sync_lock);
> > dev_dbg(map->dev, "Syncing %s cache\n",
> > map->cache_ops->name);
> > name = map->cache_ops->name;
> > @@ -254,6 +257,7 @@ int regcache_sync(struct regmap *map)
> > }
> > out:
> > trace_regcache_sync(map->dev, name, "stop");
> > + mutex_unlock(&map->sync_lock);
>
> Shouldn't we either be taking the regmap lock when doing the sync or
> otherwise guarding against something other than a cache sync?
Em the main issue is that the sync() implementation will use
regmap_write() which will grab map->lock. To avoid this we could have
gone and used directly the lockless _regmap_write() but that's
static. To be honest, it feels cleaner to have only 1 lock to guard
the map so maybe we should get rid of map->sync_lock.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/7] regmap: Add a mutex to guard the sync operation
2011-09-28 14:24 ` Dimitris Papastamos
@ 2011-09-28 15:19 ` Mark Brown
2011-09-28 15:52 ` Dimitris Papastamos
0 siblings, 1 reply; 14+ messages in thread
From: Mark Brown @ 2011-09-28 15:19 UTC (permalink / raw)
To: Dimitris Papastamos; +Cc: linux-kernel, Lars-Peter Clausen
On Wed, Sep 28, 2011 at 03:24:06PM +0100, Dimitris Papastamos wrote:
> On Wed, Sep 28, 2011 at 01:52:50PM +0100, Mark Brown wrote:
> > Shouldn't we either be taking the regmap lock when doing the sync or
> > otherwise guarding against something other than a cache sync?
> Em the main issue is that the sync() implementation will use
> regmap_write() which will grab map->lock. To avoid this we could have
> gone and used directly the lockless _regmap_write() but that's
> static. To be honest, it feels cleaner to have only 1 lock to guard
> the map so maybe we should get rid of map->sync_lock.
Making _regmap_write() global seems fine, it's not an externally visible
API but the cache code is part of the infrastructure.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/7] regmap: Add a mutex to guard the sync operation
2011-09-28 15:19 ` Mark Brown
@ 2011-09-28 15:52 ` Dimitris Papastamos
0 siblings, 0 replies; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-28 15:52 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Lars-Peter Clausen
On Wed, Sep 28, 2011 at 04:19:37PM +0100, Mark Brown wrote:
> On Wed, Sep 28, 2011 at 03:24:06PM +0100, Dimitris Papastamos wrote:
> > On Wed, Sep 28, 2011 at 01:52:50PM +0100, Mark Brown wrote:
>
> > > Shouldn't we either be taking the regmap lock when doing the sync or
> > > otherwise guarding against something other than a cache sync?
>
> > Em the main issue is that the sync() implementation will use
> > regmap_write() which will grab map->lock. To avoid this we could have
> > gone and used directly the lockless _regmap_write() but that's
> > static. To be honest, it feels cleaner to have only 1 lock to guard
> > the map so maybe we should get rid of map->sync_lock.
>
> Making _regmap_write() global seems fine, it's not an externally visible
> API but the cache code is part of the infrastructure.
Okay, so I'll go forth and revert the use of map->sync_lock
and just grab map->lock.
Thanks,
Dimitris
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/7] regmap: Save/restore the bypass state upon syncing
2011-09-28 10:43 [PATCH 1/7] regmap: Fix regcache_sync generic implementation Dimitris Papastamos
2011-09-28 10:43 ` [PATCH 2/7] regmap: Modify map->cache_bypass directly Dimitris Papastamos
2011-09-28 10:43 ` [PATCH 3/7] regmap: Add a mutex to guard the sync operation Dimitris Papastamos
@ 2011-09-28 10:43 ` Dimitris Papastamos
2011-09-28 10:43 ` [PATCH 5/7] regmap: Grab the lock in regcache_cache_only() Dimitris Papastamos
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-28 10:43 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Lars-Peter Clausen
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 b843b5f..695802c 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -229,10 +229,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->sync_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;
@@ -257,6 +260,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->sync_lock);
return ret;
--
1.7.6.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/7] regmap: Grab the lock in regcache_cache_only()
2011-09-28 10:43 [PATCH 1/7] regmap: Fix regcache_sync generic implementation Dimitris Papastamos
` (2 preceding siblings ...)
2011-09-28 10:43 ` [PATCH 4/7] regmap: Save/restore the bypass state upon syncing Dimitris Papastamos
@ 2011-09-28 10:43 ` Dimitris Papastamos
2011-09-28 12:57 ` Mark Brown
2011-09-28 10:43 ` [PATCH 6/7] regmap: Implement regcache_cache_bypass helper function Dimitris Papastamos
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-28 10:43 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Lars-Peter Clausen
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 695802c..8900897 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -282,7 +282,9 @@ 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 6/7] regmap: Implement regcache_cache_bypass helper function
2011-09-28 10:43 [PATCH 1/7] regmap: Fix regcache_sync generic implementation Dimitris Papastamos
` (3 preceding siblings ...)
2011-09-28 10:43 ` [PATCH 5/7] regmap: Grab the lock in regcache_cache_only() Dimitris Papastamos
@ 2011-09-28 10:43 ` Dimitris Papastamos
2011-09-28 10:43 ` [PATCH 7/7] regmap: Ensure we scream if we enable cache bypass/only at the same time Dimitris Papastamos
2011-09-28 12:56 ` [PATCH 1/7] regmap: Fix regcache_sync generic implementation Mark Brown
6 siblings, 0 replies; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-28 10:43 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Lars-Peter Clausen
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 8900897..a3f466c 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -288,6 +288,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->sync_lock);
+ map->cache_bypass = enable;
+ mutex_unlock(&map->sync_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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 7/7] regmap: Ensure we scream if we enable cache bypass/only at the same time
2011-09-28 10:43 [PATCH 1/7] regmap: Fix regcache_sync generic implementation Dimitris Papastamos
` (4 preceding siblings ...)
2011-09-28 10:43 ` [PATCH 6/7] regmap: Implement regcache_cache_bypass helper function Dimitris Papastamos
@ 2011-09-28 10:43 ` Dimitris Papastamos
2011-09-28 12:56 ` [PATCH 1/7] regmap: Fix regcache_sync generic implementation Mark Brown
6 siblings, 0 replies; 14+ messages in thread
From: Dimitris Papastamos @ 2011-09-28 10:43 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Lars-Peter Clausen
Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
drivers/base/regmap/regcache.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index a3f466c..3d59f14 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -282,6 +282,8 @@ EXPORT_SYMBOL_GPL(regcache_sync);
*/
void regcache_cache_only(struct regmap *map, bool enable)
{
+ BUG_ON(map->cache_bypass && enable);
+
mutex_lock(&map->sync_lock);
map->cache_only = enable;
mutex_unlock(&map->sync_lock);
@@ -301,6 +303,8 @@ EXPORT_SYMBOL_GPL(regcache_cache_only);
*/
void regcache_cache_bypass(struct regmap *map, bool enable)
{
+ BUG_ON(map->cache_only && enable);
+
mutex_lock(&map->sync_lock);
map->cache_bypass = enable;
mutex_unlock(&map->sync_lock);
--
1.7.6.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/7] regmap: Fix regcache_sync generic implementation
2011-09-28 10:43 [PATCH 1/7] regmap: Fix regcache_sync generic implementation Dimitris Papastamos
` (5 preceding siblings ...)
2011-09-28 10:43 ` [PATCH 7/7] regmap: Ensure we scream if we enable cache bypass/only at the same time Dimitris Papastamos
@ 2011-09-28 12:56 ` Mark Brown
6 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2011-09-28 12:56 UTC (permalink / raw)
To: Dimitris Papastamos; +Cc: linux-kernel, Lars-Peter Clausen
On Wed, Sep 28, 2011 at 11:43:41AM +0100, Dimitris Papastamos wrote:
> We want to use regmap_write() to actually write anything
> to the HW.
Applied, thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread