linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] regmap: Format data for raw write in regmap_bulk_write
@ 2018-02-22 12:59 Charles Keepax
  2018-02-22 12:59 ` [PATCH 2/5] regmap: Remove unnecessary printk for failed allocation Charles Keepax
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Charles Keepax @ 2018-02-22 12:59 UTC (permalink / raw)
  To: broonie; +Cc: linux-kernel, patches

In the case were the bulk transaction is split up into smaller chunks
data is passed directly to regmap_raw_write. However regmap_bulk_write
uses data in host endian and regmap_raw_write expects data in device
endian. As such if the host and device differ in endian the wrong data
will be written to the device. Correct this issue using a similar
approach to the single raw write case below it, duplicate the data
into a new buffer and use parse_inplace to format the data correctly.

Fixes: adaac459759d ("regmap: Introduce max_raw_read/write for regmap_bulk_read/write")
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/base/regmap/regmap.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index b38ba6fdecbc..8a6de76f89bb 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2005,6 +2005,17 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		int chunk_stride = map->reg_stride;
 		size_t chunk_size = val_bytes;
 		size_t chunk_count = val_count;
+		void *wval;
+
+		if (!val_count)
+			return -EINVAL;
+
+		wval = kmemdup(val, val_count * val_bytes, map->alloc_flags);
+		if (!wval)
+			return -ENOMEM;
+
+		for (i = 0; i < val_count * val_bytes; i += val_bytes)
+			map->format.parse_inplace(wval + i);
 
 		if (!map->use_single_write) {
 			chunk_size = map->max_raw_write;
@@ -2019,7 +2030,7 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		for (i = 0; i < chunk_count; i++) {
 			ret = _regmap_raw_write(map,
 						reg + (i * chunk_stride),
-						val + (i * chunk_size),
+						wval + (i * chunk_size),
 						chunk_size);
 			if (ret)
 				break;
@@ -2028,10 +2039,12 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		/* Write remaining bytes */
 		if (!ret && chunk_size * i < total_size) {
 			ret = _regmap_raw_write(map, reg + (i * chunk_stride),
-						val + (i * chunk_size),
+						wval + (i * chunk_size),
 						total_size - i * chunk_size);
 		}
 		map->unlock(map->lock_arg);
+
+		kfree(wval);
 	} else {
 		void *wval;
 
-- 
2.11.0

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

* [PATCH 2/5] regmap: Remove unnecessary printk for failed allocation
  2018-02-22 12:59 [PATCH 1/5] regmap: Format data for raw write in regmap_bulk_write Charles Keepax
@ 2018-02-22 12:59 ` Charles Keepax
  2018-02-22 12:59 ` [PATCH 3/5] regmap: Move the handling for max_raw_write into regmap_raw_write Charles Keepax
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Charles Keepax @ 2018-02-22 12:59 UTC (permalink / raw)
  To: broonie; +Cc: linux-kernel, patches

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/base/regmap/regmap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 8a6de76f89bb..c84c4d5d2df7 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2052,10 +2052,9 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 			return -EINVAL;
 
 		wval = kmemdup(val, val_count * val_bytes, map->alloc_flags);
-		if (!wval) {
-			dev_err(map->dev, "Error in memory allocation\n");
+		if (!wval)
 			return -ENOMEM;
-		}
+
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(wval + i);
 
-- 
2.11.0

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

* [PATCH 3/5] regmap: Move the handling for max_raw_write into regmap_raw_write
  2018-02-22 12:59 [PATCH 1/5] regmap: Format data for raw write in regmap_bulk_write Charles Keepax
  2018-02-22 12:59 ` [PATCH 2/5] regmap: Remove unnecessary printk for failed allocation Charles Keepax
