[PATCH 0/4] Add support for the LTM8054 voltage regulator

[PATCH 0/4] Add support for the LTM8054 voltage regulator

[Romain Gantois]

Hello everyone,

This series 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>
---
Romain Gantois (4):
      regulator: dt-bindings: Add Linear Technology LTM8054 regulator
      iio: add processed write API
      regulator: Support the LTM8054 voltage regulator
      regulator: ltm8054: Support output current limit control

 .../bindings/regulator/lltc,ltm8054.yaml           |  77 +++++++
 MAINTAINERS                                        |   6 +
 drivers/iio/inkern.c                               |  99 +++++++++
 drivers/regulator/Kconfig                          |   9 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/ltm8054-regulator.c              | 229 +++++++++++++++++++++
 include/linux/iio/consumer.h                       |  17 ++
 7 files changed, 438 insertions(+)
---
base-commit: f87c2b34768828c4ff535ba7d50dd938838caab9
change-id: 20250728-ltm8054-driver-11cfa4741065

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

[PATCH 1/4] regulator: dt-bindings: Add Linear Technology LTM8054 regulator

[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>
---
 .../bindings/regulator/lltc,ltm8054.yaml           | 77 ++++++++++++++++++++++
 MAINTAINERS                                        |  5 ++
 2 files changed, 82 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/lltc,ltm8054.yaml b/Documentation/devicetree/bindings/regulator/lltc,ltm8054.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..4db11be178b0e662ec51f3d3d73202f8c32625d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/lltc,ltm8054.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/lltc,ltm8054.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Linear Technology 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: lltc,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
+
+  lltc,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 = "lltc,ltm8054";
+
+            enable-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+
+            lltc,fb-voltage-divider = <1000000 68000>;
+
+            lltc,iout-rsense-micro-ohms = <20000>;
+
+            io-channels = <&dac 1>;
+            io-channel-names = "ctl";
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index fe168477caa45799dfe07de2f54de6d6a1ce0615..7160179e6bf9d45a241582c1b6df8c0ebf6c3641 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14517,6 +14517,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/lltc,ltm8054.yaml
+
 LTP (Linux Test Project)
 M:	Andrea Cervesato <andrea.cervesato@suse.com>
 M:	Cyril Hrubis <chrubis@suse.cz>

-- 
2.51.0

Re: [PATCH 1/4] regulator: dt-bindings: Add Linear Technology LTM8054 regulator

[Rob Herring (Arm)]

On Tue, 16 Sep 2025 12:24:06 +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>
> ---
>  .../bindings/regulator/lltc,ltm8054.yaml           | 77 ++++++++++++++++++++++
>  MAINTAINERS                                        |  5 ++
>  2 files changed, 82 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/lltc,ltm8054.example.dtb: /: 'compatible' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/lltc,ltm8054.example.dtb: /: 'model' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/lltc,ltm8054.example.dtb: /: '#address-cells' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/lltc,ltm8054.example.dtb: /: '#size-cells' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250916-ltm8054-driver-v1-1-fd4e781d33b9@bootlin.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/4] regulator: dt-bindings: Add Linear Technology LTM8054 regulator

[David Lechner]

On 9/16/25 5:24 AM, 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>
> ---
>  .../bindings/regulator/lltc,ltm8054.yaml           | 77 ++++++++++++++++++++++
>  MAINTAINERS                                        |  5 ++
>  2 files changed, 82 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/lltc,ltm8054.yaml b/Documentation/devicetree/bindings/regulator/lltc,ltm8054.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..4db11be178b0e662ec51f3d3d73202f8c32625d3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/lltc,ltm8054.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/lltc,ltm8054.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Linear Technology 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: lltc,ltm8054

