devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Add support for the LTM8054 voltage regulator
@ 2025-11-06 14:11 Romain Gantois
  2025-11-06 14:11 ` [PATCH v3 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator Romain Gantois
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Romain Gantois @ 2025-11-06 14:11 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,

This is version three 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 most 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.

Moreover, the IIO read/write operations are done in quite a roundabout way
in the driver's regulator callbacks. This is due to a unsafe locking
interaction: the regulator callbacks are called under regulator lock, which
means they have an active ww_mutex reservation ID. Or, some IIO drivers
will perform regulator operations when read/written to, thus taking a new
ww_mutex reservation ID.  Taking two consecutive reservation IDs for the
same ww_mutex context without freeing the first ID is forbidden (and
lockdep will complain about this). The most straightforward solution I've
found is to move the actual IIO read/write operations to a different
thread, and to wait for them to complete with a timeout from the main
callback thread.

Please let me know what you think.

Thanks,

Romain

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
Changes in v3:
- Made IIO operations to an asynchronous context to avoid locking issue.
- Made CTL DAC control optional.
- Make various style adjustments to the LTM8054 driver.
- Link to v2: https://lore.kernel.org/r/20250925-ltm8054-driver-v2-0-bb61a401a0dc@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
      iio: test: 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 |  71 ++++
 MAINTAINERS                                        |   6 +
 drivers/iio/inkern.c                               | 129 +++++++
 drivers/iio/test/Kconfig                           |  12 +
 drivers/iio/test/Makefile                          |   1 +
 drivers/iio/test/iio-test-divide.c                 | 215 ++++++++++++
 drivers/regulator/Kconfig                          |   9 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/ltm8054-regulator.c              | 386 +++++++++++++++++++++
 include/linux/iio/consumer.h                       |  37 ++
 10 files changed, 867 insertions(+)
---
base-commit: 959f5c38d89070dc078ec2097161f871397ea3f1
change-id: 20250728-ltm8054-driver-11cfa4741065

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


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

* [PATCH v3 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator
  2025-11-06 14:11 [PATCH v3 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
@ 2025-11-06 14:11 ` Romain Gantois
  2025-11-06 17:23   ` Conor Dooley
  2025-11-06 14:11 ` [PATCH v3 2/5] iio: add processed write API Romain Gantois
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Romain Gantois @ 2025-11-06 14:11 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 | 71 ++++++++++++++++++++++
 MAINTAINERS                                        |  5 ++
 2 files changed, 76 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml b/Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml
new file mode 100644
index 000000000000..a982daecb4cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/adi,ltm8054.yaml
@@ -0,0 +1,71 @@
+# 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
+
+  regulator-fb-voltage-divider-ohms:
+    description: Feedback voltage divider resistor values, in Ohms.
+    items:
+      - description: Top resistor value.
+      - description: Bottom resistor value.
+
+  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
+  - regulator-fb-voltage-divider-ohms
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    regulator {
+        compatible = "adi,ltm8054";
+
+        enable-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+
+        regulator-fb-voltage-divider-ohms = <1000000 68000>;
+
+        adi,iout-rsense-micro-ohms = <20000>;
+
+        io-channels = <&dac 1>;
+        io-channel-names = "ctl";
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 8fd3be0162dc..96552be2fcdd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14785,6 +14785,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.2


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

* [PATCH v3 2/5] iio: add processed write API
  2025-11-06 14:11 [PATCH v3 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
  2025-11-06 14:11 ` [PATCH v3 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator Romain Gantois
@ 2025-11-06 14:11 ` Romain Gantois
  2025-11-06 16:07   ` Andy Shevchenko
  2025-11-06 14:11 ` [PATCH v3 3/5] iio: test: Add kunit tests for iio_divide_by_value() Romain Gantois
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Romain Gantois @ 2025-11-06 14:11 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         | 129 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/consumer.h |  37 +++++++++++++
 2 files changed, 166 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 1e5eb5a41271..bd0bbaef6045 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -635,6 +635,57 @@ int iio_multiply_value(int *result, s64 multiplier,
 }
 EXPORT_SYMBOL_NS_GPL(iio_multiply_value, "IIO_UNIT_TEST");
 
+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 *= 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)
@@ -703,6 +754,64 @@ 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, half_step = 0;
+
+	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:
+			half_step = MICRO / 2;
+			break;
+		case IIO_VAL_INT_PLUS_NANO:
+			half_step = NANO / 2;
+			break;
+		case IIO_VAL_FRACTIONAL:
+			offset_val = DIV_ROUND_CLOSEST(offset_val, offset_val2);
+			break;
+		case IIO_VAL_FRACTIONAL_LOG2:
+			offset_val >>= offset_val2;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		/* Round fractional part to closest to reduce rounding bias. */
+		if (half_step) {
+			if (offset_val2 >= half_step)
+				*raw -= 1;
+			else if (offset_val2 <= -half_step)
+				*raw += 1;
+		}
+
+		*raw -= offset_val;
+	}
+
+	return 0;
+}
+
 int iio_read_channel_attribute(struct iio_channel *chan, int *val, int *val2,
 			       enum iio_chan_info_enum attribute)
 {
@@ -1039,3 +1148,23 @@ 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;
+
+	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;
+
+	return iio_channel_write(chan, processed, 0, IIO_CHAN_INFO_RAW);
+}
+EXPORT_SYMBOL_GPL(iio_write_channel_processed_scale);
+
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index a38b277c2c02..29d08b57bac9 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,23 @@ 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:		Processed value is divided by this scale factor 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.
+ *
+ * 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