@ 2018-02-22 12:59 ` Charles Keepax
  2018-02-26 11:17   ` Applied "regmap: Move the handling for max_raw_write into regmap_raw_write" to the regmap tree Mark Brown
  2018-02-22 12:59 ` [PATCH 4/5] regmap: Tidy up regmap_raw_write chunking code Charles Keepax
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Charles Keepax @ 2018-02-22 12:59 UTC (permalink / raw)
  To: broonie; +Cc: linux-kernel, patches

Currently regmap_bulk_write will split a write into chunks before
calling regmap_raw_write if max_raw_write is set. It is more logical
for this handling to be inside regmap_raw_write itself, as this
removes the need to keep re-implementing the chunking code, which
would be the same for all users of regmap_raw_write.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/base/regmap/regmap.c | 117 ++++++++++++++++++++-----------------------
 1 file changed, 54 insertions(+), 63 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index c84c4d5d2df7..dd17e2e7a3d7 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1440,8 +1440,8 @@ static void regmap_set_work_buf_flag_mask(struct regmap *map, int max_bytes,
 		buf[i] |= (mask >> (8 * i)) & 0xff;
 }
 
-int _regmap_raw_write(struct regmap *map, unsigned int reg,
-		      const void *val, size_t val_len)
+static int _regmap_raw_write_impl(struct regmap *map, unsigned int reg,
+				  const void *val, size_t val_len)
 {
 	struct regmap_range_node *range;
 	unsigned long flags;
@@ -1492,8 +1492,9 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
 		while (val_num > win_residue) {
 			dev_dbg(map->dev, "Writing window %d/%zu\n",
 				win_residue, val_len / map->format.val_bytes);
-			ret = _regmap_raw_write(map, reg, val, win_residue *
-						map->format.val_bytes);
+			ret = _regmap_raw_write_impl(map, reg, val,
+						     win_residue *
+						     map->format.val_bytes);
 			if (ret != 0)
 				return ret;
 
@@ -1709,11 +1710,11 @@ static int _regmap_bus_raw_write(void *context, unsigned int reg,
 
 	map->format.format_val(map->work_buf + map->format.reg_bytes
 			       + map->format.pad_bytes, val, 0);
-	return _regmap_raw_write(map, reg,
-				 map->work_buf +
-				 map->format.reg_bytes +
-				 map->format.pad_bytes,
-				 map->format.val_bytes);
+	return _regmap_raw_write_impl(map, reg,
+				      map->work_buf +
+				      map->format.reg_bytes +
+				      map->format.pad_bytes,
+				      map->format.val_bytes);
 }
 
 static inline void *_regmap_map_get_context(struct regmap *map)
@@ -1808,6 +1809,49 @@ int regmap_write_async(struct regmap *map, unsigned int reg, unsigned int val)
 }
 EXPORT_SYMBOL_GPL(regmap_write_async);
 
+int _regmap_raw_write(struct regmap *map, unsigned int reg,
+		      const void *val, size_t val_len)
+{
+	size_t val_bytes = map->format.val_bytes;
+	size_t val_count = val_len / val_bytes;
+	int chunk_stride = map->reg_stride;
+	size_t chunk_size = val_bytes;
+	size_t chunk_count = val_count;
+	int ret, i;
+
+	if (!val_count)
+		return -EINVAL;
+
+	if (!map->use_single_write) {
+		if (map->max_raw_write)
+			chunk_size = map->max_raw_write;
+		else
+			chunk_size = val_len;
+		if (chunk_size % val_bytes)
+			chunk_size -= chunk_size % val_bytes;
+		chunk_count = val_len / chunk_size;
+		chunk_stride *= chunk_size / val_bytes;
+	}
+
+	/* Write as many bytes as possible with chunk_size */
+	for (i = 0; i < chunk_count; i++) {
+		ret = _regmap_raw_write_impl(map,
+					     reg + (i * chunk_stride),
+					     val + (i * chunk_size),
+					     chunk_size);
+		if (ret)
+			return ret;
+	}
+
+	/* Write remaining bytes */
+	if (!ret && chunk_size * i < val_len)
+		ret = _regmap_raw_write_impl(map, reg + (i * chunk_stride),
+					     val + (i * chunk_size),
+					     val_len - i * chunk_size);
+
+	return ret;
+}
+
 /**
  * regmap_raw_write() - Write raw values to one or more registers
  *
@@ -1833,8 +1877,6 @@ int regmap_raw_write(struct regmap *map, unsigned int reg,
 		return -EINVAL;
 	if (val_len % map->format.val_bytes)
 		return -EINVAL;
-	if (map->max_raw_write && map->max_raw_write < val_len)
-		return -E2BIG;
 
 	map->lock(map->lock_arg);
 
@@ -1925,7 +1967,6 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 {
 	int ret = 0, i;
 	size_t val_bytes = map->format.val_bytes;
-	size_t total_size = val_bytes * val_count;
 
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
@@ -2000,57 +2041,9 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 			if (ret)
 				return ret;
 		}
-	} else if (map->use_single_write ||
-		   (map->max_raw_write && map->max_raw_write < total_size)) {
-		int chunk_stride = map->reg_stride;
-		size_t chunk_size = val_bytes;
-		size_t chunk_count = val_count;
-		void *wval;
-
-		if (!val_count)
-			return -EINVAL;
-
-		wval = kmemdup(val, val_count * val_bytes, map->alloc_flags);
-		if (!wval)
-			return -ENOMEM;
-
-		for (i = 0; i < val_count * val_bytes; i += val_bytes)
-			map->format.parse_inplace(wval + i);
-
-		if (!map->use_single_write) {
-			chunk_size = map->max_raw_write;
-			if (chunk_size % val_bytes)
-				chunk_size -= chunk_size % val_bytes;
-			chunk_count = total_size / chunk_size;
-			chunk_stride *= chunk_size / val_bytes;
-		}
-
-		map->lock(map->lock_arg);
-		/* Write as many bytes as possible with chunk_size */
-		for (i = 0; i < chunk_count; i++) {
-			ret = _regmap_raw_write(map,
-						reg + (i * chunk_stride),
-						wval + (i * chunk_size),
-						chunk_size);
-			if (ret)
-				break;
-		}
-
-		/* Write remaining bytes */
-		if (!ret && chunk_size * i < total_size) {
-			ret = _regmap_raw_write(map, reg + (i * chunk_stride),
-						wval + (i * chunk_size),
-						total_size - i * chunk_size);
-		}
-		map->unlock(map->lock_arg);
-
-		kfree(wval);
 	} else {
 		void *wval;
 
-		if (!val_count)
-			return -EINVAL;
-
 		wval = kmemdup(val, val_count * val_bytes, map->alloc_flags);
 		if (!wval)
 			return -ENOMEM;
@@ -2058,9 +2051,7 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(wval + i);
 
-		map->lock(map->lock_arg);
-		ret = _regmap_raw_write(map, reg, wval, val_bytes * val_count);
-		map->unlock(map->lock_arg);
+		ret = regmap_raw_write(map, reg, wval, val_bytes * val_count);
 
 		kfree(wval);
 	}
-- 
2.11.0

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

* [PATCH 4/5] regmap: Tidy up regmap_raw_write chunking code
  2018-02-22 12:59 [PATCH 1/5] regmap: Format data for raw write in regmap_bulk_write Charles Keepax
  2018-02-22 12:59 ` [PATCH 2/5] regmap: Remove unnecessary printk for failed allocation Charles Keepax
  2018-02-22 12:59 ` [PATCH 3/5] regmap: Move the handling for max_raw_write into regmap_raw_write Charles Keepax
@ 2018-02-22 12:59 ` Charles Keepax
  2018-02-26 11:17   ` Applied "regmap: Tidy up regmap_raw_write chunking code" to the regmap tree Mark Brown
  2018-02-22 12:59 ` [PATCH 5/5] regmap: Merge redundant handling in regmap_bulk_write Charles Keepax
  2018-02-26 11:18 ` Applied "regmap: Format data for raw write " Mark Brown
  4 siblings, 1 reply; 9+ messages in thread
From: Charles Keepax @ 2018-02-22 12:59 UTC (permalink / raw)
  To: broonie; +Cc: linux-kernel, patches

Raw writes may need to be split into small chunks if max_raw_write is
set. Tidy up the code implementing this, the new code is slightly
clearer, slightly shorter and slightly more efficient.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/base/regmap/regmap.c | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index dd17e2e7a3d7..6809b33d6ab1 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1814,40 +1814,35 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
 {
 	size_t val_bytes = map->format.val_bytes;
 	size_t val_count = val_len / val_bytes;
-	int chunk_stride = map->reg_stride;
-	size_t chunk_size = val_bytes;
-	size_t chunk_count = val_count;
+	size_t chunk_count, chunk_bytes;
+	size_t chunk_regs = val_count;
 	int ret, i;
 
 	if (!val_count)
 		return -EINVAL;
 
-	if (!map->use_single_write) {
-		if (map->max_raw_write)
-			chunk_size = map->max_raw_write;
-		else
-			chunk_size = val_len;
-		if (chunk_size % val_bytes)
-			chunk_size -= chunk_size % val_bytes;
-		chunk_count = val_len / chunk_size;
-		chunk_stride *= chunk_size / val_bytes;
-	}
+	if (map->use_single_write)
+		chunk_regs = 1;
+	else if (map->max_raw_write && val_len > map->max_raw_write)
+		chunk_regs = map->max_raw_write / val_bytes;
+
+	chunk_count = val_count / chunk_regs;
+	chunk_bytes = chunk_regs * val_bytes;
 
 	/* Write as many bytes as possible with chunk_size */
 	for (i = 0; i < chunk_count; i++) {
-		ret = _regmap_raw_write_impl(map,
-					     reg + (i * chunk_stride),
-					     val + (i * chunk_size),
-					     chunk_size);
+		ret = _regmap_raw_write_impl(map, reg, val, chunk_bytes);
 		if (ret)
 			return ret;
+
+		reg += regmap_get_offset(map, chunk_regs);
+		val += chunk_bytes;
+		val_len -= chunk_bytes;
 	}
 
 	/* Write remaining bytes */
-	if (!ret && chunk_size * i < val_len)
-		ret = _regmap_raw_write_impl(map, reg + (i * chunk_stride),
-					     val + (i * chunk_size),
-					     val_len - i * chunk_size);
+	if (val_len)
+		ret = _regmap_raw_write_impl(map, reg, val, val_len);
 
 	return ret;
 }
-- 
2.11.0

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

* [PATCH 5/5] regmap: Merge redundant handling in regmap_bulk_write
  2018-02-22 12:59 [PATCH 1/5] regmap: Format data for raw write in regmap_bulk_write Charles Keepax
                   ` (2 preceding siblings ...)
  2018-02-22 12:59 ` [PATCH 4/5] regmap: Tidy up regmap_raw_write chunking code Charles Keepax
@ 2018-02-22 12:59 ` Charles Keepax
  2018-02-26 11:17   ` Applied "regmap: Merge redundant handling in regmap_bulk_write" to the regmap tree Mark Brown
  2018-02-26 11:18 ` Applied "regmap: Format data for raw write " Mark Brown
  4 siblings, 1 reply; 9+ messages in thread
