devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Add support for the LTM8054 voltage regulator
@ 2025-09-25 12:37 Romain Gantois
  2025-09-25 12:37 ` [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator Romain Gantois
                   ` (4 more replies)
  0 siblings, 5 replies; 23+ messages in thread
From: Romain Gantois @ 2025-09-25 12:37 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio, Romain Gantois

Hello everyone,

These patches depend on the following series from Hans de Goede:

iio: processed channel handling fixes + Intel Dollar Cove TI PMIC ADC driver
(v4) https://lore.kernel.org/all/20250811155453.31525-1-hansg@kernel.org/

This is version two of my series which adds initial support of the Linear
Technology LTM8054 voltage regulator. The driver supports a fixed voltage and a
tunable output current limit using a DAC-controlled pin.

I'd say that the only unusual part of this series is the usage of the IIO
consumer API in a regulator driver. I think this makes sense here, since
the regulator driver has to access a DAC to read/set the output current
limit.

Since the regulator driver writes microvolts and the IIO consumer API takes
millivolts, the reads and writes to the CTL DAC have to be scaled by a
factor of 1000. Scaled reads are already supported in IIO, but scaled
writes are not, which is why I've implemented them in patch 2/4.

Please let me know what you think.

Thanks,

Romain

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
Changes in v2:
- Refactored iio_convert_processed_to_raw() to match what was done in Hans'
  series.
- Added unit tests for IIO division.
- Fixed coding style issues and removed unnecessary casts.
- Link to v1: https://lore.kernel.org/r/20250916-ltm8054-driver-v1-0-fd4e781d33b9@bootlin.com

---
Romain Gantois (5):
      regulator: dt-bindings: Add Linear Technology LTM8054 regulator
      iio: add processed write API
      Add kunit tests for iio_divide_by_value()
      regulator: Support the LTM8054 voltage regulator
      regulator: ltm8054: Support output current limit control

 .../devicetree/bindings/regulator/adi,ltm8054.yaml |  73 +++++++
 MAINTAINERS                                        |   6 +
 drivers/iio/inkern.c                               | 120 +++++++++++
 drivers/iio/test/Kconfig                           |  12 ++
 drivers/iio/test/Makefile                          |   1 +
 drivers/iio/test/iio-test-divide.c                 | 212 +++++++++++++++++++
 drivers/regulator/Kconfig                          |   9 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/ltm8054-regulator.c              | 235 +++++++++++++++++++++
 include/linux/iio/consumer.h                       |  36 ++++
 10 files changed, 705 insertions(+)
---
base-commit: bd89f4b281945a63659687ef5c70c4442d7e4940
change-id: 20250728-ltm8054-driver-11cfa4741065

Best regards,
-- 
Romain Gantois <romain.gantois@bootlin.com>


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