Looks like they got bought by Analog Devices Inc., so 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
> +
> +  lltc,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>
> +
> +    / {

Examples usually don't have a root node. Probably explains
the bot errors. (but you should have seen the same errors
locally with make dt_binding_check.)

> +
> +        regulator {
> +            compatible = "lltc,ltm8054";
> +
> +            enable-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
> +
> +            lltc,fb-voltage-divider = <1000000 68000>;
> +
> +            lltc,iout-rsense-micro-ohms = <20000>;
> +
> +            io-channels = <&dac 1>;
> +            io-channel-names = "ctl";
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fe168477caa45799dfe07de2f54de6d6a1ce0615..7160179e6bf9d45a241582c1b6df8c0ebf6c3641 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14517,6 +14517,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/lltc,ltm8054.yaml
> +
>  LTP (Linux Test Project)
>  M:	Andrea Cervesato <andrea.cervesato@suse.com>
>  M:	Cyril Hrubis <chrubis@suse.cz>
> 

Re: [PATCH 1/4] regulator: dt-bindings: Add Linear Technology LTM8054 regulator

[Romain Gantois]

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

On Tuesday, 16 September 2025 21:24:42 CEST David Lechner wrote:
> On 9/16/25 5:24 AM, 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 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: lltc,ltm8054
> 
> Looks like they got bought by Analog Devices Inc., so adi,ltm8054.
> 

Ah yes, I'll also change the Kconfig and driver descriptions in that case. I'd 
like to keep the lltc,fb-voltage-divider property name though, since that's 
already used in an identical way by two other regulator bindings.

...
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    / {
> 
> Examples usually don't have a root node. Probably explains
> the bot errors. (but you should have seen the same errors
> locally with make dt_binding_check.)
> 

I didn't see the errors but I'm guessing that's due to an out-of-date dtschema
dependency on my system, thanks for pointing it out.

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 --]

Re: [PATCH 1/4] regulator: dt-bindings: Add Linear Technology LTM8054 regulator

[Romain Gantois]

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

On Wednesday, 17 September 2025 17:51:05 CEST Romain Gantois wrote:
> On Tuesday, 16 September 2025 21:24:42 CEST David Lechner wrote:
> > On 9/16/25 5:24 AM, Romain Gantois wrote:
...
> > 
> > Examples usually don't have a root node. Probably explains
> > the bot errors. (but you should have seen the same errors
> > locally with make dt_binding_check.)
> 
> I didn't see the errors but I'm guessing that's due to an out-of-date
> dtschema dependency on my system, thanks for pointing it out.

Just a quick update: turns out this wasn't due to an out-of-date dependency, 
it was because I was missing "DT_CHECKER_FLAGS=-m"

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 --]

[PATCH 2/4] iio: add processed write API

[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         | 99 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/consumer.h | 17 ++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c174ebb7d5e6d183674b7ffb15c4ce0f65fa3aed..6486fdb7c66a4c84312541f0f42cc24469972a9c 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -598,6 +598,85 @@ int iio_read_channel_average_raw(struct iio_channel *chan, int *val)
 }
 EXPORT_SYMBOL_GPL(iio_read_channel_average_raw);
 
+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;
+	s64 tmp_num, tmp_den;
+
+	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
+				      IIO_CHAN_INFO_SCALE);
+	if (scale_type >= 0) {
+		switch (scale_type) {
+		case IIO_VAL_INT:
+			tmp_num = processed;
+			tmp_den = scale_val;
+			break;
+		case IIO_VAL_INT_PLUS_MICRO:
+			tmp_num = (s64)processed * 1000000LL;
+
+			if (scale_val2 < 0) {
+				tmp_den = (s64)scale_val * 1000000LL - (s64)scale_val2;
+				tmp_den *= -1;
+			} else {
+				tmp_den = (s64)scale_val * 1000000LL + (s64)scale_val2;
+			}
+
+			break;
+		case IIO_VAL_INT_PLUS_NANO:
+			tmp_num = (s64)processed * 1000000000LL;
+
+			if (scale_val2 < 0) {
+				tmp_den = (s64)scale_val * 1000000000LL - (s64)scale_val2;
+				tmp_den *= -1;
+			} else {
+				tmp_den = (s64)scale_val * 1000000000LL + (s64)scale_val2;
+			}
+
+			break;
+		case IIO_VAL_FRACTIONAL:
+			tmp_num = (s64)processed * (s64)scale_val2;
+			tmp_den = scale_val;
+			break;
+		case IIO_VAL_FRACTIONAL_LOG2:
+			tmp_num = (s64)processed << scale_val2;
+			tmp_den = scale_val;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		tmp_den *= scale;
+
+		*raw = div64_s64(tmp_num, tmp_den);
+	}
+
+	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;
+}
+
 static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
 						 int raw, int *processed,
 						 unsigned int scale)
@@ -1028,3 +1107,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 6a44796164792b2dd930f8168b14de327a80a6f7..79c4804a73b0652d4c16ee5ad07c4543bccd6c92 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -451,4 +451,21 @@ 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
+ *
+ * Returns an error code or 0.
+ *
+ * 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.
+ */
+int iio_write_channel_processed_scale(struct iio_channel *chan, int val,
+				      unsigned int scale);
+
 #endif

-- 
2.51.0

Re: [PATCH 2/4] iio: add processed write API

[Mark Brown]

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

On Tue, Sep 16, 2025 at 12:24:07PM +0200, Romain Gantois wrote:
> Add a function to allow IIO consumers to write a processed value to a
> channel.

This seems unrelated to the rest of the series?

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

Re: [PATCH 2/4] iio: add processed write API

[Romain Gantois]

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

Hello Mark,

On Tuesday, 16 September 2025 13:19:10 CEST Mark Brown wrote:
> On Tue, Sep 16, 2025 at 12:24:07PM +0200, Romain Gantois wrote:
> > Add a function to allow IIO consumers to write a processed value to a
> > channel.
> 
> This seems unrelated to the rest of the series?

This adds the iio_write_channel_processed_scale() function which is used in 
patch 4/4 of this series in the set_current_limit() callback.

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 --]

Re: [PATCH 2/4] iio: add processed write API

[Andy Shevchenko]

On Tue, Sep 16, 2025 at 12:24:07PM +0200, Romain Gantois wrote:
> Add a function to allow IIO consumers to write a processed value to a
> channel.

...

> +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;
> +	s64 tmp_num, tmp_den;
> +
> +	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
> +				      IIO_CHAN_INFO_SCALE);
> +	if (scale_type >= 0) {
> +		switch (scale_type) {
> +		case IIO_VAL_INT:
> +			tmp_num = processed;
> +			tmp_den = scale_val;
> +			break;
> +		case IIO_VAL_INT_PLUS_MICRO:
> +			tmp_num = (s64)processed * 1000000LL;

If you go with this, in IIO we heavily use units.h and here something like
(s64)MICRO would be appropriate. Similar to the rest of the code where one
or another constant may be used.

> +			if (scale_val2 < 0) {
> +				tmp_den = (s64)scale_val * 1000000LL - (s64)scale_val2;
> +				tmp_den *= -1;
> +			} else {
> +				tmp_den = (s64)scale_val * 1000000LL + (s64)scale_val2;
> +			}
> +
> +			break;
> +		case IIO_VAL_INT_PLUS_NANO:
> +			tmp_num = (s64)processed * 1000000000LL;
> +
> +			if (scale_val2 < 0) {
> +				tmp_den = (s64)scale_val * 1000000000LL - (s64)scale_val2;
> +				tmp_den *= -1;
> +			} else {
> +				tmp_den = (s64)scale_val * 1000000000LL + (s64)scale_val2;
> +			}
> +
> +			break;
> +		case IIO_VAL_FRACTIONAL:
> +			tmp_num = (s64)processed * (s64)scale_val2;
> +			tmp_den = scale_val;
> +			break;
> +		case IIO_VAL_FRACTIONAL_LOG2:
> +			tmp_num = (s64)processed << scale_val2;
> +			tmp_den = scale_val;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		tmp_den *= scale;
> +
> +		*raw = div64_s64(tmp_num, tmp_den);
> +	}
> +
> +	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;
> +}