-- 
2.51.2


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

* [PATCH v3 3/5] iio: test: Add kunit tests for iio_divide_by_value()
  2025-11-06 14:11 [PATCH v3 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
  2025-11-06 14:11 ` [PATCH v3 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator Romain Gantois
  2025-11-06 14:11 ` [PATCH v3 2/5] iio: add processed write API Romain Gantois
@ 2025-11-06 14:11 ` Romain Gantois
  2025-11-06 16:10   ` Andy Shevchenko
  2025-11-06 14:11 ` [PATCH v3 4/5] regulator: Support the LTM8054 voltage regulator Romain Gantois
  2025-11-06 14:11 ` [PATCH v3 5/5] regulator: ltm8054: Support output current limit control Romain Gantois
  4 siblings, 1 reply; 16+ messages in thread
From: Romain Gantois @ 2025-11-06 14:11 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 | 215 +++++++++++++++++++++++++++++++++++++
 3 files changed, 228 insertions(+)

diff --git a/drivers/iio/test/Kconfig b/drivers/iio/test/Kconfig
index 6e65e929791c..3aa1fc78966c 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 0c846bc21acd..16344eedc46a 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 000000000000..d117f868dd76
--- /dev/null
+++ b/drivers/iio/test/iio-test-divide.c
@@ -0,0 +1,215 @@
+// 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, -EDOM);
+}
+
+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;
+	val2 = 0;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, -EDOM);
+}
+
+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;
+	val2 = 0;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, -EDOM);
+}
+
+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 * 1024), 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 * 1024), 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 * 1024), 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 * 1024), val));
+
+	/* Zero */
+	val = 0;
+	val2 = 0;
+	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, val2);
+	KUNIT_EXPECT_EQ(test, ret, -EDOM);
+}
+
+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.2


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