* [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator
  2025-09-25 12:37 [PATCH v2 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
@ 2025-09-25 12:37 ` Romain Gantois
  2025-09-25 19:27   ` Conor Dooley
  2025-09-25 12:37 ` [PATCH v2 2/5] iio: add processed write API Romain Gantois
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 23+ messages in thread
From: Romain Gantois @ 2025-09-25 12:37 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio, Romain Gantois

The Linear Technology LTM8054 is a Buck-Boost voltage regulator with an
input range of 5V to 36V and an output range of 1.2V to 36V.

The LTM8054's output voltage level is typically set using a voltage divider
between the Vout and FB pins, the FB pin being constantly regulated to
1.2V.

The output current limit of the LTM8054 may be statically set by placing a
sense resistor on a dedicated pin. This limit can then be lowered by
controlling the voltage level on the CTL pin.

Describe the LTM8054 voltage regulator.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 .../devicetree/bindings/regulator/adi,ltm8054.yaml | 73 ++++++++++++++++++++++
 MAINTAINERS                                        |  5 ++
 2 files changed, 78 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml b/Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..8ca8fc4e80b5722f58b4cbe9de22c16d4fd91670
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/adi,ltm8054.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices LTM8054 buck-boost regulator
+
+maintainers:
+  - Romain Gantois <romain.gantois@bootlin.com>
+
+description:
+  This regulator operates over an input voltage range of 5V to 36V, and can
+  output from 1.2V to 36V. The output voltage level is typically set with a
+  voltage divider between the Vout pin and the FB pin which is internally
+  regulated to 1.2V.
+
+  The output current of the LTM8054 can be limited by tying the Iout pin to a
+  current sense resistor. This limit can be further lowered by applying a
+  voltage below 1.2V to the CTL pin.
+
+allOf:
+  - $ref: /schemas/regulator/regulator.yaml#
+
+properties:
+  compatible:
+    const: adi,ltm8054
+
+  enable-gpios:
+    description: GPIO connected to the RUN pin.
+    maxItems: 1
+
+  lltc,fb-voltage-divider:
+    description:
+      An array of two integers containing the resistor values
+      R1 and R2 of the feedback voltage divider in Ohms.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 2
+    maxItems: 2
+
+  adi,iout-rsense-micro-ohms:
+    description:
+      Value of the output current sense resistor, in micro Ohms.
+
+  io-channels:
+    items:
+      - description: DAC controlling the voltage level of the CTL pin.
+
+  io-channel-names:
+    items:
+      - const: ctl
+
+required:
+  - compatible
+  - lltc,fb-voltage-divider
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    regulator {
+        compatible = "adi,ltm8054";
+
+        enable-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+
+        lltc,fb-voltage-divider = <1000000 68000>;
+
+        adi,iout-rsense-micro-ohms = <20000>;
+
+        io-channels = <&dac 1>;
+        io-channel-names = "ctl";
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 3763f9fc9e4ed62bc8b273756a25f9c921570bee..69bcba82808bb815af436232fab50f70713fd533 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14534,6 +14534,11 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
 F:	drivers/i2c/muxes/i2c-mux-ltc4306.c
 
+LTM8054 REGULATOR DRIVER
+M:	Romain Gantois <romain.gantois@bootlin.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml
+
 LTP (Linux Test Project)
 M:	Andrea Cervesato <andrea.cervesato@suse.com>
 M:	Cyril Hrubis <chrubis@suse.cz>

-- 
2.51.0


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

* [PATCH v2 2/5] iio: add processed write API
  2025-09-25 12:37 [PATCH v2 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
  2025-09-25 12:37 ` [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator Romain Gantois
@ 2025-09-25 12:37 ` Romain Gantois
  2025-09-25 21:10   ` David Lechner
  2025-09-28  9:07   ` Jonathan Cameron
  2025-09-25 12:37 ` [PATCH v2 3/5] Add kunit tests for iio_divide_by_value() Romain Gantois
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 23+ messages in thread
From: Romain Gantois @ 2025-09-25 12:37 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio, Romain Gantois

Add a function to allow IIO consumers to write a processed value to a
channel.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/iio/inkern.c         | 120 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/consumer.h |  36 +++++++++++++
 2 files changed, 156 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 2a1ecef2b82007f5ee8e8d0f8b35846bc4d2f94a..a6ec724b7c1fb197e6261c1162f8315deda20fd7 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -631,6 +631,57 @@ int iio_multiply_value(int *result, s64 multiplier,
 }
 EXPORT_SYMBOL_GPL(iio_multiply_value);
 
+int iio_divide_by_value(int *result, s64 numerator,
+			unsigned int type, int val, int val2)
+{
+	s64 tmp_num, tmp_den;
+
+	switch (type) {
+	case IIO_VAL_INT:
+		tmp_num = numerator;
+		tmp_den = val;
+		break;
+	case IIO_VAL_INT_PLUS_MICRO:
+	case IIO_VAL_INT_PLUS_NANO:
+		switch (type) {
+		case IIO_VAL_INT_PLUS_MICRO:
+			tmp_num = MICRO;
+			tmp_den = MICRO;
+			break;
+
+		case IIO_VAL_INT_PLUS_NANO:
+			tmp_num = NANO;
+			tmp_den = NANO;
+			break;
+		}
+
+		tmp_num *= (s64)numerator;
+		tmp_den = (s64)abs(val) * tmp_den + (s64)abs(val2);
+
+		if (val < 0 || val2 < 0)
+			tmp_num *= -1;
+
+		break;
+	case IIO_VAL_FRACTIONAL:
+		tmp_num = (s64)numerator * (s64)val2;
+		tmp_den = val;
+		break;
+	case IIO_VAL_FRACTIONAL_LOG2:
+		tmp_num = (s64)numerator << val2;
+		tmp_den = val;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (!tmp_den)
+		return -ERANGE;
+
+	*result = div64_s64(tmp_num, tmp_den);
+
+	return IIO_VAL_INT;
+}
+
 static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
 						 int raw, int *processed,
 						 unsigned int scale)
@@ -699,6 +750,53 @@ int iio_convert_raw_to_processed(struct iio_channel *chan, int raw,
 }
 EXPORT_SYMBOL_GPL(iio_convert_raw_to_processed);
 
+static int iio_convert_processed_to_raw_unlocked(struct iio_channel *chan,
+						 int processed, int *raw,
+						 unsigned int scale)
+{
+	int scale_type, scale_val, scale_val2;
+	int offset_type, offset_val, offset_val2;
+	int ret;
+
+	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
+				      IIO_CHAN_INFO_SCALE);
+	if (scale_type >= 0) {
+		ret = iio_divide_by_value(raw, processed, scale_type, scale_val, scale_val2);
+		if (ret < 0)
+			return ret;
+	} else {
+		*raw = processed;
+	}
+
+	if (!scale)
+		return -ERANGE;
+
+	*raw = div_s64(*raw, scale);
+
+	offset_type = iio_channel_read(chan, &offset_val, &offset_val2,
+				       IIO_CHAN_INFO_OFFSET);
+	if (offset_type >= 0) {
+		switch (offset_type) {
+		case IIO_VAL_INT:
+		case IIO_VAL_INT_PLUS_MICRO:
+		case IIO_VAL_INT_PLUS_NANO:
+			break;
+		case IIO_VAL_FRACTIONAL:
+			offset_val /= offset_val2;
+			break;
+		case IIO_VAL_FRACTIONAL_LOG2:
+			offset_val >>= offset_val2;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		*raw -= offset_val;
+	}
+
+	return 0;
+}
+
 int iio_read_channel_attribute(struct iio_channel *chan, int *val, int *val2,
 			       enum iio_chan_info_enum attribute)
 {
@@ -1035,3 +1133,25 @@ ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf)
 	return do_iio_read_channel_label(chan->indio_dev, chan->channel, buf);
 }
 EXPORT_SYMBOL_GPL(iio_read_channel_label);
+
+int iio_write_channel_processed_scale(struct iio_channel *chan, int val,
+				      unsigned int scale)
+{
+	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan->indio_dev);
+	int ret, processed;
+
+	scoped_guard(mutex, &iio_dev_opaque->info_exist_lock) {
+		if (!chan->indio_dev->info)
+			return -ENODEV;
+
+		ret = iio_convert_processed_to_raw_unlocked(chan, val, &processed, scale);
+		if (ret)
+			return ret;
+
+		ret = iio_channel_write(chan, processed, 0, IIO_CHAN_INFO_RAW);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iio_write_channel_processed_scale);
+
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index c8c6261c81f934480e16854412e269207be60adc..dc84b8b4c61911d1a58427f1a9c798cae3954ac1 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -399,6 +399,24 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val,
 int iio_multiply_value(int *result, s64 multiplier,
 		       unsigned int type, int val, int val2);
 
+/**
+ * iio_divide_by_value() - Divide by an IIO value
+ * @result:	Destination pointer for the division result
+ * @numerator:	Numerator.
+ * @type:	One of the IIO_VAL_* constants. This decides how the @val
+ *		and @val2 parameters are interpreted.
+ * @val:	Denominator.
+ * @val2:	Denominator. @val2 use depends on type.
+ *
+ * Divide an s64 number by an IIO value, storing the result as
+ * IIO_VAL_INT. This is typically used for scaling.
+ *
+ * Returns:
+ * IIO_VAL_INT on success or a negative error-number on failure.
+ */
+int iio_divide_by_value(int *result, s64 numerator,
+			unsigned int type, int val, int val2);
+
 /**
  * iio_convert_raw_to_processed() - Converts a raw value to a processed value
  * @chan:		The channel being queried
@@ -469,4 +487,22 @@ ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
  */
 ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf);
 
+/**
+ * iio_write_channel_processed_scale() - scale and write processed value to a given channel
+ * @chan:		The channel being queried.
+ * @val:		Value to write.
+ * @scale:		Scale factor to apply during the conversion
+ *
+ * This function writes a processed value to a channel. A processed value means
+ * that this value will have the correct unit and not some device internal
+ * representation. If the device does not support writing a processed value, the
+ * function will query the channel's scale and offset and write an appropriately
+ * transformed raw value.
+ *
+ * Return: an error code or 0.
+ *
+ */
+int iio_write_channel_processed_scale(struct iio_channel *chan, int val,
+				      unsigned int scale);
+
 #endif

-- 
2.51.0


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

* [PATCH v2 3/5] Add kunit tests for iio_divide_by_value()
  2025-09-25 12:37 [PATCH v2 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
  2025-09-25 12:37 ` [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator Romain Gantois
  2025-09-25 12:37 ` [PATCH v2 2/5] iio: add processed write API Romain Gantois
@ 2025-09-25 12:37 ` Romain Gantois
  2025-09-25 20:26   ` David Lechner
  2025-09-25 12:37 ` [PATCH v2 4/5] regulator: Support the LTM8054 voltage regulator Romain Gantois
  2025-09-25 12:37 ` [PATCH v2 5/5] regulator: ltm8054: Support output current limit control Romain Gantois
  4 siblings, 1 reply; 23+ messages in thread
From: Romain Gantois @ 2025-09-25 12:37 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio, Romain Gantois

Add kunit tests for iio_divide_by_value(), these are similar to the
existing tests for iio_multiply_value(), but the operand values used differ
slightly.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/iio/test/Kconfig           |  12 +++
 drivers/iio/test/Makefile          |   1 +
 drivers/iio/test/iio-test-divide.c | 212 +++++++++++++++++++++++++++++++++++++
 3 files changed, 225 insertions(+)

diff --git a/drivers/iio/test/Kconfig b/drivers/iio/test/Kconfig
index 6e65e929791ca247df9ac993fddbb4da557d5dfa..3aa1fc78966cf72554e069db75de1c6eff532850 100644
--- a/drivers/iio/test/Kconfig
+++ b/drivers/iio/test/Kconfig
@@ -4,6 +4,18 @@
 #
 
 # Keep in alphabetical order
+config IIO_DIVIDE_KUNIT_TEST
+	tristate "Test IIO division functions" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  build unit tests for the IIO division functions.
+
+	  For more information on KUnit and unit tests in general, please refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
+
 config IIO_GTS_KUNIT_TEST
 	tristate "Test IIO gain-time-scale helpers" if !KUNIT_ALL_TESTS
 	depends on KUNIT
diff --git a/drivers/iio/test/Makefile b/drivers/iio/test/Makefile
index 0c846bc21acda5a487b3a6977a8e9feaef20159a..16344eedc46a5ebb4d57468b3e549d8f65b85432 100644
--- a/drivers/iio/test/Makefile
+++ b/drivers/iio/test/Makefile
@@ -5,6 +5,7 @@
 
 # Keep in alphabetical order
 obj-$(CONFIG_IIO_RESCALE_KUNIT_TEST) += iio-test-rescale.o
+obj-$(CONFIG_IIO_DIVIDE_KUNIT_TEST) += iio-test-divide.o
 obj-$(CONFIG_IIO_FORMAT_KUNIT_TEST) += iio-test-format.o
 obj-$(CONFIG_IIO_GTS_KUNIT_TEST) += iio-test-gts.o
 obj-$(CONFIG_IIO_MULTIPLY_KUNIT_TEST) += iio-test-multiply.o
diff --git a/drivers/iio/test/iio-test-divide.c b/drivers/iio/test/iio-test-divide.c
new file mode 100644
index 0000000000000000000000000000000000000000..dd4f53bf7223750e2ac583b6e2392abb42fb045b
--- /dev/null
+++ b/drivers/iio/test/iio-test-divide.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Unit tests for IIO division functions
+ *
+ * Copyright (c) 2025 Bootlin
+ * Based on iio-test-multiply.c which is:
+ * Copyright (c) 2025 Hans de Goede <hans@hansg.org>
+ * Based on iio-test-format.c which is:
+ * Copyright (c) 2020 Lars-Peter Clausen <lars@metafoo.de>
+ */
+
+#include <kunit/test.h>
+#include <linux/iio/consumer.h>
+
+static void __iio_test_iio_divide_by_integer(struct kunit *test, s64 numerator)
+{
+	int ret, result, val;
+
+	val = 42;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT, val, 0);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator, val));
+
+	val = -23;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT, val, 0);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator, val));
+
+	val = 0;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT, val, 0);
+	KUNIT_EXPECT_EQ(test, ret, -ERANGE);
+}
+
+static void iio_test_iio_divide_by_integer(struct kunit *test)
+{
+	__iio_test_iio_divide_by_integer(test, 2000);
+	__iio_test_iio_divide_by_integer(test, -2000);
+}
+
+static void __iio_test_iio_divide_by_fixedpoint(struct kunit *test, s64 numerator)
+{
+	int ret, result, val, val2;
+
+	/* positive >= 1 (1.5) */
+	val = 1;
+	val2 = 500000;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 15));
+
+	val = 1;
+	val2 = 500000000;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 15));
+
+	/* positive < 1 (0.5) */
+	val = 0;
+	val2 = 500000;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 5));
+
+	val = 0;
+	val2 = 500000000;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 5));
+
+	/* negative <= -1 (-1.5) */
+	val = -1;
+	val2 = 500000;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 15));
+
+	val = -1;
+	val2 = 500000000;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 15));
+
+	/* negative > -1 (-0.5) */
+	val = 0;
+	val2 = -500000;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 5));
+
+	val = 0;
+	val2 = -500000000;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 5));
+
+	/* Zero */
+	val = 0;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, 0);
+	KUNIT_EXPECT_EQ(test, ret, -ERANGE);
+}
+
+static void iio_test_iio_divide_by_fixedpoint(struct kunit *test)
+{
+	__iio_test_iio_divide_by_fixedpoint(test, 2000);
+	__iio_test_iio_divide_by_fixedpoint(test, -2000);
+}
+
+static void __iio_test_iio_divide_by_fractional(struct kunit *test, s64 numerator)
+{
+	int ret, result, val, val2;
+
+	/* positive < 1 (1/10)*/
+	val = 1;
+	val2 = 10;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * val2, val));
+
+	/* positive >= 1 (100/3)*/
+	val = 100;
+	val2 = 3;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * val2, val));
+
+	/* negative > -1 (-1/10) */
+	val = -1;
+	val2 = 10;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * val2, val));
+
+	/* negative <= -1 (-200/3)*/
+	val = -200;
+	val2 = 3;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * val2, val));
+
+	/* Zero */
+	val = 0;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL, val, 0);
+	KUNIT_EXPECT_EQ(test, ret, -ERANGE);
+}
+
+static void iio_test_iio_divide_by_fractional(struct kunit *test)
+{
+	__iio_test_iio_divide_by_fractional(test, 2000);
+	__iio_test_iio_divide_by_fractional(test, -2000);
+}
+
+static void __iio_test_iio_divide_by_fractional_log2(struct kunit *test, s64 numerator)
+{
+	int ret, result, val, val2;
+
+	/* positive < 1 (123/1024) */
+	val = 123;
+	val2 = 10;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64((numerator << val2), val));
+
+	/* positive >= 1 (1234567/1024) */
+	val = 1234567;
+	val2 = 10;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64((numerator << val2), val));
+
+	/* negative > -1 (-123/1024) */
+	val = -123;
+	val2 = 10;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64((numerator << val2), val));
+
+	/* negative <= -1 (-1234567/1024) */
+	val = -1234567;
+	val2 = 10;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
+	KUNIT_EXPECT_EQ(test, result, div_s64((numerator << val2), val));
+
+	/* Zero */
+	val = 0;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, 0);
+	KUNIT_EXPECT_EQ(test, ret, -ERANGE);
+}
+
+static void iio_test_iio_divide_by_fractional_log2(struct kunit *test)
+{
+	__iio_test_iio_divide_by_fractional_log2(test, 2000);
+	__iio_test_iio_divide_by_fractional_log2(test, -2000);
+}
+
+static struct kunit_case iio_divide_test_cases[] = {
+		KUNIT_CASE(iio_test_iio_divide_by_integer),
+		KUNIT_CASE(iio_test_iio_divide_by_fixedpoint),
+		KUNIT_CASE(iio_test_iio_divide_by_fractional),
+		KUNIT_CASE(iio_test_iio_divide_by_fractional_log2),
+		{ }
+};
+
+static struct kunit_suite iio_divide_test_suite = {
+	.name = "iio-divide",
+	.test_cases = iio_divide_test_cases,
+};
+
+kunit_test_suite(iio_divide_test_suite);
+
+MODULE_AUTHOR("Romain Gantois <romain.gantois@bootlin.com>");
+MODULE_DESCRIPTION("Test IIO division functions");
+MODULE_LICENSE("GPL");

-- 
2.51.0


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