...

> +/**
> + * 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
> + *
> + * Returns an error code or 0.
> + *
> + * 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.
> + */

This needs a run via kernel-doc validator (warning: Missing Return section).
Also note, in accordance with kernel-doc documentation the Return section must
be last in the big description.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/4] iio: add processed write API

[David Lechner]

On 9/16/25 5:24 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         | 99 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/consumer.h | 17 ++++++++
>  2 files changed, 116 insertions(+)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index c174ebb7d5e6d183674b7ffb15c4ce0f65fa3aed..6486fdb7c66a4c84312541f0f42cc24469972a9c 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -598,6 +598,85 @@ int iio_read_channel_average_raw(struct iio_channel *chan, int *val)
>  }
>  EXPORT_SYMBOL_GPL(iio_read_channel_average_raw);
>  
> +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;
> +	s64 tmp_num, tmp_den;
> +
> +	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
> +				      IIO_CHAN_INFO_SCALE);
> +	if (scale_type >= 0) {
> +		switch (scale_type) {
> +		case IIO_VAL_INT:
> +			tmp_num = processed;
> +			tmp_den = scale_val;
> +			break;
> +		case IIO_VAL_INT_PLUS_MICRO:
> +			tmp_num = (s64)processed * 1000000LL;
> +
> +			if (scale_val2 < 0) {
> +				tmp_den = (s64)scale_val * 1000000LL - (s64)scale_val2;
> +				tmp_den *= -1;
> +			} else {
> +				tmp_den = (s64)scale_val * 1000000LL + (s64)scale_val2;
> +			}
> +
> +			break;
> +		case IIO_VAL_INT_PLUS_NANO:
> +			tmp_num = (s64)processed * 1000000000LL;
> +
> +			if (scale_val2 < 0) {
> +				tmp_den = (s64)scale_val * 1000000000LL - (s64)scale_val2;
> +				tmp_den *= -1;
> +			} else {
> +				tmp_den = (s64)scale_val * 1000000000LL + (s64)scale_val2;
> +			}
> +
> +			break;
> +		case IIO_VAL_FRACTIONAL:
> +			tmp_num = (s64)processed * (s64)scale_val2;
> +			tmp_den = scale_val;
> +			break;
> +		case IIO_VAL_FRACTIONAL_LOG2:
> +			tmp_num = (s64)processed << scale_val2;
> +			tmp_den = scale_val;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		tmp_den *= scale;
> +
> +		*raw = div64_s64(tmp_num, tmp_den);
> +	}

It can be quite tricky to get all of these combinations right. I would
prefer if added some unit tests like we did in [1].

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=c732e60ee10ed0611a59513cbf9c8d35fbe7cf65

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

Re: [PATCH 2/4] iio: add processed write API

[Romain Gantois]

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

Hello David,

On Tuesday, 16 September 2025 21:23:04 CEST David Lechner wrote:
> On 9/16/25 5:24 AM, Romain Gantois wrote:
> > Add a function to allow IIO consumers to write a processed value to a
...
> > +		case IIO_VAL_FRACTIONAL:
> > +			tmp_num = (s64)processed * (s64)scale_val2;
> > +			tmp_den = scale_val;
> > +			break;
> > +		case IIO_VAL_FRACTIONAL_LOG2:
> > +			tmp_num = (s64)processed << scale_val2;
> > +			tmp_den = scale_val;
> > +			break;
> > +		default:
> > +			return -EINVAL;
> > +		}
> > +
> > +		tmp_den *= scale;
> > +
> > +		*raw = div64_s64(tmp_num, tmp_den);
> > +	}
> 
> It can be quite tricky to get all of these combinations right. I would
> prefer if added some unit tests like we did in [1].
> 
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=tes
> ting&id=c732e60ee10ed0611a59513cbf9c8d35fbe7cf65

Agreed, that would be nice, I'll look into 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 --]

Re: [PATCH 2/4] iio: add processed write API

[Jonathan Cameron]

On Wed, 17 Sep 2025 17:46:00 +0200
Romain Gantois <romain.gantois@bootlin.com> wrote:

