[PATCH v3 0/4] regmap: i2c block support

[PATCH v3 0/4] regmap: i2c block support

[Markus Pargmann]

Hi,

This series adds support for i2c block read/writes. To support the maximum 32
byte read/write operations, the regmap core is extended by max_raw_read and
max_raw_write. bulk operations are splitted depending of the size of
max_raw_read/write.

The last patch needs testing before it can be applied.

Best Regards,

Markus


Changes in v3:
- Rebased onto latest version of "regmap: fixes" series

Changes in v2:
- max_raw_io splitted into max_raw_read/write
- Use E2BIG as error value in the block read/write functions

Markus Pargmann (4):
  regmap: Introduce max_raw_read/write for regmap_bulk_read/write
  regmap: regmap max_raw_read/write getter functions
  regmap: Add raw_write/read checks for max_raw_write/read sizes
  regmap-i2c: Add smbus i2c block support

 drivers/base/regmap/internal.h   |   4 ++
 drivers/base/regmap/regmap-i2c.c |  49 +++++++++++++++++
 drivers/base/regmap/regmap.c     | 113 +++++++++++++++++++++++++++++++++------
 include/linux/regmap.h           |   6 +++
 4 files changed, 157 insertions(+), 15 deletions(-)

-- 
2.5.0

[PATCH v3 1/4] regmap: Introduce max_raw_read/write for regmap_bulk_read/write

[Markus Pargmann]

There are some buses which have a limit on the maximum number of bytes
that can be send/received. An example for this is
I2C_FUNC_SMBUS_I2C_BLOCK which does not support any reads/writes of more
than 32 bytes. The regmap_bulk operations should still be able to
utilize the full 32 bytes in this case.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/base/regmap/internal.h |  4 ++
 drivers/base/regmap/regmap.c   | 85 ++++++++++++++++++++++++++++++++++--------
 include/linux/regmap.h         |  4 ++
 3 files changed, 78 insertions(+), 15 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index d744ae3926dd..fc554e357c5d 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -146,6 +146,10 @@ struct regmap {
 	/* if set, the device supports multi write mode */
 	bool can_multi_write;
 
+	/* if set, raw reads/writes are limited to this size */
+	size_t max_raw_read;
+	size_t max_raw_write;
+
 	struct rb_root range_tree;
 	void *selector_work_buf;	/* Scratch buffer used for selector */
 };
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 47fe0dfbbc46..c4ebc6bce46c 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -576,6 +576,8 @@ struct regmap *regmap_init(struct device *dev,
 	map->use_single_read = config->use_single_rw || !bus || !bus->read;
 	map->use_single_write = config->use_single_rw || !bus || !bus->write;
 	map->can_multi_write = config->can_multi_write && bus && bus->write;
+	map->max_raw_read = bus->max_raw_read;
+	map->max_raw_write = bus->max_raw_write;
 	map->dev = dev;
 	map->bus = bus;
 	map->bus_context = bus_context;
@@ -1671,6 +1673,7 @@ 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 (map->bus && !map->format.parse_inplace)
 		return -EINVAL;
@@ -1719,16 +1722,37 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		}
 out:
 		map->unlock(map->lock_arg);
-	} else if (map->use_single_write) {
+	} 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;
+
+		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);
-		for (i = 0; i < val_count; i++) {
+		/* Write as many bytes as possible with chunk_size */
+		for (i = 0; i < chunk_count; i++) {
 			ret = _regmap_raw_write(map,
-						reg + (i * map->reg_stride),
-						val + (i * val_bytes),
-						val_bytes);
+						reg + (i * chunk_stride),
+						val + (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),
+						val + (i * chunk_size),
+						total_size - i * chunk_size);
+		}
 		map->unlock(map->lock_arg);
 	} else {
 		void *wval;
@@ -2316,20 +2340,51 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 		 * Some devices does not support bulk read, for
 		 * them we have a series of single read operations.
 		 */
-		if (map->use_single_read) {
-			for (i = 0; i < val_count; i++) {
-				ret = regmap_raw_read(map,
-						reg + (i * map->reg_stride),
-						val + (i * val_bytes),
-						val_bytes);
-				if (ret != 0)
-					return ret;
-			}
-		} else {
+		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;
+			}
 		}
 
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 73fc34d0c4c2..327b8f291d3f 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -311,6 +311,8 @@ typedef void (*regmap_hw_free_context)(void *context);
  * @val_format_endian_default: Default endianness for formatted register
  *     values. Used when the regmap_config specifies DEFAULT. If this is
  *     DEFAULT, BIG is assumed.