* [PATCH v3 4/5] regulator: Support the LTM8054 voltage regulator
  2025-11-06 14:11 [PATCH v3 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
                   ` (2 preceding siblings ...)
  2025-11-06 14:11 ` [PATCH v3 3/5] iio: test: Add kunit tests for iio_divide_by_value() Romain Gantois
@ 2025-11-06 14:11 ` Romain Gantois
  2025-11-06 18:08   ` Andy Shevchenko
  2025-11-06 14:11 ` [PATCH v3 5/5] regulator: ltm8054: Support output current limit control Romain Gantois
  4 siblings, 1 reply; 16+ messages in thread
From: Romain Gantois @ 2025-11-06 14:11 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 | 125 ++++++++++++++++++++++++++++++++++
 4 files changed, 135 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 96552be2fcdd..45906509508d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14789,6 +14789,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 403890a76070..f5c6d4a21a88 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -585,6 +585,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 b3101376029d..f2687755c291 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -71,6 +71,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 000000000000..b5783f6629e3
--- /dev/null
+++ b/drivers/regulator/ltm8054-regulator.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices LTM8054 Buck-Boost regulator driver
+ *
+ * Copyright (C) 2025 Bootlin
+ */
+
+#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>
+
+#include <linux/regulator/consumer.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/types.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)
+{
+	u32 r[2];
+	int ret;
+
+	ret = device_property_read_u32_array(dev, "regulator-fb-voltage-divider-ohms",
+					     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->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;
+
+	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.2


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

* [PATCH v3 5/5] regulator: ltm8054: Support output current limit control
  2025-11-06 14:11 [PATCH v3 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
                   ` (3 preceding siblings ...)
  2025-11-06 14:11 ` [PATCH v3 4/5] regulator: Support the LTM8054 voltage regulator Romain Gantois
@ 2025-11-06 14:11 ` Romain Gantois
  2025-11-06 18:32   ` Andy Shevchenko
  4 siblings, 1 reply; 16+ messages in thread
From: Romain Gantois @ 2025-11-06 14:11 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 | 273 +++++++++++++++++++++++++++++++++-
 2 files changed, 268 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index f5c6d4a21a88..aad8c523420a 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -587,6 +587,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 b5783f6629e3..38072231b8e4 100644
--- a/drivers/regulator/ltm8054-regulator.c
+++ b/drivers/regulator/ltm8054-regulator.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/array_size.h>
+#include <linux/completion.h>
 #include <linux/device.h>
 #include <linux/device/devres.h>
 #include <linux/device/driver.h>
@@ -15,7 +16,11 @@
 #include <linux/errno.h>
 
 #include <linux/gpio/consumer.h>
+#include <linux/iio/consumer.h>
+#include <linux/jiffies.h>
+#include <linux/lockdep.h>
 #include <linux/math64.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -26,10 +31,42 @@
 #include <linux/regulator/of_regulator.h>
 #include <linux/types.h>
 
+#include <linux/units.h>
+#include <linux/workqueue.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
+
+#define LTM8054_CTL_RW_TIMEOUT msecs_to_jiffies(500)
+
+/* CTL pin read/write transaction */
+struct ltm8054_ctl_pin_work {
+	struct work_struct work;
+	unsigned int ctl_val;
+	bool write;
+	int ret;
+};
+
 struct ltm8054_priv {
+	struct device *dev;
+
+	struct iio_channel *ctl_dac;
+	struct ltm8054_ctl_pin_work ctl_work;
+	/* Lock for ctl_work. */
+	struct mutex ctl_work_lock;
+	struct completion ctl_rw_done;
+
+	int min_uA;
+	int max_uA;
+
 	struct regulator_desc rdesc;
 };
 
@@ -43,14 +80,190 @@ static int ltm8054_scale(unsigned int uV, u32 r1, u32 r2)
 	return uV + tmp;
 }
 
-static const struct regulator_ops ltm8054_regulator_ops = { };
+static void ltm8054_do_ctl_work(struct work_struct *work)
+{
+	struct ltm8054_ctl_pin_work *ctl_work = container_of_const(work,
+								   struct ltm8054_ctl_pin_work,
+								   work);
+	struct ltm8054_priv *priv = container_of_const(ctl_work,
+						       struct ltm8054_priv,
+						       ctl_work);
+	unsigned int val;
+	bool write;
+	int ret;
+
+	lockdep_assert_not_held(&priv->ctl_work_lock);
+
+	mutex_lock(&priv->ctl_work_lock);
+	val = ctl_work->ctl_val;
+	write = ctl_work->write;
+	mutex_unlock(&priv->ctl_work_lock);
+
+	/* Standard IIO voltage unit is mV, scale accordingly. */
+	if (write)
+		ret = iio_write_channel_processed_scale(priv->ctl_dac,
+							val, 1000);
+	else
+		ret = iio_read_channel_processed_scale(priv->ctl_dac,
+						       &val, 1000);
+
+	pr_debug("LTM8054: %s CTL IO channel, val: %duV\n", write ? "wrote" : "reading", val);
+
+	mutex_lock(&priv->ctl_work_lock);
+	ctl_work->ret = ret;
+	ctl_work->ctl_val = val;
+	mutex_unlock(&priv->ctl_work_lock);
+
+	complete(&priv->ctl_rw_done);
+}
+
+static int ltm8054_ctl_pin_rw(struct ltm8054_priv *priv, bool write, unsigned int *ctl_val)
+{
+	struct ltm8054_ctl_pin_work *ctl_work = &priv->ctl_work;
+	int ret = 0;
+
+	lockdep_assert_not_held(&priv->ctl_work_lock);
+
+	/* The get/set_current_limit() callbacks have an active regulator core
+	 * reservation ID (obtained with ww_acquire_init()).
+	 *
+	 * Or, the IO channel driver may call something like
+	 * regulator_enable(), meaning this thread would acquire a new
+	 * regulator core reservation ID before the current one is dropped
+	 * (using ww_acquire_fini()). This is forbidden.
+	 *
+	 * Thus, perform the IO channel read/write in a different thread, and
+	 * wait for it to complete, with a timeout to avoid deadlocking.
+	 */
+
+	scoped_guard(mutex, &priv->ctl_work_lock) {
+		if (work_busy(&ctl_work->work))
+			return -EBUSY;
+
+		if (write) {
+			ctl_work->ctl_val = *ctl_val;
+			ctl_work->write = 1;
+		} else {
+			ctl_work->write = 0;
+		}
+
+		schedule_work(&ctl_work->work);
+	}
+
+	ret = wait_for_completion_timeout(&priv->ctl_rw_done, LTM8054_CTL_RW_TIMEOUT);
+	reinit_completion(&priv->ctl_rw_done);
+
+	if (unlikely(!ret))
+		return -ETIMEDOUT;
+
+	scoped_guard(mutex, &priv->ctl_work_lock) {
+		ret = ctl_work->ret;
+
+		if (!ret && !write)
+			*ctl_val = ctl_work->ctl_val;
+	}
+
+	return ret;
+}
+
+static int ltm8054_set_current_limit(struct regulator_dev *rdev, int min_uA, int max_uA)
+{
+	struct ltm8054_priv *priv = rdev_get_drvdata(rdev);
+	unsigned int ctl_val;
+	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 %lluuV\n", min_uA, vdac_uV);
+
+	ctl_val = vdac_uV;
+
+	return ltm8054_ctl_pin_rw(priv, 1, &ctl_val);
+}
+
+static int ltm8054_get_current_limit(struct regulator_dev *rdev)
+{
+	struct ltm8054_priv *priv = rdev_get_drvdata(rdev);
+	unsigned int ctl_val;
+	int ret;
+	u64 uA;
+
+	ret = ltm8054_ctl_pin_rw(priv, 0, &ctl_val);
+	if (ret)
+		return ret;
+
+	uA = (u64)ctl_val * priv->max_uA;
+	do_div(uA, LTM8054_MAX_CTL_uV);
+
+	return uA;
+}
+
+static const struct regulator_ops ltm8054_no_ctl_ops = { };
+
+static const struct regulator_ops ltm8054_ctl_ops = {
+	.set_current_limit = ltm8054_set_current_limit,
+	.get_current_limit = ltm8054_get_current_limit,
+};
+
+static struct iio_channel *ltm8054_init_ctl_dac(struct platform_device *pdev)
+{
+	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)) {
+		if (PTR_ERR(ctl_dac) == -ENODEV)
+			return ERR_PTR(-EPROBE_DEFER);
+
+		return ctl_dac;
+	}
+
+	ret = iio_get_channel_type(ctl_dac, &type);
+	if (ret)
+		return ERR_PTR(ret);
+
+	if (type != IIO_VOLTAGE)
+		return ERR_PTR(-EINVAL);
+
+	return ctl_dac;
+}
 
 static int ltm8054_of_parse(struct device *dev, struct ltm8054_priv *priv,
 			    struct regulator_config *config)
 {
+	u32 rsense;
 	u32 r[2];
+	u64 tmp;
 	int ret;
 
+	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, "regulator-fb-voltage-divider-ohms",
 					     r, ARRAY_SIZE(r));
 	if (ret)
@@ -60,6 +273,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->of_node = dev_of_node(dev);
 	config->init_data = of_get_regulator_init_data(dev,
 						       config->of_node,
@@ -77,32 +293,76 @@ static int ltm8054_of_parse(struct device *dev, struct ltm8054_priv *priv,
 static int ltm8054_probe(struct platform_device *pdev)
 {
 	struct regulator_config config = { };
+	struct iio_channel *ctl_dac = NULL;
 	struct device *dev = &pdev->dev;
 	struct regulator_dev *rdev;
 	struct ltm8054_priv *priv;
 	int ret;
 
+	/* Do this first, as it might defer. */
+	if (device_property_match_string(dev, "io-channel-names", "ctl") >= 0) {
+		ctl_dac = ltm8054_init_ctl_dac(pdev);
+		if (IS_ERR(ctl_dac))
+			return dev_err_probe(dev, PTR_ERR(ctl_dac),
+					     "failed to initialize CTL DAC\n");
+	}
+
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, priv);
+
+	priv->dev = dev;
 	priv->rdesc.name = "ltm8054-regulator";
-	priv->rdesc.ops = &ltm8054_regulator_ops;
+	priv->rdesc.ops = &ltm8054_no_ctl_ops;
 	priv->rdesc.type = REGULATOR_VOLTAGE;
 	priv->rdesc.owner = THIS_MODULE;
 
+	if (ctl_dac) {
+		priv->ctl_dac = ctl_dac;
+
+		INIT_WORK(&priv->ctl_work.work, ltm8054_do_ctl_work);
+		init_completion(&priv->ctl_rw_done);
+		mutex_init(&priv->ctl_work_lock);
+
+		priv->rdesc.ops = &ltm8054_ctl_ops;
+	}
+
 	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");
+	if (ret) {
+		ret = dev_err_probe(dev, ret, "failed to parse device tree\n");
+		goto out_err;
+	}
 
 	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");
+	if (IS_ERR(rdev)) {
+		ret = dev_err_probe(dev, PTR_ERR(rdev), "failed to register regulator\n");
+		goto out_err;
+	}
 
 	return 0;
+
+out_err:
+	if (ctl_dac) {
+		cancel_work_sync(&priv->ctl_work.work);
+		mutex_destroy(&priv->ctl_work_lock);
+	}
+
+	return ret;
+}
+
+static void ltm8054_remove(struct platform_device *pdev)
+{
+	struct ltm8054_priv *priv = platform_get_drvdata(pdev);
+
+	if (priv->ctl_dac) {
+		cancel_work_sync(&priv->ctl_work.work);
+		mutex_destroy(&priv->ctl_work_lock);
+	}
 }
 
 static const struct of_device_id ltm8054_of_match[] = {
@@ -113,6 +373,7 @@ MODULE_DEVICE_TABLE(of, ltm8054_of_match);
 
 static struct platform_driver ltm8054_driver = {
 	.probe = ltm8054_probe,
+	.remove = ltm8054_remove,
 	.driver = {
 		.name  = "ltm8054",
 		.of_match_table = ltm8054_of_match,

-- 
2.51.2


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

* Re: [PATCH v3 2/5] iio: add processed write API
  2025-11-06 14:11 ` [PATCH v3 2/5] iio: add processed write API Romain Gantois
@ 2025-11-06 16:07   ` Andy Shevchenko
  2025-11-18 15:16     ` Romain Gantois
  0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2025-11-06 16:07 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, Nov 06, 2025 at 03:11:47PM +0100, Romain Gantois wrote:
> Add a function to allow IIO consumers to write a processed value to a
> channel.

...

> +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 *= numerator;
> +		tmp_den = (s64)abs(val) * tmp_den + (s64)abs(val2);

Here is a subtle bug. The problematic piece is abs(). See
https://lore.kernel.org/r/20251106152051.2361551-1-andriy.shevchenko@linux.intel.com
for the answer.

> +		if (val < 0 || val2 < 0)
> +			tmp_num *= -1;

Drop that duplication of the switches above and split the calculations. Note,
with the split done, the confusing assignments of tmp_den will gone as well.

> +		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;
> +}