> Hello David,
> 
> On Tuesday, 16 September 2025 21:23:04 CEST David Lechner wrote:
> > On 9/16/25 5:24 AM, Romain Gantois wrote:  
> > > Add a function to allow IIO consumers to write a processed value to a  
> ...
> > > +		case IIO_VAL_FRACTIONAL:
> > > +			tmp_num = (s64)processed * (s64)scale_val2;
> > > +			tmp_den = scale_val;
> > > +			break;
> > > +		case IIO_VAL_FRACTIONAL_LOG2:
> > > +			tmp_num = (s64)processed << scale_val2;
> > > +			tmp_den = scale_val;
> > > +			break;
> > > +		default:
> > > +			return -EINVAL;
> > > +		}
> > > +
> > > +		tmp_den *= scale;
> > > +
> > > +		*raw = div64_s64(tmp_num, tmp_den);
> > > +	}  
> > 
> > It can be quite tricky to get all of these combinations right. I would
> > prefer if added some unit tests like we did in [1].
> > 
> > [1]:
> > https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=tes
> > ting&id=c732e60ee10ed0611a59513cbf9c8d35fbe7cf65  
> 
> Agreed, that would be nice, I'll look into it.
> 
> Thanks,
> 

Overall code looks fine to me but unit tests would indeed be great.
Thanks,

Jonathan

[PATCH 3/4] regulator: Support the LTM8054 voltage regulator

[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 | 120 ++++++++++++++++++++++++++++++++++
 4 files changed, 130 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7160179e6bf9d45a241582c1b6df8c0ebf6c3641..4bc1a0e4c087060295a927da02f56c332269035f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14521,6 +14521,7 @@ LTM8054 REGULATOR DRIVER
 M:	Romain Gantois <romain.gantois@bootlin.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/regulator/lltc,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 eaa6df1c9f806652a21942bcb48084ba63f942d9..15fb71193b67d0b2daa631b69778dde9323aedd2 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 Linear Technology 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 be98b29d6675d8be1ca984c2d137bdfc4ba2de87..0e61ef826c08f64ea638d19bf10e69abf1526aa7 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..e41bd95da55fb87912e2cdf70bae231133c25745
--- /dev/null
+++ b/drivers/regulator/ltm8054-regulator.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Linear Technology LTM8054 Buck-Boost regulator driver
+ *
+ * Copyright (C) 2025 Bootlin
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
+
+/* The LTM8054 regulates its FB pin to 1.2V */
+#define LTM8054_FB_V 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 + (unsigned int)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 = of_property_read_u32_array(np, "lltc,fb-voltage-divider", r, 2);
+	if (ret) {
+		dev_err(dev, "Failed to parse voltage divider\n");
+		return ret;
+	}
+
+	priv->rdesc.fixed_uV = ltm8054_scale(LTM8054_FB_V, 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) {
+		dev_err(dev, "failed to parse init data\n");
+		return -EINVAL;
+	}
+
+	config->ena_gpiod = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(config->ena_gpiod)) {
+		dev_err(dev, "unable to acquire enable gpio\n");
+		return PTR_ERR(config->ena_gpiod);
+	}
+
+	return 0;
+}
+
+static int ltm8054_probe(struct platform_device *pdev)
+{
+	struct regulator_config config = { 0 };
+	struct regulator_dev *rdev;
+	struct ltm8054_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->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 = &pdev->dev;
+	config.driver_data = priv;
+
+	ret = ltm8054_of_parse(&pdev->dev, priv, &config);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to parse device tree\n");
+
+	rdev = devm_regulator_register(&pdev->dev, &priv->rdesc, &config);
+	if (IS_ERR(rdev))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rdev), "failed to register regulator\n");
+
+	return 0;
+}
+
+static const struct of_device_id __maybe_unused ltm8054_of_match[] = {
+	{ .compatible = "lltc,ltm8054", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ltm8054_of_match);
+
+static struct platform_driver ltm8054_driver = {
+	.probe = ltm8054_probe,
+	.driver = {
+		.name  = "ltm8054",
+		.of_match_table = of_match_ptr(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

Re: [PATCH 3/4] regulator: Support the LTM8054 voltage regulator

[Andy Shevchenko]

On Tue, Sep 16, 2025 at 12:24:08PM +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.

...

> +#include <linux/module.h>
> +#include <linux/of.h>

I think we have already something agnostic in regulator API to get a regulator
from a firmware node (rather than from specific OF/etc one).

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

Can you keep it ordered? This way it's easy to maintain and avoid potential
duplication (note, there are also many headers are missing here, but Mark
usually not insisting in following IWYU principle [1])

...

> +/* The LTM8054 regulates its FB pin to 1.2V */
> +#define LTM8054_FB_V 1200000

It's actually _mV

#define LTM8054_FB_mV	1200000

...

> +static int ltm8054_scale(unsigned int uV, u32 r1, u32 r2)
> +{
> +	u64 tmp;
> +
> +	tmp = (u64)uV * r1;
> +	do_div(tmp, r2);
> +
> +	return uV + (unsigned int)tmp;

Why one needs a casting here?

> +}

...

> +static const struct regulator_ops ltm8054_regulator_ops = {
> +};

Why it can be simply as

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 = of_property_read_u32_array(np, "lltc,fb-voltage-divider", r, 2);

device_property_read_u32_array() ?

ARRAY_SIZE() instead of 2

> +	if (ret) {
> +		dev_err(dev, "Failed to parse voltage divider\n");
> +		return ret;
> +	}
> +
> +	priv->rdesc.fixed_uV = ltm8054_scale(LTM8054_FB_V, 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) {
> +		dev_err(dev, "failed to parse init data\n");
> +		return -EINVAL;
> +	}
> +
> +	config->ena_gpiod = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
> +	if (IS_ERR(config->ena_gpiod)) {
> +		dev_err(dev, "unable to acquire enable gpio\n");
> +		return PTR_ERR(config->ena_gpiod);

All messages in cases of EPROBE_DEFER are problematic (for sure with GPIO),
as it may well flood the logs.
Solution: Use

		return dev_err_probe(...);

pattern instead,

> +	}
> +
> +	return 0;
> +}

...

> +static int ltm8054_probe(struct platform_device *pdev)
> +{
> +	struct regulator_config config = { 0 };

'0' is not required. The { } will have the same effect.

> +	struct regulator_dev *rdev;
> +	struct ltm8054_priv *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(&pdev->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 = &pdev->dev;
> +	config.driver_data = priv;
> +
> +	ret = ltm8054_of_parse(&pdev->dev, priv, &config);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "failed to parse device tree\n");
> +
> +	rdev = devm_regulator_register(&pdev->dev, &priv->rdesc, &config);
> +	if (IS_ERR(rdev))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(rdev), "failed to register regulator\n");

Using

	struct device *dev = &pdev->dev;

at the top will allow to make a few lines shorter.

> +	return 0;
> +}
> +
> +static const struct of_device_id __maybe_unused ltm8054_of_match[] = {
> +	{ .compatible = "lltc,ltm8054", },

Inner comma is not required.

> +	{},

Drop the trailing comma here as it's a terminator entry. The absence of it will
give a hint to the compiler as well.

> +};

...

> +static struct platform_driver ltm8054_driver = {
> +	.probe = ltm8054_probe,
> +	.driver = {
> +		.name  = "ltm8054",
> +		.of_match_table = of_match_ptr(ltm8054_of_match),

Please, do not use of_match_ptr() and/or ACPI_PTR() in a new code.

> +	},
> +};

> +

Unneeded blank line.

> +module_platform_driver(ltm8054_driver);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] regulator: Support the LTM8054 voltage regulator

[Andy Shevchenko]

On Tue, Sep 16, 2025 at 04:12:37PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 16, 2025 at 12:24:08PM +0200, Romain Gantois wrote:

...

> there are also many headers are missing here, but Mark
> usually not insisting in following IWYU principle [1])

