[PATCH 1/5] regmap: Move the handling for max_raw_read into regmap_raw_read

[PATCH 1/5] regmap: Move the handling for max_raw_read into regmap_raw_read

[Charles Keepax]

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

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

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 6342eaefc218..f78ddaa9a1e8 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2542,18 +2542,45 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 
 	if (regmap_volatile_range(map, reg, val_count) || map->cache_bypass ||
 	    map->cache_type == REGCACHE_NONE) {
+		int chunk_stride = map->reg_stride;
+		size_t chunk_size = val_bytes;
+		size_t chunk_count = val_count;
+
 		if (!map->bus->read) {
 			ret = -ENOTSUPP;
 			goto out;
 		}
-		if (map->max_raw_read && map->max_raw_read < val_len) {
-			ret = -E2BIG;
-			goto out;
+
+		if (!map->use_single_read) {
+			if (map->max_raw_read)
+				chunk_size = map->max_raw_read;
+			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;
 		}
 
-		/* Physical block read if there's no cache involved */
-		ret = _regmap_raw_read(map, reg, val, val_len);
+		/* Read bytes that fit into a multiple of chunk_size */
+		for (i = 0; i < chunk_count; i++) {
+			ret = _regmap_raw_read(map,
+					       reg + (i * chunk_stride),
+					       val + (i * chunk_size),
+					       chunk_size);
+			if (ret != 0)
+				return ret;
+		}
 
+		/* Read remaining bytes */
+		if (chunk_size * i < val_len) {
+			ret = _regmap_raw_read(map,
+					       reg + (i * chunk_stride),
+					       val + (i * chunk_size),
+					       val_len - i * chunk_size);
+			if (ret != 0)
+				return ret;
+		}
 	} else {
 		/* Otherwise go word by word for the cache; should be low
 		 * cost as we expect to hit the cache.
@@ -2655,56 +2682,9 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 		return -EINVAL;
 
 	if (map->bus && map->format.parse_inplace && (vol || map->cache_type == REGCACHE_NONE)) {
-		/*
-		 * Some devices does not support bulk read, for
-		 * them we have a series of single read operations.
-		 */
-		size_t total_size = val_bytes * val_count;
-
-		if (!map->use_single_read &&
-		    (!map->max_raw_read || map->max_raw_read > total_size)) {
-			ret = regmap_raw_read(map, reg, val,
-					      val_bytes * val_count);
-			if (ret != 0)
-				return ret;
-		} else {
-			/*
-			 * Some devices do not support bulk read or do not
-			 * support large bulk reads, for them we have a series
-			 * of read operations.
-			 */
-			int chunk_stride = map->reg_stride;
-			size_t chunk_size = val_bytes;
-			size_t chunk_count = val_count;
-
-			if (!map->use_single_read) {
-				chunk_size = map->max_raw_read;
-				if (chunk_size % val_bytes)
-					chunk_size -= chunk_size % val_bytes;
-				chunk_count = total_size / chunk_size;
-				chunk_stride *= chunk_size / val_bytes;
-			}
-
-			/* Read bytes that fit into a multiple of chunk_size */
-			for (i = 0; i < chunk_count; i++) {
-				ret = regmap_raw_read(map,
-						      reg + (i * chunk_stride),
-						      val + (i * chunk_size),
-						      chunk_size);
-				if (ret != 0)
-					return ret;
-			}
-
-			/* Read remaining bytes */
-			if (chunk_size * i < total_size) {
-				ret = regmap_raw_read(map,
-						      reg + (i * chunk_stride),
-						      val + (i * chunk_size),
-						      total_size - i * chunk_size);
-				if (ret != 0)
-					return ret;
-			}
-		}
+		ret = regmap_raw_read(map, reg, val, val_bytes * val_count);
+		if (ret != 0)
+			return ret;
 
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(val + i);
-- 
2.11.0

[PATCH 2/5] regmap: Tidy up regmap_raw_read chunking code

[Charles Keepax]

Raw reads may need to be split into small chunks if max_raw_read 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 | 44 +++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index f78ddaa9a1e8..93d4218c5569 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2542,44 +2542,38 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 
 	if (regmap_volatile_range(map, reg, val_count) || map->cache_bypass ||
 	    map->cache_type == REGCACHE_NONE) {
-		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;
 
 		if (!map->bus->read) {
 			ret = -ENOTSUPP;
 			goto out;
 		}
 
-		if (!map->use_single_read) {
-			if (map->max_raw_read)
-				chunk_size = map->max_raw_read;
-			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_read)
+			chunk_regs = 1;
+		else if (map->max_raw_read && val_len > map->max_raw_read)
+			chunk_regs = map->max_raw_read / val_bytes;
 
-		/* Read bytes that fit into a multiple of chunk_size */
+		chunk_count = val_count / chunk_regs;
+		chunk_bytes = chunk_regs * val_bytes;
+
+		/* Read bytes that fit into whole chunks */
 		for (i = 0; i < chunk_count; i++) {
-			ret = _regmap_raw_read(map,
-					       reg + (i * chunk_stride),
-					       val + (i * chunk_size),
-					       chunk_size);
+			ret = _regmap_raw_read(map, reg, val, chunk_bytes);
 			if (ret != 0)
-				return ret;
+				goto out;
+
+			reg += regmap_get_offset(map, chunk_regs);
+			val += chunk_bytes;
+			val_len -= chunk_bytes;
 		}
 
 		/* Read remaining bytes */
-		if (chunk_size * i < val_len) {
-			ret = _regmap_raw_read(map,
-					       reg + (i * chunk_stride),
-					       val + (i * chunk_size),
-					       val_len - i * chunk_size);
+		if (val_len) {
+			ret = _regmap_raw_read(map, reg, val, val_len);
 			if (ret != 0)
-				return ret;
+				goto out;
 		}
 	} else {
 		/* Otherwise go word by word for the cache; should be low
-- 
2.11.0

Applied "regmap: Tidy up regmap_raw_read chunking code" to the regmap tree

[Mark Brown]

The patch

   regmap: Tidy up regmap_raw_read 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 1b079ca2c2e9a4652051bc4b62a5ef83d59d86bb Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.cirrus.com>
Date: Thu, 15 Feb 2018 17:52:17 +0000
Subject: [PATCH] regmap: Tidy up regmap_raw_read chunking code

Raw reads may need to be split into small chunks if max_raw_read 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 | 44 +++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 0cc7387008c9..ff30a9157de5 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2542,44 +2542,38 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 
 	if (regmap_volatile_range(map, reg, val_count) || map->cache_bypass ||
 	    map->cache_type == REGCACHE_NONE) {
-		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;
 
 		if (!map->bus->read) {
 			ret = -ENOTSUPP;
 			goto out;
 		}
 
-		if (!map->use_single_read) {
-			if (map->max_raw_read)
-				chunk_size = map->max_raw_read;
-			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_read)
+			chunk_regs = 1;
+		else if (map->max_raw_read && val_len > map->max_raw_read)
+			chunk_regs = map->max_raw_read / val_bytes;
 
-		/* Read bytes that fit into a multiple of chunk_size */
+		chunk_count = val_count / chunk_regs;
+		chunk_bytes = chunk_regs * val_bytes;
+
+		/* Read bytes that fit into whole chunks */
 		for (i = 0; i < chunk_count; i++) {
-			ret = _regmap_raw_read(map,
-					       reg + (i * chunk_stride),
-					       val + (i * chunk_size),
-					       chunk_size);
+			ret = _regmap_raw_read(map, reg, val, chunk_bytes);
 			if (ret != 0)
-				return ret;
+				goto out;
+
+			reg += regmap_get_offset(map, chunk_regs);
+			val += chunk_bytes;
+			val_len -= chunk_bytes;
 		}
 
 		/* Read remaining bytes */
-		if (chunk_size * i < val_len) {
-			ret = _regmap_raw_read(map,
-					       reg + (i * chunk_stride),
-					       val + (i * chunk_size),
-					       val_len - i * chunk_size);
+		if (val_len) {
+			ret = _regmap_raw_read(map, reg, val, val_len);
 			if (ret != 0)
-				return ret;
+				goto out;
 		}
 	} else {
 		/* Otherwise go word by word for the cache; should be low
-- 
2.16.1

[PATCH 3/5] regmap: Use _regmap_read in regmap_bulk_read

[Charles Keepax]

Bulk reads may potentially read a lot of registers and regmap_read will
take and release the regmap lock for each register. Avoid bouncing
the lock so frequently by holding the lock locally and calling
_regmap_read instead. This also has the nice side-effect that all the
reads will be done atomically so no other threads can sneak a write in
during the regmap_bulk_read.

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

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 93d4218c5569..872be065e3fe 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2674,6 +2674,8 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
+	if (val_count == 0)
+		return -EINVAL;
 
 	if (map->bus && map->format.parse_inplace && (vol || map->cache_type == REGCACHE_NONE)) {
 		ret = regmap_raw_read(map, reg, val, val_bytes * val_count);
@@ -2690,13 +2692,15 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 		u16 *u16 = val;
 		u8 *u8 = val;
 
+		map->lock(map->lock_arg);
+
 		for (i = 0; i < val_count; i++) {
 			unsigned int ival;
 
-			ret = regmap_read(map, reg + regmap_get_offset(map, i),
-					  &ival);
+			ret = _regmap_read(map, reg + regmap_get_offset(map, i),
+					   &ival);
 			if (ret != 0)
-				return ret;
+				goto out;
 
 			switch (map->format.val_bytes) {
 #ifdef CONFIG_64BIT
@@ -2714,12 +2718,16 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 				u8[i] = ival;
 				break;
 			default:
-				return -EINVAL;
+				ret = -EINVAL;
+				goto out;
 			}
 		}
+
+out:
+		map->unlock(map->lock_arg);
 	}
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_bulk_read);
 
-- 
2.11.0

Applied "regmap: Use _regmap_read in regmap_bulk_read" to the regmap tree

[Mark Brown]

The patch

   regmap: Use _regmap_read in regmap_bulk_read

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 186ba2eec275a5e4ee09d4b6a77c619e46fab9fd Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.cirrus.com>
Date: Thu, 15 Feb 2018 17:52:18 +0000
Subject: [PATCH] regmap: Use _regmap_read in regmap_bulk_read

Bulk reads may potentially read a lot of registers and regmap_read will
take and release the regmap lock for each register. Avoid bouncing
the lock so frequently by holding the lock locally and calling
_regmap_read instead. This also has the nice side-effect that all the
reads will be done atomically so no other threads can sneak a write in
during the regmap_bulk_read.

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

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index ff30a9157de5..258a40e2a1d3 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2674,6 +2674,8 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
+	if (val_count == 0)
+		return -EINVAL;
 
 	if (map->bus && map->format.parse_inplace && (vol || map->cache_type == REGCACHE_NONE)) {
 		ret = regmap_raw_read(map, reg, val, val_bytes * val_count);
@@ -2690,13 +2692,15 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 		u16 *u16 = val;
 		u8 *u8 = val;
 
+		map->lock(map->lock_arg);
+
 		for (i = 0; i < val_count; i++) {
 			unsigned int ival;
 
-			ret = regmap_read(map, reg + regmap_get_offset(map, i),
-					  &ival);
+			ret = _regmap_read(map, reg + regmap_get_offset(map, i),
+					   &ival);
 			if (ret != 0)
-				return ret;
+				goto out;
 
 			switch (map->format.val_bytes) {
 #ifdef CONFIG_64BIT
@@ -2714,12 +2718,16 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 				u8[i] = ival;
 				break;
 			default:
-				return -EINVAL;
+				ret = -EINVAL;
+				goto out;
 			}
 		}
+
+out:
+		map->unlock(map->lock_arg);
 	}
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_bulk_read);
 
-- 
2.16.1

[PATCH 4/5] iio: accel: bcm150: Remove handling for regmap raw_read_max

[Charles Keepax]

The regmap core now handles splitting up transactions according to
max_raw_read, so this code is no longer required in client drivers.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/iio/accel/bmc150-accel-core.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 870f92ef61c2..3d2e6b501bf9 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -838,29 +838,12 @@ static int bmc150_accel_fifo_transfer(struct bmc150_accel_data *data,
 	int sample_length = 3 * 2;
 	int ret;
 	int total_length = samples * sample_length;
-	int i;
-	size_t step = regmap_get_raw_read_max(data->regmap);
-
-	if (!step || step > total_length)
-		step = total_length;
-	else if (step < total_length)
-		step = sample_length;
-
-	/*
-	 * Seems we have a bus with size limitation so we have to execute
-	 * multiple reads
-	 */
-	for (i = 0; i < total_length; i += step) {
-		ret = regmap_raw_read(data->regmap, BMC150_ACCEL_REG_FIFO_DATA,
-				      &buffer[i], step);
-		if (ret)
-			break;
-	}
 
+	ret = regmap_raw_read(data->regmap, BMC150_ACCEL_REG_FIFO_DATA,
+			      buffer, total_length);
 	if (ret)
 		dev_err(dev,
-			"Error transferring data from fifo in single steps of %zu\n",
-			step);
+			"Error transferring data from fifo: %d\n", ret);
 
 	return ret;
 }
-- 
2.11.0

Re: [PATCH 4/5] iio: accel: bcm150: Remove handling for regmap raw_read_max

[Jonathan Cameron]

On Thu, 15 Feb 2018 17:52:19 +0000
Charles Keepax <ckeepax@opensource.cirrus.com> wrote:

> The regmap core now handles splitting up transactions according to
> max_raw_read, so this code is no longer required in client drivers.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Hi Charles,

This looks fine to me.  I'll need to wait for Mark's patches to work
their way through to mainline before I can apply this however.

Please give me a poke when that has happened if it looks like I have
forgotten it.

Thanks,

Jonathan

> ---
>  drivers/iio/accel/bmc150-accel-core.c | 23 +++--------------------
>  1 file changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> index 870f92ef61c2..3d2e6b501bf9 100644
> --- a/drivers/iio/accel/bmc150-accel-core.c
> +++ b/drivers/iio/accel/bmc150-accel-core.c
> @@ -838,29 +838,12 @@ static int bmc150_accel_fifo_transfer(struct bmc150_accel_data *data,
>  	int sample_length = 3 * 2;
>  	int ret;
>  	int total_length = samples * sample_length;
> -	int i;
> -	size_t step = regmap_get_raw_read_max(data->regmap);
> -
> -	if (!step || step > total_length)
> -		step = total_length;
> -	else if (step < total_length)
> -		step = sample_length;
> -
> -	/*
> -	 * Seems we have a bus with size limitation so we have to execute
> -	 * multiple reads
> -	 */
> -	for (i = 0; i < total_length; i += step) {
> -		ret = regmap_raw_read(data->regmap, BMC150_ACCEL_REG_FIFO_DATA,
> -				      &buffer[i], step);
> -		if (ret)
> -			break;
> -	}
>  
> +	ret = regmap_raw_read(data->regmap, BMC150_ACCEL_REG_FIFO_DATA,
> +			      buffer, total_length);
>  	if (ret)
>  		dev_err(dev,
> -			"Error transferring data from fifo in single steps of %zu\n",
> -			step);
> +			"Error transferring data from fifo: %d\n", ret);
>  
>  	return ret;
>  }

Re: [PATCH 4/5] iio: accel: bcm150: Remove handling for regmap raw_read_max

[Charles Keepax]

On Sat, Feb 17, 2018 at 02:09:35PM +0000, Jonathan Cameron wrote:
> On Thu, 15 Feb 2018 17:52:19 +0000
> Charles Keepax <ckeepax@opensource.cirrus.com> wrote:
> 
> > The regmap core now handles splitting up transactions according to
> > max_raw_read, so this code is no longer required in client drivers.
> > 
> > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Hi Charles,
> 
> This looks fine to me.  I'll need to wait for Mark's patches to work
> their way through to mainline before I can apply this however.
> 
> Please give me a poke when that has happened if it looks like I have
> forgotten it.
> 

Yeah can do.

Thanks,
Charles

[PATCH 5/5] regmap: Remove regmap_get_raw_read_max

[Charles Keepax]

Since the handling for max_raw_read is now inside regmap_raw_read there
should be no need for client drivers to query as max_raw_read.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/base/regmap/regmap.c | 11 -----------
 include/linux/regmap.h       |  1 -
 2 files changed, 12 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 872be065e3fe..a7ee885fce89 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1641,17 +1641,6 @@ bool regmap_can_raw_write(struct regmap *map)
 EXPORT_SYMBOL_GPL(regmap_can_raw_write);
 
 /**
- * regmap_get_raw_read_max - Get the maximum size we can read
- *
- * @map: Map to check.
- */
-size_t regmap_get_raw_read_max(struct regmap *map)
-{
-	return map->max_raw_read;
-}
-EXPORT_SYMBOL_GPL(regmap_get_raw_read_max);
-
-/**
  * regmap_get_raw_write_max - Get the maximum size we can read
  *
  * @map: Map to check.
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 6a3aeba40e9e..0382fb787c10 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -936,7 +936,6 @@ int regmap_get_max_register(struct regmap *map);
 int regmap_get_reg_stride(struct regmap *map);
 int regmap_async_complete(struct regmap *map);
 bool regmap_can_raw_write(struct regmap *map);
-size_t regmap_get_raw_read_max(struct regmap *map);
 size_t regmap_get_raw_write_max(struct regmap *map);
 
 int regcache_sync(struct regmap *map);
-- 
2.11.0

Re: [PATCH 5/5] regmap: Remove regmap_get_raw_read_max

[Mark Brown]

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

On Thu, Feb 15, 2018 at 05:52:20PM +0000, Charles Keepax wrote:
> Since the handling for max_raw_read is now inside regmap_raw_read there
> should be no need for client drivers to query as max_raw_read.

No, they might need to know if things are going to be combined into a
single transaction - some devices have weird sensitivities about bulk
I/O and so the driver may need to know exactly what's going on when it
interacts with the device.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 5/5] regmap: Remove regmap_get_raw_read_max

[Charles Keepax]

On Fri, Feb 16, 2018 at 12:03:11PM +0000, Mark Brown wrote:
> On Thu, Feb 15, 2018 at 05:52:20PM +0000, Charles Keepax wrote:
> > Since the handling for max_raw_read is now inside regmap_raw_read there
> > should be no need for client drivers to query as max_raw_read.
> 
> No, they might need to know if things are going to be combined into a
> single transaction - some devices have weird sensitivities about bulk
> I/O and so the driver may need to know exactly what's going on when it
> interacts with the device.

True, no objections from me for dropping this one from the chain.

Thanks,
Charles

Applied "regmap: Move the handling for max_raw_read into regmap_raw_read" to the regmap tree

[Mark Brown]

The patch

   regmap: Move the handling for max_raw_read into regmap_raw_read

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 0645ba4331c2b02ba9907b1591ba722535890e9f Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.cirrus.com>
Date: Thu, 15 Feb 2018 17:52:16 +0000
Subject: [PATCH] regmap: Move the handling for max_raw_read into
 regmap_raw_read

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

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

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index f075c05859b0..0cc7387008c9 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2542,18 +2542,45 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 
 	if (regmap_volatile_range(map, reg, val_count) || map->cache_bypass ||
 	    map->cache_type == REGCACHE_NONE) {
+		int chunk_stride = map->reg_stride;
+		size_t chunk_size = val_bytes;
+		size_t chunk_count = val_count;
+
 		if (!map->bus->read) {
 			ret = -ENOTSUPP;
 			goto out;
 		}
-		if (map->max_raw_read && map->max_raw_read < val_len) {
-			ret = -E2BIG;
-			goto out;
+
+		if (!map->use_single_read) {
+			if (map->max_raw_read)
+				chunk_size = map->max_raw_read;
+			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;
 		}
 
-		/* Physical block read if there's no cache involved */
-		ret = _regmap_raw_read(map, reg, val, val_len);
+		/* Read bytes that fit into a multiple of chunk_size */
+		for (i = 0; i < chunk_count; i++) {
+			ret = _regmap_raw_read(map,
+					       reg + (i * chunk_stride),
+					       val + (i * chunk_size),
+					       chunk_size);
+			if (ret != 0)
+				return ret;
+		}
 
+		/* Read remaining bytes */
+		if (chunk_size * i < val_len) {
+			ret = _regmap_raw_read(map,
+					       reg + (i * chunk_stride),
+					       val + (i * chunk_size),
+					       val_len - i * chunk_size);
+			if (ret != 0)
+				return ret;
+		}
 	} else {
 		/* Otherwise go word by word for the cache; should be low
 		 * cost as we expect to hit the cache.
@@ -2655,56 +2682,9 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 		return -EINVAL;
 
 	if (map->bus && map->format.parse_inplace && (vol || map->cache_type == REGCACHE_NONE)) {
-		/*
-		 * Some devices does not support bulk read, for
-		 * them we have a series of single read operations.
-		 */
-		size_t total_size = val_bytes * val_count;
-
-		if (!map->use_single_read &&
-		    (!map->max_raw_read || map->max_raw_read > total_size)) {
-			ret = regmap_raw_read(map, reg, val,
-					      val_bytes * val_count);
-			if (ret != 0)
-				return ret;
-		} else {
-			/*
-			 * Some devices do not support bulk read or do not
-			 * support large bulk reads, for them we have a series
-			 * of read operations.
-			 */
-			int chunk_stride = map->reg_stride;
-			size_t chunk_size = val_bytes;
-			size_t chunk_count = val_count;
-
-			if (!map->use_single_read) {
-				chunk_size = map->max_raw_read;
-				if (chunk_size % val_bytes)
-					chunk_size -= chunk_size % val_bytes;
-				chunk_count = total_size / chunk_size;
-				chunk_stride *= chunk_size / val_bytes;
-			}
-
-			/* Read bytes that fit into a multiple of chunk_size */
-			for (i = 0; i < chunk_count; i++) {
-				ret = regmap_raw_read(map,
-						      reg + (i * chunk_stride),
-						      val + (i * chunk_size),
-						      chunk_size);
-				if (ret != 0)
-					return ret;
-			}
-
-			/* Read remaining bytes */
-			if (chunk_size * i < total_size) {
-				ret = regmap_raw_read(map,
-						      reg + (i * chunk_stride),
-						      val + (i * chunk_size),
-						      total_size - i * chunk_size);
-				if (ret != 0)
-					return ret;
-			}
-		}
+		ret = regmap_raw_read(map, reg, val, val_bytes * val_count);
+		if (ret != 0)
+			return ret;
 
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(val + i);
-- 
2.16.1