...

> +	offset_type = iio_channel_read(chan, &offset_val, &offset_val2,

> +	if (offset_type >= 0) {

Why?

> +		switch (offset_type) {
> +		case IIO_VAL_INT:
> +		case IIO_VAL_INT_PLUS_MICRO:
> +			half_step = MICRO / 2;
> +			break;
> +		case IIO_VAL_INT_PLUS_NANO:
> +			half_step = NANO / 2;
> +			break;
> +		case IIO_VAL_FRACTIONAL:
> +			offset_val = DIV_ROUND_CLOSEST(offset_val, offset_val2);
> +			break;
> +		case IIO_VAL_FRACTIONAL_LOG2:
> +			offset_val >>= offset_val2;
> +			break;

> +		default:

You probably wanted to check it here.

> +			return -EINVAL;



> +		}
> +
> +		/* Round fractional part to closest to reduce rounding bias. */
> +		if (half_step) {
> +			if (offset_val2 >= half_step)
> +				*raw -= 1;
> +			else if (offset_val2 <= -half_step)
> +				*raw += 1;
> +		}
> +
> +		*raw -= offset_val;
> +	}

...

> +EXPORT_SYMBOL_GPL(iio_write_channel_processed_scale);

Can we start using namespaced exports?

...

> +/**
> + * 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

s64 number --> @numerator

> + * IIO_VAL_INT. This is typically used for scaling.
> + *
> + * Returns:
> + * IIO_VAL_INT on success or a negative error-number on failure.

Use % for the constants. It will be rendered differently (font) when
applicable. Same for other constants in all of the kernel-doc you add.

> + */

...

> +/**
> + * iio_write_channel_processed_scale() - scale and write processed value to a given channel
> + * @chan:		The channel being queried.
> + * @val:		Value to write.
> + * @scale:		Processed value is divided by this scale factor 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.

> + * Context: May sleep.

The above kernel-doc doesn't have this!

> + * Return: an error code or 0.

Be consistent with the existing code, and even in your own change.

("Return" section name, "Context" section presence, etc.)

Use Perl (original) kernel-doc for now, the Python has a significant regression
(the fix is pending to go to Linus' branch).

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 3/5] iio: test: Add kunit tests for iio_divide_by_value()
  2025-11-06 14:11 ` [PATCH v3 3/5] iio: test: Add kunit tests for iio_divide_by_value() Romain Gantois
@ 2025-11-06 16:10   ` Andy Shevchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2025-11-06 16:10 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, Nov 06, 2025 at 03:11:48PM +0100, Romain Gantois wrote:
> 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.

When use abs() in the code, always add a check for the *_MIN.
This will give you a few surprises.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator
  2025-11-06 14:11 ` [PATCH v3 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator Romain Gantois
@ 2025-11-06 17:23   ` Conor Dooley
  0 siblings, 0 replies; 16+ messages in thread
From: Conor Dooley @ 2025-11-06 17:23 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: 768 bytes --]

On Thu, Nov 06, 2025 at 03:11:46PM +0100, 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>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

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

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

* Re: [PATCH v3 4/5] regulator: Support the LTM8054 voltage regulator
  2025-11-06 14:11 ` [PATCH v3 4/5] regulator: Support the LTM8054 voltage regulator Romain Gantois
@ 2025-11-06 18:08   ` Andy Shevchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2025-11-06 18:08 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, Nov 06, 2025 at 03:11:49PM +0100, 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.

...

The blank lines below are in strange places.

> +#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>
> +
> +#include <linux/regulator/consumer.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/of_regulator.h>
> +#include <linux/types.h>

I expected to see

#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>
#include <linux/types.h>

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

...


Other than above LGTM, FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 5/5] regulator: ltm8054: Support output current limit control
  2025-11-06 14:11 ` [PATCH v3 5/5] regulator: ltm8054: Support output current limit control Romain Gantois
@ 2025-11-06 18:32   ` Andy Shevchenko
  2025-11-07 14:54     ` Romain Gantois
  0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2025-11-06 18:32 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, Nov 06, 2025 at 03:11:50PM +0100, Romain Gantois wrote:
> 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.

...

>  #include <linux/array_size.h>
> +#include <linux/completion.h>
>  #include <linux/device.h>
>  #include <linux/device/devres.h>
>  #include <linux/device/driver.h>

>  #include <linux/errno.h>
>  
>  #include <linux/gpio/consumer.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/jiffies.h>
> +#include <linux/lockdep.h>
>  #include <linux/math64.h>
> +#include <linux/minmax.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>

>  #include <linux/regulator/of_regulator.h>
>  #include <linux/types.h>
>  
> +#include <linux/units.h>
> +#include <linux/workqueue.h>

This will be updated accordingly.

...

> +struct ltm8054_ctl_pin_work {
> +	struct work_struct work;
> +	unsigned int ctl_val;
> +	bool write;
> +	int ret;
> +};

Have you ran `pahole`? It might suggest a better layout to save a few bytes.

...

> +static void ltm8054_do_ctl_work(struct work_struct *work)
> +{
> +	struct ltm8054_ctl_pin_work *ctl_work = container_of_const(work,
> +								   struct ltm8054_ctl_pin_work,
> +								   work);
> +	struct ltm8054_priv *priv = container_of_const(ctl_work,
> +						       struct ltm8054_priv,
> +						       ctl_work);

These read better in slightly different split:

	struct ltm8054_ctl_pin_work *ctl_work =
		container_of_const(work, struct ltm8054_ctl_pin_work, work);
	struct ltm8054_priv *priv =
		container_of_const(ctl_work, struct ltm8054_priv, ctl_work);

...

> +	mutex_lock(&priv->ctl_work_lock);
> +	val = ctl_work->ctl_val;
> +	write = ctl_work->write;
> +	mutex_unlock(&priv->ctl_work_lock);

Why not scoped_guard() from cleanup,h?

...

> +	/* Standard IIO voltage unit is mV, scale accordingly. */
> +	if (write)
> +		ret = iio_write_channel_processed_scale(priv->ctl_dac,
> +							val, 1000);

One line. It just 82 characters.

> +	else
> +		ret = iio_read_channel_processed_scale(priv->ctl_dac,
> +						       &val, 1000);

Ditto.

And perhaps use MILLI/KILO?

...

> +	pr_debug("LTM8054: %s CTL IO channel, val: %duV\n", write ? "wrote" : "reading", val);

Besides str_write_read() from string_choices.h this should be dev_dbg().

> +	mutex_lock(&priv->ctl_work_lock);
> +	ctl_work->ret = ret;
> +	ctl_work->ctl_val = val;
> +	mutex_unlock(&priv->ctl_work_lock);

scoped_guard()

> +	complete(&priv->ctl_rw_done);
> +}