Forgot to put a link: https://include-what-you-use.org/

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] regulator: Support the LTM8054 voltage regulator

[Romain Gantois]

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

Hello Andy,

On Tuesday, 16 September 2025 15:12:37 CEST Andy Shevchenko wrote:
> On Tue, Sep 16, 2025 at 12:24:08PM +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.
> 
> ...
> 
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> 
> I think we have already something agnostic in regulator API to get a
> regulator from a firmware node (rather than from specific OF/etc one).
> 

IIRC the "of_match" regulator descriptor property can be used for this, I'll 
have a second look and see if I can use that instead.

> > +#include <linux/mod_devicetable.h>
> > +#include <linux/regulator/driver.h>
> > +#include <linux/regulator/of_regulator.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/gpio/consumer.h>
> 
> Can you keep it ordered? This way it's easy to maintain and avoid potential
> duplication (note, there are also many headers are missing here, but Mark
> usually not insisting in following IWYU principle [1])
> 
> ...

I usually also try to follow IWYU for header inclusions so I'll look for those 
that I missed.

> 
> > +/* The LTM8054 regulates its FB pin to 1.2V */
> > +#define LTM8054_FB_V 1200000
> 
> It's actually _mV
> 
> #define LTM8054_FB_mV	1200000
> 
> ...
> 
> > +static int ltm8054_scale(unsigned int uV, u32 r1, u32 r2)
> > +{
> > +	u64 tmp;
> > +
> > +	tmp = (u64)uV * r1;
> > +	do_div(tmp, r2);
> > +
> > +	return uV + (unsigned int)tmp;
> 
> Why one needs a casting here?
> 

Both of those are unsigned so the cast here is indeed unnecessary.

> > +}
> 
> ...
> 
> > +static const struct regulator_ops ltm8054_regulator_ops = {
> > +};
> 
> Why it can be simply as
> 
> static const struct regulator_ops ltm8054_regulator_ops;
> 

Yeah, this was mostly to have a clean diff on patch 4/4, I'll see if I can drop 
this struct and introduce it in patch 4/4. I wouldn't want to use it 
uninitialized though.

> ...
> 
> > +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 = of_property_read_u32_array(np, "lltc,fb-voltage-divider", r, 2);
> 
> device_property_read_u32_array() ?
> 
> ARRAY_SIZE() instead of 2
> 

Yes, indeed.