+ * @max_raw_read: Max raw read size that can be used on the bus.
+ * @max_raw_write: Max raw write size that can be used on the bus.
  */
 struct regmap_bus {
 	bool fast_io;
@@ -325,6 +327,8 @@ struct regmap_bus {
 	u8 read_flag_mask;
 	enum regmap_endian reg_format_endian_default;
 	enum regmap_endian val_format_endian_default;
+	size_t max_raw_read;
+	size_t max_raw_write;
 };
 
 struct regmap *regmap_init(struct device *dev,
-- 
2.5.0

[PATCH v3 2/4] regmap: regmap max_raw_read/write getter functions

[Markus Pargmann]

Add functions to access the maximum size we can read/write using
regmap_raw_read/write().

This helps drivers that need to know how much they can write with the
raw functions without problems. There are some devices (e.g. bmc150)
that have fifos as registers which need to be read in specific chunks
otherwise samples are dropped.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/base/regmap/regmap.c | 22 ++++++++++++++++++++++
 include/linux/regmap.h       |  2 ++
 2 files changed, 24 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index c4ebc6bce46c..b4d12fe15f21 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1390,6 +1390,28 @@ 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.
+ */
+size_t regmap_get_raw_write_max(struct regmap *map)
+{
+	return map->max_raw_write;
+}
+EXPORT_SYMBOL_GPL(regmap_get_raw_write_max);
+
 static int _regmap_bus_formatted_write(void *context, unsigned int reg,
 				       unsigned int val)
 {
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 327b8f291d3f..6724d0e3819e 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -444,6 +444,8 @@ 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);
 int regcache_sync_region(struct regmap *map, unsigned int min,
-- 
2.5.0

[PATCH v3 3/4] regmap: Add raw_write/read checks for max_raw_write/read sizes

[Markus Pargmann]

Check in regmap_raw_read() and regmap_raw_write() for correct maximum
sizes of the operations. Return -E2BIG if this size is not supported
because it is too big.

Also this patch causes an uninitialized variable warning so it
initializes ret (although not necessary).

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/base/regmap/regmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index b4d12fe15f21..157fe79a7d06 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1581,6 +1581,8 @@ 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);
 
@@ -2253,6 +2255,10 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 			ret = -ENOTSUPP;
 			goto out;
 		}
+		if (map->max_raw_read && map->max_raw_read < val_len) {
+			ret = -E2BIG;
+			goto out;
+		}
 
 		/* Physical block read if there's no cache involved */
 		ret = _regmap_raw_read(map, reg, val, val_len);
-- 
2.5.0

[PATCH v3 4/4] regmap-i2c: Add smbus i2c block support

[Markus Pargmann]

This allows to read/write up to 32 bytes of data and is to be prefered
if supported before the register read/write smbus support.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/base/regmap/regmap-i2c.c | 49 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index 4b76e33110a2..ddb9b0efb724 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -209,11 +209,60 @@ static struct regmap_bus regmap_i2c = {
 	.val_format_endian_default = REGMAP_ENDIAN_BIG,
 };
 
+static int regmap_i2c_smbus_i2c_write(void *context, const void *data,
+				      size_t count)
+{
+	struct device *dev = context;
+	struct i2c_client *i2c = to_i2c_client(dev);
+
+	if (count < 1)
+		return -EINVAL;
+	if (count >= I2C_SMBUS_BLOCK_MAX)
+		return -E2BIG;
+
+	--count;
+	return i2c_smbus_write_i2c_block_data(i2c, ((u8 *)data)[0], count,
+					      ((u8 *)data + 1));
+}
+
+static int regmap_i2c_smbus_i2c_read(void *context, const void *reg,
+				     size_t reg_size, void *val,
+				     size_t val_size)
+{
+	struct device *dev = context;
+	struct i2c_client *i2c = to_i2c_client(dev);
+	int ret;
+
+	if (reg_size != 1 || val_size < 1)
+		return -EINVAL;
+	if (val_size >= I2C_SMBUS_BLOCK_MAX)
+		return -E2BIG;
+
+	ret = i2c_smbus_read_i2c_block_data(i2c, ((u8 *)reg)[0], val_size, val);
+	if (ret == val_size)
+		return 0;
+	else if (ret < 0)
+		return ret;
+	else
+		return -EIO;
+}
+
+static struct regmap_bus regmap_i2c_smbus_i2c_block = {
+	.write = regmap_i2c_smbus_i2c_write,
+	.read = regmap_i2c_smbus_i2c_read,
+	.max_raw_read = I2C_SMBUS_BLOCK_MAX,
+	.max_raw_write = I2C_SMBUS_BLOCK_MAX,
+};
+
 static const struct regmap_bus *regmap_get_i2c_bus(struct i2c_client *i2c,
 					const struct regmap_config *config)
 {
 	if (i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C))
 		return &regmap_i2c;
+	else if (config->reg_bits == 8 &&
+		 i2c_check_functionality(i2c->adapter,
+					 I2C_FUNC_SMBUS_I2C_BLOCK))
+		return &regmap_i2c_smbus_i2c_block;
 	else if (config->val_bits == 16 && config->reg_bits == 8 &&
 		 i2c_check_functionality(i2c->adapter,
 					 I2C_FUNC_SMBUS_WORD_DATA))
-- 
2.5.0

[PATCH v3 4/4] regmap-i2c: Add smbus i2c block support

[Nikolaus Voss]

Hi Markus,

On Thu, 27 Aug 2015, Markus Pargmann wrote:
> This allows to read/write up to 32 bytes of data and is to be prefered
> if supported before the register read/write smbus support.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
> drivers/base/regmap/regmap-i2c.c | 49 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
> index 4b76e33110a2..ddb9b0efb724 100644
> --- a/drivers/base/regmap/regmap-i2c.c
> +++ b/drivers/base/regmap/regmap-i2c.c
> @@ -209,11 +209,60 @@ static struct regmap_bus regmap_i2c = {
> 	.val_format_endian_default = REGMAP_ENDIAN_BIG,
> };
>
> +static int regmap_i2c_smbus_i2c_write(void *context, const void *data,
> +				      size_t count)
> +{
> +	struct device *dev = context;
> +	struct i2c_client *i2c = to_i2c_client(dev);
> +
> +	if (count < 1)
> +		return -EINVAL;
> +	if (count >= I2C_SMBUS_BLOCK_MAX)
> +		return -E2BIG;
> +
> +	--count;
> +	return i2c_smbus_write_i2c_block_data(i2c, ((u8 *)data)[0], count,
> +					      ((u8 *)data + 1));
> +}
> +
> +static int regmap_i2c_smbus_i2c_read(void *context, const void *reg,
> +				     size_t reg_size, void *val,
> +				     size_t val_size)
> +{
> +	struct device *dev = context;
> +	struct i2c_client *i2c = to_i2c_client(dev);
> +	int ret;
> +
> +	if (reg_size != 1 || val_size < 1)
> +		return -EINVAL;
> +	if (val_size >= I2C_SMBUS_BLOCK_MAX)
> +		return -E2BIG;
> +
> +	ret = i2c_smbus_read_i2c_block_data(i2c, ((u8 *)reg)[0], val_size, val);
> +	if (ret == val_size)
> +		return 0;
> +	else if (ret < 0)
> +		return ret;
> +	else
> +		return -EIO;
> +}
> +
> +static struct regmap_bus regmap_i2c_smbus_i2c_block = {
> +	.write = regmap_i2c_smbus_i2c_write,
> +	.read = regmap_i2c_smbus_i2c_read,
> +	.max_raw_read = I2C_SMBUS_BLOCK_MAX,
> +	.max_raw_write = I2C_SMBUS_BLOCK_MAX,
> +};
> +
> static const struct regmap_bus *regmap_get_i2c_bus(struct i2c_client *i2c,
> 					const struct regmap_config *config)
> {
> 	if (i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C))
> 		return &regmap_i2c;
> +	else if (config->reg_bits == 8 &&
> +		 i2c_check_functionality(i2c->adapter,
> +					 I2C_FUNC_SMBUS_I2C_BLOCK))
> +		return &regmap_i2c_smbus_i2c_block;
> 	else if (config->val_bits == 16 && config->reg_bits == 8 &&
> 		 i2c_check_functionality(i2c->adapter,
> 					 I2C_FUNC_SMBUS_WORD_DATA))
>

using i2c_smbus_read/write_i2c_block_data() instead of
read/write_word_data() or i2c_transfer() actually changes what is
transferred on the bus. SMBus block transfers have a different
register model, they additionally read or write the byte count after
transmitting the register/command code, see SMBus spec:

+---+-------------+---+--+-------------+--+---------------+--+------------+----+
| S | Slave Addr  | Wr| A| Command Code| A| Byte Count = N| A| Data Byte 1| A  |
+---+-------------+---+--+-------------+--+---------------+--+------------+----+

So a driver using regmap must implement different transfers depending on
the i2c adapter capabilities, is this intentionally done so?

Specifically, there is no way of getting rid of the additional byte count
field if the adapter doesn't support I2C_FUNC_I2C although it would be
technically possible.

I would prefer regmap to always use the same register model (i.e. without
the byte count field), can you explain the rationale for this?

Niko
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3493 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180906/7b7f3661/attachment-0001.p7s>

[PATCH v3 0/4] regmap: i2c block support

[Mark Brown]

On Thu, Aug 27, 2015 at 08:44:28AM +0200, Markus Pargmann wrote:

> Changes in v3:
> - Rebased onto latest version of "regmap: fixes" series

New code not intended as bug fixes needs to apply against the latest
development code, not a fixes branch (and especially not some out of
tree patch series which may have ended up in multiple branches or
something).  git says it doesn't know what you generated this
against...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150828/060ceb9d/attachment.sig>

[PATCH v3 0/4] regmap: i2c block support

[Mark Brown]

On Thu, Aug 27, 2015 at 08:44:28AM +0200, Markus Pargmann wrote:
> Hi,
> 
> This series adds support for i2c block read/writes. To support the maximum 32
> byte read/write operations, the regmap core is extended by max_raw_read and
> max_raw_write. bulk operations are splitted depending of the size of
> max_raw_read/write.

...and reverted because it broke the build, probably because it was not
a patch against current development trees:

  http://kernelci.org/build/broonie-regmap/kernel/v4.2-rc8-34-gd535ae53edec/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150829/ab2985c7/attachment.sig>