...

> +static int ltm8054_ctl_pin_rw(struct ltm8054_priv *priv, bool write, unsigned int *ctl_val)
> +{
> +	struct ltm8054_ctl_pin_work *ctl_work = &priv->ctl_work;

> +	int ret = 0;

Redundant assignment.

> +	lockdep_assert_not_held(&priv->ctl_work_lock);
> +
> +	/* The get/set_current_limit() callbacks have an active regulator core

/*
 * The proper style of multi-line comment
 * is depicted in this example. Use it.
 */

> +	 * reservation ID (obtained with ww_acquire_init()).
> +	 *
> +	 * Or, the IO channel driver may call something like
> +	 * regulator_enable(), meaning this thread would acquire a new
> +	 * regulator core reservation ID before the current one is dropped
> +	 * (using ww_acquire_fini()). This is forbidden.
> +	 *
> +	 * Thus, perform the IO channel read/write in a different thread, and
> +	 * wait for it to complete, with a timeout to avoid deadlocking.
> +	 */
> +
> +	scoped_guard(mutex, &priv->ctl_work_lock) {
> +		if (work_busy(&ctl_work->work))
> +			return -EBUSY;
> +
> +		if (write) {
> +			ctl_work->ctl_val = *ctl_val;
> +			ctl_work->write = 1;
> +		} else {
> +			ctl_work->write = 0;
> +		}
> +
> +		schedule_work(&ctl_work->work);
> +	}
> +
> +	ret = wait_for_completion_timeout(&priv->ctl_rw_done, LTM8054_CTL_RW_TIMEOUT);
> +	reinit_completion(&priv->ctl_rw_done);
> +
> +	if (unlikely(!ret))
> +		return -ETIMEDOUT;
> +
> +	scoped_guard(mutex, &priv->ctl_work_lock) {
> +		ret = ctl_work->ret;

> +		if (!ret && !write)
> +			*ctl_val = ctl_work->ctl_val;

Return directly.

		if (ret)
			return ret;

		if (!write)
			...

> +	}

> +	return ret;

	return 0;

> +}