> > +	if (ret) {
> > +		dev_err(dev, "Failed to parse voltage divider\n");
> > +		return ret;
> > +	}
> > +
> > +	priv->rdesc.fixed_uV = ltm8054_scale(LTM8054_FB_V, 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) {
> > +		dev_err(dev, "failed to parse init data\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	config->ena_gpiod = devm_gpiod_get_optional(dev, "enable",
> > GPIOD_OUT_LOW); +	if (IS_ERR(config->ena_gpiod)) {
> > +		dev_err(dev, "unable to acquire enable gpio\n");
> > +		return PTR_ERR(config->ena_gpiod);
> 
> All messages in cases of EPROBE_DEFER are problematic (for sure with GPIO),
> as it may well flood the logs.
> Solution: Use
> 
> 		return dev_err_probe(...);
> 
> pattern instead,
> 

Yes, I used that in the caller function but it doesn't solve the flooding 
issue, so I'll move dev_err_probe() to this function instead.

> > +	}
> > +
> > +	return 0;
> > +}
> 
> ...
> 
> > +static int ltm8054_probe(struct platform_device *pdev)
> > +{
> > +	struct regulator_config config = { 0 };
> 
> '0' is not required. The { } will have the same effect.
> 
> > +	struct regulator_dev *rdev;
> > +	struct ltm8054_priv *priv;
> > +	int ret;
> > +
> > +	priv = devm_kzalloc(&pdev->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 = &pdev->dev;
> > +	config.driver_data = priv;
> > +
> > +	ret = ltm8054_of_parse(&pdev->dev, priv, &config);
> > +	if (ret)
> > +		return dev_err_probe(&pdev->dev, ret, "failed to parse device 
tree\n");
> > +
> > +	rdev = devm_regulator_register(&pdev->dev, &priv->rdesc, &config);
> > +	if (IS_ERR(rdev))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(rdev), "failed to 
register
> > regulator\n");
> Using
> 
> 	struct device *dev = &pdev->dev;
> 
> at the top will allow to make a few lines shorter.
> 
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id __maybe_unused ltm8054_of_match[] = {
> > +	{ .compatible = "lltc,ltm8054", },
> 
> Inner comma is not required.
> 
> > +	{},
> 
> Drop the trailing comma here as it's a terminator entry. The absence of it
> will give a hint to the compiler as well.
> 
> > +};
> 
> ...
> 
> > +static struct platform_driver ltm8054_driver = {
> > +	.probe = ltm8054_probe,
> > +	.driver = {
> > +		.name  = "ltm8054",
> > +		.of_match_table = of_match_ptr(ltm8054_of_match),
> 
> Please, do not use of_match_ptr() and/or ACPI_PTR() in a new code.
> 
> > +	},
> > +};
> > 
> > +
> 
> Unneeded blank line.
> 
> > +module_platform_driver(ltm8054_driver);

Thanks for the review,

-- 
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 --]

Re: [PATCH 3/4] regulator: Support the LTM8054 voltage regulator

[Romain Gantois]

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

On Tuesday, 16 September 2025 16:17:56 CEST Romain Gantois wrote:
> Hello Andy,
> 
> On Tuesday, 16 September 2025 15:12:37 CEST Andy Shevchenko wrote:
> > On Tue, Sep 16, 2025 at 12:24:08PM +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.
> > 
> > ...
> > 
> > > +#include <linux/module.h>
> > > +#include <linux/of.h>
> > 
> > I think we have already something agnostic in regulator API to get a
> > regulator from a firmware node (rather than from specific OF/etc one).
> 
> IIRC the "of_match" regulator descriptor property can be used for this, I'll
> have a second look and see if I can use that instead.
> 

Looks like I misread your comment sorry, the "of_match" property is pretty 
much irrelevant to using fwnode_* wrappers, and I didn't find any of those in 
the regulator subsystem. I'm missing a Kconfig dependency on "OF" though, I'll 
have to add 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 --]

Re: [PATCH 3/4] regulator: Support the LTM8054 voltage regulator

[Andy Shevchenko]

On Thu, Sep 18, 2025 at 11:31:50AM +0200, Romain Gantois wrote:
> On Tuesday, 16 September 2025 16:17:56 CEST Romain Gantois wrote:
> > On Tuesday, 16 September 2025 15:12:37 CEST Andy Shevchenko wrote:
> > > On Tue, Sep 16, 2025 at 12:24:08PM +0200, Romain Gantois wrote:

...

> > > > +#include <linux/of.h>
> > > 
> > > I think we have already something agnostic in regulator API to get a
> > > regulator from a firmware node (rather than from specific OF/etc one).
> > 
> > IIRC the "of_match" regulator descriptor property can be used for this, I'll
> > have a second look and see if I can use that instead.
> 
> Looks like I misread your comment sorry, the "of_match" property is pretty 
> much irrelevant to using fwnode_* wrappers, and I didn't find any of those in 
> the regulator subsystem. I'm missing a Kconfig dependency on "OF" though, I'll 
> have to add that.

Why do we need to add that dependency? Yes, probably it won't function,
but then it will decrease test coverage at compile time.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] regulator: Support the LTM8054 voltage regulator

[Romain Gantois]

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

On Thursday, 18 September 2025 21:17:59 CEST Andy Shevchenko wrote:
> On Thu, Sep 18, 2025 at 11:31:50AM +0200, Romain Gantois wrote:
> > On Tuesday, 16 September 2025 16:17:56 CEST Romain Gantois wrote:
> > > On Tuesday, 16 September 2025 15:12:37 CEST Andy Shevchenko wrote:
> > > > On Tue, Sep 16, 2025 at 12:24:08PM +0200, Romain Gantois wrote:
...
> > > > I think we have already something agnostic in regulator API to get a
> > > > regulator from a firmware node (rather than from specific OF/etc one).
> > > 
> > > IIRC the "of_match" regulator descriptor property can be used for this,
> > > I'll have a second look and see if I can use that instead.
> > 
> > Looks like I misread your comment sorry, the "of_match" property is pretty
> > much irrelevant to using fwnode_* wrappers, and I didn't find any of those
> > in the regulator subsystem. I'm missing a Kconfig dependency on "OF"
> > though, I'll have to add that.
> 
> Why do we need to add that dependency? Yes, probably it won't function,
> but then it will decrease test coverage at compile time.

Oh I didn't see it that way, in that case I'll just go the LTC3676 way and 
leave the OF dependency out.

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 --]

[PATCH 4/4] regulator: ltm8054: Support output current limit control

