linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] regmap: Mark reg_defaults in regmap_multi_reg_write as const
@ 2014-02-25 13:45 Charles Keepax
  2014-02-25 13:45 ` [PATCH 2/4] regmap: Add bypassed version of regmap_multi_reg_write Charles Keepax
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Charles Keepax @ 2014-02-25 13:45 UTC (permalink / raw)
  To: broonie; +Cc: gregkh, sameo, lee.jones, linux-kernel, patches

There should be no need for the writes supplied to this function to be
edited by it so mark them as const.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regmap.c |    4 ++--
 include/linux/regmap.h       |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 2b4e72f..1ebc6df 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1607,8 +1607,8 @@ EXPORT_SYMBOL_GPL(regmap_bulk_write);
  * A value of zero will be returned on success, a negative errno will
  * be returned in error cases.
  */
-int regmap_multi_reg_write(struct regmap *map, struct reg_default *regs,
-				int num_regs)
+int regmap_multi_reg_write(struct regmap *map, const struct reg_default *regs,
+			   int num_regs)
 {
 	int ret = 0, i;
 
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index fa4d079..ccb2ed0 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -388,7 +388,7 @@ int regmap_raw_write(struct regmap *map, unsigned int reg,
 		     const void *val, size_t val_len);
 int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 			size_t val_count);
-int regmap_multi_reg_write(struct regmap *map, struct reg_default *regs,
+int regmap_multi_reg_write(struct regmap *map, const struct reg_default *regs,
 			int num_regs);
 int regmap_raw_write_async(struct regmap *map, unsigned int reg,
 			   const void *val, size_t val_len);
-- 
1.7.2.5


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

* [PATCH 2/4] regmap: Add bypassed version of regmap_multi_reg_write
  2014-02-25 13:45 [PATCH 1/4] regmap: Mark reg_defaults in regmap_multi_reg_write as const Charles Keepax
@ 2014-02-25 13:45 ` Charles Keepax
  2014-02-25 13:45 ` [PATCH 3/4] regmap: Base regmap_register_patch on _regmap_multi_reg_write Charles Keepax
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Charles Keepax @ 2014-02-25 13:45 UTC (permalink / raw)
  To: broonie; +Cc: gregkh, sameo, lee.jones, linux-kernel, patches

Devices with more complex boot proceedures may occasionally apply the
register patch manual. regmap_multi_reg_write is a logical way to do so,
however the patch must be applied with cache bypass on, such that it
doesn't override any user settings. This patch adds a
regmap_multi_reg_write_bypassed function that applies a set of writes
with the bypass enabled.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regmap.c |   75 ++++++++++++++++++++++++++++++++++-------
 include/linux/regmap.h       |    3 ++
 2 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 1ebc6df..6b07a97 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1591,6 +1591,26 @@ out:
 }
 EXPORT_SYMBOL_GPL(regmap_bulk_write);
 
+static int _regmap_multi_reg_write(struct regmap *map,
+				   const struct reg_default *regs,
+				   int num_regs)
+{
+	int i, ret;
+
+	for (i = 0; i < num_regs; i++) {
+		if (regs[i].reg % map->reg_stride)
+			return -EINVAL;
+		ret = _regmap_write(map, regs[i].reg, regs[i].def);
+		if (ret != 0) {
+			dev_err(map->dev, "Failed to write %x = %x: %d\n",
+				regs[i].reg, regs[i].def, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 /*
  * regmap_multi_reg_write(): Write multiple registers to the device
  *
@@ -1610,28 +1630,57 @@ EXPORT_SYMBOL_GPL(regmap_bulk_write);
 int regmap_multi_reg_write(struct regmap *map, const struct reg_default *regs,
 			   int num_regs)
 {
-	int ret = 0, i;
-
-	for (i = 0; i < num_regs; i++) {
-		int reg = regs[i].reg;
-		if (reg % map->reg_stride)
-			return -EINVAL;
-	}
+	int ret;
 
 	map->lock(map->lock_arg);
 
-	for (i = 0; i < num_regs; i++) {
-		ret = _regmap_write(map, regs[i].reg, regs[i].def);
-		if (ret != 0)
-			goto out;
-	}
-out:
+	ret = _regmap_multi_reg_write(map, regs, num_regs);
+
 	map->unlock(map->lock_arg);
 
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_multi_reg_write);
 
+/*
+ * regmap_multi_reg_write_bypassed(): Write multiple registers to the
+ *                                    device but not the cache
+ *
+ * where the set of register are supplied in any order
+ *
+ * @map: Register map to write to
+ * @regs: Array of structures containing register,value to be written
+ * @num_regs: Number of registers to write
+ *
+ * This function is intended to be used for writing a large block of data
+ * atomically to the device in single transfer for those I2C client devices
+ * that implement this alternative block write mode.
+ *
+ * A value of zero will be returned on success, a negative errno will
+ * be returned in error cases.
+ */
+int regmap_multi_reg_write_bypassed(struct regmap *map,
+				    const struct reg_default *regs,
+				    int num_regs)
+{
+	int ret;
+	bool bypass;
+
+	map->lock(map->lock_arg);
+
+	bypass = map->cache_bypass;
+	map->cache_bypass = true;
+
+	ret = _regmap_multi_reg_write(map, regs, num_regs);
+
+	map->cache_bypass = bypass;
+
+	map->unlock(map->lock_arg);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(regmap_multi_reg_write_bypassed);
+
 /**
  * regmap_raw_write_async(): Write raw values to one or more registers
  *                           asynchronously
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index ccb2ed0..15a6c2c 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -390,6 +390,9 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 			size_t val_count);
 int regmap_multi_reg_write(struct regmap *map, const struct reg_default *regs,
 			int num_regs);
+int regmap_multi_reg_write_bypassed(struct regmap *map,
+				    const struct reg_default *regs,
+				    int num_regs);
 int regmap_raw_write_async(struct regmap *map, unsigned int reg,
 			   const void *val, size_t val_len);
 int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val);
-- 
1.7.2.5


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

* [PATCH 3/4] regmap: Base regmap_register_patch on _regmap_multi_reg_write
  2014-02-25 13:45 [PATCH 1/4] regmap: Mark reg_defaults in regmap_multi_reg_write as const Charles Keepax
  2014-02-25 13:45 ` [PATCH 2/4] regmap: Add bypassed version of regmap_multi_reg_write Charles Keepax
