[RFC v2] regmap: Add regmap_pipe_read API

[Crestez Dan Leonard <leonard.crestez@xxxxxxxxx>]

The regmap API usually assumes that bulk read operations will read a
range of registers but some I2C/SPI devices have certain registers for
which a such a read operation will return data from an internal FIFO or
some other kind of buffer. Add an explicit API to support bulk read with
"output pipe" rather than range semantics.

Some linux drivers use regmap_bulk_read or regmap_raw_read for such
registers, for example mpu6050 or bmi150 from IIO. This only happens to
work because when caching is disabled a single short regmap read op will
map to a single bus read op and behave as intended. This breaks if
caching is enabled and reg+1 happens to be a cacheable register.

Without regmap support refactoring a driver to enable regmap caching
requires separate i2c and spi paths. This is exactly what regmap is
supposed to help avoid.

Suggested-by: Jonathan Cameron <jic23@xxxxxxxxxx>
Signed-off-by: Crestez Dan Leonard <leonard.crestez@xxxxxxxxx>

---
Changes since V1:
* Improve comments. I think "pipe" is the correct word here.
* Rely on void* pointer arithmetic instead of casting to u8*

The SPMI implementations of regmap_bus have read functions which
auto-increment internally and this will not work correctly with
regmap_pipe_read. I think the correct fix would be for regmap-spmi to
only implement reg_read. This can be considered an acceptable limitation
because in order to run into this somebody would have to write new code
to call this new API with an SPMI device.

I attempted to also support this when the underlying bus only supports
reg_read (for example an adapter with only I2C_FUNC_SMBUS_BYTE_DATA) but
it's not clear how to do this on top of _regmap_read. Apparently this
returns the value as an "unsigned int" which needs to be formatted
according to val_bytes. Unfortunately map->format.format_val is not
always available. Presumably the code dealing with this from regmap_bulk_read
should be refactored into a separate function?

It's not clear why regmap doesn't just always initialize format_val.

 drivers/base/regmap/regmap.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h       |  9 +++++++
 2 files changed, 73 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index df2d2ef..55c3090 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2408,6 +2408,70 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 EXPORT_SYMBOL_GPL(regmap_raw_read);
 
 /**
+ * regmap_pipe_read(): Read data from a register with pipe semantics
+ *
+ * @map: Register map to read from
+ * @reg: Register to read from
+ * @val: Pointer to data buffer
+ * @val_len: Length of output buffer in bytes.
+ *
+ * The regmap API usually assumes that bulk bus read operations will read a
+ * range of registers. Some devices have certain registers for which a read
+ * operation will read a block of data from an internal buffer.
+ *
+ * The target register must be volatile but registers after it can be
+ * completely unrelated cacheable registers.
+ *
+ * This will attempt multiple reads as required to read val_len bytes.
+ *
+ * This function is not supported over SPMI.
+ *
+ * A value of zero will be returned on success, a negative errno will be
+ * returned in error cases.
+ */
+int regmap_pipe_read(struct regmap *map, unsigned int reg,
+		     void *val, size_t val_len)
+{
+	size_t read_len;
+	int ret;
+
+	if (!map->bus)
+		return -EINVAL;
+	if (!map->bus->read)
+		return -ENOTSUPP;
+	if (val_len % map->format.val_bytes)
+		return -EINVAL;
+	if (!IS_ALIGNED(reg, map->reg_stride))
+		return -EINVAL;
+	if (val_len == 0)
+		return -EINVAL;
+
+	map->lock(map->lock_arg);
+
+	if (!regmap_volatile(map, reg)) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	while (val_len) {
+		if (map->max_raw_read && map->max_raw_read < val_len)
+			read_len = map->max_raw_read;
+		else
+			read_len = val_len;
+		ret = _regmap_raw_read(map, reg, val, read_len);
+		if (ret)
+			goto out_unlock;
+		val += read_len;
+		val_len -= read_len;
+	}
+
+out_unlock:
+	map->unlock(map->lock_arg);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(regmap_pipe_read);
+
+/**
  * regmap_field_read(): Read a value to a single register field
  *
  * @field: Register field to read from
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 3dc08ce..18ee90e 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -719,6 +719,8 @@ int regmap_raw_write_async(struct regmap *map, unsigned int reg,
 int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val);
 int regmap_raw_read(struct regmap *map, unsigned int reg,
 		    void *val, size_t val_len);
+int regmap_pipe_read(struct regmap *map, unsigned int reg,
+		     void *val, size_t val_len);
 int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 		     size_t val_count);
 int regmap_update_bits_base(struct regmap *map, unsigned int reg,
@@ -955,6 +957,13 @@ static inline int regmap_raw_read(struct regmap *map, unsigned int reg,
 	return -EINVAL;
 }
 
+static inline int regmap_pipe_read(struct regmap *map, unsigned int reg,
+				   void *val, size_t val_len)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
 static inline int regmap_bulk_read(struct regmap *map, unsigned int reg,
 				   void *val, size_t val_count)
 {
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html