[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 | 109 ++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 15fb71193b67d0b2daa631b69778dde9323aedd2..22cf0e980351f21e3ef5b6611a39cb48aeb503ea 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 Linear Technology 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 e41bd95da55fb87912e2cdf70bae231133c25745..3b7b826e29cfb37415a7fb7cab678cc33494d184 100644
--- a/drivers/regulator/ltm8054-regulator.c
+++ b/drivers/regulator/ltm8054-regulator.c
@@ -11,12 +11,27 @@
 #include <linux/regulator/driver.h>
 #include <linux/regulator/of_regulator.h>
 #include <linux/platform_device.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/types.h>
 #include <linux/gpio/consumer.h>
 
+/* Threshold voltage between the Vout and Iout pins which triggers current
+ * limiting, in microvolts
+ */
+#define LTM8054_VOUT_IOUT_MAX 58000
+
+#define LTM8054_MAX_CTL_V 1200000
+#define LTM8054_MIN_CTL_V 50000
+
 /* The LTM8054 regulates its FB pin to 1.2V */
 #define LTM8054_FB_V 1200000
 
 struct ltm8054_priv {
+	struct iio_channel *ctl_dac;
+
+	int min_uA;
+	int max_uA;
+
 	struct regulator_desc rdesc;
 };
 
@@ -30,18 +45,105 @@ static int ltm8054_scale(unsigned int uV, u32 r1, u32 r2)
 	return uV + (unsigned int)tmp;
 }
 
+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_V;
+	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_V);
+
+	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 < 0)
+		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 = of_property_read_u32(np, "lltc,iout-rsense-micro-ohms", &rsense);
+	if (ret < 0) {
+		dev_err(dev, "failed to get sense resistor value\n");
+		return ret;
+	}
+
+	if (rsense == 0) {
+		dev_err(dev, "invalid value zero for sense resistor\n");
+		return -EINVAL;
+	}
+
+	/* The maximum output current limit is the one set by the Rsense resistor */
+	tmp = 1000000 * (u64)LTM8054_VOUT_IOUT_MAX;
+	do_div(tmp, rsense);
+	priv->max_uA = tmp;
+
+	/* Applying a voltage below LTM8054_MAX_CTL_V on the CTL pin reduces
+	 * the output current limit. If this level drops below
+	 * LTM8054_MIN_CTL_V the regulator stops switching
+	 */
+
+	tmp = LTM8054_MIN_CTL_V * (u64)priv->max_uA;
+	do_div(tmp, (u32)LTM8054_MAX_CTL_V);
+	priv->min_uA = tmp;
+
 	ret = of_property_read_u32_array(np, "lltc,fb-voltage-divider", r, 2);
 	if (ret) {
 		dev_err(dev, "Failed to parse voltage divider\n");
@@ -52,6 +154,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);
@@ -92,6 +197,10 @@ static int ltm8054_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "failed to parse device tree\n");
 
+	ret = ltm8054_init_ctl_dac(pdev, priv);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to initialize CTL DAC\n");
+
 	rdev = devm_regulator_register(&pdev->dev, &priv->rdesc, &config);
 	if (IS_ERR(rdev))
 		return dev_err_probe(&pdev->dev, PTR_ERR(rdev), "failed to register regulator\n");

-- 
2.51.0

Re: [PATCH 4/4] regulator: ltm8054: Support output current limit control

[Andy Shevchenko]

On Tue, Sep 16, 2025 at 12:24:09PM +0200, 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.

...

> in microvolts

Yeah, using _mV postfix will make it visible that those are in micro-Volts.

...

> +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_V;
> +	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);

Why casting?

> +	/* Standard IIO voltage unit is mV, scale accordingly. */
> +	return iio_write_channel_processed_scale(priv->ctl_dac, vdac_uV, 1000);
> +}

...

> +	ret = of_property_read_u32(np, "lltc,iout-rsense-micro-ohms", &rsense);

device_property_read_u32()

> +	if (ret < 0) {

Be consistent with a style, in the previous patch it was 'if (ret)'.

> +		dev_err(dev, "failed to get sense resistor value\n");
> +		return ret;
> +	}
> +
> +	if (rsense == 0) {
> +		dev_err(dev, "invalid value zero for sense resistor\n");
> +		return -EINVAL;
> +	}
> +
> +	/* The maximum output current limit is the one set by the Rsense resistor */
> +	tmp = 1000000 * (u64)LTM8054_VOUT_IOUT_MAX;

Yo may use MICRO and drop the casting.

> +	do_div(tmp, rsense);
> +	priv->max_uA = tmp;
> +
> +	/* Applying a voltage below LTM8054_MAX_CTL_V on the CTL pin reduces
> +	 * the output current limit. If this level drops below
> +	 * LTM8054_MIN_CTL_V the regulator stops switching
> +	 */

/*
 * Besides missing period at the end this is not correct multi-line style of
 * the comments. Use this example.
 */

> +	tmp = LTM8054_MIN_CTL_V * (u64)priv->max_uA;
> +	do_div(tmp, (u32)LTM8054_MAX_CTL_V);

Why casting?

> +	priv->min_uA = tmp;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/4] regulator: ltm8054: Support output current limit control

[Romain Gantois]

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

On Tuesday, 16 September 2025 15:19:16 CEST Andy Shevchenko wrote:
> On Tue, Sep 16, 2025 at 12:24:09PM +0200, 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.
> 
> ...
> 
> > in microvolts
> 
> Yeah, using _mV postfix will make it visible that those are in micro-Volts.
> 
> ...
> 
> > +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_V;
> > +	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);
> Why casting?
> 

This one is indeed unnecessary.

