[RFC PATCH 0/3] regmap: Implement regmap_multi_reg_read()

[RFC PATCH 0/3] regmap: Implement regmap_multi_reg_read()

[Guenter Roeck]

regmap_multi_reg_read() is similar to regmap_bilk_read() but reads from
an array of non-sequential registers. It is helpful if multiple non-
sequential registers need to be read in a single operation which would
otherwise have to be mutex protected.

The name of the new function was chosen to match the existing function
regmap_multi_reg_write().

The first patch of the series introduces the new function. The following
two patches provide examples for its use.

----------------------------------------------------------------
Guenter Roeck (3):
      regmap: Implement regmap_multi_reg_read()
      hwmon: (adt7470) Use multi-byte regmap operations
      hwmon: (tmp401) Use multi-byte regmap operations

 drivers/base/regmap/regmap.c | 103 ++++++++++++++++++++++++++++---------------
 drivers/hwmon/adt7470.c      |  22 +++------
 drivers/hwmon/tmp401.c       |  19 +++-----
 include/linux/regmap.h       |   2 +
 4 files changed, 84 insertions(+), 62 deletions(-)

[RFC PATCH 1/3] regmap: Implement regmap_multi_reg_read()

[Guenter Roeck]

regmap_multi_reg_read() is similar to regmap_bilk_read() but reads from
an array of non-sequential registers.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/base/regmap/regmap.c | 103 +++++++++++++++++++++++------------
 include/linux/regmap.h       |   2 +
 2 files changed, 70 insertions(+), 35 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 0a34dd3c4f38..dff6c515305a 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -3101,8 +3101,53 @@ int regmap_fields_read(struct regmap_field *field, unsigned int id,
 }
 EXPORT_SYMBOL_GPL(regmap_fields_read);
 