@ 2014-02-25 13:45 ` Charles Keepax
  2014-02-25 13:45 ` [PATCH 4/4] mfd: arizona: Use new regmap features for manual register patch Charles Keepax
  2014-02-25 23:58 ` [PATCH 1/4] regmap: Mark reg_defaults in regmap_multi_reg_write as const Mark Brown
  3 siblings, 0 replies; 6+ messages in thread
From: Charles Keepax @ 2014-02-25 13:45 UTC (permalink / raw)
  To: broonie; +Cc: gregkh, sameo, lee.jones, linux-kernel, patches

Since we now have an internal version of regmap_multi_reg_write use this
to apply the register patch.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regmap.c |   18 ++++--------------
 1 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 6b07a97..0e5c833 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2245,7 +2245,7 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs,
 			  int num_regs)
 {
 	struct reg_default *p;
-	int i, ret;
+	int ret;
 	bool bypass;
 
 	if (WARN_ONCE(num_regs <= 0, "invalid registers number (%d)\n",
@@ -2259,19 +2259,9 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs,
 	map->cache_bypass = true;
 	map->async = true;
 
-	/* Write out first; it's useful to apply even if we fail later. */
-	for (i = 0; i < num_regs; i++) {
-		if (regs[i].reg % map->reg_stride) {
-			ret = -EINVAL;
-			goto out;
-		}
-		ret = _regmap_write(map, regs[i].reg, regs[i].def);
-		if (ret != 0) {
-			dev_err(map->dev, "Failed to write %x = %x: %d\n",
-				regs[i].reg, regs[i].def, ret);
-			goto out;
-		}
-	}
+	ret = _regmap_multi_reg_write(map, regs, num_regs);
+	if (ret != 0)
+		goto out;
 
 	p = krealloc(map->patch,
 		     sizeof(struct reg_default) * (map->patch_regs + num_regs),
-- 
1.7.2.5


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

* [PATCH 4/4] mfd: arizona: Use new regmap features for manual register patch
  2014-02-25 13:45 [PATCH 1/4] regmap: Mark reg_defaults in regmap_multi_reg_write as const Charles Keepax
  2014-02-25 13:45 ` [PATCH 2/4] regmap: Add bypassed version of regmap_multi_reg_write Charles Keepax
  2014-02-25 13:45 ` [PATCH 3/4] regmap: Base regmap_register_patch on _regmap_multi_reg_write Charles Keepax
@ 2014-02-25 13:45 ` Charles Keepax
  2014-02-25 14:27   ` Lee Jones
  2014-02-25 23:58 ` [PATCH 1/4] regmap: Mark reg_defaults in regmap_multi_reg_write as const Mark Brown
  3 siblings, 1 reply; 6+ messages in thread
From: Charles Keepax @ 2014-02-25 13:45 UTC (permalink / raw)
  To: broonie; +Cc: gregkh, sameo, lee.jones, linux-kernel, patches

On the wm5102 the register patches are applied manually, rather than by
the regmap core. This application is wrapped in calls to
regcache_cache_bypass. However, this is dangerous as other threads may
be accessing the hardware at the same time as the pm_runtime operations
and if they do so during the period whilst cache_bypass is enabled those
writes will miss the cache when they shouldn't.

Apply the register patch using the new regmap_multi_reg_write_bypassed
function to avoid this problem. Also remove the call to
regcache_cache_bypass from the hardware patch application as it is
unneeded there and creates a similar window for writes to miss the
cache.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c  |    4 ----
 drivers/mfd/wm5102-tables.c |   21 ++++-----------------
 2 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index a45aab9..1c3ae57 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -251,8 +251,6 @@ static int arizona_apply_hardware_patch(struct arizona* arizona)
 	unsigned int fll, sysclk;
 	int ret, err;
 