> > +	/* Standard IIO voltage unit is mV, scale accordingly. */
> > +	return iio_write_channel_processed_scale(priv->ctl_dac, vdac_uV, 
1000);
> > +}
> 
> ...
> 
> > +	ret = of_property_read_u32(np, "lltc,iout-rsense-micro-ohms", 
&rsense);
> 
> device_property_read_u32()
> 
> > +	if (ret < 0) {
> 
> Be consistent with a style, in the previous patch it was 'if (ret)'.
> 
> > +		dev_err(dev, "failed to get sense resistor value\n");
> > +		return ret;
> > +	}
> > +
> > +	if (rsense == 0) {
> > +		dev_err(dev, "invalid value zero for sense resistor\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* The maximum output current limit is the one set by the Rsense
> > resistor */ +	tmp = 1000000 * (u64)LTM8054_VOUT_IOUT_MAX;
> 
> Yo may use MICRO and drop the casting.
> 
> > +	do_div(tmp, rsense);
> > +	priv->max_uA = tmp;
> > +
> > +	/* Applying a voltage below LTM8054_MAX_CTL_V on the CTL pin reduces
> > +	 * the output current limit. If this level drops below
> > +	 * LTM8054_MIN_CTL_V the regulator stops switching
> > +	 */
> 
> /*
>  * Besides missing period at the end this is not correct multi-line style of
> * the comments. Use this example.
>  */
> 
> > +	tmp = LTM8054_MIN_CTL_V * (u64)priv->max_uA;

This cast avoids an overflow of the multiplication, since the result may 
easily exceed 32 bytes in size.

> > +	do_div(tmp, (u32)LTM8054_MAX_CTL_V);
> 
> Why casting?

Since do_div() is a macro, I casted the second argument just to be safe, but 
it seems that do_div() already does this internally, so I'll just drop the 
cast.

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 --]

Re: [PATCH 4/4] regulator: ltm8054: Support output current limit control

[Andy Shevchenko]

On Tue, Sep 16, 2025 at 04:27:25PM +0200, Romain Gantois wrote:
> On Tuesday, 16 September 2025 15:19:16 CEST Andy Shevchenko wrote:
> > On Tue, Sep 16, 2025 at 12:24:09PM +0200, Romain Gantois wrote:

First of all, please remove unneeded context (which is assumed you agree with)
in the replies. It makes reviewers' life much easier.

...

> > /*
> >  * Besides missing period at the end this is not correct multi-line style of
> > * the comments. Use this example.
> >  */
> > 
> > > +	tmp = LTM8054_MIN_CTL_V * (u64)priv->max_uA;
> 
> This cast avoids an overflow of the multiplication, since the result may
> easily exceed 32 bytes in size.

It's better to read in a way of

	tmp = (u64)priv->max_uA * LTM8054_MIN_CTL_mV;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/4] regulator: ltm8054: Support output current limit control

[Andy Shevchenko]

On Wed, Sep 17, 2025 at 10:02:03AM +0300, Andy Shevchenko wrote:
> On Tue, Sep 16, 2025 at 04:27:25PM +0200, Romain Gantois wrote:
> > On Tuesday, 16 September 2025 15:19:16 CEST Andy Shevchenko wrote:
> > > On Tue, Sep 16, 2025 at 12:24:09PM +0200, Romain Gantois wrote:

...

> > > /*
> > >  * Besides missing period at the end this is not correct multi-line style of
> > > * the comments. Use this example.
> > >  */
> > > 
> > > > +	tmp = LTM8054_MIN_CTL_V * (u64)priv->max_uA;
> > 
> > This cast avoids an overflow of the multiplication, since the result may
> > easily exceed 32 bytes in size.
> 
> It's better to read in a way of
> 
> 	tmp = (u64)priv->max_uA * LTM8054_MIN_CTL_mV;

I just realised that in previous mails and here I meant _uV postfix for the
predefined voltage thresholds. Sorry for the confusion.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/4] regulator: ltm8054: Support output current limit control

[Romain Gantois]

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

On Tuesday, 16 September 2025 16:27:25 CEST Romain Gantois wrote:
> On Tuesday, 16 September 2025 15:19:16 CEST Andy Shevchenko wrote:
> > On Tue, Sep 16, 2025 at 12:24:09PM +0200, Romain Gantois wrote:
...
> > > CTL pin voltage */ +	vdac_uV = (u64)min_uA * LTM8054_MAX_CTL_V;
> > > +	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);
> > 
> > Why casting?
> 
> This one is indeed unnecessary.

My mistake, this cast is required to avoid a compiler warning;

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 --]

Re: [PATCH 4/4] regulator: ltm8054: Support output current limit control

[Andy Shevchenko]

On Thu, Sep 25, 2025 at 09:54:19AM +0200, Romain Gantois wrote:
> On Tuesday, 16 September 2025 16:27:25 CEST Romain Gantois wrote:
> > On Tuesday, 16 September 2025 15:19:16 CEST Andy Shevchenko wrote:
> > > On Tue, Sep 16, 2025 at 12:24:09PM +0200, Romain Gantois wrote:
...
> > > > CTL pin voltage */ +	vdac_uV = (u64)min_uA * LTM8054_MAX_CTL_V;
> > > > +	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);
> > > 
> > > Why casting?
> > 
> > This one is indeed unnecessary.
> 
> My mistake, this cast is required to avoid a compiler warning;

THen provide a proper specifier instead. Casting in printf() is in 99.9% cases
just a pure mistake.

-- 
With Best Regards,
Andy Shevchenko