+static int _regmap_bulk_read(struct regmap *map, unsigned int reg,
+			     unsigned int *regs, void *val, size_t val_count)
+{
+	u32 *u32 = val;
+	u16 *u16 = val;
+	u8 *u8 = val;
+	int ret, i;
+
+	map->lock(map->lock_arg);
+
+	for (i = 0; i < val_count; i++) {
+		unsigned int ival;
+
+		if (regs) {
+			if (!IS_ALIGNED(regs[i], map->reg_stride)) {
+				ret = -EINVAL;
+				goto out;
+			}
+			ret = _regmap_read(map, regs[i], &ival);
+		} else {
+			ret = _regmap_read(map, reg + regmap_get_offset(map, i), &ival);
+		}
+		if (ret != 0)
+			goto out;
+
+		switch (map->format.val_bytes) {
+		case 4:
+			u32[i] = ival;
+			break;
+		case 2:
+			u16[i] = ival;
+			break;
+		case 1:
+			u8[i] = ival;
+			break;
+		default:
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+out:
+	map->unlock(map->lock_arg);
+	return ret;
+}
+
 /**
- * regmap_bulk_read() - Read multiple registers from the device
+ * regmap_bulk_read() - Read multiple sequential registers from the device
  *
  * @map: Register map to read from
  * @reg: First register to be read from
@@ -3132,47 +3177,35 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(val + i);
 	} else {
-		u32 *u32 = 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);
-			if (ret != 0)
-				goto out;
-
-			switch (map->format.val_bytes) {
-			case 4:
-				u32[i] = ival;
-				break;
-			case 2:
-				u16[i] = ival;
-				break;
-			case 1:
-				u8[i] = ival;
-				break;
-			default:
-				ret = -EINVAL;
-				goto out;
-			}
-		}
-
-out:
-		map->unlock(map->lock_arg);
+		ret = _regmap_bulk_read(map, reg, NULL, val, val_count);
 	}
-
 	if (!ret)
 		trace_regmap_bulk_read(map, reg, val, val_bytes * val_count);
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_bulk_read);
 
+/**
+ * regmap_multi_reg_read() - Read multiple non-sequential registers from the device
+ *
+ * @map: Register map to read from
+ * @regs: Array of registers to read from
+ * @val: Pointer to store read value, in native register size for device
+ * @val_count: Number of registers to read
+ *
+ * A value of zero will be returned on success, a negative errno will
+ * be returned in error cases.
+ */
+int regmap_multi_reg_read(struct regmap *map, unsigned int *regs, void *val,
+			  size_t val_count)
+{
+	if (val_count == 0)
+		return -EINVAL;
+
+	return _regmap_bulk_read(map, 0, regs, val, val_count);
+}
+EXPORT_SYMBOL_GPL(regmap_multi_reg_read);
+
 static int _regmap_update_bits(struct regmap *map, unsigned int reg,
 			       unsigned int mask, unsigned int val,
 			       bool *change, bool force_write)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index a6bc2980a98b..9c1ddbf82719 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1237,6 +1237,8 @@ int regmap_noinc_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_multi_reg_read(struct regmap *map, unsigned int *reg, void *val,
+			  size_t val_count);
 int regmap_update_bits_base(struct regmap *map, unsigned int reg,
 			    unsigned int mask, unsigned int val,
 			    bool *change, bool async, bool force);
-- 
2.39.2

[RFC PATCH 2/3] hwmon: (adt7470) Use multi-byte regmap operations

[Guenter Roeck]

Use multi-byte regmap operations where possible to reduce code size
and the need for mutex protection.

No functional change.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/adt7470.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index 517248d2994e..dbee6926fa05 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -728,30 +728,22 @@ static const int adt7470_freq_map[] = {
 static int pwm1_freq_get(struct device *dev)
 {
 	struct adt7470_data *data = dev_get_drvdata(dev);
-	unsigned int cfg_reg_1, cfg_reg_2;
+	unsigned int regs[2] = {ADT7470_REG_CFG, ADT7470_REG_CFG_2};
+	u8 cfg_reg[2];
 	int index;
 	int err;
 
-	mutex_lock(&data->lock);
-	err = regmap_read(data->regmap, ADT7470_REG_CFG, &cfg_reg_1);
-	if (err < 0)
-		goto out;
-	err = regmap_read(data->regmap, ADT7470_REG_CFG_2, &cfg_reg_2);
-	if (err < 0)
-		goto out;
-	mutex_unlock(&data->lock);
+	err = regmap_multi_reg_read(data->regmap, regs, cfg_reg, 2);
+	if (err)
+		return err;
 
-	index = (cfg_reg_2 & ADT7470_FREQ_MASK) >> ADT7470_FREQ_SHIFT;
-	if (!(cfg_reg_1 & ADT7470_CFG_LF))
+	index = (cfg_reg[1] & ADT7470_FREQ_MASK) >> ADT7470_FREQ_SHIFT;
+	if (!(cfg_reg[0] & ADT7470_CFG_LF))
 		index += 8;
 	if (index >= ARRAY_SIZE(adt7470_freq_map))
 		index = ARRAY_SIZE(adt7470_freq_map) - 1;
 
 	return adt7470_freq_map[index];
-
-out:
-	mutex_unlock(&data->lock);
-	return err;
 }
 
 static int adt7470_pwm_read(struct device *dev, u32 attr, int channel, long *val)
-- 
2.39.2

[RFC PATCH 3/3] hwmon: (tmp401) Use multi-byte regmap operations

[Guenter Roeck]

Use multi-byte regmap operations where possible to reduce code size
and the need for mutex protection.

No functional change.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/tmp401.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/hwmon/tmp401.c b/drivers/hwmon/tmp401.c
index df1b45a62e80..8a1497cca2d2 100644
--- a/drivers/hwmon/tmp401.c
+++ b/drivers/hwmon/tmp401.c
@@ -308,7 +308,9 @@ static int tmp401_temp_read(struct device *dev, u32 attr, int channel, long *val
 {
 	struct tmp401_data *data = dev_get_drvdata(dev);
 	struct regmap *regmap = data->regmap;
+	unsigned int regs[2];
 	unsigned int regval;
+	u16 regvals[2];
 	int reg, ret;
 
 	switch (attr) {
@@ -325,20 +327,13 @@ static int tmp401_temp_read(struct device *dev, u32 attr, int channel, long *val
 		*val = tmp401_register_to_temp(regval, data->extended_range);
 		break;
 	case hwmon_temp_crit_hyst:
-		mutex_lock(&data->update_lock);
-		reg = TMP401_TEMP_MSB[3][channel];
-		ret = regmap_read(regmap, reg, &regval);
-		if (ret < 0)
-			goto unlock;
-		*val = tmp401_register_to_temp(regval, data->extended_range);
-		ret = regmap_read(regmap, TMP401_TEMP_CRIT_HYST, &regval);
-		if (ret < 0)
-			goto unlock;
-		*val -= regval * 1000;
-unlock:
-		mutex_unlock(&data->update_lock);
+		regs[0] = TMP401_TEMP_MSB[3][channel];
+		regs[1] = TMP401_TEMP_CRIT_HYST;
+		ret = regmap_multi_reg_read(regmap, regs, regvals, 2);
 		if (ret < 0)
 			return ret;
+		*val = tmp401_register_to_temp(regvals[0], data->extended_range) -
+							(regvals[1] * 1000);
 		break;
 	case hwmon_temp_fault:
 	case hwmon_temp_min_alarm:
-- 
2.39.2

Re: (subset) [RFC PATCH 0/3] regmap: Implement regmap_multi_reg_read()

[Mark Brown]

On Tue, 09 Jul 2024 18:56:19 -0700, Guenter Roeck wrote:
> regmap_multi_reg_read() is similar to regmap_bilk_read() but reads from
> an array of non-sequential registers. It is helpful if multiple non-
> sequential registers need to be read in a single operation which would
> otherwise have to be mutex protected.
> 
> The name of the new function was chosen to match the existing function
> regmap_multi_reg_write().
> 
> [...]

Applied to

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

Thanks!

[1/3] regmap: Implement regmap_multi_reg_read()
      commit: 450a60ef607900bb9affb0e822fea9726679c512

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

Re: [RFC PATCH 0/3] regmap: Implement regmap_multi_reg_read()

[Mark Brown]

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

On Tue, Jul 09, 2024 at 06:56:19PM -0700, Guenter Roeck wrote:
> regmap_multi_reg_read() is similar to regmap_bilk_read() but reads from
> an array of non-sequential registers. It is helpful if multiple non-
> sequential registers need to be read in a single operation which would
> otherwise have to be mutex protected.

It would be nice to have KUnit coverage for the new function too.

The following changes since commit f2661062f16b2de5d7b6a5c42a9a5c96326b8454:

  Linux 6.10-rc5 (2024-06-23 17:08:54 -0400)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/regmap-multi-reg-write

for you to fetch changes up to 3c1ff93b4deea502cd8b0869839557cab2a28b71:

  regmap: Implement regmap_multi_reg_read() (2024-07-10 18:45:34 +0100)

----------------------------------------------------------------
regmap: Implement regmap_multi_reg_read()

A new API function from Guenter Roeck:

regmap_multi_reg_read() is similar to regmap_bulk_read() but reads from
an array of non-sequential registers. It is helpful if multiple non-
sequential registers need to be read in a single operation which would
otherwise have to be mutex protected.

The name of the new function was chosen to match the existing function
regmap_multi_reg_write().

----------------------------------------------------------------
Guenter Roeck (1):
      regmap: Implement regmap_multi_reg_read()

 drivers/base/regmap/regmap.c | 103 ++++++++++++++++++++++++++++---------------
 include/linux/regmap.h       |   2 +
 2 files changed, 70 insertions(+), 35 deletions(-)

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

Re: [RFC PATCH 0/3] regmap: Implement regmap_multi_reg_read()

[Guenter Roeck]

On 7/10/24 18:14, Mark Brown wrote:
> On Tue, Jul 09, 2024 at 06:56:19PM -0700, Guenter Roeck wrote:
>> regmap_multi_reg_read() is similar to regmap_bilk_read() but reads from
>> an array of non-sequential registers. It is helpful if multiple non-
>> sequential registers need to be read in a single operation which would
>> otherwise have to be mutex protected.
> 
> It would be nice to have KUnit coverage for the new function too.
> 

Agreed, makes sense. I'll have a look and see what I can come up with.
I assume you'd also like to have a unit test for regmap_multi_reg_write() ?

Thanks,
Guenter

Re: [RFC PATCH 0/3] regmap: Implement regmap_multi_reg_read()

[Mark Brown]

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

On Wed, Jul 10, 2024 at 08:33:42PM -0700, Guenter Roeck wrote:
> On 7/10/24 18:14, Mark Brown wrote:

> > It would be nice to have KUnit coverage for the new function too.

> Agreed, makes sense. I'll have a look and see what I can come up with.
> I assume you'd also like to have a unit test for regmap_multi_reg_write() ?

Yes, indeed - thanks for sending that.  In general it'd be nice to fill
out the whole API and things like formatting as well.

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