...

> +static struct iio_channel *ltm8054_init_ctl_dac(struct platform_device *pdev)
> +{
> +	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)) {

> +		if (PTR_ERR(ctl_dac) == -ENODEV)
> +			return ERR_PTR(-EPROBE_DEFER);

Hmm... Are you sure about this?

> +
> +		return ctl_dac;
> +	}
> +
> +	ret = iio_get_channel_type(ctl_dac, &type);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	if (type != IIO_VOLTAGE)
> +		return ERR_PTR(-EINVAL);
> +
> +	return ctl_dac;
> +}

...

>  static int ltm8054_probe(struct platform_device *pdev)
>  {
>  	struct regulator_config config = { };
> +	struct iio_channel *ctl_dac = NULL;
>  	struct device *dev = &pdev->dev;
>  	struct regulator_dev *rdev;
>  	struct ltm8054_priv *priv;
>  	int ret;
>  
> +	/* Do this first, as it might defer. */
> +	if (device_property_match_string(dev, "io-channel-names", "ctl") >= 0) {
> +		ctl_dac = ltm8054_init_ctl_dac(pdev);
> +		if (IS_ERR(ctl_dac))
> +			return dev_err_probe(dev, PTR_ERR(ctl_dac),
> +					     "failed to initialize CTL DAC\n");
> +	}
> +
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;

> +	platform_set_drvdata(pdev, priv);

Do we need this? I think "no". See below how.

> +	priv->dev = dev;
>  	priv->rdesc.name = "ltm8054-regulator";
> -	priv->rdesc.ops = &ltm8054_regulator_ops;
> +	priv->rdesc.ops = &ltm8054_no_ctl_ops;
>  	priv->rdesc.type = REGULATOR_VOLTAGE;
>  	priv->rdesc.owner = THIS_MODULE;
>  
> +	if (ctl_dac) {
> +		priv->ctl_dac = ctl_dac;

> +		INIT_WORK(&priv->ctl_work.work, ltm8054_do_ctl_work);
> +		init_completion(&priv->ctl_rw_done);

Do devm-helpers.h APIs help with something here? Does
devm_add_action_or_reset() help with not covered cases?

> +		mutex_init(&priv->ctl_work_lock);

Use devm_mutex_init() and don't forget the error check.

> +		priv->rdesc.ops = &ltm8054_ctl_ops;
> +	}
> +
>  	config.dev = dev;
>  	config.driver_data = priv;


From this...

>  	ret = ltm8054_of_parse(dev, priv, &config);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "failed to parse device tree\n");
> +	if (ret) {
> +		ret = dev_err_probe(dev, ret, "failed to parse device tree\n");
> +		goto out_err;
> +	}
>  
>  	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");
> +	if (IS_ERR(rdev)) {
> +		ret = dev_err_probe(dev, PTR_ERR(rdev), "failed to register regulator\n");
> +		goto out_err;
> +	}
>  
>  	return 0;
> +
> +out_err:
> +	if (ctl_dac) {
> +		cancel_work_sync(&priv->ctl_work.work);
> +		mutex_destroy(&priv->ctl_work_lock);
> +	}
> +
> +	return ret;
> +}
> +
> +static void ltm8054_remove(struct platform_device *pdev)
> +{
> +	struct ltm8054_priv *priv = platform_get_drvdata(pdev);
> +
> +	if (priv->ctl_dac) {
> +		cancel_work_sync(&priv->ctl_work.work);
> +		mutex_destroy(&priv->ctl_work_lock);
> +	}
>  }