* [PATCH v2 4/5] regulator: Support the LTM8054 voltage regulator
  2025-09-25 12:37 [PATCH v2 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
                   ` (2 preceding siblings ...)
  2025-09-25 12:37 ` [PATCH v2 3/5] Add kunit tests for iio_divide_by_value() Romain Gantois
@ 2025-09-25 12:37 ` Romain Gantois
  2025-10-22 14:44   ` Andy Shevchenko
  2025-09-25 12:37 ` [PATCH v2 5/5] regulator: ltm8054: Support output current limit control Romain Gantois
  4 siblings, 1 reply; 23+ messages in thread
From: Romain Gantois @ 2025-09-25 12:37 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio, Romain Gantois

Add a stub driver for the  Linear Technology LTM8054 Buck-Boost voltage
regulator. This version only supports enabling/disabling the regulator via
a GPIO, and reporting the output voltage level from the resistor divider
values given in the device tree.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 MAINTAINERS                           |   1 +
 drivers/regulator/Kconfig             |   8 +++
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/ltm8054-regulator.c | 126 ++++++++++++++++++++++++++++++++++
 4 files changed, 136 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 69bcba82808bb815af436232fab50f70713fd533..310dac4f60ab1e6e82e4dd667482f46723964992 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14538,6 +14538,7 @@ LTM8054 REGULATOR DRIVER
 M:	Romain Gantois <romain.gantois@bootlin.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml
+F:	drivers/regulator/ltm8054-regulator.c
 
 LTP (Linux Test Project)
 M:	Andrea Cervesato <andrea.cervesato@suse.com>
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index e252bb11ae6615dc9154908bc237905b97e739e5..c48b2af350974b3715a1ecf05dec656a92268294 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -577,6 +577,14 @@ config REGULATOR_LTC3676
 	  This enables support for the LTC3676
 	  8-output regulators controlled via I2C.
 
+config REGULATOR_LTM8054
+	tristate "LTM8054 Buck-Boost voltage regulator"
+	help
+	  This driver provides support for the Analog Devices LTM8054
+	  Buck-Boost micromodule regulator. The LTM8054 has an adjustable
+	  output current limitation and a feedback pin for setting the
+	  output voltage level.
+
 config REGULATOR_MAX14577
 	tristate "Maxim 14577/77836 regulator"
 	depends on MFD_MAX14577
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 76b02d12b758c607028d2c8879017afc6d1b244e..afcc9ffcc72f268a9e999a185e3024d67261249a 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_REGULATOR_LP8788) += lp8788-ldo.o
 obj-$(CONFIG_REGULATOR_LP8755) += lp8755.o
 obj-$(CONFIG_REGULATOR_LTC3589) += ltc3589.o
 obj-$(CONFIG_REGULATOR_LTC3676) += ltc3676.o
+obj-$(CONFIG_REGULATOR_LTM8054) += ltm8054-regulator.o
 obj-$(CONFIG_REGULATOR_MAX14577) += max14577-regulator.o
 obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o
 obj-$(CONFIG_REGULATOR_MAX5970) += max5970-regulator.o
diff --git a/drivers/regulator/ltm8054-regulator.c b/drivers/regulator/ltm8054-regulator.c
new file mode 100644
index 0000000000000000000000000000000000000000..bc8cf98b5a3b5663481d148330de70a8165e5981
--- /dev/null
+++ b/drivers/regulator/ltm8054-regulator.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices LTM8054 Buck-Boost regulator driver
+ *
+ * Copyright (C) 2025 Bootlin
+ */
+
+#include <asm/div64.h>
+
+#include <linux/array_size.h>
+
+#include <linux/device.h>
+#include <linux/device/devres.h>
+#include <linux/device/driver.h>
+
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+
+#include <linux/regulator/consumer.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+/* The LTM8054 regulates its FB pin to 1.2V */
+#define LTM8054_FB_uV 1200000
+
+struct ltm8054_priv {
+	struct regulator_desc rdesc;
+};
+
+static int ltm8054_scale(unsigned int uV, u32 r1, u32 r2)
+{
+	u64 tmp;
+
+	tmp = (u64)uV * r1;
+	do_div(tmp, r2);
+
+	return uV + tmp;
+}
+
+static const struct regulator_ops ltm8054_regulator_ops = { };
+
+static int ltm8054_of_parse(struct device *dev, struct ltm8054_priv *priv,
+			    struct regulator_config *config)
+{
+	struct device_node *np = dev->of_node;
+	u32 r[2];
+	int ret;
+
+	config->of_node = np;
+
+	ret = device_property_read_u32_array(dev, "lltc,fb-voltage-divider", r, ARRAY_SIZE(r));
+	if (ret)
+		return ret;
+
+	priv->rdesc.fixed_uV = ltm8054_scale(LTM8054_FB_uV, r[0], r[1]);
+	priv->rdesc.min_uV = priv->rdesc.fixed_uV;
+	priv->rdesc.n_voltages = 1;
+
+	config->init_data = of_get_regulator_init_data(dev,
+						       np,
+						       &priv->rdesc);
+	if (!config->init_data)
+		return -EINVAL;
+
+	config->ena_gpiod = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(config->ena_gpiod))
+		return PTR_ERR(config->ena_gpiod);
+
+	return 0;
+}
+
+static int ltm8054_probe(struct platform_device *pdev)
+{
+	struct regulator_config config = { };
+	struct device *dev = &pdev->dev;
+	struct regulator_dev *rdev;
+	struct ltm8054_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->rdesc.name = "ltm8054-regulator",
+	priv->rdesc.ops = &ltm8054_regulator_ops,
+	priv->rdesc.type = REGULATOR_VOLTAGE,
+	priv->rdesc.owner = THIS_MODULE,
+
+	config.dev = dev;
+	config.driver_data = priv;
+
+	ret = ltm8054_of_parse(dev, priv, &config);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to parse device tree\n");
+
+	rdev = devm_regulator_register(dev, &priv->rdesc, &config);
+	if (IS_ERR(rdev))
+		return dev_err_probe(dev, PTR_ERR(rdev), "failed to register regulator\n");
+
+	return 0;
+}
+
+static const struct of_device_id ltm8054_of_match[] = {
+	{ .compatible = "adi,ltm8054" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ltm8054_of_match);
+
+static struct platform_driver ltm8054_driver = {
+	.probe = ltm8054_probe,
+	.driver = {
+		.name  = "ltm8054",
+		.of_match_table = ltm8054_of_match,
+	},
+};
+module_platform_driver(ltm8054_driver);
+
+MODULE_DESCRIPTION("LTM8054 regulator driver");
+MODULE_AUTHOR("Romain Gantois <romain.gantois@bootlin.com>");
+MODULE_LICENSE("GPL");

-- 
2.51.0


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

* [PATCH v2 5/5] regulator: ltm8054: Support output current limit control
  2025-09-25 12:37 [PATCH v2 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
                   ` (3 preceding siblings ...)
  2025-09-25 12:37 ` [PATCH v2 4/5] regulator: Support the LTM8054 voltage regulator Romain Gantois
@ 2025-09-25 12:37 ` Romain Gantois
  2025-10-22  8:05   ` Romain Gantois
  4 siblings, 1 reply; 23+ messages in thread
From: Romain Gantois @ 2025-09-25 12:37 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio, Romain Gantois

The LTM8054 supports setting a fixed output current limit using a sense
resistor connected to a dedicated pin. This limit can then be lowered
dynamically by varying the voltage level of the CTL pin.

Support controlling the LTM8054's output current limit.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/regulator/Kconfig             |   1 +
 drivers/regulator/ltm8054-regulator.c | 113 +++++++++++++++++++++++++++++++++-
 2 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index c48b2af350974b3715a1ecf05dec656a92268294..e9ee6ed9fe3587c542223a6d6be78412e96797ee 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -579,6 +579,7 @@ config REGULATOR_LTC3676
 
 config REGULATOR_LTM8054
 	tristate "LTM8054 Buck-Boost voltage regulator"
+	depends on IIO
 	help
 	  This driver provides support for the Analog Devices LTM8054
 	  Buck-Boost micromodule regulator. The LTM8054 has an adjustable
diff --git a/drivers/regulator/ltm8054-regulator.c b/drivers/regulator/ltm8054-regulator.c
index bc8cf98b5a3b5663481d148330de70a8165e5981..172ec32c5a9517c6fb38ded8095ffc8e1acf55f0 100644
--- a/drivers/regulator/ltm8054-regulator.c
+++ b/drivers/regulator/ltm8054-regulator.c
@@ -17,6 +17,8 @@
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/gpio/consumer.h>
+#include <linux/iio/consumer.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -26,10 +28,25 @@
 #include <linux/regulator/driver.h>
 #include <linux/regulator/of_regulator.h>
 
+#include <linux/units.h>
+
 /* The LTM8054 regulates its FB pin to 1.2V */
 #define LTM8054_FB_uV 1200000
 
+/* Threshold voltage between the Vout and Iout pins which triggers current
+ * limiting.
+ */
+#define LTM8054_VOUT_IOUT_MAX_uV 58000
+
+#define LTM8054_MAX_CTL_uV 1200000
+#define LTM8054_MIN_CTL_uV 50000
+
 struct ltm8054_priv {
+	struct iio_channel *ctl_dac;
+
+	int min_uA;
+	int max_uA;
+
 	struct regulator_desc rdesc;
 };
 
@@ -43,18 +60,103 @@ static int ltm8054_scale(unsigned int uV, u32 r1, u32 r2)
 	return uV + tmp;
 }
 
-static const struct regulator_ops ltm8054_regulator_ops = { };
+static int ltm8054_set_current_limit(struct regulator_dev *rdev, int min_uA, int max_uA)
+{
+	struct ltm8054_priv *priv = rdev_get_drvdata(rdev);
+	u64 vdac_uV;
+
+	min_uA = clamp_t(int, min_uA, priv->min_uA, priv->max_uA);
+
+	/* adjusted current limit = Rsense current limit * CTL pin voltage / max CTL pin voltage */
+	vdac_uV = (u64)min_uA * LTM8054_MAX_CTL_uV;
+	do_div(vdac_uV, priv->max_uA);
+
+	dev_dbg(&rdev->dev,
+		"Setting current limit to %duA, CTL pin to %duV\n", min_uA, (int)vdac_uV);
+
+	/* Standard IIO voltage unit is mV, scale accordingly. */
+	return iio_write_channel_processed_scale(priv->ctl_dac, vdac_uV, 1000);
+}
+
+static int ltm8054_get_current_limit(struct regulator_dev *rdev)
+{
+	struct ltm8054_priv *priv = rdev_get_drvdata(rdev);
+	int ret, vdac_uv;
+	u64 uA;
+
+	ret = iio_read_channel_processed_scale(priv->ctl_dac, &vdac_uv, 1000);
+	if (ret < 0) {
+		dev_err(&rdev->dev, "failed to read CTL DAC voltage, err %d\n", ret);
+		return ret;
+	}
+
+	uA = (u64)vdac_uv * priv->max_uA;
+	do_div(uA, LTM8054_MAX_CTL_uV);
+
+	return uA;
+}
+
+static const struct regulator_ops ltm8054_regulator_ops = {
+	.set_current_limit = ltm8054_set_current_limit,
+	.get_current_limit = ltm8054_get_current_limit,
+};
+
+static int ltm8054_init_ctl_dac(struct platform_device *pdev, struct ltm8054_priv *priv)
+{
+	struct iio_channel *ctl_dac;
+	enum iio_chan_type type;
+	int ret;
+
+	ctl_dac = devm_iio_channel_get(&pdev->dev, "ctl");
+	if (IS_ERR(ctl_dac))
+		return PTR_ERR(ctl_dac);
+
+	ret = iio_get_channel_type(ctl_dac, &type);
+	if (ret)
+		return ret;
+
+	if (type != IIO_VOLTAGE)
+		return -EINVAL;
+
+	priv->ctl_dac = ctl_dac;
+
+	return 0;
+}
 
 static int ltm8054_of_parse(struct device *dev, struct ltm8054_priv *priv,
 			    struct regulator_config *config)
 {
 	struct device_node *np = dev->of_node;
+	u32 rsense;
 	u32 r[2];
+	u64 tmp;
 	int ret;
 
 	config->of_node = np;
 
-	ret = device_property_read_u32_array(dev, "lltc,fb-voltage-divider", r, ARRAY_SIZE(r));
+	ret = device_property_read_u32(dev, "adi,iout-rsense-micro-ohms", &rsense);
+	if (ret)
+		return ret;
+
+	if (rsense == 0)
+		return -EINVAL;
+
+	/* The maximum output current limit is the one set by the Rsense resistor */
+	tmp = (u64)LTM8054_VOUT_IOUT_MAX_uV * MICRO;
+	do_div(tmp, rsense);
+	priv->max_uA = tmp;
+
+	/*
+	 * Applying a voltage below LTM8054_MAX_CTL_uV on the CTL pin reduces
+	 * the output current limit. If this level drops below
+	 * LTM8054_MIN_CTL_uV the regulator stops switching.
+	 */
+
+	tmp = (u64)priv->max_uA * LTM8054_MIN_CTL_uV;
+	do_div(tmp, LTM8054_MAX_CTL_uV);
+	priv->min_uA = tmp;
+
+	ret = device_property_read_u32_array(dev, "lltc,fb-voltage-divider", r, 2);
 	if (ret)
 		return ret;
 
@@ -62,6 +164,9 @@ static int ltm8054_of_parse(struct device *dev, struct ltm8054_priv *priv,
 	priv->rdesc.min_uV = priv->rdesc.fixed_uV;
 	priv->rdesc.n_voltages = 1;
 
+	dev_dbg(dev, "max_uA: %d min_uA: %d fixed_uV: %d\n",
+		priv->max_uA, priv->min_uA, priv->rdesc.fixed_uV);
+
 	config->init_data = of_get_regulator_init_data(dev,
 						       np,
 						       &priv->rdesc);
@@ -99,6 +204,10 @@ static int ltm8054_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to parse device tree\n");
 
+	ret = ltm8054_init_ctl_dac(pdev, priv);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to initialize CTL DAC\n");
+
 	rdev = devm_regulator_register(dev, &priv->rdesc, &config);
 	if (IS_ERR(rdev))
 		return dev_err_probe(dev, PTR_ERR(rdev), "failed to register regulator\n");

-- 
2.51.0


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

* Re: [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator
  2025-09-25 12:37 ` [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator Romain Gantois
@ 2025-09-25 19:27   ` Conor Dooley
  2025-09-26 15:59     ` Romain Gantois
  0 siblings, 1 reply; 23+ messages in thread
From: Conor Dooley @ 2025-09-25 19:27 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Hans de Goede, Thomas Petazzoni, linux-kernel,
	devicetree, linux-iio

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

On Thu, Sep 25, 2025 at 02:37:33PM +0200, Romain Gantois wrote:
> The Linear Technology LTM8054 is a Buck-Boost voltage regulator with an
> input range of 5V to 36V and an output range of 1.2V to 36V.
> 
> The LTM8054's output voltage level is typically set using a voltage divider
> between the Vout and FB pins, the FB pin being constantly regulated to
> 1.2V.
> 
> The output current limit of the LTM8054 may be statically set by placing a
> sense resistor on a dedicated pin. This limit can then be lowered by
> controlling the voltage level on the CTL pin.
> 
> Describe the LTM8054 voltage regulator.
> 
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> ---
>  .../devicetree/bindings/regulator/adi,ltm8054.yaml | 73 ++++++++++++++++++++++
>  MAINTAINERS                                        |  5 ++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml b/Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..8ca8fc4e80b5722f58b4cbe9de22c16d4fd91670
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/adi,ltm8054.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices LTM8054 buck-boost regulator
> +
> +maintainers:
> +  - Romain Gantois <romain.gantois@bootlin.com>
> +
> +description:
> +  This regulator operates over an input voltage range of 5V to 36V, and can
> +  output from 1.2V to 36V. The output voltage level is typically set with a
> +  voltage divider between the Vout pin and the FB pin which is internally
> +  regulated to 1.2V.
> +
> +  The output current of the LTM8054 can be limited by tying the Iout pin to a
> +  current sense resistor. This limit can be further lowered by applying a
> +  voltage below 1.2V to the CTL pin.
> +
> +allOf:
> +  - $ref: /schemas/regulator/regulator.yaml#
> +
> +properties:
> +  compatible:
> +    const: adi,ltm8054
> +
> +  enable-gpios:
> +    description: GPIO connected to the RUN pin.
> +    maxItems: 1
> +
> +  lltc,fb-voltage-divider:

Why does this property have a ?linear? vendor prefix?
Shouldn't it be adi to match the other property and compatible?

> +    description:
> +      An array of two integers containing the resistor values
> +      R1 and R2 of the feedback voltage divider in Ohms.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 2
> +    maxItems: 2
> +
> +  adi,iout-rsense-micro-ohms:
> +    description:
> +      Value of the output current sense resistor, in micro Ohms.
> +
> +  io-channels:
> +    items:
> +      - description: DAC controlling the voltage level of the CTL pin.
> +
> +  io-channel-names:
> +    items:
> +      - const: ctl
> +
> +required:
> +  - compatible
> +  - lltc,fb-voltage-divider
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    regulator {
> +        compatible = "adi,ltm8054";
> +
> +        enable-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
> +
> +        lltc,fb-voltage-divider = <1000000 68000>;
> +
> +        adi,iout-rsense-micro-ohms = <20000>;
> +
> +        io-channels = <&dac 1>;
> +        io-channel-names = "ctl";
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3763f9fc9e4ed62bc8b273756a25f9c921570bee..69bcba82808bb815af436232fab50f70713fd533 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14534,6 +14534,11 @@ W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
>  F:	drivers/i2c/muxes/i2c-mux-ltc4306.c
>  
> +LTM8054 REGULATOR DRIVER
> +M:	Romain Gantois <romain.gantois@bootlin.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml
> +
>  LTP (Linux Test Project)
>  M:	Andrea Cervesato <andrea.cervesato@suse.com>
>  M:	Cyril Hrubis <chrubis@suse.cz>
> 
> -- 
> 2.51.0
> 

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

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

* Re: [PATCH v2 3/5] Add kunit tests for iio_divide_by_value()
  2025-09-25 12:37 ` [PATCH v2 3/5] Add kunit tests for iio_divide_by_value() Romain Gantois
@ 2025-09-25 20:26   ` David Lechner
  0 siblings, 0 replies; 23+ messages in thread
From: David Lechner @ 2025-09-25 20:26 UTC (permalink / raw)
  To: Romain Gantois, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron, Nuno Sá,
	Andy Shevchenko
  Cc: Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio

On 9/25/25 7:37 AM, Romain Gantois wrote:

...

> +static void __iio_test_iio_divide_by_integer(struct kunit *test, s64 numerator)
> +{
> +	int ret, result, val;
> +
> +	val = 42;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT, val, 0);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator, val));
> +
> +	val = -23;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT, val, 0);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator, val));
> +
> +	val = 0;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT, val, 0);
> +	KUNIT_EXPECT_EQ(test, ret, -ERANGE);