From: Charles Keepax @ 2018-02-22 12:59 UTC (permalink / raw)
  To: broonie; +Cc: linux-kernel, patches

The handling for the first two cases in regmap_bulk_write is
essentially identical. The first case is just a better implementation of
the second, supporting 8 byte registers and doing the locking manually to
avoid bouncing the lock for each register. Drop some redundant code by
removing the second of these cases and allowing both situations to be
handled by the same code.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/base/regmap/regmap.c | 39 +++------------------------------------
 1 file changed, 3 insertions(+), 36 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 6809b33d6ab1..3bc84885eb91 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1967,17 +1967,10 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		return -EINVAL;
 
 	/*
-	 * Some devices don't support bulk write, for
-	 * them we have a series of single write operations in the first two if
-	 * blocks.
-	 *
-	 * The first if block is used for memory mapped io. It does not allow
-	 * val_bytes of 3 for example.
-	 * The second one is for busses that do not provide raw I/O.
-	 * The third one is used for busses which do not have these limitations
-	 * and can write arbitrary value lengths.
+	 * Some devices don't support bulk write, for them we have a series of
+	 * single write operations.
 	 */
-	if (!map->bus) {
+	if (!map->bus || !map->format.parse_inplace) {
 		map->lock(map->lock_arg);
 		for (i = 0; i < val_count; i++) {
 			unsigned int ival;
@@ -2010,32 +2003,6 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		}
 out:
 		map->unlock(map->lock_arg);
-	} else if (map->bus && !map->format.parse_inplace) {
-		const u8 *u8 = val;
-		const u16 *u16 = val;
-		const u32 *u32 = val;
-		unsigned int ival;
-
-		for (i = 0; i < val_count; i++) {
-			switch (map->format.val_bytes) {
-			case 4:
-				ival = u32[i];
-				break;
-			case 2:
-				ival = u16[i];
-				break;
-			case 1:
-				ival = u8[i];
-				break;
-			default:
-				return -EINVAL;
-			}
-
-			ret = regmap_write(map, reg + regmap_get_offset(map, i),
-					   ival);
-			if (ret)
-				return ret;
-		}
 	} else {
 		void *wval;
 
-- 
2.11.0

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

* Applied "regmap: Merge redundant handling in regmap_bulk_write" to the regmap tree
  2018-02-22 12:59 ` [PATCH 5/5] regmap: Merge redundant handling in regmap_bulk_write Charles Keepax
@ 2018-02-26 11:17   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2018-02-26 11:17 UTC (permalink / raw)
  To: Charles Keepax; +Cc: Mark Brown, broonie, linux-kernel, patches, linux-kernel

The patch

   regmap: Merge redundant handling in regmap_bulk_write

has been applied to the regmap tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From fb44f3cec35c6e71865012fa281ba6d4ff50a99a Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.cirrus.com>
Date: Thu, 22 Feb 2018 12:59:14 +0000
Subject: [PATCH] regmap: Merge redundant handling in regmap_bulk_write

The handling for the first two cases in regmap_bulk_write is
essentially identical. The first case is just a better implementation of
the second, supporting 8 byte registers and doing the locking manually to
avoid bouncing the lock for each register. Drop some redundant code by
removing the second of these cases and allowing both situations to be
handled by the same code.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/regmap.c | 39 +++------------------------------------
 1 file changed, 3 insertions(+), 36 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index bfdd66dd3766..fafee9251d65 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1965,17 +1965,10 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		return -EINVAL;
 
 	/*
-	 * Some devices don't support bulk write, for
-	 * them we have a series of single write operations in the first two if
-	 * blocks.
-	 *
-	 * The first if block is used for memory mapped io. It does not allow
-	 * val_bytes of 3 for example.
-	 * The second one is for busses that do not provide raw I/O.
-	 * The third one is used for busses which do not have these limitations
-	 * and can write arbitrary value lengths.
+	 * Some devices don't support bulk write, for them we have a series of
+	 * single write operations.
 	 */
-	if (!map->bus) {
+	if (!map->bus || !map->format.parse_inplace) {
 		map->lock(map->lock_arg);
 		for (i = 0; i < val_count; i++) {
 			unsigned int ival;
@@ -2008,32 +2001,6 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		}
 out:
 		map->unlock(map->lock_arg);
-	} else if (map->bus && !map->format.parse_inplace) {
-		const u8 *u8 = val;
-		const u16 *u16 = val;
-		const u32 *u32 = val;
-		unsigned int ival;
-
-		for (i = 0; i < val_count; i++) {
-			switch (map->format.val_bytes) {
-			case 4:
-				ival = u32[i];
-				break;
-			case 2:
-				ival = u16[i];
-				break;
-			case 1:
-				ival = u8[i];
-				break;
-			default:
-				return -EINVAL;
-			}
-
-			ret = regmap_write(map, reg + regmap_get_offset(map, i),
-					   ival);
-			if (ret)
-				return ret;
-		}
 	} else {
 		void *wval;
 
-- 
2.16.1

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

* Applied "regmap: Tidy up regmap_raw_write chunking code" to the regmap tree
  2018-02-22 12:59 ` [PATCH 4/5] regmap: Tidy up regmap_raw_write chunking code Charles Keepax
@ 2018-02-26 11:17   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2018-02-26 11:17 UTC (permalink / raw)
  To: Charles Keepax; +Cc: Mark Brown, broonie, linux-kernel, patches, linux-kernel

The patch

   regmap: Tidy up regmap_raw_write chunking code

has been applied to the regmap tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 364e378b8d1679f91a29a9537a881bba17931cfb Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.cirrus.com>
Date: Thu, 22 Feb 2018 12:59:13 +0000
Subject: [PATCH] regmap: Tidy up regmap_raw_write chunking code

Raw writes may need to be split into small chunks if max_raw_write is
set. Tidy up the code implementing this, the new code is slightly
clearer, slightly shorter and slightly more efficient.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/regmap.c | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index e82ea77849fb..bfdd66dd3766 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1812,40 +1812,35 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
 {
 	size_t val_bytes = map->format.val_bytes;
 	size_t val_count = val_len / val_bytes;
-	int chunk_stride = map->reg_stride;
-	size_t chunk_size = val_bytes;
-	size_t chunk_count = val_count;
+	size_t chunk_count, chunk_bytes;
+	size_t chunk_regs = val_count;
 	int ret, i;
 
 	if (!val_count)
 		return -EINVAL;
 
-	if (!map->use_single_write) {
-		if (map->max_raw_write)
-			chunk_size = map->max_raw_write;
-		else
-			chunk_size = val_len;
-		if (chunk_size % val_bytes)
-			chunk_size -= chunk_size % val_bytes;
-		chunk_count = val_len / chunk_size;
-		chunk_stride *= chunk_size / val_bytes;
-	}
+	if (map->use_single_write)
+		chunk_regs = 1;
+	else if (map->max_raw_write && val_len > map->max_raw_write)
+		chunk_regs = map->max_raw_write / val_bytes;
+
+	chunk_count = val_count / chunk_regs;
+	chunk_bytes = chunk_regs * val_bytes;
 
 	/* Write as many bytes as possible with chunk_size */
 	for (i = 0; i < chunk_count; i++) {
-		ret = _regmap_raw_write_impl(map,
-					     reg + (i * chunk_stride),
-					     val + (i * chunk_size),
-					     chunk_size);
+		ret = _regmap_raw_write_impl(map, reg, val, chunk_bytes);
 		if (ret)
 			return ret;
+
+		reg += regmap_get_offset(map, chunk_regs);
+		val += chunk_bytes;
+		val_len -= chunk_bytes;
 	}
 
 	/* Write remaining bytes */
-	if (!ret && chunk_size * i < val_len)
-		ret = _regmap_raw_write_impl(map, reg + (i * chunk_stride),
-					     val + (i * chunk_size),
-					     val_len - i * chunk_size);
+	if (val_len)
+		ret = _regmap_raw_write_impl(map, reg, val, val_len);
 
 	return ret;
 }
-- 
2.16.1

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

* Applied "regmap: Move the handling for max_raw_write into regmap_raw_write" to the regmap tree
  2018-02-22 12:59 ` [PATCH 3/5] regmap: Move the handling for max_raw_write into regmap_raw_write Charles Keepax
@ 2018-02-26 11:17   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2018-02-26 11:17 UTC (permalink / raw)
  To: Charles Keepax; +Cc: Mark Brown, broonie, linux-kernel, patches, linux-kernel

The patch

   regmap: Move the handling for max_raw_write into regmap_raw_write

has been applied to the regmap tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 7ef2c6b8689a084954cffbd102ee49c2fb72cbd4 Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.cirrus.com>
Date: Thu, 22 Feb 2018 12:59:12 +0000
Subject: [PATCH] regmap: Move the handling for max_raw_write into
 regmap_raw_write

Currently regmap_bulk_write will split a write into chunks before
calling regmap_raw_write if max_raw_write is set. It is more logical
for this handling to be inside regmap_raw_write itself, as this
removes the need to keep re-implementing the chunking code, which
would be the same for all users of regmap_raw_write.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/regmap.c | 117 ++++++++++++++++++++-----------------------
 1 file changed, 54 insertions(+), 63 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 707b0450ad72..e82ea77849fb 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1438,8 +1438,8 @@ static void regmap_set_work_buf_flag_mask(struct regmap *map, int max_bytes,
 		buf[i] |= (mask >> (8 * i)) & 0xff;
 }
 
-int _regmap_raw_write(struct regmap *map, unsigned int reg,
-		      const void *val, size_t val_len)
+static int _regmap_raw_write_impl(struct regmap *map, unsigned int reg,
+				  const void *val, size_t val_len)
 {
 	struct regmap_range_node *range;
 	unsigned long flags;
@@ -1490,8 +1490,9 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
 		while (val_num > win_residue) {
 			dev_dbg(map->dev, "Writing window %d/%zu\n",
 				win_residue, val_len / map->format.val_bytes);
-			ret = _regmap_raw_write(map, reg, val, win_residue *
-						map->format.val_bytes);
+			ret = _regmap_raw_write_impl(map, reg, val,
+						     win_residue *
+						     map->format.val_bytes);
 			if (ret != 0)
 				return ret;
 
@@ -1707,11 +1708,11 @@ static int _regmap_bus_raw_write(void *context, unsigned int reg,
 
 	map->format.format_val(map->work_buf + map->format.reg_bytes
 			       + map->format.pad_bytes, val, 0);
-	return _regmap_raw_write(map, reg,
-				 map->work_buf +
-				 map->format.reg_bytes +
-				 map->format.pad_bytes,
-				 map->format.val_bytes);
+	return _regmap_raw_write_impl(map, reg,
+				      map->work_buf +
+				      map->format.reg_bytes +
+				      map->format.pad_bytes,
+				      map->format.val_bytes);
 }
 
 static inline void *_regmap_map_get_context(struct regmap *map)
@@ -1806,6 +1807,49 @@ int regmap_write_async(struct regmap *map, unsigned int reg, unsigned int val)
 }
 EXPORT_SYMBOL_GPL(regmap_write_async);
 
+int _regmap_raw_write(struct regmap *map, unsigned int reg,
+		      const void *val, size_t val_len)
+{
+	size_t val_bytes = map->format.val_bytes;
+	size_t val_count = val_len / val_bytes;
+	int chunk_stride = map->reg_stride;
+	size_t chunk_size = val_bytes;
+	size_t chunk_count = val_count;
+	int ret, i;
+
+	if (!val_count)
+		return -EINVAL;
+
+	if (!map->use_single_write) {
+		if (map->max_raw_write)
+			chunk_size = map->max_raw_write;
+		else
+			chunk_size = val_len;
+		if (chunk_size % val_bytes)
+			chunk_size -= chunk_size % val_bytes;
+		chunk_count = val_len / chunk_size;
+		chunk_stride *= chunk_size / val_bytes;
+	}
+
+	/* Write as many bytes as possible with chunk_size */
+	for (i = 0; i < chunk_count; i++) {
+		ret = _regmap_raw_write_impl(map,
+					     reg + (i * chunk_stride),
+					     val + (i * chunk_size),
+					     chunk_size);
+		if (ret)
+			return ret;
+	}
+
+	/* Write remaining bytes */
+	if (!ret && chunk_size * i < val_len)
+		ret = _regmap_raw_write_impl(map, reg + (i * chunk_stride),
+					     val + (i * chunk_size),
+					     val_len - i * chunk_size);
+
+	return ret;
+}
+
 /**
  * regmap_raw_write() - Write raw values to one or more registers
  *
@@ -1831,8 +1875,6 @@ int regmap_raw_write(struct regmap *map, unsigned int reg,
 		return -EINVAL;
 	if (val_len % map->format.val_bytes)
 		return -EINVAL;
-	if (map->max_raw_write && map->max_raw_write < val_len)
-		return -E2BIG;
 
 	map->lock(map->lock_arg);
 
@@ -1923,7 +1965,6 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 {
 	int ret = 0, i;
 	size_t val_bytes = map->format.val_bytes;
-	size_t total_size = val_bytes * val_count;
 
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
@@ -1998,57 +2039,9 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 			if (ret)
 				return ret;
 		}
-	} else if (map->use_single_write ||
-		   (map->max_raw_write && map->max_raw_write < total_size)) {
-		int chunk_stride = map->reg_stride;
-		size_t chunk_size = val_bytes;
-		size_t chunk_count = val_count;
-		void *wval;
-
-		if (!val_count)
-			return -EINVAL;
-
-		wval = kmemdup(val, val_count * val_bytes, map->alloc_flags);
-		if (!wval)
-			return -ENOMEM;
-
-		for (i = 0; i < val_count * val_bytes; i += val_bytes)
-			map->format.parse_inplace(wval + i);
-
-		if (!map->use_single_write) {
-			chunk_size = map->max_raw_write;
-			if (chunk_size % val_bytes)
-				chunk_size -= chunk_size % val_bytes;
-			chunk_count = total_size / chunk_size;
-			chunk_stride *= chunk_size / val_bytes;
-		}
-
-		map->lock(map->lock_arg);
-		/* Write as many bytes as possible with chunk_size */
-		for (i = 0; i < chunk_count; i++) {
-			ret = _regmap_raw_write(map,
-						reg + (i * chunk_stride),
-						wval + (i * chunk_size),
-						chunk_size);
-			if (ret)
-				break;
-		}
-
-		/* Write remaining bytes */
-		if (!ret && chunk_size * i < total_size) {
-			ret = _regmap_raw_write(map, reg + (i * chunk_stride),
-						wval + (i * chunk_size),
-						total_size - i * chunk_size);
-		}
-		map->unlock(map->lock_arg);
-
-		kfree(wval);
 	} else {
 		void *wval;
 
-		if (!val_count)
-			return -EINVAL;
-
 		wval = kmemdup(val, val_count * val_bytes, map->alloc_flags);
 		if (!wval)
 			return -ENOMEM;
@@ -2056,9 +2049,7 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(wval + i);
 
-		map->lock(map->lock_arg);
-		ret = _regmap_raw_write(map, reg, wval, val_bytes * val_count);
-		map->unlock(map->lock_arg);
+		ret = regmap_raw_write(map, reg, wval, val_bytes * val_count);
 
 		kfree(wval);
 	}
-- 
2.16.1

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

* Applied "regmap: Format data for raw write in regmap_bulk_write" to the regmap tree
  2018-02-22 12:59 [PATCH 1/5] regmap: Format data for raw write in regmap_bulk_write Charles Keepax
                   ` (3 preceding siblings ...)
  2018-02-22 12:59 ` [PATCH 5/5] regmap: Merge redundant handling in regmap_bulk_write Charles Keepax
@ 2018-02-26 11:18 ` Mark Brown
  4 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2018-02-26 11:18 UTC (permalink / raw)
  To: Charles Keepax; +Cc: Mark Brown, broonie, linux-kernel, patches, linux-kernel

The patch

   regmap: Format data for raw write in regmap_bulk_write

has been applied to the regmap tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 0812d8ffa9955251ba0077488d4408d8987ec091 Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.cirrus.com>
Date: Thu, 22 Feb 2018 12:59:10 +0000
Subject: [PATCH] regmap: Format data for raw write in regmap_bulk_write

In the case were the bulk transaction is split up into smaller chunks
data is passed directly to regmap_raw_write. However regmap_bulk_write
uses data in host endian and regmap_raw_write expects data in device
endian. As such if the host and device differ in endian the wrong data
will be written to the device. Correct this issue using a similar
approach to the single raw write case below it, duplicate the data
into a new buffer and use parse_inplace to format the data correctly.

Fixes: adaac459759d ("regmap: Introduce max_raw_read/write for regmap_bulk_read/write")
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/regmap.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index f5d653663626..8fe6e08fa41e 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2003,6 +2003,17 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		int chunk_stride = map->reg_stride;
 		size_t chunk_size = val_bytes;
 		size_t chunk_count = val_count;
+		void *wval;
+
+		if (!val_count)
+			return -EINVAL;
+
+		wval = kmemdup(val, val_count * val_bytes, map->alloc_flags);
+		if (!wval)
+			return -ENOMEM;
+
+		for (i = 0; i < val_count * val_bytes; i += val_bytes)
+			map->format.parse_inplace(wval + i);
 
 		if (!map->use_single_write) {
 			chunk_size = map->max_raw_write;
@@ -2017,7 +2028,7 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		for (i = 0; i < chunk_count; i++) {
 			ret = _regmap_raw_write(map,
 						reg + (i * chunk_stride),
-						val + (i * chunk_size),
+						wval + (i * chunk_size),
 						chunk_size);
 			if (ret)
 				break;
@@ -2026,10 +2037,12 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		/* Write remaining bytes */
 		if (!ret && chunk_size * i < total_size) {
 			ret = _regmap_raw_write(map, reg + (i * chunk_stride),
-						val + (i * chunk_size),
+						wval + (i * chunk_size),
 						total_size - i * chunk_size);
 		}
 		map->unlock(map->lock_arg);
+
+		kfree(wval);
 	} else {
 		void *wval;
 
-- 
2.16.1

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

end of thread, other threads:[~2018-02-26 11:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-22 12:59 [PATCH 1/5] regmap: Format data for raw write in regmap_bulk_write Charles Keepax
2018-02-22 12:59 ` [PATCH 2/5] regmap: Remove unnecessary printk for failed allocation Charles Keepax
2018-02-22 12:59 ` [PATCH 3/5] regmap: Move the handling for max_raw_write into regmap_raw_write Charles Keepax
2018-02-26 11:17   ` Applied "regmap: Move the handling for max_raw_write into regmap_raw_write" to the regmap tree Mark Brown
2018-02-22 12:59 ` [PATCH 4/5] regmap: Tidy up regmap_raw_write chunking code Charles Keepax
2018-02-26 11:17   ` Applied "regmap: Tidy up regmap_raw_write chunking code" to the regmap tree Mark Brown
2018-02-22 12:59 ` [PATCH 5/5] regmap: Merge redundant handling in regmap_bulk_write Charles Keepax
2018-02-26 11:17   ` Applied "regmap: Merge redundant handling in regmap_bulk_write" to the regmap tree Mark Brown
2018-02-26 11:18 ` Applied "regmap: Format data for raw write " 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).