...to this no changes are needed.

...

>  	.probe = ltm8054_probe,
> +	.remove = ltm8054_remove,

Neither is this.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 5/5] regulator: ltm8054: Support output current limit control
  2025-11-06 18:32   ` Andy Shevchenko
@ 2025-11-07 14:54     ` Romain Gantois
  2025-11-07 15:28       ` Andy Shevchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Romain Gantois @ 2025-11-07 14:54 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

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

Hello Andy,

On Thursday, 6 November 2025 19:32:29 CET Andy Shevchenko wrote:
> On Thu, Nov 06, 2025 at 03:11:50PM +0100, Romain Gantois wrote:
> > 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.
...
> > +	ctl_dac = devm_iio_channel_get(&pdev->dev, "ctl");
> > +	if (IS_ERR(ctl_dac)) {
> > 
> > +		if (PTR_ERR(ctl_dac) == -ENODEV)
> > +			return ERR_PTR(-EPROBE_DEFER);
> 
> Hmm... Are you sure about this?

The only case where I want to defer is if the IO channel hasn't been created 
yet. From what I've read in iio_channel_get(), -ENODEV is returned specifically 
in this case. For example in fwnode_iio_channel_get_by_name() you have:

```
if (!IS_ERR(chan) || PTR_ERR(chan) != -ENODEV)
		return chan;
```

> > 
> >  	priv->rdesc.type = REGULATOR_VOLTAGE;
> >  	priv->rdesc.owner = THIS_MODULE;
> > 
> > +	if (ctl_dac) {
> > +		priv->ctl_dac = ctl_dac;
> > 
> > +		INIT_WORK(&priv->ctl_work.work, ltm8054_do_ctl_work);
> > +		init_completion(&priv->ctl_rw_done);
> 
> Do devm-helpers.h APIs help with something here? Does
> devm_add_action_or_reset() help with not covered cases?

I could definitely use a cleanup action to flush the ctl pin work item instead 
of doing it in a remove() function. It would also remove the gotos below.

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] 16+ messages in thread

* Re: [PATCH v3 5/5] regulator: ltm8054: Support output current limit control
  2025-11-07 14:54     ` Romain Gantois
@ 2025-11-07 15:28       ` Andy Shevchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2025-11-07 15:28 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Andy Shevchenko, 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 Fri, Nov 7, 2025 at 4:54 PM Romain Gantois
<romain.gantois@bootlin.com> wrote:
> On Thursday, 6 November 2025 19:32:29 CET Andy Shevchenko wrote:
> > On Thu, Nov 06, 2025 at 03:11:50PM +0100, Romain Gantois wrote:

...

> > > +   ctl_dac = devm_iio_channel_get(&pdev->dev, "ctl");
> > > +   if (IS_ERR(ctl_dac)) {
> > >
> > > +           if (PTR_ERR(ctl_dac) == -ENODEV)
> > > +                   return ERR_PTR(-EPROBE_DEFER);
> >
> > Hmm... Are you sure about this?
>
> The only case where I want to defer is if the IO channel hasn't been created
> yet. From what I've read in iio_channel_get(), -ENODEV is returned specifically
> in this case. For example in fwnode_iio_channel_get_by_name() you have:
>
> ```
> if (!IS_ERR(chan) || PTR_ERR(chan) != -ENODEV)
>                 return chan;