I would expect EDOM for divide by 0 rather than ERANGE. The function is
undefined at that point.

> +}
> +
> +static void iio_test_iio_divide_by_integer(struct kunit *test)
> +{
> +	__iio_test_iio_divide_by_integer(test, 2000);
> +	__iio_test_iio_divide_by_integer(test, -2000);
> +}
> +
> +static void __iio_test_iio_divide_by_fixedpoint(struct kunit *test, s64 numerator)
> +{
> +	int ret, result, val, val2;
> +
> +	/* positive >= 1 (1.5) */
> +	val = 1;
> +	val2 = 500000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 15));
> +
> +	val = 1;
> +	val2 = 500000000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 15));
> +
> +	/* positive < 1 (0.5) */
> +	val = 0;
> +	val2 = 500000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 5));
> +
> +	val = 0;
> +	val2 = 500000000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 5));
> +
> +	/* negative <= -1 (-1.5) */
> +	val = -1;
> +	val2 = 500000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 15));
> +
> +	val = -1;
> +	val2 = 500000000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 15));
> +
> +	/* negative > -1 (-0.5) */
> +	val = 0;
> +	val2 = -500000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 5));
> +
> +	val = 0;
> +	val2 = -500000000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 5));
> +
> +	/* Zero */
> +	val = 0;

Odd to break the pattern an not have `val2 = 0;` here.

> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, 0);
> +	KUNIT_EXPECT_EQ(test, ret, -ERANGE);
> +}

...

> +static void __iio_test_iio_divide_by_fractional_log2(struct kunit *test, s64 numerator)
> +{
> +	int ret, result, val, val2;
> +
> +	/* positive < 1 (123/1024) */
> +	val = 123;
> +	val2 = 10;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64((numerator << val2), val));

My instinct would be to write it like this:

	div_s64((numerator * 1024), 123)

This follows how it was done in __iio_test_iio_divide_by_fixedpoint() and
makes it easier to see that it matchs exactly the value in the comment.


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

* Re: [PATCH v2 2/5] iio: add processed write API
  2025-09-25 12:37 ` [PATCH v2 2/5] iio: add processed write API Romain Gantois
@ 2025-09-25 21:10   ` David Lechner
  2025-10-01  7:19     ` Romain Gantois
  2025-09-28  9:07   ` Jonathan Cameron
  1 sibling, 1 reply; 23+ messages in thread
From: David Lechner @ 2025-09-25 21:10 UTC (permalink / raw)
  To: Romain Gantois, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron, Nuno Sá,
	Andy Shevchenko
  Cc: Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio

On 9/25/25 7:37 AM, Romain Gantois wrote:
> Add a function to allow IIO consumers to write a processed value to a
> channel.
> 
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> ---
>  drivers/iio/inkern.c         | 120 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/consumer.h |  36 +++++++++++++
>  2 files changed, 156 insertions(+)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 2a1ecef2b82007f5ee8e8d0f8b35846bc4d2f94a..a6ec724b7c1fb197e6261c1162f8315deda20fd7 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -631,6 +631,57 @@ int iio_multiply_value(int *result, s64 multiplier,
>  }
>  EXPORT_SYMBOL_GPL(iio_multiply_value);
>  
> +int iio_divide_by_value(int *result, s64 numerator,
> +			unsigned int type, int val, int val2)
> +{
> +	s64 tmp_num, tmp_den;
> +
> +	switch (type) {
> +	case IIO_VAL_INT:
> +		tmp_num = numerator;
> +		tmp_den = val;
> +		break;
> +	case IIO_VAL_INT_PLUS_MICRO:
> +	case IIO_VAL_INT_PLUS_NANO:
> +		switch (type) {
> +		case IIO_VAL_INT_PLUS_MICRO:
> +			tmp_num = MICRO;
> +			tmp_den = MICRO;
> +			break;
> +
> +		case IIO_VAL_INT_PLUS_NANO:
> +			tmp_num = NANO;
> +			tmp_den = NANO;
> +			break;
> +		}
> +
> +		tmp_num *= (s64)numerator;
> +		tmp_den = (s64)abs(val) * tmp_den + (s64)abs(val2);
> +
> +		if (val < 0 || val2 < 0)
> +			tmp_num *= -1;
> +
> +		break;
> +	case IIO_VAL_FRACTIONAL:
> +		tmp_num = (s64)numerator * (s64)val2;
> +		tmp_den = val;
> +		break;
> +	case IIO_VAL_FRACTIONAL_LOG2:
> +		tmp_num = (s64)numerator << val2;
> +		tmp_den = val;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (!tmp_den)
> +		return -ERANGE;
> +
> +	*result = div64_s64(tmp_num, tmp_den);
> +
> +	return IIO_VAL_INT;
> +}
> +
>  static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
>  						 int raw, int *processed,
>  						 unsigned int scale)
> @@ -699,6 +750,53 @@ int iio_convert_raw_to_processed(struct iio_channel *chan, int raw,
>  }
>  EXPORT_SYMBOL_GPL(iio_convert_raw_to_processed);
>  
> +static int iio_convert_processed_to_raw_unlocked(struct iio_channel *chan,
> +						 int processed, int *raw,
> +						 unsigned int scale)
> +{
> +	int scale_type, scale_val, scale_val2;
> +	int offset_type, offset_val, offset_val2;
> +	int ret;
> +
> +	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
> +				      IIO_CHAN_INFO_SCALE);
> +	if (scale_type >= 0) {
> +		ret = iio_divide_by_value(raw, processed, scale_type, scale_val, scale_val2);
> +		if (ret < 0)
> +			return ret;
> +	} else {
> +		*raw = processed;
> +	}
> +
> +	if (!scale)
> +		return -ERANGE;
> +
> +	*raw = div_s64(*raw, scale);
> +
> +	offset_type = iio_channel_read(chan, &offset_val, &offset_val2,
> +				       IIO_CHAN_INFO_OFFSET);
> +	if (offset_type >= 0) {
> +		switch (offset_type) {
> +		case IIO_VAL_INT:
> +		case IIO_VAL_INT_PLUS_MICRO:
> +		case IIO_VAL_INT_PLUS_NANO:
> +			break;
> +		case IIO_VAL_FRACTIONAL:
> +			offset_val /= offset_val2;
> +			break;
> +		case IIO_VAL_FRACTIONAL_LOG2:
> +			offset_val >>= offset_val2;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		*raw -= offset_val;
> +	}

There are some rounding biases in this function, but I'm not sure if
it is worth trying to make a perfectly fair function.

> +
> +	return 0;
> +}
> +
>  int iio_read_channel_attribute(struct iio_channel *chan, int *val, int *val2,
>  			       enum iio_chan_info_enum attribute)
>  {
> @@ -1035,3 +1133,25 @@ ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf)
>  	return do_iio_read_channel_label(chan->indio_dev, chan->channel, buf);
>  }
>  EXPORT_SYMBOL_GPL(iio_read_channel_label);
> +
> +int iio_write_channel_processed_scale(struct iio_channel *chan, int val,
> +				      unsigned int scale)
> +{
> +	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan->indio_dev);
> +	int ret, processed;
> +
> +	scoped_guard(mutex, &iio_dev_opaque->info_exist_lock) {

Can just use regular guard(mutex)() here and save some indent.

> +		if (!chan->indio_dev->info)
> +			return -ENODEV;
> +
> +		ret = iio_convert_processed_to_raw_unlocked(chan, val, &processed, scale);
> +		if (ret)
> +			return ret;
> +
> +		ret = iio_channel_write(chan, processed, 0, IIO_CHAN_INFO_RAW);

And this can return directly.

> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_write_channel_processed_scale);
> +
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index c8c6261c81f934480e16854412e269207be60adc..dc84b8b4c61911d1a58427f1a9c798cae3954ac1 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -399,6 +399,24 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val,
>  int iio_multiply_value(int *result, s64 multiplier,
>  		       unsigned int type, int val, int val2);
>  
> +/**
> + * iio_divide_by_value() - Divide by an IIO value
> + * @result:	Destination pointer for the division result
> + * @numerator:	Numerator.
> + * @type:	One of the IIO_VAL_* constants. This decides how the @val
> + *		and @val2 parameters are interpreted.
> + * @val:	Denominator.
> + * @val2:	Denominator. @val2 use depends on type.
> + *
> + * Divide an s64 number by an IIO value, storing the result as
> + * IIO_VAL_INT. This is typically used for scaling.
> + *
> + * Returns:
> + * IIO_VAL_INT on success or a negative error-number on failure.
> + */
> +int iio_divide_by_value(int *result, s64 numerator,
> +			unsigned int type, int val, int val2);
> +
>  /**
>   * iio_convert_raw_to_processed() - Converts a raw value to a processed value
>   * @chan:		The channel being queried
> @@ -469,4 +487,22 @@ ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
>   */
>  ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf);
>  
> +/**
> + * iio_write_channel_processed_scale() - scale and write processed value to a given channel
> + * @chan:		The channel being queried.
> + * @val:		Value to write.
> + * @scale:		Scale factor to apply during the conversion

This could be more explicit about what sort of scaling this is.
The function divides by this factor rather than multiplies by it.

> + *
> + * This function writes a processed value to a channel. A processed value means
> + * that this value will have the correct unit and not some device internal
> + * representation. If the device does not support writing a processed value, the
> + * function will query the channel's scale and offset and write an appropriately
> + * transformed raw value.
> + *

Could be helpful even if obvious to most people...

	Context: May sleep.

> + * Return: an error code or 0.
> + *
> + */
> +int iio_write_channel_processed_scale(struct iio_channel *chan, int val,
> +				      unsigned int scale);
> +
>  #endif
> 


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

* Re: [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator
  2025-09-25 19:27   ` Conor Dooley
@ 2025-09-26 15:59     ` Romain Gantois
  2025-09-27 22:31       ` Conor Dooley
  0 siblings, 1 reply; 23+ messages in thread
From: Romain Gantois @ 2025-09-26 15:59 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Hans de Goede, Thomas Petazzoni, linux-kernel,
	devicetree, linux-iio

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

Hello Conor,

On Thursday, 25 September 2025 21:27:06 CEST Conor Dooley wrote:
> On Thu, Sep 25, 2025 at 02:37:33PM +0200, Romain Gantois wrote:
...
> > +properties:
> > +  compatible:
> > +    const: adi,ltm8054
> > +
> > +  enable-gpios:
> > +    description: GPIO connected to the RUN pin.
> > +    maxItems: 1
> > +
> 
> > +  lltc,fb-voltage-divider:
> Why does this property have a ?linear? vendor prefix?
> Shouldn't it be adi to match the other property and compatible?

This component was originally from Linear Technology, before it was acquired 
by Analog Devices. The new properties and compatibles have the Analog Devices 
prefix, but the "fb-voltage-divider" property is already used by the LTC3676 
and LTC3589 regulators, so I left the Linear Technology prefix for this one to 
avoid introducing a new property just to specify a vendor prefix change.

I don't have a strong opinion about this though.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator
  2025-09-26 15:59     ` Romain Gantois
@ 2025-09-27 22:31       ` Conor Dooley
  2025-10-01  7:11         ` Romain Gantois
  0 siblings, 1 reply; 23+ messages in thread
From: Conor Dooley @ 2025-09-27 22:31 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Hans de Goede, Thomas Petazzoni, linux-kernel,
	devicetree, linux-iio

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

On Fri, Sep 26, 2025 at 05:59:48PM +0200, Romain Gantois wrote:
> Hello Conor,
> 
> On Thursday, 25 September 2025 21:27:06 CEST Conor Dooley wrote:
> > On Thu, Sep 25, 2025 at 02:37:33PM +0200, Romain Gantois wrote:
> ...
> > > +properties:
> > > +  compatible:
> > > +    const: adi,ltm8054
> > > +
> > > +  enable-gpios:
> > > +    description: GPIO connected to the RUN pin.
> > > +    maxItems: 1
> > > +
> > 
> > > +  lltc,fb-voltage-divider:
> > Why does this property have a ?linear? vendor prefix?
> > Shouldn't it be adi to match the other property and compatible?
> 
> This component was originally from Linear Technology, before it was acquired 
> by Analog Devices. The new properties and compatibles have the Analog Devices 
> prefix, but the "fb-voltage-divider" property is already used by the LTC3676 
> and LTC3589 regulators, so I left the Linear Technology prefix for this one to 
> avoid introducing a new property just to specify a vendor prefix change.
> 
> I don't have a strong opinion about this though.

Do they share the same driver?

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

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

* Re: [PATCH v2 2/5] iio: add processed write API
  2025-09-25 12:37 ` [PATCH v2 2/5] iio: add processed write API Romain Gantois
  2025-09-25 21:10   ` David Lechner
@ 2025-09-28  9:07   ` Jonathan Cameron
  1 sibling, 0 replies; 23+ messages in thread
From: Jonathan Cameron @ 2025-09-28  9:07 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, David Lechner, Nuno Sá, Andy Shevchenko,
	Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio

On Thu, 25 Sep 2025 14:37:34 +0200
Romain Gantois <romain.gantois@bootlin.com> wrote:

> Add a function to allow IIO consumers to write a processed value to a
> channel.
> 
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
Just one trivial thing I noticed.
Thanks for adding the unit tests btw.  Makes me much more confident we don't have
the corner case sign errors that plagued the similar functions before unit tests and fixes
were applied.

J
> ---
>  drivers/iio/inkern.c         | 120 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/consumer.h |  36 +++++++++++++
>  2 files changed, 156 insertions(+)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 2a1ecef2b82007f5ee8e8d0f8b35846bc4d2f94a..a6ec724b7c1fb197e6261c1162f8315deda20fd7 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -631,6 +631,57 @@ int iio_multiply_value(int *result, s64 multiplier,
>  }
>  EXPORT_SYMBOL_GPL(iio_multiply_value);
>  
> +int iio_divide_by_value(int *result, s64 numerator,
> +			unsigned int type, int val, int val2)
> +{
> +	s64 tmp_num, tmp_den;
> +
> +	switch (type) {
> +	case IIO_VAL_INT:
> +		tmp_num = numerator;
> +		tmp_den = val;
> +		break;
> +	case IIO_VAL_INT_PLUS_MICRO:
> +	case IIO_VAL_INT_PLUS_NANO:
> +		switch (type) {
> +		case IIO_VAL_INT_PLUS_MICRO:
> +			tmp_num = MICRO;
> +			tmp_den = MICRO;
> +			break;
> +
> +		case IIO_VAL_INT_PLUS_NANO:
> +			tmp_num = NANO;
> +			tmp_den = NANO;
> +			break;
> +		}
> +
> +		tmp_num *= (s64)numerator;

Seems to be casting an s64 to an s64.

> +		tmp_den = (s64)abs(val) * tmp_den + (s64)abs(val2);
> +
> +		if (val < 0 || val2 < 0)
> +			tmp_num *= -1;
> +
> +		break;
> +	case IIO_VAL_FRACTIONAL:
> +		tmp_num = (s64)numerator * (s64)val2;
> +		tmp_den = val;
> +		break;
> +	case IIO_VAL_FRACTIONAL_LOG2:
> +		tmp_num = (s64)numerator << val2;
> +		tmp_den = val;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (!tmp_den)
> +		return -ERANGE;
> +
> +	*result = div64_s64(tmp_num, tmp_den);
> +
> +	return IIO_VAL_INT;
> +}


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

* Re: [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator
  2025-09-27 22:31       ` Conor Dooley
@ 2025-10-01  7:11         ` Romain Gantois
  2025-10-01 11:18           ` David Lechner
  0 siblings, 1 reply; 23+ messages in thread
From: Romain Gantois @ 2025-10-01  7:11 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Hans de Goede, Thomas Petazzoni, linux-kernel,
	devicetree, linux-iio

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

On Sunday, 28 September 2025 00:31:05 CEST Conor Dooley wrote:
...
> > > 
> > > > +  lltc,fb-voltage-divider:
> > > Why does this property have a ?linear? vendor prefix?
> > > Shouldn't it be adi to match the other property and compatible?
> > 
> > This component was originally from Linear Technology, before it was
> > acquired by Analog Devices. The new properties and compatibles have the
> > Analog Devices prefix, but the "fb-voltage-divider" property is already
> > used by the LTC3676 and LTC3589 regulators, so I left the Linear
> > Technology prefix for this one to avoid introducing a new property just
> > to specify a vendor prefix change.
> > 
> > I don't have a strong opinion about this though.
> 
> Do they share the same driver?

They do not. However, they use it in the exact same way, and I would've
liked to factor out the handling of this property in a future patch. This
would also make it easier to handle other types of feedback pin circuits
and have a generic binding for "regulators using a feedback pin connected
to some kind of analog circuit".

For example:

Vout----+      
        |      
        |      
       +++     
       | |     
       | | Rtop
       | |     
       +++     
        |      
        |      
 FB ----+      
        |      
     +--+--+   
     |  |  |   
     |  |  |CCS
     +--v--+   
        |      
        |      
       -+-     
        -      
 
This is all speculation at this point though, so I don't mind changing the
property to "adi,fb-voltage-divider" and handling the different compatibles
when it comes to it.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 2/5] iio: add processed write API
  2025-09-25 21:10   ` David Lechner
@ 2025-10-01  7:19     ` Romain Gantois
  2025-10-01 10:43       ` David Lechner
       [not found]       ` <CAMknhBG_o=jTKtHHDyK=bq7wcHMnDM1ZHaYAfX0K2hjHfkX3Bg@mail.gmail.com>
  0 siblings, 2 replies; 23+ messages in thread