-	regcache_cache_bypass(arizona->regmap, true);
-
 	/* Cache existing FLL and SYSCLK settings */
 	ret = regmap_read(arizona->regmap, ARIZONA_FLL1_CONTROL_1, &fll);
 	if (ret != 0) {
@@ -322,8 +320,6 @@ err_fll:
 			err);
 	}
 
-	regcache_cache_bypass(arizona->regmap, false);
-
 	if (ret != 0)
 		return ret;
 	else
diff --git a/drivers/mfd/wm5102-tables.c b/drivers/mfd/wm5102-tables.c
index 1e9a4b2..bffc584 100644
--- a/drivers/mfd/wm5102-tables.c
+++ b/drivers/mfd/wm5102-tables.c
@@ -80,8 +80,7 @@ static const struct reg_default wm5102_revb_patch[] = {
 int wm5102_patch(struct arizona *arizona)
 {
 	const struct reg_default *wm5102_patch;
-	int ret = 0;
-	int i, patch_size;
+	int patch_size;
 
 	switch (arizona->rev) {
 	case 0:
@@ -92,21 +91,9 @@ int wm5102_patch(struct arizona *arizona)
 		patch_size = ARRAY_SIZE(wm5102_revb_patch);
 	}
 
-	regcache_cache_bypass(arizona->regmap, true);
-
-	for (i = 0; i < patch_size; i++) {
-		ret = regmap_write(arizona->regmap, wm5102_patch[i].reg,
-				   wm5102_patch[i].def);
-		if (ret != 0) {
-			dev_err(arizona->dev, "Failed to write %x = %x: %d\n",
-				wm5102_patch[i].reg, wm5102_patch[i].def, ret);
-			goto out;
-		}
-	}
-
-out:
-	regcache_cache_bypass(arizona->regmap, false);
-	return ret;
+	return regmap_multi_reg_write_bypassed(arizona->regmap,
+					       wm5102_patch,
+					       patch_size);
 }
 
 static const struct regmap_irq wm5102_aod_irqs[ARIZONA_NUM_IRQ] = {
-- 
1.7.2.5


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

* Re: [PATCH 4/4] mfd: arizona: Use new regmap features for manual register patch
  2014-02-25 13:45 ` [PATCH 4/4] mfd: arizona: Use new regmap features for manual register patch Charles Keepax
@ 2014-02-25 14:27   ` Lee Jones
  0 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2014-02-25 14:27 UTC (permalink / raw)
  To: Charles Keepax; +Cc: broonie, gregkh, sameo, linux-kernel, patches

On Tue, 25 Feb 2014, Charles Keepax wrote:

> On the wm5102 the register patches are applied manually, rather than by
> the regmap core. This application is wrapped in calls to
> regcache_cache_bypass. However, this is dangerous as other threads may
> be accessing the hardware at the same time as the pm_runtime operations
> and if they do so during the period whilst cache_bypass is enabled those
> writes will miss the cache when they shouldn't.
> 
> Apply the register patch using the new regmap_multi_reg_write_bypassed
> function to avoid this problem. Also remove the call to
> regcache_cache_bypass from the hardware patch application as it is
> unneeded there and creates a similar window for writes to miss the
> cache.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/mfd/arizona-core.c  |    4 ----
>  drivers/mfd/wm5102-tables.c |   21 ++++-----------------
>  2 files changed, 4 insertions(+), 21 deletions(-)

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 1/4] regmap: Mark reg_defaults in regmap_multi_reg_write as const
  2014-02-25 13:45 [PATCH 1/4] regmap: Mark reg_defaults in regmap_multi_reg_write as const Charles Keepax
                   ` (2 preceding siblings ...)
  2014-02-25 13:45 ` [PATCH 4/4] mfd: arizona: Use new regmap features for manual register patch Charles Keepax
@ 2014-02-25 23:58 ` Mark Brown
  3 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-02-25 23:58 UTC (permalink / raw)
  To: Charles Keepax; +Cc: gregkh, sameo, lee.jones, linux-kernel, patches

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

On Tue, Feb 25, 2014 at 01:45:49PM +0000, Charles Keepax wrote:
> There should be no need for the writes supplied to this function to be
> edited by it so mark them as const.

Applied all, thanks.

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

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

end of thread, other threads:[~2014-02-25 23:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-25 13:45 [PATCH 1/4] regmap: Mark reg_defaults in regmap_multi_reg_write as const Charles Keepax
2014-02-25 13:45 ` [PATCH 2/4] regmap: Add bypassed version of regmap_multi_reg_write Charles Keepax
2014-02-25 13:45 ` [PATCH 3/4] regmap: Base regmap_register_patch on _regmap_multi_reg_write Charles Keepax
2014-02-25 13:45 ` [PATCH 4/4] mfd: arizona: Use new regmap features for manual register patch Charles Keepax
2014-02-25 14:27   ` Lee Jones
2014-02-25 23:58 ` [PATCH 1/4] regmap: Mark reg_defaults in regmap_multi_reg_write as const Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).