Yes, but my point is that -ENODEV != -EPROBE_DEFER.
The latter may create an unbound driver in some cases. This needs a
very good justification explaining the metamorphoses.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v3 2/5] iio: add processed write API
  2025-11-06 16:07   ` Andy Shevchenko
@ 2025-11-18 15:16     ` Romain Gantois
  2025-11-18 15:30       ` Andy Shevchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Romain Gantois @ 2025-11-18 15:16 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

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

Hello Andy,

On Thursday, 6 November 2025 17:07:18 CET Andy Shevchenko wrote:
> On Thu, Nov 06, 2025 at 03:11:47PM +0100, Romain Gantois wrote:
> > Add a function to allow IIO consumers to write a processed value to a
> > channel.
> 
> ...
> 
> > +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 *= numerator;
> > +		tmp_den = (s64)abs(val) * tmp_den + (s64)abs(val2);
> 
> Here is a subtle bug. The problematic piece is abs(). See
> https://lore.kernel.org/r/20251106152051.2361551-1-andriy.shevchenko@linux.i
> ntel.com for the answer.

Oh wow, that's a nasty one indeed.

> > +EXPORT_SYMBOL_GPL(iio_write_channel_processed_scale);
> 
> Can we start using namespaced exports?
> 

Sounds good, but won't it look strange to have only 
iio_write_channel_processed_scale() use a namespaced export?

...
> > +/**
> > + * iio_write_channel_processed_scale() - scale and write processed value
> > to a given channel + * @chan:		The channel being queried.
> > + * @val:		Value to write.
> > + * @scale:		Processed value is divided by this scale factor 
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.
> > 
> > + * Context: May sleep.
> 
> The above kernel-doc doesn't have this!
> 
> > + * Return: an error code or 0.
> 
> Be consistent with the existing code, and even in your own change.
> 
> ("Return" section name, "Context" section presence, etc.)

I'll match the "Return" section with what I used for iio_divide_by_value() 
then, since the format used for iio_read_channel_label() is broken.

> 
> Use Perl (original) kernel-doc for now, the Python has a significant
> regression (the fix is pending to go to Linus' branch).

Thanks for the heads up.

-- 
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] 16+ messages in thread

* Re: [PATCH v3 2/5] iio: add processed write API
  2025-11-18 15:16     ` Romain Gantois
@ 2025-11-18 15:30       ` Andy Shevchenko
  2025-11-18 18:21         ` Jonathan Cameron
  0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2025-11-18 15:30 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Andy Shevchenko, 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 Tue, Nov 18, 2025 at 5:16 PM Romain Gantois
<romain.gantois@bootlin.com> wrote:
> On Thursday, 6 November 2025 17:07:18 CET Andy Shevchenko wrote:
> > On Thu, Nov 06, 2025 at 03:11:47PM +0100, Romain Gantois wrote:

...

> > > +EXPORT_SYMBOL_GPL(iio_write_channel_processed_scale);
> >
> > Can we start using namespaced exports?
>
> Sounds good, but won't it look strange to have only
> iio_write_channel_processed_scale() use a namespaced export?

Nope, somebody needs to start this mission, everybody so far has this
excuse :-) I think now it's time.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v3 2/5] iio: add processed write API
  2025-11-18 15:30       ` Andy Shevchenko
@ 2025-11-18 18:21         ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2025-11-18 18:21 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Romain Gantois, Andy Shevchenko, 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 Tue, 18 Nov 2025 17:30:09 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Nov 18, 2025 at 5:16 PM Romain Gantois
> <romain.gantois@bootlin.com> wrote:
> > On Thursday, 6 November 2025 17:07:18 CET Andy Shevchenko wrote:  
> > > On Thu, Nov 06, 2025 at 03:11:47PM +0100, Romain Gantois wrote:  
> 
> ...
> 
> > > > +EXPORT_SYMBOL_GPL(iio_write_channel_processed_scale);  
> > >
> > > Can we start using namespaced exports?  
> >
> > Sounds good, but won't it look strange to have only
> > iio_write_channel_processed_scale() use a namespaced export?  
> 
> Nope, somebody needs to start this mission, everybody so far has this
> excuse :-) I think now it's time.
Choose a namespace that is narrow, so only covers the consumer interface
IIO_CONSUMER perhaps works. I haven't thought much about it so feel
free to propose something else!

J
> 


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

end of thread, other threads:[~2025-11-18 18:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-06 14:11 [PATCH v3 0/5] Add support for the LTM8054 voltage regulator Romain Gantois
2025-11-06 14:11 ` [PATCH v3 1/5] regulator: dt-bindings: Add Linear Technology LTM8054 regulator Romain Gantois
2025-11-06 17:23   ` Conor Dooley
2025-11-06 14:11 ` [PATCH v3 2/5] iio: add processed write API Romain Gantois
2025-11-06 16:07   ` Andy Shevchenko
2025-11-18 15:16     ` Romain Gantois
2025-11-18 15:30       ` Andy Shevchenko
2025-11-18 18:21         ` Jonathan Cameron
2025-11-06 14:11 ` [PATCH v3 3/5] iio: test: Add kunit tests for iio_divide_by_value() Romain Gantois
2025-11-06 16:10   ` Andy Shevchenko
2025-11-06 14:11 ` [PATCH v3 4/5] regulator: Support the LTM8054 voltage regulator Romain Gantois
2025-11-06 18:08   ` Andy Shevchenko
2025-11-06 14:11 ` [PATCH v3 5/5] regulator: ltm8054: Support output current limit control Romain Gantois
2025-11-06 18:32   ` Andy Shevchenko
2025-11-07 14:54     ` Romain Gantois
2025-11-07 15:28       ` Andy Shevchenko

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