From: Romain Gantois @ 2025-10-01  7:19 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
	David Lechner
  Cc: Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio

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

Hello David,

On Thursday, 25 September 2025 23:10:14 CEST David Lechner wrote:
> On 9/25/25 7:37 AM, Romain Gantois wrote:
> > Add a function to allow IIO consumers to write a processed value to a
...
> > +static int iio_convert_processed_to_raw_unlocked(struct iio_channel
> > *chan,
> > +						 int processed, int *raw,
> > +						 unsigned int scale)
> > +{
> > +	int scale_type, scale_val, scale_val2;
> > +	int offset_type, offset_val, offset_val2;
> > +	int ret;
> > +
> > +	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
> > +				      IIO_CHAN_INFO_SCALE);
> > +	if (scale_type >= 0) {
> > +		ret = iio_divide_by_value(raw, processed, scale_type, 
scale_val,
> > scale_val2); +		if (ret < 0)
> > +			return ret;
> > +	} else {
> > +		*raw = processed;
> > +	}
> > +
> > +	if (!scale)
> > +		return -ERANGE;
> > +
> > +	*raw = div_s64(*raw, scale);
> > +
> > +	offset_type = iio_channel_read(chan, &offset_val, &offset_val2,
> > +				       IIO_CHAN_INFO_OFFSET);
> > +	if (offset_type >= 0) {
> > +		switch (offset_type) {
> > +		case IIO_VAL_INT:
> > +		case IIO_VAL_INT_PLUS_MICRO:
> > +		case IIO_VAL_INT_PLUS_NANO:
> > +			break;
> > +		case IIO_VAL_FRACTIONAL:
> > +			offset_val /= offset_val2;
> > +			break;
> > +		case IIO_VAL_FRACTIONAL_LOG2:
> > +			offset_val >>= offset_val2;
> > +			break;
> > +		default:
> > +			return -EINVAL;
> > +		}
> > +
> > +		*raw -= offset_val;
> > +	}
> 
> There are some rounding biases in this function, but I'm not sure if
> it is worth trying to make a perfectly fair function.

I'm unfamiliar with the notion of rounding bias, does it mean that nested 
calls of this function would tend to amplify rounding errors? In this case, 
would rounding to the nearest integer instead of whatever is being done by the 
integer division here be a good solution?

Maybe I'm misunderstanding what you mean, in that case please let me know.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 2/5] iio: add processed write API
  2025-10-01  7:19     ` Romain Gantois
@ 2025-10-01 10:43       ` David Lechner
       [not found]       ` <CAMknhBG_o=jTKtHHDyK=bq7wcHMnDM1ZHaYAfX0K2hjHfkX3Bg@mail.gmail.com>
  1 sibling, 0 replies; 23+ messages in thread
From: David Lechner @ 2025-10-01 10:43 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
	Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio

Apologies if you are receiving the same message twice. Re-sending as
text email so that the mailing lists will pick up the reply.

On Wed, Oct 1, 2025 at 9:19 AM Romain Gantois
<romain.gantois@bootlin.com> wrote:
>
> Hello David,
>
> On Thursday, 25 September 2025 23:10:14 CEST David Lechner wrote:
> > On 9/25/25 7:37 AM, Romain Gantois wrote:
> > > Add a function to allow IIO consumers to write a processed value to a
> ...
> > > +static int iio_convert_processed_to_raw_unlocked(struct iio_channel
> > > *chan,
> > > +                                            int processed, int *raw,
> > > +                                            unsigned int scale)
> > > +{
> > > +   int scale_type, scale_val, scale_val2;
> > > +   int offset_type, offset_val, offset_val2;
> > > +   int ret;
> > > +
> > > +   scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
> > > +                                 IIO_CHAN_INFO_SCALE);
> > > +   if (scale_type >= 0) {
> > > +           ret = iio_divide_by_value(raw, processed, scale_type,
> scale_val,
> > > scale_val2); +              if (ret < 0)
> > > +                   return ret;
> > > +   } else {
> > > +           *raw = processed;
> > > +   }
> > > +
> > > +   if (!scale)
> > > +           return -ERANGE;
> > > +
> > > +   *raw = div_s64(*raw, scale);
> > > +
> > > +   offset_type = iio_channel_read(chan, &offset_val, &offset_val2,
> > > +                                  IIO_CHAN_INFO_OFFSET);
> > > +   if (offset_type >= 0) {
> > > +           switch (offset_type) {
> > > +           case IIO_VAL_INT:
> > > +           case IIO_VAL_INT_PLUS_MICRO:
> > > +           case IIO_VAL_INT_PLUS_NANO:
> > > +                   break;
> > > +           case IIO_VAL_FRACTIONAL:
> > > +                   offset_val /= offset_val2;
> > > +                   break;
> > > +           case IIO_VAL_FRACTIONAL_LOG2:
> > > +                   offset_val >>= offset_val2;
> > > +                   break;
> > > +           default:
> > > +                   return -EINVAL;
> > > +           }
> > > +
> > > +           *raw -= offset_val;
> > > +   }
> >
> > There are some rounding biases in this function, but I'm not sure if
> > it is worth trying to make a perfectly fair function.
>
> I'm unfamiliar with the notion of rounding bias, does it mean that nested
> calls of this function would tend to amplify rounding errors? In this case,
> would rounding to the nearest integer instead of whatever is being done by the
> integer division here be a good solution?

In this case, the issue is when you are taking multiple samples. When you
look at the average of all of the samples, you will be able to see the
bias. For example, in one of the drivers I was looking at there is an
offset of xxxx.6. Since the IIO_VAL_INT_PLUS_MICRO case is just dropping
any fractional part, the raw value will be on average 0.6 lsb lower that
the requested value. This could be a problem in an application where high
precision is required. But probably not noticeable in cases where 1 lsb is
less than the noise level.

The floor division for IIO_VAL_FRACTIONAL creates a similar bias.
DIV_ROUND_CLOSEST can help there, but even that has a small bias because
values of exactly 0.5 always get rounded in the same direction. That kind
of bias is much smaller though, so easier to ignore.


>
> Maybe I'm misunderstanding what you mean, in that case please let me know.
>
> Thanks,
>
> --
> Romain Gantois, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

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

* Re: [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator
  2025-10-01  7:11         ` Romain Gantois
@ 2025-10-01 11:18           ` David Lechner
  2025-10-01 18:40             ` Conor Dooley
  0 siblings, 1 reply; 23+ messages in thread
From: David Lechner @ 2025-10-01 11:18 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Conor Dooley, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron, Nuno Sá,
	Andy Shevchenko, Hans de Goede, Thomas Petazzoni, linux-kernel,
	devicetree, linux-iio

On Wed, Oct 1, 2025 at 9:12 AM Romain Gantois
<romain.gantois@bootlin.com> wrote:
>
> On Sunday, 28 September 2025 00:31:05 CEST Conor Dooley wrote:
> ...
> > > >
> > > > > +  lltc,fb-voltage-divider:
> > > > Why does this property have a ?linear? vendor prefix?
> > > > Shouldn't it be adi to match the other property and compatible?
> > >
> > > This component was originally from Linear Technology, before it was
> > > acquired by Analog Devices. The new properties and compatibles have the
> > > Analog Devices prefix, but the "fb-voltage-divider" property is already
> > > used by the LTC3676 and LTC3589 regulators, so I left the Linear
> > > Technology prefix for this one to avoid introducing a new property just
> > > to specify a vendor prefix change.
> > >
> > > I don't have a strong opinion about this though.
> >
> > Do they share the same driver?
>
> They do not. However, they use it in the exact same way, and I would've
> liked to factor out the handling of this property in a future patch. This
> would also make it easier to handle other types of feedback pin circuits
> and have a generic binding for "regulators using a feedback pin connected
> to some kind of analog circuit".
>
> For example:
>
> Vout----+
>         |
>         |
>        +++
>        | |
>        | | Rtop
>        | |
>        +++
>         |
>         |
>  FB ----+
>         |
>      +--+--+
>      |  |  |
>      |  |  |CCS
>      +--v--+
>         |
>         |
>        -+-
>         -
>
> This is all speculation at this point though, so I don't mind changing the
> property to "adi,fb-voltage-divider" and handling the different compatibles
> when it comes to it.
>

Could we just make it `fb-voltage-divider-ohms`? The -ohms suffix
makes it match the standard property-units suffix which already has
the uint32-array type. There are a couple of bindings that have
`vout-voltage-divider` without a vendor prefix, so it sounds like this
pattern is considered somewhat of a standard property already. But I
think it would be better with the -ohms suffix. For example, there is
already `gw,voltage-divider-ohms` as well. But there are so many
similar properties without the suffix, it is kind of the defacto
standard already, so might be better to stick with that rather than
making it even more different variants than there already are.

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

* Re: [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator
  2025-10-01 11:18           ` David Lechner
@ 2025-10-01 18:40             ` Conor Dooley
  2025-10-02  7:11               ` David Lechner
  0 siblings, 1 reply; 23+ messages in thread
From: Conor Dooley @ 2025-10-01 18:40 UTC (permalink / raw)
  To: David Lechner
  Cc: Romain Gantois, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron, Nuno Sá,
	Andy Shevchenko, Hans de Goede, Thomas Petazzoni, linux-kernel,
	devicetree, linux-iio

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

On Wed, Oct 01, 2025 at 01:18:51PM +0200, David Lechner wrote:
> On Wed, Oct 1, 2025 at 9:12 AM Romain Gantois
> <romain.gantois@bootlin.com> wrote:
> >
> > On Sunday, 28 September 2025 00:31:05 CEST Conor Dooley wrote:
> > ...
> > > > >
> > > > > > +  lltc,fb-voltage-divider:
> > > > > Why does this property have a ?linear? vendor prefix?
> > > > > Shouldn't it be adi to match the other property and compatible?
> > > >
> > > > This component was originally from Linear Technology, before it was
> > > > acquired by Analog Devices. The new properties and compatibles have the
> > > > Analog Devices prefix, but the "fb-voltage-divider" property is already
> > > > used by the LTC3676 and LTC3589 regulators, so I left the Linear
> > > > Technology prefix for this one to avoid introducing a new property just
> > > > to specify a vendor prefix change.
> > > >
> > > > I don't have a strong opinion about this though.
> > >
> > > Do they share the same driver?
> >
> > They do not. However, they use it in the exact same way, and I would've
> > liked to factor out the handling of this property in a future patch. This
> > would also make it easier to handle other types of feedback pin circuits
> > and have a generic binding for "regulators using a feedback pin connected
> > to some kind of analog circuit".
> >
> > For example:
> >
> > Vout----+
> >         |
> >         |
> >        +++
> >        | |
> >        | | Rtop
> >        | |
> >        +++
> >         |
> >         |
> >  FB ----+
> >         |
> >      +--+--+
> >      |  |  |
> >      |  |  |CCS
> >      +--v--+
> >         |
> >         |
> >        -+-
> >         -
> >
> > This is all speculation at this point though, so I don't mind changing the
> > property to "adi,fb-voltage-divider" and handling the different compatibles
> > when it comes to it.
> >
> 
> Could we just make it `fb-voltage-divider-ohms`? The -ohms suffix
> makes it match the standard property-units suffix which already has
> the uint32-array type. There are a couple of bindings that have
> `vout-voltage-divider` without a vendor prefix, so it sounds like this
> pattern is considered somewhat of a standard property already. But I
> think it would be better with the -ohms suffix. For example, there is
> already `gw,voltage-divider-ohms` as well. But there are so many
> similar properties without the suffix, it is kind of the defacto
> standard already, so might be better to stick with that rather than
> making it even more different variants than there already are.

Ye, by all means standardise it. I suppose that means insertion into
regulator.yaml, which usually also means a regulator- prefix - unless
you're eyeing something wider than that?

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

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

* Re: [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator
  2025-10-01 18:40             ` Conor Dooley
@ 2025-10-02  7:11               ` David Lechner
  0 siblings, 0 replies; 23+ messages in thread
From: David Lechner @ 2025-10-02  7:11 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Romain Gantois, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron, Nuno Sá,
	Andy Shevchenko, Hans de Goede, Thomas Petazzoni, linux-kernel,
	devicetree, linux-iio

On Wed, Oct 1, 2025 at 8:40 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Wed, Oct 01, 2025 at 01:18:51PM +0200, David Lechner wrote:
> > On Wed, Oct 1, 2025 at 9:12 AM Romain Gantois
> > <romain.gantois@bootlin.com> wrote:
> > >
> > > On Sunday, 28 September 2025 00:31:05 CEST Conor Dooley wrote:
> > > ...
> > > > > >
> > > > > > > +  lltc,fb-voltage-divider:
> > > > > > Why does this property have a ?linear? vendor prefix?
> > > > > > Shouldn't it be adi to match the other property and compatible?
> > > > >
> > > > > This component was originally from Linear Technology, before it was
> > > > > acquired by Analog Devices. The new properties and compatibles have the
> > > > > Analog Devices prefix, but the "fb-voltage-divider" property is already
> > > > > used by the LTC3676 and LTC3589 regulators, so I left the Linear
> > > > > Technology prefix for this one to avoid introducing a new property just
> > > > > to specify a vendor prefix change.
> > > > >
> > > > > I don't have a strong opinion about this though.
> > > >
> > > > Do they share the same driver?
> > >
> > > They do not. However, they use it in the exact same way, and I would've
> > > liked to factor out the handling of this property in a future patch. This
> > > would also make it easier to handle other types of feedback pin circuits
> > > and have a generic binding for "regulators using a feedback pin connected
> > > to some kind of analog circuit".
> > >
> > > For example:
> > >
> > > Vout----+
> > >         |
> > >         |
> > >        +++
> > >        | |
> > >        | | Rtop
> > >        | |
> > >        +++
> > >         |
> > >         |
> > >  FB ----+
> > >         |
> > >      +--+--+
> > >      |  |  |
> > >      |  |  |CCS
> > >      +--v--+
> > >         |
> > >         |
> > >        -+-
> > >         -
> > >
> > > This is all speculation at this point though, so I don't mind changing the
> > > property to "adi,fb-voltage-divider" and handling the different compatibles
> > > when it comes to it.
> > >
> >
> > Could we just make it `fb-voltage-divider-ohms`? The -ohms suffix
> > makes it match the standard property-units suffix which already has
> > the uint32-array type. There are a couple of bindings that have
> > `vout-voltage-divider` without a vendor prefix, so it sounds like this
> > pattern is considered somewhat of a standard property already. But I
> > think it would be better with the -ohms suffix. For example, there is
> > already `gw,voltage-divider-ohms` as well. But there are so many
> > similar properties without the suffix, it is kind of the defacto
> > standard already, so might be better to stick with that rather than
> > making it even more different variants than there already are.
>
> Ye, by all means standardise it. I suppose that means insertion into
> regulator.yaml, which usually also means a regulator- prefix - unless
> you're eyeing something wider than that?

Yes, there are also hwmon and iio bindings that are already already
using variations of ([a-z]+,)?(([a-x]+-)voltage-divider(-ohms)?.
Although `fb-voltage-divider` specifically seems to only be common for
regulators, so could make sense to just have
`regulator-fb-voltage-divider-ohms`.

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

* Re: [PATCH v2 2/5] iio: add processed write API
       [not found]       ` <CAMknhBG_o=jTKtHHDyK=bq7wcHMnDM1ZHaYAfX0K2hjHfkX3Bg@mail.gmail.com>
@ 2025-10-03 14:35         ` Romain Gantois
  0 siblings, 0 replies; 23+ messages in thread
From: Romain Gantois @ 2025-10-03 14:35 UTC (permalink / raw)
  To: David Lechner
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
	Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio

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

On Wednesday, 1 October 2025 12:03:21 CEST David Lechner wrote:
> On Wed, Oct 1, 2025 at 9:19 AM Romain Gantois <romain.gantois@bootlin.com>
> 
...
> > > > +           case IIO_VAL_INT_PLUS_MICRO:
> > > > +           case IIO_VAL_INT_PLUS_NANO:
> > > > +                   break;
> > > > +           case IIO_VAL_FRACTIONAL:
> > > > +                   offset_val /= offset_val2;
> > > > +                   break;
> > > > +           case IIO_VAL_FRACTIONAL_LOG2:
> > > > +                   offset_val >>= offset_val2;
> > > > +                   break;
> > > > +           default:
> > > > +                   return -EINVAL;
> > > > +           }
> > > > +
> > > > +           *raw -= offset_val;
> > > > +   }
> > > 
> > > There are some rounding biases in this function, but I'm not sure if
> > > it is worth trying to make a perfectly fair function.
> > 
> > I'm unfamiliar with the notion of rounding bias, does it mean that nested
> > calls of this function would tend to amplify rounding errors? In this
> > case,
> > would rounding to the nearest integer instead of whatever is being done by
> > the
> > integer division here be a good solution?
> 
> In this case, the issue is when you are taking multiple samples. When you
> look at the average of all of the samples, you will be able to see the
> bias. For example, in one of the drivers I was looking at there is an
> offset of xxxx.6. Since the IIO_VAL_INT_PLUS_MICRO case is just dropping
> any fractional part, the raw value will be on average 0.6 lsb lower that
> the requested value. This could be a problem in an application where high
> precision is required. But probably not noticeable in cases where 1 lsb is
> less than the noise level.
> 

Thanks a lot for the detailed explanation. For the IIO_VAL_INT_PLUS_MICRO/NANO 
cases, I think that scaling by MICRO/NANO, then subtracting the offset, then 
dividing and rounding to the closest would give a small precision improvement 
in some cases. It would be a bit slower though, but for low sample-rate 
devices like the ones in IIO I don't think it would be noticeable. I'll give 
it a try.

> The floor division for IIO_VAL_FRACTIONAL creates a similar bias.
> DIV_ROUND_CLOSEST can help there, but even that has a small bias because
> values of exactly 0.5 always get rounded in the same direction. That kind
> of bias is much smaller though, so easier to ignore.
> 

DIV_ROUND_CLOSEST would indeed reduce the bias at no substantial cost, 
so I think I'll go with that.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 5/5] regulator: ltm8054: Support output current limit control
  2025-09-25 12:37 ` [PATCH v2 5/5] regulator: ltm8054: Support output current limit control Romain Gantois
@ 2025-10-22  8:05   ` Romain Gantois
  2025-10-22 16:37     ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Romain Gantois @ 2025-10-22  8:05 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: Hans de Goede, Thomas Petazzoni, linux-kernel, devicetree,
	linux-iio, Herve Codina

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

Hello everyone,

I've encountered a rather troublesome issue with this particular patch,
which has delayed version 3 of this series. I'd like to describe it here,
so that you can tell me if you have any suggestions for an upstreamable
solution.
 
The problem concerns the set_current_limit() and get_current_limit()
callbacks:

On Thursday, 25 September 2025 14:37:37 CEST Romain Gantois wrote:
...
> -static const struct regulator_ops ltm8054_regulator_ops = { };
> +static int ltm8054_set_current_limit(struct regulator_dev *rdev, int
> min_uA, int max_uA) +{
> +	struct ltm8054_priv *priv = rdev_get_drvdata(rdev);
> +	u64 vdac_uV;
> +
> +	min_uA = clamp_t(int, min_uA, priv->min_uA, priv->max_uA);
> +
> +	/* adjusted current limit = Rsense current limit * CTL pin voltage / 
max
> CTL pin voltage */ +	vdac_uV = (u64)min_uA * LTM8054_MAX_CTL_uV;
> +	do_div(vdac_uV, priv->max_uA);
> +
> +	dev_dbg(&rdev->dev,
> +		"Setting current limit to %duA, CTL pin to %duV\n", min_uA,
> (int)vdac_uV); +
> +	/* Standard IIO voltage unit is mV, scale accordingly. */
> +	return iio_write_channel_processed_scale(priv->ctl_dac, vdac_uV, 
1000);
> +}
> +
> +static int ltm8054_get_current_limit(struct regulator_dev *rdev)
> +{
> +	struct ltm8054_priv *priv = rdev_get_drvdata(rdev);
> +	int ret, vdac_uv;
> +	u64 uA;
> +
> +	ret = iio_read_channel_processed_scale(priv->ctl_dac, &vdac_uv, 1000);
> +	if (ret < 0) {
> +		dev_err(&rdev->dev, "failed to read CTL DAC voltage, err %d\n", 
ret);
> +		return ret;
> +	}
> +
> +	uA = (u64)vdac_uv * priv->max_uA;
> +	do_div(uA, LTM8054_MAX_CTL_uV);
> +
> +	return uA;
> +}
> +
> +static const struct regulator_ops ltm8054_regulator_ops = {
> +	.set_current_limit = ltm8054_set_current_limit,
> +	.get_current_limit = ltm8054_get_current_limit,
> +};
> +
...

I've encountered a lockdep splat while testing these callbacks. I've
included a summary of the splat at the end of this email [1].
 
After investigating, it seems like the issue lies with IIO callbacks in the
ad5592r driver being called with the LTM8054 regulator device lock held.
 
The ad5592r callbacks themselves call into the regulator core to enable the
DAC's regulators, which might try the LTM8054 lock again in the same
thread, causing a deadlock. This would only happen if the LTM8054 was
supplying voltage to the ad5592r.
 
There are two parts to this issue:

1. Making sure that the CTL IIO channel used by an LTM8054 device isn't
supplied by the LTM8054 itself (or a consumer of the LTM8054). Solving this 
removes the risk of an actual deadlock.
 
2. Silencing the lockdep splat. The splat seems to be triggered by the IIO
driver taking the general regulator ww_mutex context, which means it will
still occur even if we've made sure that the IIO channel isn't a consumer
of the LTM8054 regulator.

For part 1., a potential solution would be to create a device link with the
LTM8054 device as a consumer and the CTL IIO channel as a supplier. IIUC
device links do not tolerate cycles, so this should ensure that the IIO
channel isn't a direct or indirect consumer of the LTM8054.

However, the LTM8054 driver cannot access the IIO device struct to create the 
link, so adding a new IIO consumer API function could be necessary.
 
For part 2., I'm having more trouble finding a proper solution. One
potential fix would be to put the IIO channel reads/writes in a LTM8054
driver work item and have them run without the regulator lock held. This
would incidentally also solve part 1., however it would make the current
limit operations asynchronous, and it seems like a lot of unnecessary
complexity.
 
Please tell me if you have any suggestions for solving this, I'll keep
searching on my side in the meantime.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[1] lockdep splat summary
```
WARNING: possible circular locking dependency detected
6.17.0-rc6+ #9 Not tainted
------------------------------------------------------
kworker/u17:0/34 is trying to acquire lock:
(&iio_dev_opaque->info_exist_lock){+.+.}-{4:4}, at: 
iio_read_channel_processed_scale+0x40/0x120
   
but task is already holding lock:
(regulator_ww_class_mutex){+.+.}-{4:4}, at: ww_mutex_trylock+0x184/0x3a0
   
which lock already depends on the new lock.
   
   
the existing dependency chain (in reverse order) is:
   
-> #2 (regulator_ww_class_mutex){+.+.}-{4:4}:
       lock_acquire+0xf0/0x2c0
       regulator_lock_dependent+0x120/0x270
       regulator_enable+0x38/0xd0
       ad5592r_probe+0xcc/0x630
       ad5593r_i2c_probe+0x58/0x80
   ...
       ret_from_fork+0x10/0x20
   
-> #1 (regulator_ww_class_acquire){+.+.}-{0:0}:
       reacquire_held_locks+0xd4/0x1c0
       lock_release+0x148/0x2c0
       __mutex_unlock_slowpath+0x3c/0x2f0
       mutex_unlock+0x1c/0x30
       regulator_lock_dependent+0x1d4/0x270
       regulator_get_voltage+0x34/0xd0
       ad5592r_read_raw+0x154/0x2f0
       iio_channel_read.isra.0+0xac/0xd0
       iio_write_channel_processed_scale+0x64/0x1e0
       ltm8054_set_current_limit+0x70/0xd0
   ...
       ret_from_fork+0x10/0x20
   
-> #0 (&iio_dev_opaque->info_exist_lock){+.+.}-{4:4}:
       check_prev_add+0x104/0xc60
       __lock_acquire+0x12a4/0x15c0
       lock_acquire+0xf0/0x2c0
       __mutex_lock+0x90/0xc80
       mutex_lock_nested+0x28/0x40
       iio_read_channel_processed_scale+0x40/0x120
       ltm8054_get_current_limit+0x34/0xa0
       kthread+0x11c/0x1f0
   ...
       ret_from_fork+0x10/0x20
   
other info that might help us debug this:
   
Chain exists of:
  &iio_dev_opaque->info_exist_lock --> regulator_ww_class_acquire --> 
regulator_ww_class_mutex
   
 Possible unsafe locking scenario:
   
       CPU0                    CPU1
       ----                    ----
  lock(regulator_ww_class_mutex);
                               lock(regulator_ww_class_acquire);
                               lock(regulator_ww_class_mutex);
  lock(&iio_dev_opaque->info_exist_lock);
   
 *** DEADLOCK ***
```

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 4/5] regulator: Support the LTM8054 voltage regulator
  2025-09-25 12:37 ` [PATCH v2 4/5] regulator: Support the LTM8054 voltage regulator Romain Gantois
@ 2025-10-22 14:44   ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2025-10-22 14:44 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Hans de Goede, Thomas Petazzoni, linux-kernel,
	devicetree, linux-iio

On Thu, Sep 25, 2025 at 02:37:36PM +0200, Romain Gantois wrote:
> Add a stub driver for the  Linear Technology LTM8054 Buck-Boost voltage
> regulator. This version only supports enabling/disabling the regulator via
> a GPIO, and reporting the output voltage level from the resistor divider
> values given in the device tree.

...

It's a bit an interesting grouping of headers...

> +#include <asm/div64.h>

...starting with leading asm/*.

> +#include <linux/array_size.h>
> +
> +#include <linux/device.h>
> +#include <linux/device/devres.h>
> +#include <linux/device/driver.h>
> +
> +#include <linux/dev_printk.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>

I would expect above to be (but not limited to)

#include <linux/array_size.h>
#include <linux/device.h>
#include <linux/device/devres.h>
#include <linux/device/driver.h>
#include <linux/dev_printk.h>
#include <linux/err.h>

#include <linux/errno.h>

#include <linux/gpio/consumer.h>
#include <linux/math64.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/property.h>

Also missing
types.h

> +#include <linux/regulator/consumer.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/of_regulator.h>

...

> +static int ltm8054_of_parse(struct device *dev, struct ltm8054_priv *priv,
> +			    struct regulator_config *config)
> +{
> +	struct device_node *np = dev->of_node;

No need, see below how.

> +	u32 r[2];
> +	int ret;

> +	config->of_node = np;

Better to move it...

> +
> +	ret = device_property_read_u32_array(dev, "lltc,fb-voltage-divider", r, ARRAY_SIZE(r));
> +	if (ret)
> +		return ret;
> +
> +	priv->rdesc.fixed_uV = ltm8054_scale(LTM8054_FB_uV, r[0], r[1]);
> +	priv->rdesc.min_uV = priv->rdesc.fixed_uV;
> +	priv->rdesc.n_voltages = 1;

...here and reuse.

> +	config->init_data = of_get_regulator_init_data(dev,
> +						       np,
> +						       &priv->rdesc);

	config->of_node = dev_of_node(dev);
	config->init_data = of_get_regulator_init_data(dev,
						       config->of_node,
						       &priv->rdesc);

> +	if (!config->init_data)
> +		return -EINVAL;
> +
> +	config->ena_gpiod = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
> +	if (IS_ERR(config->ena_gpiod))
> +		return PTR_ERR(config->ena_gpiod);
> +
> +	return 0;
> +}

> +static int ltm8054_probe(struct platform_device *pdev)
> +{
> +	struct regulator_config config = { };
> +	struct device *dev = &pdev->dev;
> +	struct regulator_dev *rdev;
> +	struct ltm8054_priv *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->rdesc.name = "ltm8054-regulator",
> +	priv->rdesc.ops = &ltm8054_regulator_ops,
> +	priv->rdesc.type = REGULATOR_VOLTAGE,
> +	priv->rdesc.owner = THIS_MODULE,

The commas should be replaced by semicolons.

> +	config.dev = dev;
> +	config.driver_data = priv;
> +
> +	ret = ltm8054_of_parse(dev, priv, &config);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to parse device tree\n");
> +
> +	rdev = devm_regulator_register(dev, &priv->rdesc, &config);
> +	if (IS_ERR(rdev))
> +		return dev_err_probe(dev, PTR_ERR(rdev), "failed to register regulator\n");
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v2 5/5] regulator: ltm8054: Support output current limit control
  2025-10-22  8:05   ` Romain Gantois
@ 2025-10-22 16:37     ` Andy Shevchenko
  2025-10-24  7:55       ` Romain Gantois
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2025-10-22 16:37 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Hans de Goede, Thomas Petazzoni, linux-kernel,
	devicetree, linux-iio, Herve Codina

On Wed, Oct 22, 2025 at 11:06 AM Romain Gantois
<romain.gantois@bootlin.com> wrote:

...

> I've encountered a lockdep splat while testing these callbacks. I've
> included a summary of the splat at the end of this email [1].
>
> After investigating, it seems like the issue lies with IIO callbacks in the
> ad5592r driver being called with the LTM8054 regulator device lock held.
>
> The ad5592r callbacks themselves call into the regulator core to enable the
> DAC's regulators, which might try the LTM8054 lock again in the same
> thread, causing a deadlock. This would only happen if the LTM8054 was
> supplying voltage to the ad5592r.
>
> There are two parts to this issue:
>
> 1. Making sure that the CTL IIO channel used by an LTM8054 device isn't
> supplied by the LTM8054 itself (or a consumer of the LTM8054). Solving this
> removes the risk of an actual deadlock.
>
> 2. Silencing the lockdep splat. The splat seems to be triggered by the IIO
> driver taking the general regulator ww_mutex context, which means it will
> still occur even if we've made sure that the IIO channel isn't a consumer
> of the LTM8054 regulator.
>
> For part 1., a potential solution would be to create a device link with the
> LTM8054 device as a consumer and the CTL IIO channel as a supplier. IIUC
> device links do not tolerate cycles, so this should ensure that the IIO
> channel isn't a direct or indirect consumer of the LTM8054.
>
> However, the LTM8054 driver cannot access the IIO device struct to create the
> link, so adding a new IIO consumer API function could be necessary.
>
> For part 2., I'm having more trouble finding a proper solution. One
> potential fix would be to put the IIO channel reads/writes in a LTM8054
> driver work item and have them run without the regulator lock held. This
> would incidentally also solve part 1., however it would make the current
> limit operations asynchronous, and it seems like a lot of unnecessary
> complexity.

Interesting that locking a single  regulator, there is no context and
hence the lock class is global. Hence whoever calls a regulator will
have the same lockdep splat, even when false positive. Basically the
solution for those cases (and I don't know if yours / this one falls
into the category) is to enable context for the single regulator
locking and set up a lockdep class (so the regulator core should call
lockdep_set_class() at mutex initialisation).

> Please tell me if you have any suggestions for solving this, I'll keep
> searching on my side in the meantime.

--
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v2 5/5] regulator: ltm8054: Support output current limit control
  2025-10-22 16:37     ` Andy Shevchenko
@ 2025-10-24  7:55       ` Romain Gantois
  0 siblings, 0 replies; 23+ messages in thread
From: Romain Gantois @ 2025-10-24  7:55 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Hans de Goede, Thomas Petazzoni, linux-kernel,
	devicetree, linux-iio, Herve Codina

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

On Wednesday, 22 October 2025 18:37:31 CEST Andy Shevchenko wrote:
> On Wed, Oct 22, 2025 at 11:06 AM Romain Gantois
> <romain.gantois@bootlin.com> wrote:
> 
> ...
> > For part 2., I'm having more trouble finding a proper solution. One
> > potential fix would be to put the IIO channel reads/writes in a LTM8054
> > driver work item and have them run without the regulator lock held. This
> > would incidentally also solve part 1., however it would make the current
> > limit operations asynchronous, and it seems like a lot of unnecessary
> > complexity.
> 
> Interesting that locking a single  regulator, there is no context and
> hence the lock class is global. Hence whoever calls a regulator will
> have the same lockdep splat, even when false positive. Basically the
> solution for those cases (and I don't know if yours / this one falls
> into the category) is to enable context for the single regulator
> locking and set up a lockdep class (so the regulator core should call
> lockdep_set_class() at mutex initialisation).

The strange part is that this "global lock" is actually a lockdep-provided
mutex which isn't taken when lockdep is disabled. It seems to be there to
ensure that ww_mutex contexts are not taken recursively, but then again the
IIO driver is calling regulator_lock_*nested*() so it sounds like
recursively taking the ww_mutex context might be expected here...

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2025-10-24  7:55 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-25 12:37 [PATCH v2 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
2025-09-25 12:37 ` [PATCH v2 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator Romain Gantois
2025-09-25 19:27   ` Conor Dooley
2025-09-26 15:59     ` Romain Gantois
2025-09-27 22:31       ` Conor Dooley
2025-10-01  7:11         ` Romain Gantois
2025-10-01 11:18           ` David Lechner
2025-10-01 18:40             ` Conor Dooley
2025-10-02  7:11               ` David Lechner
2025-09-25 12:37 ` [PATCH v2 2/5] iio: add processed write API Romain Gantois
2025-09-25 21:10   ` David Lechner
2025-10-01  7:19     ` Romain Gantois
2025-10-01 10:43       ` David Lechner
     [not found]       ` <CAMknhBG_o=jTKtHHDyK=bq7wcHMnDM1ZHaYAfX0K2hjHfkX3Bg@mail.gmail.com>
2025-10-03 14:35         ` Romain Gantois
2025-09-28  9:07   ` Jonathan Cameron
2025-09-25 12:37 ` [PATCH v2 3/5] Add kunit tests for iio_divide_by_value() Romain Gantois
2025-09-25 20:26   ` David Lechner
2025-09-25 12:37 ` [PATCH v2 4/5] regulator: Support the LTM8054 voltage regulator Romain Gantois
2025-10-22 14:44   ` Andy Shevchenko
2025-09-25 12:37 ` [PATCH v2 5/5] regulator: ltm8054: Support output current limit control Romain Gantois
2025-10-22  8:05   ` Romain Gantois
2025-10-22 16:37     ` Andy Shevchenko
2025-10-24  7:55       ` Romain Gantois

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).