* [PATCH v2 0/4] Add TI LM51772 buck-boost voltage regulator
@ 2026-10-27 9:39 Paul Louvel
2026-10-27 9:39 ` [PATCH v2 1/4] regulator: dt-bindings: Add TI LM51772 support Paul Louvel
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Paul Louvel @ 2026-10-27 9:39 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-kernel, devicetree, Thomas Petazzoni, Herve Codina,
Miquel Raynal, Paul Louvel
Hi,
This series provides support for the LM51772, a buck-boost controller
from TI with I2C control, designed for USB Power Delivery applications.
The output voltage is software-configurable from 1 V to 48 V, and the
peak current limit threshold is programmed through software. The
LM51772 also provides an auxiliary fixed 5 V regulator output (VCC1).
Core changes
------------
Patch #2:
The voltage selector helpers can now handle regulators that keep their
selector in two consecutive 8-bit registers. The implementation
preserves bits outside the selector field.
Rejects the unsupported case where an oversized selector shares a
register with the range selector.
Patch #3:
Some regulators have an absolute OVP threshold instead of an offset
that can be programmed once at registration.
The core now has a way for such drivers to have the protection limit
reprogrammed after a voltage change. The callback runs after the ramp
delay, so the new limit is applied once the output has settled.
Thanks,
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
Changes in v2:
- Dropped the regulator-boot-force-off binding and core patches.
After discussion, this property should not be in the binding because
it is Linux specific. Also, the situation described in the v1 cover
letter can be resolved by the driver getting exclusive ownership of
the regulator it uses.
- Added the fixed OVP core change. The custom .set_voltage_sel callback
for the LM51772 is no longer needed since this case can be handled by
the core now.
- Updated the LM51772 binding to the standard unit suffixes and
property names, and fixed the schema and example issues found.
- Reworked the two-register selector helper so it preserves unrelated
bits. Make it atomic by using a bulk read and write instead of two
separate regmap_update_bits().
- Cleaned up the driver around units, current-limit validation,
volatile fault status, IRQ fault reporting, voltage timing, and nINT
handling.
The .renable callback is not really needed when the nFLT/nINT pin is
used as nINT, as the device drives the pin low only when a change to
the status byte occurs.
- Link to v1: https://patch.msgid.link/20261021-paul-lm51772-v1-0-25f0ffbffa8d@bootlin.com
---
Paul Louvel (4):
regulator: dt-bindings: Add TI LM51772 support
regulator: Support regulators where the voltage selector spans two registers
regulator: core: Call set_over_voltage_protection() if OVP limit is fixed after setting new voltage
regulator: Add TI LM51772 support
.../devicetree/bindings/regulator/ti,lm51772.yaml | 232 ++++++
MAINTAINERS | 6 +
drivers/regulator/Kconfig | 11 +
drivers/regulator/Makefile | 1 +
drivers/regulator/core.c | 10 +
drivers/regulator/helpers.c | 87 ++-
drivers/regulator/lm51772-regulator.c | 850 +++++++++++++++++++++
include/linux/regulator/driver.h | 6 +
8 files changed, 1188 insertions(+), 15 deletions(-)
---
base-commit: 0785f7172040512cba759838b27286646773c735
change-id: 20261021-paul-lm51772-c96bbce762a8
Best regards,
--
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/4] regulator: dt-bindings: Add TI LM51772 support
2026-10-27 9:39 [PATCH v2 0/4] Add TI LM51772 buck-boost voltage regulator Paul Louvel
@ 2026-10-27 9:39 ` Paul Louvel
2026-10-29 8:41 ` Krzysztof Kozlowski
2026-10-27 9:39 ` [PATCH v2 2/4] regulator: Support regulators where the voltage selector spans two registers Paul Louvel
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Paul Louvel @ 2026-10-27 9:39 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-kernel, devicetree, Thomas Petazzoni, Herve Codina,
Miquel Raynal, Paul Louvel
Add device tree binding for the TI LM51772 buck-boost voltage regulator,
which consist of a primary VOUT output, and a fixed 5V LDO.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
.../devicetree/bindings/regulator/ti,lm51772.yaml | 232 +++++++++++++++++++++
MAINTAINERS | 5 +
2 files changed, 237 insertions(+)
diff --git a/Documentation/devicetree/bindings/regulator/ti,lm51772.yaml b/Documentation/devicetree/bindings/regulator/ti,lm51772.yaml
new file mode 100644
index 000000000000..ec6dd45e0c71
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/ti,lm51772.yaml
@@ -0,0 +1,232 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/ti,lm51772.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI LM51772 buck-boost voltage regulator
+
+maintainers:
+ - Paul Louvel <paul.louvel@bootlin.com>
+
+description: |
+ The LM51772 is a buck-boost controller with I2C control, designed for
+ USB Power Delivery applications. The output voltage is software
+ configurable between 1V to 48V, with:
+
+ – 1V up to 24V in 10mV monotonous steps.
+ – 3.3V up to 48V in 20mV monotonous steps.
+
+ The peak current limit threshold is programmed through software, between 0.5A
+ to 7A.
+ The LM51772 also provides an auxiliary fixed 5 V regulator output
+ (VCC1).
+
+properties:
+ compatible:
+ const: ti,lm51772
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+ description: Fault / warning irq.
+
+ vin-supply:
+ description: Regulator supply for the chip.
+
+ regulators:
+ type: object
+ description: List of regulators provided by this PMIC.
+
+ properties:
+ vout:
+ type: object
+ $ref: regulator.yaml#
+ description: Main output voltage.
+
+ properties:
+ ti,active-discharge-strength-microamp:
+ description: Discharge current for VOUT.
+ multipleOf: 25000
+ minimum: 25000
+ maximum: 75000
+ default: 25000
+
+ ti,iset-pin:
+ description: |
+ Select the ISET pin as the source of the current limit, instead of
+ the ILIM DAC. When this property is present, the ISET pin is forced
+ in its active configuration and the ILIM DAC is disabled, so the
+ current limit can no longer be programmed through the
+ dedicated register.
+ type: boolean
+
+ ti,pcm-window-low-permille:
+ description: |
+ When VIN is inside a programmable window around VOUT, the PWM
+ logic stops switching entirely and the regulator connect VIN to VOUT.
+ This mode is called the Programmable Conduction Mode (PCM), and is an
+ efficiency features allowing the unit to avoid unnecessary switching
+ operation when VIN ≈ VOUT.
+
+ Lower threshold of the Programmable Conduction Mode (PCM)
+ window, expressed in permille (1/1000) below VOUT target.
+ If this property is zero, PCM is disabled.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ multipleOf: 25
+ minimum: 0
+ maximum: 775
+ default: 300
+
+ shunt-resistor-micro-ohms:
+ description: |
+ Value of the current sense resistor connected between the ISNSP
+ and ISNSN pins, used to translate the ILIM threshold voltage into
+ a current limit and vice versa.
+ TI recommands a value of 10000.
+ default: 10000
+
+ regulator-temp-warn-kelvin:
+ enum: [0, 1, 368, 383, 398, 413]
+
+ regulator-ramp-delay:
+ enum: [40000, 20000, 1000, 500]
+
+ regulator-min-microvolt:
+ minimum: 1000000
+ maximum: 48000000
+
+ regulator-max-microvolt:
+ minimum: 1000000
+ maximum: 48000000
+
+ regulator-min-microamp:
+ minimum: 500000
+ maximum: 7000000
+
+ regulator-max-microamp:
+ minimum: 500000
+ maximum: 7000000
+
+ unevaluatedProperties: false
+
+ vcc1:
+ type: object
+ $ref: regulator.yaml#
+ description: |
+ LDO regulator for auxiliary use.
+ It can used to power, for example, a PD controller that will drive this PMIC.
+
+ properties:
+ regulator-min-microvolt:
+ minimum: 5000000
+ maximum: 5000000
+
+ regulator-max-microvolt:
+ minimum: 5000000
+ maximum: 5000000
+
+ unevaluatedProperties: false
+
+ additionalProperties: false
+
+ ti,sync-pin:
+ description: |
+ Selects the function of the SYNC pin, used when several converters
+ operate in parallel.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [input-rising, input-falling, output-rising, output-falling]
+ default: input-rising
+
+ ti,ivp-thres-microvolt:
+ description: |
+ Input Overvoltage Protection and Regulation Threshold.
+ It shall avoid any damage to the device in case the current flows from the
+ output to the input and the input source cannot sink current.
+ This threshold is used for IVP (Input Voltage Protection) and IVR (Input
+ Voltage Regulation).
+ oneOf:
+ - multipleOf: 125000
+ minimum: 4750000
+ maximum: 23250000
+ - multipleOf: 250000
+ minimum: 23500000
+ maximum: 50000000
+ default: 50000000
+
+ ti,ivp:
+ description: Enable the IVP feature of the regulator.
+ type: boolean
+
+ ti,ivr:
+ description: Enable the IVR feature of the regulator.
+ type: boolean
+
+ ti,nint:
+ description: |
+ Configures the nFLT/nINT pin to act as an interrupt pin.
+
+ nFLT mode: The pin remains low until the cause of the status change ceases.
+ In this mode use an level irq trigger.
+
+ nINT mode: The pin is asserted low to signal a status change.
+ In this mode use an edge irq trigger.
+ type: boolean
+
+ ti,cable-drop-compensation:
+ description: |
+ The cable drop compensation feature helps to keep the output voltage at the
+ nominal value over a wide range of load current without the need for
+ additional remote sensing. The cable drop compensation measures the
+ current and offsets the output voltage proportionally to the measured
+ current.
+ type: boolean
+
+dependencies:
+ ti,ivr: ["ti,ivp"]
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmic@6a {
+ compatible = "ti,lm51772";
+ reg = <0x6a>;
+ interrupt-parent = <&irqc>;
+ interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
+
+ ti,nint;
+
+ regulators {
+ vcc1 {
+ regulator-max-microvolt = <5000000>;
+ regulator-min-microvolt = <5000000>;
+ regulator-name = "vcc1";
+ };
+
+ vout {
+ regulator-max-microamp = <5000000>;
+ regulator-max-microvolt = <20000000>;
+ regulator-min-microamp = <1000000>;
+ regulator-min-microvolt = <5000000>;
+ regulator-name = "vout";
+ regulator-ov-protection-microvolt = <1000000>;
+ regulator-temp-warn-kelvin = <368>;
+ };
+ };
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 57285ff29a16..c958e116cd66 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27461,6 +27461,11 @@ S: Maintained
F: sound/soc/codecs/isabelle*
F: sound/soc/codecs/lm49453*
+TI LM51772 BUCK-BOOST REGULATOR DRIVER
+M: Paul Louvel <paul.louvel@bootlin.com>
+S: Maintained
+F: Documentation/devicetree/bindings/regulator/ti,lm51772.yaml
+
TI LMP92064 ADC DRIVER
M: Leonard Göhrs <l.goehrs@pengutronix.de>
R: kernel@pengutronix.de
--
2.56.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/4] regulator: Support regulators where the voltage selector spans two registers
2026-10-27 9:39 [PATCH v2 0/4] Add TI LM51772 buck-boost voltage regulator Paul Louvel
2026-10-27 9:39 ` [PATCH v2 1/4] regulator: dt-bindings: Add TI LM51772 support Paul Louvel
@ 2026-10-27 9:39 ` Paul Louvel
2026-10-27 9:39 ` [PATCH v2 3/4] regulator: core: Call set_over_voltage_protection() if OVP limit is fixed after setting new voltage Paul Louvel
2026-10-27 9:39 ` [PATCH v2 4/4] regulator: Add TI LM51772 support Paul Louvel
3 siblings, 0 replies; 10+ messages in thread
From: Paul Louvel @ 2026-10-27 9:39 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-kernel, devicetree, Thomas Petazzoni, Herve Codina,
Miquel Raynal, Paul Louvel
Some devices uses 8-bit registers to store the voltage selector across
two registers: one for the least significant bits and one for the most
significant bits.
Introduce vsel_reg_oversize to support voltage selectors that span two
registers. Write the selector in bulk across both registers using
little-endian encoding.
Assume that future hardware will require at most one additional register
to store the voltage selector, and that only a device with 8-bit
register will require such flag.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
drivers/regulator/helpers.c | 87 +++++++++++++++++++++++++++++++++-------
include/linux/regulator/driver.h | 2 +
2 files changed, 74 insertions(+), 15 deletions(-)
diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
index 0def82eb8b46..d4b53803ae12 100644
--- a/drivers/regulator/helpers.c
+++ b/drivers/regulator/helpers.c
@@ -118,6 +118,35 @@ static int regulator_range_selector_to_index(struct regulator_dev *rdev,
return -EINVAL;
}
+static int get_voltage_sel_reg(struct regulator_dev *rdev, unsigned int *sel)
+{
+ unsigned int val;
+ __le16 buf;
+ int ret;
+
+ if (rdev->desc->vsel_reg_oversize) {
+ if (regmap_get_val_bytes(rdev->regmap) != 1)
+ return -EINVAL;
+
+ ret = regmap_bulk_read(rdev->regmap, rdev->desc->vsel_reg, &buf, 2);
+ if (ret != 0)
+ return ret;
+
+ val = le16_to_cpu(buf);
+ } else {
+ ret = regmap_read(rdev->regmap, rdev->desc->vsel_reg, &val);
+ if (ret != 0)
+ return ret;
+ }
+
+ val &= rdev->desc->vsel_mask;
+ val >>= ffs(rdev->desc->vsel_mask) - 1;
+
+ *sel = val;
+
+ return 0;
+}
+
/**
* regulator_get_voltage_sel_pickable_regmap - pickable range get_voltage_sel
*
@@ -140,7 +169,7 @@ int regulator_get_voltage_sel_pickable_regmap(struct regulator_dev *rdev)
if (!r)
return -EINVAL;
- ret = regmap_read(rdev->regmap, rdev->desc->vsel_reg, &val);
+ ret = get_voltage_sel_reg(rdev, &val);
if (ret != 0)
return ret;
@@ -148,9 +177,6 @@ int regulator_get_voltage_sel_pickable_regmap(struct regulator_dev *rdev)
if (ret != 0)
return ret;
- val &= rdev->desc->vsel_mask;
- val >>= ffs(rdev->desc->vsel_mask) - 1;
-
range = regulator_range_selector_to_index(rdev, r_val);
if (range < 0)
return -EINVAL;
@@ -161,6 +187,29 @@ int regulator_get_voltage_sel_pickable_regmap(struct regulator_dev *rdev)
}
EXPORT_SYMBOL_GPL(regulator_get_voltage_sel_pickable_regmap);
+static int set_oversize_voltage_sel(struct regulator_dev *rdev,
+ unsigned int sel)
+{
+ unsigned int mask = rdev->desc->vsel_mask;
+ __le16 buf;
+ u16 val;
+ int ret;
+
+ if (regmap_get_val_bytes(rdev->regmap) != 1)
+ return -EINVAL;
+
+ ret = regmap_bulk_read(rdev->regmap, rdev->desc->vsel_reg, &buf, 2);
+ if (ret)
+ return ret;
+
+ val = le16_to_cpu(buf);
+ val = (val & ~mask) | (sel & mask);
+
+ buf = cpu_to_le16(val);
+
+ return regmap_bulk_write(rdev->regmap, rdev->desc->vsel_reg, &buf, 2);
+}
+
static int write_separate_vsel_and_range(struct regulator_dev *rdev,
unsigned int sel, unsigned int range)
{
@@ -173,15 +222,17 @@ static int write_separate_vsel_and_range(struct regulator_dev *rdev,
if (ret)
return ret;
+ if (rdev->desc->vsel_reg_oversize)
+ return set_oversize_voltage_sel(rdev, sel);
+
/*
* Some PMICs treat the vsel_reg same as apply-bit. Force it to be
* written if the range changed, even if the old selector was same as
* the new one
*/
if (rdev->desc->range_applied_by_vsel && range_updated)
- return regmap_write_bits(rdev->regmap,
- rdev->desc->vsel_reg,
- rdev->desc->vsel_mask, sel);
+ return regmap_write_bits(rdev->regmap, rdev->desc->vsel_reg,
+ rdev->desc->vsel_mask, sel);
return regmap_update_bits(rdev->regmap, rdev->desc->vsel_reg,
rdev->desc->vsel_mask, sel);
@@ -225,12 +276,16 @@ int regulator_set_voltage_sel_pickable_regmap(struct regulator_dev *rdev,
range = rdev->desc->linear_range_selectors_bitfield[i];
range <<= ffs(rdev->desc->vsel_range_mask) - 1;
- if (rdev->desc->vsel_reg == rdev->desc->vsel_range_reg)
+ if (rdev->desc->vsel_reg == rdev->desc->vsel_range_reg) {
+ if (rdev->desc->vsel_reg_oversize)
+ return -EINVAL;
+
ret = regmap_update_bits(rdev->regmap, rdev->desc->vsel_reg,
rdev->desc->vsel_range_mask |
rdev->desc->vsel_mask, sel | range);
- else
+ } else {
ret = write_separate_vsel_and_range(rdev, sel, range);
+ }
if (ret)
return ret;
@@ -257,17 +312,15 @@ int regulator_get_voltage_sel_regmap(struct regulator_dev *rdev)
unsigned int val;
int ret;
- ret = regmap_read(rdev->regmap, rdev->desc->vsel_reg, &val);
+ ret = get_voltage_sel_reg(rdev, &val);
if (ret != 0)
return ret;
- val &= rdev->desc->vsel_mask;
- val >>= ffs(rdev->desc->vsel_mask) - 1;
-
return val;
}
EXPORT_SYMBOL_GPL(regulator_get_voltage_sel_regmap);
+
/**
* regulator_set_voltage_sel_regmap - standard set_voltage_sel for regmap users
*
@@ -284,8 +337,12 @@ int regulator_set_voltage_sel_regmap(struct regulator_dev *rdev, unsigned sel)
sel <<= ffs(rdev->desc->vsel_mask) - 1;
- ret = regmap_update_bits(rdev->regmap, rdev->desc->vsel_reg,
- rdev->desc->vsel_mask, sel);
+ if (rdev->desc->vsel_reg_oversize)
+ ret = set_oversize_voltage_sel(rdev, sel);
+ else
+ ret = regmap_update_bits(rdev->regmap, rdev->desc->vsel_reg,
+ rdev->desc->vsel_mask, sel);
+
if (ret)
return ret;
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 55cc519103ed..8a0f3aebdade 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -312,6 +312,7 @@ enum regulator_type {
* @range_applied_by_vsel: A flag to indicate that changes to vsel_range_reg
* are only effective after vsel_reg is written
* @vsel_reg: Register for selector when using ``regulator_map_*_voltage_*``
+ * @vsel_reg_oversize: Indicates that the vsel_reg spans on two registers
* @vsel_mask: Mask for register bitfield used for selector
* @vsel_step: Specify the resolution of selector stepping when setting
* voltage. If 0, then no stepping is done (requested selector is
@@ -405,6 +406,7 @@ struct regulator_desc {
unsigned int vsel_range_mask;
bool range_applied_by_vsel;
unsigned int vsel_reg;
+ bool vsel_reg_oversize;
unsigned int vsel_mask;
unsigned int vsel_step;
unsigned int csel_reg;
--
2.56.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/4] regulator: core: Call set_over_voltage_protection() if OVP limit is fixed after setting new voltage
2026-10-27 9:39 [PATCH v2 0/4] Add TI LM51772 buck-boost voltage regulator Paul Louvel
2026-10-27 9:39 ` [PATCH v2 1/4] regulator: dt-bindings: Add TI LM51772 support Paul Louvel
2026-10-27 9:39 ` [PATCH v2 2/4] regulator: Support regulators where the voltage selector spans two registers Paul Louvel
@ 2026-10-27 9:39 ` Paul Louvel
2026-10-27 9:50 ` sashiko-bot
2026-10-27 9:39 ` [PATCH v2 4/4] regulator: Add TI LM51772 support Paul Louvel
3 siblings, 1 reply; 10+ messages in thread
From: Paul Louvel @ 2026-10-27 9:39 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-kernel, devicetree, Thomas Petazzoni, Herve Codina,
Miquel Raynal, Paul Louvel
The over-voltage protection limit is defined as a microvolt offset from
the voltage configured on the regulator, and the hardware is programmed
with this limit once, at regulator registration.
Some devices cannot be configured with an offset because their OVP
threshold is a fixed value. For such devices, the protection must be
reprogrammed every time the output voltage changes.
Introduce a new 'fixed_ovp' member in struct regulator_desc to tell the
core that the registered regulator is such a device. The
set_over_voltage_protection() callback is then called after the
slew-rate delay, so that the protection is never programmed before the
output voltage has settled.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
drivers/regulator/core.c | 10 ++++++++++
include/linux/regulator/driver.h | 4 ++++
2 files changed, 14 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 14422cfe80e2..b1cabe50ac25 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4014,6 +4014,16 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
(void *)data);
}
+ if (old_uV != best_val) {
+ if (rdev->desc->fixed_ovp && ops->set_over_voltage_protection) {
+ ret = handle_notify_limits(
+ rdev, ops->set_over_voltage_protection,
+ &rdev->constraints->over_voltage_limits);
+ if (ret == -EOPNOTSUPP)
+ ret = 0;
+ }
+ }
+
out:
trace_regulator_set_voltage_complete(rdev_get_name(rdev), best_val);
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 8a0f3aebdade..248fd8565abc 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -311,6 +311,9 @@ enum regulator_type {
* @vsel_range_mask: Mask for register bitfield used for range selector
* @range_applied_by_vsel: A flag to indicate that changes to vsel_range_reg
* are only effective after vsel_reg is written
+ * @fixed_ovp: A flag to indicate that over voltage protection is set with
+ * hardcoded value, and need to be set each time
+ * when a new voltage is set.
* @vsel_reg: Register for selector when using ``regulator_map_*_voltage_*``
* @vsel_reg_oversize: Indicates that the vsel_reg spans on two registers
* @vsel_mask: Mask for register bitfield used for selector
@@ -405,6 +408,7 @@ struct regulator_desc {
unsigned int vsel_range_reg;
unsigned int vsel_range_mask;
bool range_applied_by_vsel;
+ bool fixed_ovp;
unsigned int vsel_reg;
bool vsel_reg_oversize;
unsigned int vsel_mask;
--
2.56.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 4/4] regulator: Add TI LM51772 support
2026-10-27 9:39 [PATCH v2 0/4] Add TI LM51772 buck-boost voltage regulator Paul Louvel
` (2 preceding siblings ...)
2026-10-27 9:39 ` [PATCH v2 3/4] regulator: core: Call set_over_voltage_protection() if OVP limit is fixed after setting new voltage Paul Louvel
@ 2026-10-27 9:39 ` Paul Louvel
2026-10-27 10:01 ` sashiko-bot
3 siblings, 1 reply; 10+ messages in thread
From: Paul Louvel @ 2026-10-27 9:39 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-kernel, devicetree, Thomas Petazzoni, Herve Codina,
Miquel Raynal, Paul Louvel
Add driver for the TI LM51772 buck-boost voltage regulator.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
MAINTAINERS | 1 +
drivers/regulator/Kconfig | 11 +
drivers/regulator/Makefile | 1 +
drivers/regulator/lm51772-regulator.c | 850 ++++++++++++++++++++++++++++++++++
4 files changed, 863 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index c958e116cd66..a4c59be2551c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27465,6 +27465,7 @@ TI LM51772 BUCK-BOOST REGULATOR DRIVER
M: Paul Louvel <paul.louvel@bootlin.com>
S: Maintained
F: Documentation/devicetree/bindings/regulator/ti,lm51772.yaml
+F: drivers/regulator/lm51772-regulator.c
TI LMP92064 ADC DRIVER
M: Leonard Göhrs <l.goehrs@pengutronix.de>
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 7d151433747b..ce43a1dea33d 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -519,6 +519,17 @@ config REGULATOR_LM363X
One boost output voltage is configurable and always on.
Other LDOs are used for the display module.
+config REGULATOR_LM51772
+ tristate "TI LM51772 voltage regulator"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ This driver supports the TI LM51772 buck-boost controller, designed
+ for USB Power Delivery applications. It provides a software
+ configurable buck-boost output voltage and a fixed auxiliary 5 V LDO
+ output, both controlled over I2C. The driver supports voltage
+ setting, current limiting and fault/thermal reporting.
+
config REGULATOR_LOCHNAGAR
tristate "Cirrus Logic Lochnagar regulator driver"
depends on MFD_LOCHNAGAR
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 6aabcc54382a..783eda233e25 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
obj-$(CONFIG_REGULATOR_ISL9305) += isl9305.o
obj-$(CONFIG_REGULATOR_FP9931) += fp9931.o
obj-$(CONFIG_REGULATOR_LM363X) += lm363x-regulator.o
+obj-$(CONFIG_REGULATOR_LM51772) += lm51772-regulator.o
obj-$(CONFIG_REGULATOR_LOCHNAGAR) += lochnagar-regulator.o
obj-$(CONFIG_REGULATOR_LP3971) += lp3971.o
obj-$(CONFIG_REGULATOR_LP3972) += lp3972.o
diff --git a/drivers/regulator/lm51772-regulator.c b/drivers/regulator/lm51772-regulator.c
new file mode 100644
index 000000000000..66f4541b5099
--- /dev/null
+++ b/drivers/regulator/lm51772-regulator.c
@@ -0,0 +1,850 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI LM51772 I2C regulator driver
+ *
+ * Datasheet: SNVSC22D (September 2025)
+ *
+ * Copyright (c) 2026 Bootlin
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/find.h>
+#include <linux/i2c.h>
+#include <linux/linear_range.h>
+#include <linux/math.h>
+#include <linux/math64.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/regulator/coupler.h>
+#include <linux/regulator/driver.h>
+#include <linux/string.h>
+#include <linux/units.h>
+
+#define LM51772_SENSE_RES_DEFAULT_UOHM 10000
+
+#define LM51772_OVP_DEFAULT 0x3F
+
+#define LM51772_VCC1_LDO_UV 5000000
+#define LM51772_VCC1_LDO_NV 1
+
+#define LM51772_DISCHARGE_STRENGTH_MIN_UA 25000
+#define LM51772_DISCHARGE_STRENGTH_STEP_UA 25000
+#define LM51772_DISCHARGE_STRENGTH_MAX_UA 75000
+
+#define LM51772_ILIM_THR_MIN 0x0A
+#define LM51772_ILIM_THR_MAX 0x8C
+#define LM51772_ILIM_THR_STEP_UV 500
+#define LM51772_ILIM_MIN_UA 500000
+#define LM51772_ILIM_MAX_UA 7000000
+#define LM51772_UA_TO_ILIM_SEL(sns_uOhm, uA) \
+ div_u64((u64)(sns_uOhm) * (uA), MICRO * LM51772_ILIM_THR_STEP_UV)
+#define LM51772_ILIM_SEL_TO_UA(sns_uOhm, sel) \
+ div_u64((u64)(sel) * LM51772_ILIM_THR_STEP_UV * MICRO, (sns_uOhm))
+
+#define LM51772_PCM_WINDOW_LOW_MAX 0x1F
+#define LM51772_PCM_WINDOW_LOW_DFLT 0xC
+#define LM51772_PCM_WINDOW_LOW_STEP_PERMILLE 25
+
+#define LM51772_IVP_VOLTAGE_DFLT 0xFF
+
+#define LM51772_DIV10_MIN_UV 1000000
+#define LM51772_DIV10_STEP_UV 10000
+
+#define LM51772_DIV20_MIN_UV 3300000
+#define LM51772_DIV20_STEP_UV 20000
+
+#define LM51772_N_REGS 2
+
+/* Register map (datasheet table 8-1) */
+#define LM51772_REG_CLEAR_FAULTS 0x03
+#define LM51772_REG_ILIM_THRESHOLD 0x0A
+#define LM51772_REG_VOUT_TARGET1_LSB 0x0C
+#define LM51772_REG_VOUT_TARGET1_MSB 0x0D
+#define LM51772_REG_USB_PD_STATUS_0 0x21
+#define LM51772_REG_STATUS_BYTE 0x78
+#define LM51772_REG_USB_PD_CONTROL_0 0x81
+#define LM51772_REG_MFR_SPECIFIC_D0 0xD0
+#define LM51772_REG_MFR_SPECIFIC_D1 0xD1
+#define LM51772_REG_MFR_SPECIFIC_D2 0xD2
+#define LM51772_REG_MFR_SPECIFIC_D3 0xD3
+#define LM51772_REG_MFR_SPECIFIC_D4 0xD4
+#define LM51772_REG_MFR_SPECIFIC_D5 0xD5
+#define LM51772_REG_MFR_SPECIFIC_D6 0xD6
+#define LM51772_REG_MFR_SPECIFIC_D7 0xD7
+#define LM51772_REG_MFR_SPECIFIC_D8 0xD8
+#define LM51772_REG_MFR_SPECIFIC_D9 0xD9
+#define LM51772_REG_IVP_VOLTAGE 0xDA
+
+#define LM51772_VOUT_TARGET1_MASK GENMASK(11, 0)
+
+/* USB_PD_CONTROL_0 bits */
+#define LM51772_FORCE_DISCH BIT(1)
+#define LM51772_CONV_EN2 BIT(0)
+
+/* STATUS_BYTE bits */
+#define LM51772_STAT_OTHER BIT(0)
+#define LM51772_STAT_CML BIT(1)
+#define LM51772_STAT_TEMP BIT(2)
+#define LM51772_STAT_IOUT BIT(4)
+#define LM51772_STAT_VOUT BIT(5)
+#define LM51772_STAT_OFF BIT(6)
+
+/* MFR_SPECIFIC_D0 bits */
+#define LM51772_EN_VCC1 BIT(5)
+#define LM51772_CONV_EN BIT(0)
+
+/* MFR_SPECIFIC_D1 bits */
+#define LM51772_EN_THER_WARN BIT(7)
+#define LM51772_THW_THRESHOLD_MASK GENMASK(6, 5)
+#define LM51772_EN_NINT BIT(4)
+
+/* MFR_SPECIFIC_D2 bits */
+#define LM51772_DISCHARGE_STRENGTH_MASK GENMASK(3, 2)
+#define LM51772_DVS_SLEW_RAMP_MASK GENMASK(5, 4)
+
+/* MFR_SPECIFIC_D3 bits */
+#define LM51772_EN_IVP BIT(7)
+#define LM51772_SEL_IVR BIT(6)
+
+/* MFR_SPECIFIC_D6 bits */
+#define LM51772_CONFIG_SYNC_PIN_MASK GENMASK(7, 6)
+
+/* MFR_SPECIFIC_D8 bits */
+#define LM51772_SEL_FB_DIV20 BIT(7)
+#define LM51772_EN_CDC BIT(6)
+
+/* MFR_SPECIFIC_D9 bits */
+#define LM51772_SEL_ISET_PIN BIT(5)
+#define LM51772_PCM_WINDOW_LOW_MASK GENMASK(4, 0)
+
+struct lm51772_priv {
+ bool iset_pin;
+ bool nint;
+ u32 sense_res_micro_ohms;
+};
+
+static const unsigned int lm51772_ramp_delays[] = { 40000, 20000, 1000, 500 };
+
+static int lm51772_regulator_set_thermal_protection(struct regulator_dev *rdev, int lim,
+ int severity, bool enable)
+{
+ static const struct linear_range thw_range_celsius = LINEAR_RANGE(95, 0, 3, 15);
+ unsigned int regval = 0, mask = 0, selector = 0;
+ struct regmap *regmap = rdev_get_regmap(rdev);
+ struct device *dev = rdev_get_dev(rdev);
+ long celsius = 0;
+ bool found;
+ int ret;
+
+ if (severity != REGULATOR_SEVERITY_WARN)
+ return -EOPNOTSUPP;
+
+ if (lim != 0) {
+ celsius = kelvin_to_celsius(lim);
+
+ ret = linear_range_get_selector_low_array(&thw_range_celsius, 1, celsius, &selector,
+ &found);
+ if (ret || !found)
+ return -EINVAL;
+
+ selector = thw_range_celsius.max_sel - selector;
+
+ regval |= FIELD_PREP(LM51772_THW_THRESHOLD_MASK, selector);
+ mask |= LM51772_THW_THRESHOLD_MASK;
+ }
+
+ regval |= FIELD_PREP(LM51772_EN_THER_WARN, enable ? 1 : 0);
+ mask |= LM51772_EN_THER_WARN;
+
+ dev_dbg(dev, "thermal protection set to %ld deg C (selector 0x%02x, enable %d)\n", celsius,
+ selector, enable);
+
+ return regmap_update_bits(regmap, LM51772_REG_MFR_SPECIFIC_D1, mask, regval);
+}
+
+static int lm51772_set_conv_en(struct regmap *regmap, bool enable)
+{
+ int ret;
+
+ ret = regmap_update_bits(regmap, LM51772_REG_USB_PD_CONTROL_0, LM51772_CONV_EN2,
+ FIELD_PREP(LM51772_CONV_EN2, enable));
+ if (ret)
+ return ret;
+
+ return regmap_update_bits(regmap, LM51772_REG_MFR_SPECIFIC_D0, LM51772_CONV_EN,
+ FIELD_PREP(LM51772_CONV_EN, enable));
+}
+
+static int lm51772_get_conv_en(struct regmap *regmap)
+{
+ unsigned int pd_ctrl, cfg0;
+ int ret;
+
+ ret = regmap_read(regmap, LM51772_REG_USB_PD_CONTROL_0, &pd_ctrl);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(regmap, LM51772_REG_MFR_SPECIFIC_D0, &cfg0);
+ if (ret)
+ return ret;
+
+ return FIELD_GET(LM51772_CONV_EN2, pd_ctrl) || FIELD_GET(LM51772_CONV_EN, cfg0);
+}
+
+static int lm51772_regulator_enable(struct regulator_dev *rdev)
+{
+ return lm51772_set_conv_en(rdev_get_regmap(rdev), true);
+}
+
+static int lm51772_regulator_disable(struct regulator_dev *rdev)
+{
+ return lm51772_set_conv_en(rdev_get_regmap(rdev), false);
+}
+
+static int lm51772_regulator_is_enabled(struct regulator_dev *rdev)
+{
+ return lm51772_get_conv_en(rdev_get_regmap(rdev));
+}
+
+static int lm51772_set_ov(struct regulator_dev *rdev, int lim_uV)
+{
+ static const struct linear_range ovp_ranges[] = {
+ LINEAR_RANGE(4000000, 0, 0x18, 500000),
+ LINEAR_RANGE(16000000, 0x18, 0x3f, 1000000),
+ };
+ struct regmap *regmap = rdev_get_regmap(rdev);
+ struct device *dev = rdev_get_dev(rdev);
+ unsigned int sel;
+ bool found;
+ int ret;
+
+ ret = linear_range_get_selector_high_array(ovp_ranges, ARRAY_SIZE(ovp_ranges), lim_uV, &sel,
+ &found);
+ if (ret || !found)
+ return -EINVAL;
+
+ dev_dbg(dev, "OVP set to selector 0x%02x\n", sel);
+
+ return regmap_write(regmap, LM51772_REG_MFR_SPECIFIC_D5, sel);
+}
+
+static int lm51772_regulator_set_over_voltage_protection(struct regulator_dev *rdev, int lim_uV,
+ int severity, bool enable)
+{
+ int ret;
+
+ if (!enable)
+ return -EOPNOTSUPP;
+
+ if (severity != REGULATOR_SEVERITY_PROT)
+ return -EOPNOTSUPP;
+
+ /* Use default OVP threshold when no limit is requested. */
+ if (!lim_uV)
+ return regmap_write(rdev_get_regmap(rdev), LM51772_REG_MFR_SPECIFIC_D5,
+ LM51772_OVP_DEFAULT);
+
+ ret = regulator_get_voltage_rdev(rdev);
+ if (ret < 0)
+ return ret;
+
+ return lm51772_set_ov(rdev, ret + lim_uV);
+}
+
+static int lm51772_regulator_get_current_limit(struct regulator_dev *rdev)
+{
+ struct lm51772_priv *priv = rdev_get_drvdata(rdev);
+ struct regmap *regmap = rdev_get_regmap(rdev);
+ unsigned int sel, current_uA;
+ int ret;
+
+ if (priv->iset_pin)
+ return -EOPNOTSUPP;
+
+ ret = regmap_read(regmap, LM51772_REG_ILIM_THRESHOLD, &sel);
+ if (ret)
+ return ret;
+
+ sel = clamp_val(sel, LM51772_ILIM_THR_MIN, LM51772_ILIM_THR_MAX);
+
+ current_uA = LM51772_ILIM_SEL_TO_UA(priv->sense_res_micro_ohms, sel);
+
+ dev_dbg(rdev_get_dev(rdev), "current limit: %d uA (ilim_reg 0x%02x, sense_res %d mOhm)\n",
+ current_uA, sel, priv->sense_res_micro_ohms);
+
+ return current_uA;
+}
+
+static int lm51772_regulator_set_current_limit(struct regulator_dev *rdev, int min_uA, int max_uA)
+{
+ struct lm51772_priv *priv = rdev_get_drvdata(rdev);
+ struct regmap *regmap = rdev_get_regmap(rdev);
+ unsigned int current_uA;
+ unsigned int sel;
+
+ if (priv->iset_pin)
+ return -EOPNOTSUPP;
+
+ if (min_uA > LM51772_ILIM_MAX_UA || max_uA < LM51772_ILIM_MIN_UA)
+ return -EINVAL;
+
+ if (max_uA > LM51772_ILIM_MAX_UA)
+ max_uA = LM51772_ILIM_MAX_UA;
+
+ sel = LM51772_UA_TO_ILIM_SEL(priv->sense_res_micro_ohms, max_uA);
+ sel = clamp_val(sel, LM51772_ILIM_THR_MIN, LM51772_ILIM_THR_MAX);
+
+ /* Check if the selected value is still within the allowed range after computation. */
+ current_uA = LM51772_ILIM_SEL_TO_UA(priv->sense_res_micro_ohms, sel);
+ if (current_uA < min_uA)
+ return -EINVAL;
+
+ dev_dbg(rdev_get_dev(rdev), "set current limit: %d uA (sel 0x%02x, sense_res %d mOhm)\n",
+ current_uA, sel, priv->sense_res_micro_ohms);
+
+ return regmap_write(regmap, LM51772_REG_ILIM_THRESHOLD, sel);
+}
+
+static unsigned int lm51772_status2err(unsigned int status_reg)
+{
+ unsigned int errors = 0;
+
+ /*
+ * The BUSY bit (bit 7) is only meaningful during device startup.
+ * The INPUT bit (bit 3) reports a VIN_UV condition, which is not a
+ * regulator output error. Ignore both.
+ */
+
+ if (status_reg & LM51772_STAT_TEMP)
+ errors |= (status_reg & LM51772_STAT_OFF) ? REGULATOR_ERROR_OVER_TEMP :
+ REGULATOR_ERROR_OVER_TEMP_WARN;
+
+ if (status_reg & LM51772_STAT_IOUT)
+ errors |= REGULATOR_ERROR_OVER_CURRENT;
+
+ if (status_reg & LM51772_STAT_VOUT)
+ errors |= REGULATOR_ERROR_OVER_VOLTAGE_WARN;
+
+ if (status_reg & LM51772_STAT_CML || status_reg & LM51772_STAT_OTHER)
+ errors |= REGULATOR_ERROR_FAIL;
+
+ return errors;
+}
+
+static int lm51772_get_error_flags(struct regulator_dev *rdev, unsigned int *flags)
+{
+ struct regmap *regmap = rdev_get_regmap(rdev);
+ unsigned int reg;
+ int ret;
+
+ ret = regmap_read(regmap, LM51772_REG_STATUS_BYTE, ®);
+ if (ret)
+ return ret;
+
+ *flags = lm51772_status2err(reg);
+
+ return 0;
+}
+
+static int lm51772_regulator_set_voltage_time(struct regulator_dev *rdev, int old_uV, int new_uV)
+{
+ struct regmap *regmap = rdev_get_regmap(rdev);
+ unsigned int reg, slew_rate_uv_per_us, sel;
+ struct device *dev = rdev_get_dev(rdev);
+ int ret, delay_us;
+
+ ret = regmap_read(regmap, LM51772_REG_MFR_SPECIFIC_D2, ®);
+ if (ret)
+ return ret;
+
+ sel = FIELD_GET(LM51772_DVS_SLEW_RAMP_MASK, reg);
+ slew_rate_uv_per_us = lm51772_ramp_delays[sel];
+
+ delay_us = DIV_ROUND_UP(abs(new_uV - old_uV), slew_rate_uv_per_us);
+
+ dev_dbg(dev, "voltage change from %d uV to %d uV, slew rate %d uV/us, delay %d us\n",
+ old_uV, new_uV, slew_rate_uv_per_us, delay_us);
+
+ return delay_us;
+}
+
+static int lm51772_irq_map_event(int irq, struct regulator_irq_data *rid, unsigned long *dev_mask)
+{
+ struct regmap *regmap = rid->data;
+ unsigned int status, reg, errors;
+ bool dev_off;
+ size_t bit;
+ int ret;
+
+ ret = regmap_read(regmap, LM51772_REG_STATUS_BYTE, &status);
+ if (ret)
+ return REGULATOR_FAILED_RETRY;
+
+ errors = lm51772_status2err(status);
+ rid->opaque = status;
+
+ dev_off = (status & LM51772_STAT_OFF) != 0;
+
+ ret = regmap_read(regmap, LM51772_REG_CLEAR_FAULTS, ®);
+ if (ret)
+ return REGULATOR_FAILED_RETRY;
+
+ for (size_t i = 0; i < rid->num_states; i++) {
+ rid->states[i].errors = errors & rid->states[i].possible_errs;
+ rid->states[i].notifs = 0;
+
+ for_each_set_bit(bit, &rid->states[i].errors, BITS_PER_TYPE(rid->states[i].errors))
+ rid->states[i].notifs |= regulator_err2notif(BIT(bit));
+
+ if (dev_off)
+ rid->states[i].notifs |= REGULATOR_EVENT_DISABLE;
+
+ if (rid->states[i].errors || dev_off)
+ *dev_mask |= BIT(i);
+ }
+
+ return 0;
+}
+
+static int lm51772_irq_renable(struct regulator_irq_data *rid)
+{
+ struct regmap *regmap = rid->data;
+ unsigned int reg;
+ int ret;
+
+ ret = regmap_read(regmap, LM51772_REG_STATUS_BYTE, ®);
+ if (ret)
+ return REGULATOR_FAILED_RETRY;
+
+ if (rid->opaque && rid->opaque == reg) {
+ ret = regmap_read(regmap, LM51772_REG_CLEAR_FAULTS, ®);
+ if (ret)
+ return REGULATOR_FAILED_RETRY;
+
+ return REGULATOR_ERROR_ON;
+ }
+
+ rid->opaque = 0;
+
+ return REGULATOR_ERROR_CLEARED;
+}
+
+static int lm51772_vout_of_parse(struct device_node *np, const struct regulator_desc *desc,
+ struct regulator_config *cfg)
+{
+ u32 sense_res_micro_ohms, active_discharge_strength, pcm_window_low;
+ struct lm51772_priv *priv = cfg->driver_data;
+ int ret;
+
+ ret = of_property_read_u32(np, "ti,active-discharge-strength-microamp",
+ &active_discharge_strength);
+ if (ret && ret != -EINVAL)
+ return ret;
+
+ if (ret == -EINVAL) {
+ active_discharge_strength = 0;
+ } else {
+ if (active_discharge_strength < LM51772_DISCHARGE_STRENGTH_MIN_UA ||
+ active_discharge_strength > LM51772_DISCHARGE_STRENGTH_MAX_UA ||
+ active_discharge_strength % LM51772_DISCHARGE_STRENGTH_STEP_UA)
+ return -EINVAL;
+
+ active_discharge_strength =
+ active_discharge_strength / LM51772_DISCHARGE_STRENGTH_STEP_UA - 1;
+ }
+
+ priv->iset_pin = of_property_read_bool(np, "ti,iset-pin");
+
+ ret = of_property_read_u32(np, "ti,pcm-window-low-permille", &pcm_window_low);
+ if (ret && ret != -EINVAL)
+ return ret;
+
+ if (ret == -EINVAL) {
+ pcm_window_low = LM51772_PCM_WINDOW_LOW_DFLT;
+ } else {
+ if (pcm_window_low % LM51772_PCM_WINDOW_LOW_STEP_PERMILLE)
+ return -EINVAL;
+
+ pcm_window_low /= LM51772_PCM_WINDOW_LOW_STEP_PERMILLE;
+ if (pcm_window_low > LM51772_PCM_WINDOW_LOW_MAX)
+ return -EINVAL;
+ }
+
+ ret = of_property_read_u32(np, "shunt-resistor-micro-ohms", &sense_res_micro_ohms);
+ if (ret && ret != -EINVAL)
+ return ret;
+
+ if (ret != -EINVAL) {
+ if (!sense_res_micro_ohms)
+ return -EINVAL;
+
+ priv->sense_res_micro_ohms = sense_res_micro_ohms;
+ }
+
+ ret = regmap_update_bits(
+ cfg->regmap, LM51772_REG_MFR_SPECIFIC_D2, LM51772_DISCHARGE_STRENGTH_MASK,
+ FIELD_PREP(LM51772_DISCHARGE_STRENGTH_MASK, active_discharge_strength));
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(cfg->regmap, LM51772_REG_MFR_SPECIFIC_D9,
+ LM51772_PCM_WINDOW_LOW_MASK | LM51772_SEL_ISET_PIN,
+ FIELD_PREP(LM51772_PCM_WINDOW_LOW_MASK, pcm_window_low) |
+ FIELD_PREP(LM51772_SEL_ISET_PIN, priv->iset_pin));
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int lm51772_of_parse(struct lm51772_priv *priv, struct device *dev, struct regmap *regmap)
+{
+ static const char *const sync_pin_modes[] = {
+ "input-rising",
+ "input-falling",
+ "output-rising",
+ "output-falling",
+ };
+ static const struct linear_range ivp_ranges[] = {
+ LINEAR_RANGE(4750000, 0x00, 0x94, 125000),
+ LINEAR_RANGE(23500000, 0x95, 0xff, 250000)
+ };
+ bool ivp, ivr, cable_drop_compensation, found;
+ unsigned int sync_pin, selector, uv;
+ const char *mode;
+ u32 ivp_thres;
+ int ret;
+
+ ivp = device_property_read_bool(dev, "ti,ivp");
+ ivr = device_property_read_bool(dev, "ti,ivr");
+ priv->nint = device_property_read_bool(dev, "ti,nint");
+ cable_drop_compensation = device_property_read_bool(dev, "ti,cable-drop-compensation");
+
+ if (ivr && !ivp)
+ return -EINVAL;
+
+ ret = device_property_read_string(dev, "ti,sync-pin", &mode);
+ if (ret && ret != -EINVAL)
+ return ret;
+
+ if (ret == -EINVAL) {
+ sync_pin = 0;
+ } else {
+ ret = match_string(sync_pin_modes, ARRAY_SIZE(sync_pin_modes), mode);
+ if (ret < 0)
+ return -EINVAL;
+
+ sync_pin = ret;
+ }
+
+ ret = device_property_read_u32(dev, "ti,ivp-thres-microvolt", &ivp_thres);
+ if (ret && ret != -EINVAL)
+ return ret;
+
+ if (ret == -EINVAL) {
+ ivp_thres = LM51772_IVP_VOLTAGE_DFLT;
+ } else {
+ ret = linear_range_get_selector_low_array(
+ ivp_ranges, ARRAY_SIZE(ivp_ranges), ivp_thres,
+ &selector, &found);
+ if (ret || !found)
+ return -EINVAL;
+
+ ret = linear_range_get_value_array(
+ ivp_ranges, ARRAY_SIZE(ivp_ranges), selector, &uv);
+ if (ret)
+ return -EINVAL;
+
+ if (uv != ivp_thres)
+ return -EINVAL;
+
+ ivp_thres = selector;
+ }
+
+ ret = regmap_update_bits(
+ regmap, LM51772_REG_MFR_SPECIFIC_D3, LM51772_EN_IVP | LM51772_SEL_IVR,
+ FIELD_PREP(LM51772_EN_IVP, ivp) | FIELD_PREP(LM51772_SEL_IVR, ivr));
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(regmap, LM51772_REG_MFR_SPECIFIC_D6, LM51772_CONFIG_SYNC_PIN_MASK,
+ FIELD_PREP(LM51772_CONFIG_SYNC_PIN_MASK, sync_pin));
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(regmap, LM51772_REG_MFR_SPECIFIC_D8, LM51772_EN_CDC,
+ FIELD_PREP(LM51772_EN_CDC, cable_drop_compensation));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(regmap, LM51772_REG_IVP_VOLTAGE, ivp_thres);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static const struct regmap_range lm51772_readable_ranges[] = {
+ regmap_reg_range(LM51772_REG_CLEAR_FAULTS, LM51772_REG_CLEAR_FAULTS),
+ regmap_reg_range(LM51772_REG_ILIM_THRESHOLD, LM51772_REG_ILIM_THRESHOLD),
+ regmap_reg_range(LM51772_REG_VOUT_TARGET1_LSB, LM51772_REG_VOUT_TARGET1_MSB),
+ regmap_reg_range(LM51772_REG_USB_PD_STATUS_0, LM51772_REG_USB_PD_STATUS_0),
+ regmap_reg_range(LM51772_REG_STATUS_BYTE, LM51772_REG_STATUS_BYTE),
+ regmap_reg_range(LM51772_REG_USB_PD_CONTROL_0, LM51772_REG_USB_PD_CONTROL_0),
+ regmap_reg_range(LM51772_REG_MFR_SPECIFIC_D0, LM51772_REG_IVP_VOLTAGE)
+};
+
+static const struct regmap_access_table lm51772_readable_table = {
+ .yes_ranges = lm51772_readable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(lm51772_readable_ranges)
+};
+
+static const struct regmap_range lm51772_writeable_ranges[] = {
+ regmap_reg_range(LM51772_REG_CLEAR_FAULTS, LM51772_REG_CLEAR_FAULTS),
+ regmap_reg_range(LM51772_REG_STATUS_BYTE, LM51772_REG_STATUS_BYTE),
+ regmap_reg_range(LM51772_REG_ILIM_THRESHOLD, LM51772_REG_ILIM_THRESHOLD),
+ regmap_reg_range(LM51772_REG_VOUT_TARGET1_LSB, LM51772_REG_VOUT_TARGET1_MSB),
+ regmap_reg_range(LM51772_REG_USB_PD_CONTROL_0, LM51772_REG_USB_PD_CONTROL_0),
+ regmap_reg_range(LM51772_REG_MFR_SPECIFIC_D0, LM51772_REG_IVP_VOLTAGE)
+};
+
+static const struct regmap_access_table lm51772_writeable_table = {
+ .yes_ranges = lm51772_writeable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(lm51772_writeable_ranges)
+};
+
+static const struct regmap_range lm51772_volatile_ranges[] = {
+ regmap_reg_range(LM51772_REG_USB_PD_STATUS_0, LM51772_REG_USB_PD_STATUS_0),
+ regmap_reg_range(LM51772_REG_STATUS_BYTE, LM51772_REG_STATUS_BYTE),
+ regmap_reg_range(LM51772_REG_CLEAR_FAULTS, LM51772_REG_CLEAR_FAULTS)
+};
+
+static const struct regmap_access_table lm51772_volatile_table = {
+ .yes_ranges = lm51772_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(lm51772_volatile_ranges)
+};
+
+static const struct reg_default lm51772_reg_defaults[] = {
+ { .reg = LM51772_REG_CLEAR_FAULTS, .def = 0x00 },
+ { .reg = LM51772_REG_ILIM_THRESHOLD, .def = 0x64 },
+ { .reg = LM51772_REG_VOUT_TARGET1_LSB, .def = 0x58 },
+ { .reg = LM51772_REG_VOUT_TARGET1_MSB, .def = 0x02 },
+ { .reg = LM51772_REG_STATUS_BYTE, .def = 0x00 },
+ { .reg = LM51772_REG_USB_PD_CONTROL_0, .def = 0x01 },
+ { .reg = LM51772_REG_MFR_SPECIFIC_D0, .def = 0x32 },
+ { .reg = LM51772_REG_MFR_SPECIFIC_D1, .def = 0x09 },
+ { .reg = LM51772_REG_MFR_SPECIFIC_D2, .def = 0x40 },
+ { .reg = LM51772_REG_MFR_SPECIFIC_D3, .def = 0x20 },
+ { .reg = LM51772_REG_MFR_SPECIFIC_D4, .def = 0x03 },
+ { .reg = LM51772_REG_MFR_SPECIFIC_D5, .def = 0x3F },
+ { .reg = LM51772_REG_MFR_SPECIFIC_D6, .def = 0x15 },
+ { .reg = LM51772_REG_MFR_SPECIFIC_D7, .def = 0x28 },
+ { .reg = LM51772_REG_MFR_SPECIFIC_D8, .def = 0x84 },
+ { .reg = LM51772_REG_MFR_SPECIFIC_D9, .def = 0x2C },
+ { .reg = LM51772_REG_IVP_VOLTAGE, .def = 0xFF }
+};
+
+static const struct regmap_config lm51772_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .rd_table = &lm51772_readable_table,
+ .wr_table = &lm51772_writeable_table,
+ .volatile_table = &lm51772_volatile_table,
+ .max_register = LM51772_REG_IVP_VOLTAGE,
+ .reg_defaults = lm51772_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(lm51772_reg_defaults),
+ .cache_type = REGCACHE_MAPLE
+};
+
+static const struct regulator_ops lm51772_reg_ops = {
+ .list_voltage = regulator_list_voltage_pickable_linear_range,
+ .map_voltage = regulator_map_voltage_pickable_linear_range,
+ .get_voltage_sel = regulator_get_voltage_sel_pickable_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_pickable_regmap,
+ .set_over_voltage_protection = lm51772_regulator_set_over_voltage_protection,
+ .set_thermal_protection = lm51772_regulator_set_thermal_protection,
+ .set_active_discharge = regulator_set_active_discharge_regmap,
+ .get_current_limit = lm51772_regulator_get_current_limit,
+ .set_current_limit = lm51772_regulator_set_current_limit,
+ .get_error_flags = lm51772_get_error_flags,
+ .set_ramp_delay = regulator_set_ramp_delay_regmap,
+ .set_voltage_time = lm51772_regulator_set_voltage_time,
+ .is_enabled = lm51772_regulator_is_enabled,
+ .enable = lm51772_regulator_enable,
+ .disable = lm51772_regulator_disable
+};
+
+static const struct regulator_ops lm51772_fixed_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap
+};
+
+/*
+ * For < 24V, voltages are set in 10mV monotonous steps.
+ * For >= 24V, voltages are set in 20mV monotonous steps.
+ */
+static const struct linear_range lm51772_voltage_ranges[] = {
+ REGULATOR_LINEAR_RANGE(LM51772_DIV10_MIN_UV, 0, 99, 0),
+ REGULATOR_LINEAR_RANGE(LM51772_DIV10_MIN_UV, 100, 2399, LM51772_DIV10_STEP_UV),
+
+ REGULATOR_LINEAR_RANGE(LM51772_DIV20_MIN_UV, 0, 164, 0),
+ REGULATOR_LINEAR_RANGE(LM51772_DIV20_MIN_UV, 165, 2400, LM51772_DIV20_STEP_UV)
+};
+
+static const unsigned int lm51772_voltage_ranges_sel[] = { 0, 0, 1, 1 };
+
+static const struct regulator_desc lm51772_regulator_descs[] = {
+ { .name = "vout",
+ .of_match = of_match_ptr("vout"),
+ .of_parse_cb = lm51772_vout_of_parse,
+ .regulators_node = of_match_ptr("regulators"),
+ .id = 0,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .ops = &lm51772_reg_ops,
+ .linear_ranges = lm51772_voltage_ranges,
+ .linear_range_selectors_bitfield = lm51772_voltage_ranges_sel,
+ .n_linear_ranges = ARRAY_SIZE(lm51772_voltage_ranges),
+ .n_voltages = 100 + 2300 + 165 + 2236,
+ .vsel_reg = LM51772_REG_VOUT_TARGET1_LSB,
+ .vsel_mask = LM51772_VOUT_TARGET1_MASK,
+ .vsel_reg_oversize = true,
+ .fixed_ovp = true,
+ .vsel_range_reg = LM51772_REG_MFR_SPECIFIC_D8,
+ .vsel_range_mask = LM51772_SEL_FB_DIV20,
+ .active_discharge_reg = LM51772_REG_USB_PD_CONTROL_0,
+ .active_discharge_mask = LM51772_FORCE_DISCH,
+ .active_discharge_on = FIELD_PREP_CONST(LM51772_FORCE_DISCH, 1),
+ .active_discharge_off = FIELD_PREP_CONST(LM51772_FORCE_DISCH, 0),
+ .ramp_reg = LM51772_REG_MFR_SPECIFIC_D2,
+ .ramp_mask = LM51772_DVS_SLEW_RAMP_MASK,
+ .ramp_delay_table = lm51772_ramp_delays,
+ .n_ramp_values = ARRAY_SIZE(lm51772_ramp_delays) },
+ {
+ .name = "vcc1",
+ .of_match = of_match_ptr("vcc1"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = 1,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .ops = &lm51772_fixed_ops,
+ .fixed_uV = LM51772_VCC1_LDO_UV,
+ .n_voltages = LM51772_VCC1_LDO_NV,
+ .enable_reg = LM51772_REG_MFR_SPECIFIC_D0,
+ .enable_mask = LM51772_EN_VCC1,
+ },
+};
+
+static int lm51772_setup_irq(struct device *dev, struct regmap *regmap, int irq,
+ struct regulator_dev *rdev[LM51772_N_REGS], bool nint)
+{
+ int common_errs = REGULATOR_ERROR_OVER_TEMP | REGULATOR_ERROR_OVER_TEMP_WARN |
+ REGULATOR_ERROR_FAIL;
+ int per_rdev_errs[] = {
+ REGULATOR_ERROR_OVER_CURRENT | REGULATOR_ERROR_OVER_VOLTAGE_WARN,
+ 0,
+ };
+ struct regulator_irq_desc irq_desc = {
+ .name = "lm51772-irq",
+ .data = regmap,
+ .map_event = lm51772_irq_map_event,
+ };
+ void *retp;
+
+ if (!nint) {
+ irq_desc.renable = lm51772_irq_renable;
+ irq_desc.irq_off_ms = 1000;
+ irq_desc.reread_ms = 200;
+ }
+
+ retp = devm_regulator_irq_helper(dev, &irq_desc, irq, 0, common_errs, per_rdev_errs, rdev,
+ LM51772_N_REGS);
+ if (IS_ERR(retp))
+ return PTR_ERR(retp);
+
+ return regmap_update_bits(regmap, LM51772_REG_MFR_SPECIFIC_D1, LM51772_EN_NINT,
+ FIELD_PREP(LM51772_EN_NINT, nint));
+}
+
+static int lm51772_probe(struct i2c_client *client)
+{
+ struct regulator_dev *rdev[LM51772_N_REGS];
+ struct regulator_config config = {};
+ struct device *dev = &client->dev;
+ struct lm51772_priv *priv;
+ struct regmap *regmap;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->sense_res_micro_ohms = LM51772_SENSE_RES_DEFAULT_UOHM;
+
+ ret = devm_regulator_get_enable_optional(dev, "vin");
+ if (ret < 0 && ret != -ENODEV)
+ return dev_err_probe(dev, ret, "unable to get vin regulator\n");
+
+ regmap = devm_regmap_init_i2c(client, &lm51772_regmap_config);
+ if (IS_ERR(regmap))
+ return dev_err_probe(dev, PTR_ERR(regmap), "unable to init regmap\n");
+
+ config.dev = dev;
+ config.driver_data = priv;
+ config.regmap = regmap;
+
+ ret = lm51772_of_parse(priv, dev, regmap);
+ if (ret)
+ return dev_err_probe(dev, ret, "unable to parse dt node\n");
+
+ for (size_t i = 0; i < LM51772_N_REGS; i++) {
+ rdev[i] = devm_regulator_register(dev, &lm51772_regulator_descs[i], &config);
+ if (IS_ERR(rdev[i]))
+ return dev_err_probe(dev, PTR_ERR(rdev[i]),
+ "unable to register regulator %s\n",
+ lm51772_regulator_descs[i].name);
+ }
+
+ ret = lm51772_setup_irq(dev, regmap, client->irq, rdev, priv->nint);
+ if (ret)
+ return dev_err_probe(dev, ret, "unable to request irq\n");
+
+ return 0;
+}
+
+static const struct of_device_id lm51772_of_match[] = {
+ { .compatible = "ti,lm51772" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, lm51772_of_match);
+
+static const struct i2c_device_id lm51772_id[] = {
+ { .name = "lm51772" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, lm51772_id);
+
+static struct i2c_driver lm51772_driver = {
+ .driver = {
+ .name = "lm51772",
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .of_match_table = lm51772_of_match,
+ },
+ .probe = lm51772_probe,
+ .id_table = lm51772_id,
+};
+
+module_i2c_driver(lm51772_driver);
+
+MODULE_AUTHOR("Paul Louvel <paul.louvel@bootlin.com>");
+MODULE_DESCRIPTION("TI LM51772 I2C regulator driver");
+MODULE_LICENSE("GPL");
--
2.56.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/4] regulator: core: Call set_over_voltage_protection() if OVP limit is fixed after setting new voltage
2026-10-27 9:39 ` [PATCH v2 3/4] regulator: core: Call set_over_voltage_protection() if OVP limit is fixed after setting new voltage Paul Louvel
@ 2026-10-27 9:50 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-10-27 9:50 UTC (permalink / raw)
To: Paul Louvel; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] regulator: core: late fixed_ovp update in _regulator_do_set_voltage()
- [Medium] regulator: core: state desync when set_over_voltage_protection() fails
--
commit 842d424c282842b568e77b33fac660e8e08d0998
Author: Paul Louvel <paul.louvel@bootlin.com>
regulator: core: Call set_over_voltage_protection() if OVP limit is fixed after setting new voltage
This commit introduces a fixed_ovp flag in struct regulator_desc for
regulators whose over-voltage protection threshold is programmed as an
absolute value rather than an offset. When the output voltage changes in
_regulator_do_set_voltage(), it calls set_over_voltage_protection() via
handle_notify_limits() after the slew-rate delay.
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 14422cfe80e2d..b1cabe50ac256 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -4014,6 +4014,16 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
> (void *)data);
> }
>
> + if (old_uV != best_val) {
> + if (rdev->desc->fixed_ovp && ops->set_over_voltage_protection) {
> + ret = handle_notify_limits(
> + rdev, ops->set_over_voltage_protection,
> + &rdev->constraints->over_voltage_limits);
> + if (ret == -EOPNOTSUPP)
> + ret = 0;
> + }
> + }
[Severity: High]
When increasing the output voltage (best_val > old_uV) above the previous
over-voltage protection threshold, won't reprogramming the OVP limit after
the voltage write and slew-rate delay trigger a spurious hardware OVP fault?
In _regulator_do_set_voltage(), the new target voltage best_val is written to
hardware first (via _regulator_call_set_voltage(),
_regulator_set_voltage_sel_step(), or _regulator_call_set_voltage_sel()), and
then fsleep(delay) waits for the output voltage to slew to best_val before
handle_notify_limits() is reached:
drivers/regulator/core.c:_regulator_do_set_voltage() {
...
/* Insert any necessary delays */
fsleep(delay);
if (best_val >= 0) {
unsigned long data = best_val;
_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE,
(void *)data);
}
if (old_uV != best_val) {
if (rdev->desc->fixed_ovp && ops->set_over_voltage_protection) {
ret = handle_notify_limits(
rdev, ops->set_over_voltage_protection,
&rdev->constraints->over_voltage_limits);
...
}
Waiting until after the slew-rate delay is necessary when decreasing the
voltage (best_val < old_uV) so the threshold is not lowered before the output
has slewed down. However, when raising the voltage (best_val > old_uV) to a
level above the old threshold (old_uV + lim_uV), the old lower OVP threshold
remains active in hardware while the physical output slews upward during
fsleep(delay), causing the rising output voltage to cross the old OVP limit
before ops->set_over_voltage_protection() is called.
Should _regulator_do_set_voltage() update the OVP threshold before raising
the hardware voltage when best_val > old_uV, and after fsleep(delay) when
best_val < old_uV? Note that because ops->set_over_voltage_protection() only
receives the offset lim_uV and drivers may read back the current hardware
voltage via regulator_get_voltage_rdev(), reprogramming OVP prior to raising
the voltage may also require passing or exposing the target best_val.
[Severity: Medium]
What happens to the regulator state if handle_notify_limits() returns an
error other than -EOPNOTSUPP here?
At this point in _regulator_do_set_voltage(), the hardware voltage has
already been changed to best_val and REGULATOR_EVENT_VOLTAGE_CHANGE has
already been emitted. Overwriting ret with a negative error from
handle_notify_limits() causes regulator_set_voltage_unlocked() ->
regulator_balance_voltage() -> regulator_do_balance_voltage() ->
regulator_set_voltage_rdev() -> _regulator_do_set_voltage() to fail without
reverting the hardware voltage.
In regulator_set_voltage_unlocked(), a negative return rolls back the
consumer's requested voltage range to old_min_uV and old_max_uV even though
the hardware remains at best_val:
drivers/regulator/core.c:regulator_set_voltage_unlocked() {
...
/* for not coupled regulators this will just set the voltage */
ret = regulator_balance_voltage(rdev, state);
if (ret < 0) {
voltage->min_uV = old_min_uV;
voltage->max_uV = old_max_uV;
}
...
}
Because voltage->min_uV and voltage->max_uV are reset to old_min_uV and
old_max_uV, a subsequent regulator_set_voltage() call attempting to restore
old_min_uV and old_max_uV will hit the early no-op check:
if (voltage->min_uV == min_uV && voltage->max_uV == max_uV)
goto out;
Additionally, in regulator_set_voltage_rdev(), a negative return from
_regulator_do_set_voltage() jumps to out before lowering the parent supply
voltage when supply_change_uV < 0:
drivers/regulator/core.c:regulator_set_voltage_rdev() {
...
if (state == PM_SUSPEND_ON)
ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
else
ret = _regulator_do_set_suspend_voltage(rdev, min_uV,
max_uV, state);
if (ret < 0)
goto out;
if (supply_change_uV < 0) {
ret = regulator_set_voltage_unlocked(rdev->supply,
best_supply_uV, INT_MAX, state);
...
}
Could this leave the software state and parent supply out of sync with the
actual hardware voltage if ops->set_over_voltage_protection() fails (for
example, due to an out-of-range OVP calculation or a register write error)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-paul-lm51772-v2-0-b2a767e34722@bootlin.com?part=3
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 4/4] regulator: Add TI LM51772 support
2026-10-27 9:39 ` [PATCH v2 4/4] regulator: Add TI LM51772 support Paul Louvel
@ 2026-10-27 10:01 ` sashiko-bot
2026-10-27 11:36 ` Paul Louvel
0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-10-27 10:01 UTC (permalink / raw)
To: Paul Louvel; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] regulator: lm51772: use logical AND in lm51772_get_conv_en()
- [High] regulator: lm51772: fix current limit bounds and max_uA check
- [High] regulator: lm51772: fix STAT_OFF and fault checks in IRQ callbacks
- [Medium] regulator: lm51772: fix cached errors and IRQ order in lm51772_setup_irq()
- [Medium] regulator: lm51772: allow OVP limits below 4V in lm51772_set_ov()
- [Medium] regulator: lm51772: OVP updated after voltage ramp-up trips false OVP fault
- [Medium] regulator: lm51772: state mismatch between priv->iset_pin and hardware default
--
commit 85d9f9bf5e7e89e913ab51cc149fc605a17772db
Author: Paul Louvel <paul.louvel@bootlin.com>
regulator: Add TI LM51772 support
This commit adds a regulator driver for the TI LM51772 I2C buck-boost
voltage regulator and its auxiliary 5 V LDO output.
> diff --git a/drivers/regulator/lm51772-regulator.c b/drivers/regulator/lm51772-regulator.c
> new file mode 100644
> index 0000000000000..66f4541b50994
> --- /dev/null
> +++ b/drivers/regulator/lm51772-regulator.c
[ ... ]
> +static int lm51772_set_conv_en(struct regmap *regmap, bool enable)
> +{
> + int ret;
> +
> + ret = regmap_update_bits(regmap, LM51772_REG_USB_PD_CONTROL_0, LM51772_CONV_EN2,
> + FIELD_PREP(LM51772_CONV_EN2, enable));
> + if (ret)
> + return ret;
> +
> + return regmap_update_bits(regmap, LM51772_REG_MFR_SPECIFIC_D0, LM51772_CONV_EN,
> + FIELD_PREP(LM51772_CONV_EN, enable));
> +}
> +
> +static int lm51772_get_conv_en(struct regmap *regmap)
> +{
> + unsigned int pd_ctrl, cfg0;
> + int ret;
> +
> + ret = regmap_read(regmap, LM51772_REG_USB_PD_CONTROL_0, &pd_ctrl);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(regmap, LM51772_REG_MFR_SPECIFIC_D0, &cfg0);
> + if (ret)
> + return ret;
> +
> + return FIELD_GET(LM51772_CONV_EN2, pd_ctrl) || FIELD_GET(LM51772_CONV_EN, cfg0);
> +}
[Severity: High]
Should lm51772_get_conv_en() use && instead of || here?
In lm51772_reg_defaults (and at hardware reset), LM51772_REG_USB_PD_CONTROL_0
is 0x01 (so LM51772_CONV_EN2 is 1) while LM51772_REG_MFR_SPECIFIC_D0 is 0x32
(so LM51772_CONV_EN is 0), and lm51772_set_conv_en() must set both bits to
enable the converter.
When a consumer calls regulator_enable() on vout for the first time:
_regulator_enable()
_regulator_is_enabled()
lm51772_regulator_is_enabled()
lm51772_get_conv_en()
Because lm51772_get_conv_en() uses ||, it returns 1 while LM51772_CONV_EN is
still 0, causing _regulator_enable() to treat vout as already enabled and
skip calling _regulator_do_enable().
[ ... ]
> +static int lm51772_set_ov(struct regulator_dev *rdev, int lim_uV)
> +{
> + static const struct linear_range ovp_ranges[] = {
> + LINEAR_RANGE(4000000, 0, 0x18, 500000),
> + LINEAR_RANGE(16000000, 0x18, 0x3f, 1000000),
> + };
> + struct regmap *regmap = rdev_get_regmap(rdev);
> + struct device *dev = rdev_get_dev(rdev);
> + unsigned int sel;
> + bool found;
> + int ret;
> +
> + ret = linear_range_get_selector_high_array(ovp_ranges, ARRAY_SIZE(ovp_ranges), lim_uV, &sel,
> + &found);
> + if (ret || !found)
> + return -EINVAL;
[Severity: Medium]
Does checking !found in lm51772_set_ov() reject valid over-voltage protection
thresholds when the target voltage plus lim_uV is below 4.0 V?
Because vout supports output voltages down to 1.0 V while ovp_ranges starts
at 4.0 V (4000000 uV), calling linear_range_get_selector_high_array() with
lim_uV < 4000000 sets sel = 0 (4.0 V, the lowest available upper-bound OVP
threshold), sets found = false, and returns 0.
Checking !found causes lm51772_set_ov() to return -EINVAL instead of
selecting 4.0 V, and since fixed_ovp is set on vout, _regulator_do_set_voltage()
will also fail with -EINVAL whenever vout + lim_uV is under 4.0 V.
> +
> + dev_dbg(dev, "OVP set to selector 0x%02x\n", sel);
> +
> + return regmap_write(regmap, LM51772_REG_MFR_SPECIFIC_D5, sel);
> +}
> +
> +static int lm51772_regulator_set_over_voltage_protection(struct regulator_dev *rdev, int lim_uV,
> + int severity, bool enable)
> +{
> + int ret;
> +
> + if (!enable)
> + return -EOPNOTSUPP;
> +
> + if (severity != REGULATOR_SEVERITY_PROT)
> + return -EOPNOTSUPP;
> +
> + /* Use default OVP threshold when no limit is requested. */
> + if (!lim_uV)
> + return regmap_write(rdev_get_regmap(rdev), LM51772_REG_MFR_SPECIFIC_D5,
> + LM51772_OVP_DEFAULT);
> +
> + ret = regulator_get_voltage_rdev(rdev);
> + if (ret < 0)
> + return ret;
> +
> + return lm51772_set_ov(rdev, ret + lim_uV);
> +}
[Severity: Medium]
When regulator-ov-protection-microvolt is configured and vout is raised to a
new voltage above the previous absolute OVP threshold, can this trip a false
hardware OVP fault during the voltage ramp?
With fixed_ovp enabled on vout in lm51772_regulator_descs,
_regulator_do_set_voltage() first writes the new voltage selector via
ops->set_voltage_sel() and sleeps for fsleep(delay) while the output slews,
and only afterwards invokes ops->set_over_voltage_protection():
_regulator_do_set_voltage()
ops->set_voltage_sel()
fsleep(delay)
handle_notify_limits(..., ops->set_over_voltage_protection, ...)
lm51772_regulator_set_over_voltage_protection()
When stepping vout up (for example from 5 V to 20 V with a 1 V OVP margin,
where LM51772_REG_MFR_SPECIFIC_D5 is initially set to 6 V), the converter
ramps past 6 V during fsleep(delay) before LM51772_REG_MFR_SPECIFIC_D5 is
updated to 21 V. Should the OVP threshold be raised before increasing the
output voltage?
[ ... ]
> +static int lm51772_regulator_set_current_limit(struct regulator_dev *rdev, int min_uA, int max_uA)
> +{
> + struct lm51772_priv *priv = rdev_get_drvdata(rdev);
> + struct regmap *regmap = rdev_get_regmap(rdev);
> + unsigned int current_uA;
> + unsigned int sel;
> +
> + if (priv->iset_pin)
> + return -EOPNOTSUPP;
> +
> + if (min_uA > LM51772_ILIM_MAX_UA || max_uA < LM51772_ILIM_MIN_UA)
> + return -EINVAL;
> +
> + if (max_uA > LM51772_ILIM_MAX_UA)
> + max_uA = LM51772_ILIM_MAX_UA;
> +
> + sel = LM51772_UA_TO_ILIM_SEL(priv->sense_res_micro_ohms, max_uA);
> + sel = clamp_val(sel, LM51772_ILIM_THR_MIN, LM51772_ILIM_THR_MAX);
> +
> + /* Check if the selected value is still within the allowed range after computation. */\n> + current_uA = LM51772_ILIM_SEL_TO_UA(priv->sense_res_micro_ohms, sel);
> + if (current_uA < min_uA)
> + return -EINVAL;
[Severity: High]
Can clamping sel up to LM51772_ILIM_THR_MIN cause
lm51772_regulator_set_current_limit() to program a current limit higher than
max_uA when a smaller sense resistor is used?
For example, if shunt-resistor-micro-ohms is 5000 (where LM51772_ILIM_THR_MIN
0x0A corresponds to 1000000 uA) and the caller requests min_uA = 500000 and
max_uA = 600000, LM51772_UA_TO_ILIM_SEL() computes sel = 6, which clamp_val()
raises to 0x0A. Recomputing current_uA gives 1000000 uA, and because only
current_uA < min_uA is checked (without checking current_uA > max_uA), the
driver programs 1.0 A into LM51772_REG_ILIM_THRESHOLD, exceeding max_uA.
Also, LM51772_ILIM_MIN_UA and LM51772_ILIM_MAX_UA hardcode the current range
for a 10 mOhm resistor rather than scaling with priv->sense_res_micro_ohms,
which rejects valid selectors when a different sense resistor is configured.
[ ... ]
> +static int lm51772_irq_map_event(int irq, struct regulator_irq_data *rid, unsigned long *dev_mask)
> +{
> + struct regmap *regmap = rid->data;
> + unsigned int status, reg, errors;
> + bool dev_off;
> + size_t bit;
> + int ret;
> +
> + ret = regmap_read(regmap, LM51772_REG_STATUS_BYTE, &status);
> + if (ret)
> + return REGULATOR_FAILED_RETRY;
> +
> + errors = lm51772_status2err(status);
> + rid->opaque = status;
> +
> + dev_off = (status & LM51772_STAT_OFF) != 0;
> +
> + ret = regmap_read(regmap, LM51772_REG_CLEAR_FAULTS, ®);
> + if (ret)
> + return REGULATOR_FAILED_RETRY;
> +
> + for (size_t i = 0; i < rid->num_states; i++) {
> + rid->states[i].errors = errors & rid->states[i].possible_errs;
> + rid->states[i].notifs = 0;
> +
> + for_each_set_bit(bit, &rid->states[i].errors, BITS_PER_TYPE(rid->states[i].errors))
> + rid->states[i].notifs |= regulator_err2notif(BIT(bit));
> +
> + if (dev_off)
> + rid->states[i].notifs |= REGULATOR_EVENT_DISABLE;
> +
> + if (rid->states[i].errors || dev_off)
> + *dev_mask |= BIT(i);
> + }
> +
> + return 0;
> +}
> +
> +static int lm51772_irq_renable(struct regulator_irq_data *rid)
> +{
> + struct regmap *regmap = rid->data;
> + unsigned int reg;
> + int ret;
> +
> + ret = regmap_read(regmap, LM51772_REG_STATUS_BYTE, ®);
> + if (ret)
> + return REGULATOR_FAILED_RETRY;
> +
> + if (rid->opaque && rid->opaque == reg) {
> + ret = regmap_read(regmap, LM51772_REG_CLEAR_FAULTS, ®);
> + if (ret)
> + return REGULATOR_FAILED_RETRY;
> +
> + return REGULATOR_ERROR_ON;
> + }
> +
> + rid->opaque = 0;
> +
> + return REGULATOR_ERROR_CLEARED;
> +}
[Severity: High]
Are LM51772_STAT_OFF and rid->opaque handled as intended in
lm51772_irq_map_event() and lm51772_irq_renable()?
In lm51772_irq_map_event(), dev_off reflects LM51772_STAT_OFF for the main
buck-boost converter (vout), including when vout is simply disabled in
software. However, the loop applies dev_off to all regulators in
rid->num_states, which sends REGULATOR_EVENT_DISABLE to the auxiliary vcc1
LDO (and to vout on warning interrupts while vout is software-disabled).
In lm51772_irq_renable(), comparing the raw status byte with
rid->opaque == reg has two issues:
1. If an interrupt occurs when status is only LM51772_STAT_OFF (for example,
an INPUT/VIN_UV event while vout is off), reading LM51772_REG_CLEAR_FAULTS
does not clear LM51772_STAT_OFF, so rid->opaque == reg stays true and
lm51772_irq_renable() returns REGULATOR_ERROR_ON in an infinite polling loop.
2. If multiple status bits were set and one bit changes while other fault
bits remain active in reg, rid->opaque == reg evaluates to false, skipping
LM51772_REG_CLEAR_FAULTS and prematurely returning REGULATOR_ERROR_CLEARED.
> +
> +static int lm51772_vout_of_parse(struct device_node *np, const struct regulator_desc *desc,
> + struct regulator_config *cfg)
> +{
> + u32 sense_res_micro_ohms, active_discharge_strength, pcm_window_low;
> + struct lm51772_priv *priv = cfg->driver_data;
> + int ret;
[ ... ]
> + priv->iset_pin = of_property_read_bool(np, "ti,iset-pin");
[Severity: Medium]
What happens to priv->iset_pin if the optional regulators/vout DT node is
absent?
In lm51772_reg_defaults, LM51772_REG_MFR_SPECIFIC_D9 defaults to 0x2C, which
has LM51772_SEL_ISET_PIN (BIT(5)) set to 1 (selecting the external ISET pin).
However, priv->iset_pin is zero-initialized to false in lm51772_probe() and
LM51772_SEL_ISET_PIN is only updated in lm51772_vout_of_parse().
If the vout child node is omitted (or on non-DT instantiation via
lm51772_id), regulator_of_get_init_data() does not invoke
lm51772_vout_of_parse(), leaving LM51772_SEL_ISET_PIN set to 1 in hardware
while priv->iset_pin is false in software. As a result,
lm51772_regulator_get_current_limit() and
lm51772_regulator_set_current_limit() will read and write the inactive
LM51772_REG_ILIM_THRESHOLD register instead of returning -EOPNOTSUPP (or
clearing LM51772_SEL_ISET_PIN).
[ ... ]
> +static int lm51772_setup_irq(struct device *dev, struct regmap *regmap, int irq,
> + struct regulator_dev *rdev[LM51772_N_REGS], bool nint)
> +{
> + int common_errs = REGULATOR_ERROR_OVER_TEMP | REGULATOR_ERROR_OVER_TEMP_WARN |
> + REGULATOR_ERROR_FAIL;
> + int per_rdev_errs[] = {
> + REGULATOR_ERROR_OVER_CURRENT | REGULATOR_ERROR_OVER_VOLTAGE_WARN,
> + 0,
> + };
> + struct regulator_irq_desc irq_desc = {
> + .name = "lm51772-irq",
> + .data = regmap,
> + .map_event = lm51772_irq_map_event,
> + };
> + void *retp;
> +
> + if (!nint) {
> + irq_desc.renable = lm51772_irq_renable;
> + irq_desc.irq_off_ms = 1000;
> + irq_desc.reread_ms = 200;
> + }
> +
> + retp = devm_regulator_irq_helper(dev, &irq_desc, irq, 0, common_errs, per_rdev_errs, rdev,
> + LM51772_N_REGS);
> + if (IS_ERR(retp))
> + return PTR_ERR(retp);
> +
> + return regmap_update_bits(regmap, LM51772_REG_MFR_SPECIFIC_D1, LM51772_EN_NINT,
> + FIELD_PREP(LM51772_EN_NINT, nint));
> +}
[Severity: Medium]
When nint is true, how do cached error flags in rdev->cached_err get cleared?
Because common_errs and per_rdev_errs are passed to
devm_regulator_irq_helper(), init_rdev_errors() sets rdev->use_cached_err =
true, and regulator_notifier_isr() latches any reported errors via
rdev_flag_err(). However, rdev_clear_err() is only called from
regulator_notifier_isr_work(), which is only scheduled when
irq_desc.irq_off_ms is non-zero. With irq_off_ms left at 0 when nint is true,
_regulator_get_error_flags() will permanently OR the latched rdev->cached_err
flags into every subsequent regulator_get_error_flags() call even after the
fault clears.
Also, should LM51772_EN_NINT be configured via regmap_update_bits() before
calling devm_regulator_irq_helper() so the IRQ handler is not active while
the pin is still in its default nFLT mode?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-paul-lm51772-v2-0-b2a767e34722@bootlin.com?part=4
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 4/4] regulator: Add TI LM51772 support
2026-10-27 10:01 ` sashiko-bot
@ 2026-10-27 11:36 ` Paul Louvel
0 siblings, 0 replies; 10+ messages in thread
From: Paul Louvel @ 2026-10-27 11:36 UTC (permalink / raw)
To: sashiko-reviews, Paul Louvel; +Cc: devicetree, robh, conor+dt
On Tue Oct 27, 2026 at 11:01 AM CET, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
> - [High] regulator: lm51772: use logical AND in lm51772_get_conv_en()
> - [High] regulator: lm51772: fix current limit bounds and max_uA check
> - [High] regulator: lm51772: fix STAT_OFF and fault checks in IRQ callbacks
> - [Medium] regulator: lm51772: fix cached errors and IRQ order in lm51772_setup_irq()
> - [Medium] regulator: lm51772: allow OVP limits below 4V in lm51772_set_ov()
> - [Medium] regulator: lm51772: OVP updated after voltage ramp-up trips false OVP fault
> - [Medium] regulator: lm51772: state mismatch between priv->iset_pin and hardware default
> --
>
> commit 85d9f9bf5e7e89e913ab51cc149fc605a17772db
> Author: Paul Louvel <paul.louvel@bootlin.com>
>
> regulator: Add TI LM51772 support
>
> This commit adds a regulator driver for the TI LM51772 I2C buck-boost
> voltage regulator and its auxiliary 5 V LDO output.
>
>> diff --git a/drivers/regulator/lm51772-regulator.c b/drivers/regulator/lm51772-regulator.c
>> new file mode 100644
>> index 0000000000000..66f4541b50994
>> --- /dev/null
>> +++ b/drivers/regulator/lm51772-regulator.c
>
> [ ... ]
>
>> +static int lm51772_set_conv_en(struct regmap *regmap, bool enable)
>> +{
>> + int ret;
>> +
>> + ret = regmap_update_bits(regmap, LM51772_REG_USB_PD_CONTROL_0, LM51772_CONV_EN2,
>> + FIELD_PREP(LM51772_CONV_EN2, enable));
>> + if (ret)
>> + return ret;
>> +
>> + return regmap_update_bits(regmap, LM51772_REG_MFR_SPECIFIC_D0, LM51772_CONV_EN,
>> + FIELD_PREP(LM51772_CONV_EN, enable));
>> +}
>> +
>> +static int lm51772_get_conv_en(struct regmap *regmap)
>> +{
>> + unsigned int pd_ctrl, cfg0;
>> + int ret;
>> +
>> + ret = regmap_read(regmap, LM51772_REG_USB_PD_CONTROL_0, &pd_ctrl);
>> + if (ret)
>> + return ret;
>> +
>> + ret = regmap_read(regmap, LM51772_REG_MFR_SPECIFIC_D0, &cfg0);
>> + if (ret)
>> + return ret;
>> +
>> + return FIELD_GET(LM51772_CONV_EN2, pd_ctrl) || FIELD_GET(LM51772_CONV_EN, cfg0);
>> +}
>
> [Severity: High]
> Should lm51772_get_conv_en() use && instead of || here?
>
> In lm51772_reg_defaults (and at hardware reset), LM51772_REG_USB_PD_CONTROL_0
> is 0x01 (so LM51772_CONV_EN2 is 1) while LM51772_REG_MFR_SPECIFIC_D0 is 0x32
> (so LM51772_CONV_EN is 0), and lm51772_set_conv_en() must set both bits to
> enable the converter.
>
> When a consumer calls regulator_enable() on vout for the first time:
>
> _regulator_enable()
> _regulator_is_enabled()
> lm51772_regulator_is_enabled()
> lm51772_get_conv_en()
>
> Because lm51772_get_conv_en() uses ||, it returns 1 while LM51772_CONV_EN is
> still 0, causing _regulator_enable() to treat vout as already enabled and
> skip calling _regulator_do_enable().
>
> [ ... ]
>
No. It might need a comment in the code to clarify this.
Both bits are internally OR'ed, so whenever one of them is set, the converter is
enabled.
When enabling / disabling, we update both bits, just to be consistent.
>> +static int lm51772_set_ov(struct regulator_dev *rdev, int lim_uV)
>> +{
>> + static const struct linear_range ovp_ranges[] = {
>> + LINEAR_RANGE(4000000, 0, 0x18, 500000),
>> + LINEAR_RANGE(16000000, 0x18, 0x3f, 1000000),
>> + };
>> + struct regmap *regmap = rdev_get_regmap(rdev);
>> + struct device *dev = rdev_get_dev(rdev);
>> + unsigned int sel;
>> + bool found;
>> + int ret;
>> +
>> + ret = linear_range_get_selector_high_array(ovp_ranges, ARRAY_SIZE(ovp_ranges), lim_uV, &sel,
>> + &found);
>> + if (ret || !found)
>> + return -EINVAL;
>
> [Severity: Medium]
> Does checking !found in lm51772_set_ov() reject valid over-voltage protection
> thresholds when the target voltage plus lim_uV is below 4.0 V?
>
> Because vout supports output voltages down to 1.0 V while ovp_ranges starts
> at 4.0 V (4000000 uV), calling linear_range_get_selector_high_array() with
> lim_uV < 4000000 sets sel = 0 (4.0 V, the lowest available upper-bound OVP
> threshold), sets found = false, and returns 0.
>
> Checking !found causes lm51772_set_ov() to return -EINVAL instead of
> selecting 4.0 V, and since fixed_ovp is set on vout, _regulator_do_set_voltage()
> will also fail with -EINVAL whenever vout + lim_uV is under 4.0 V.
>
Yes, setting it to 4.0V should be the correct behavior.
>> +
>> + dev_dbg(dev, "OVP set to selector 0x%02x\n", sel);
>> +
>> + return regmap_write(regmap, LM51772_REG_MFR_SPECIFIC_D5, sel);
>> +}
>> +
>> +static int lm51772_regulator_set_over_voltage_protection(struct regulator_dev *rdev, int lim_uV,
>> + int severity, bool enable)
>> +{
>> + int ret;
>> +
>> + if (!enable)
>> + return -EOPNOTSUPP;
>> +
>> + if (severity != REGULATOR_SEVERITY_PROT)
>> + return -EOPNOTSUPP;
>> +
>> + /* Use default OVP threshold when no limit is requested. */
>> + if (!lim_uV)
>> + return regmap_write(rdev_get_regmap(rdev), LM51772_REG_MFR_SPECIFIC_D5,
>> + LM51772_OVP_DEFAULT);
>> +
>> + ret = regulator_get_voltage_rdev(rdev);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return lm51772_set_ov(rdev, ret + lim_uV);
>> +}
>
> [Severity: Medium]
> When regulator-ov-protection-microvolt is configured and vout is raised to a
> new voltage above the previous absolute OVP threshold, can this trip a false
> hardware OVP fault during the voltage ramp?
>
> With fixed_ovp enabled on vout in lm51772_regulator_descs,
> _regulator_do_set_voltage() first writes the new voltage selector via
> ops->set_voltage_sel() and sleeps for fsleep(delay) while the output slews,
> and only afterwards invokes ops->set_over_voltage_protection():
>
> _regulator_do_set_voltage()
> ops->set_voltage_sel()
> fsleep(delay)
> handle_notify_limits(..., ops->set_over_voltage_protection, ...)
> lm51772_regulator_set_over_voltage_protection()
>
> When stepping vout up (for example from 5 V to 20 V with a 1 V OVP margin,
> where LM51772_REG_MFR_SPECIFIC_D5 is initially set to 6 V), the converter
> ramps past 6 V during fsleep(delay) before LM51772_REG_MFR_SPECIFIC_D5 is
> updated to 21 V. Should the OVP threshold be raised before increasing the
> output voltage?
>
> [ ... ]
>
Described in last review.
>> +static int lm51772_regulator_set_current_limit(struct regulator_dev *rdev, int min_uA, int max_uA)
>> +{
>> + struct lm51772_priv *priv = rdev_get_drvdata(rdev);
>> + struct regmap *regmap = rdev_get_regmap(rdev);
>> + unsigned int current_uA;
>> + unsigned int sel;
>> +
>> + if (priv->iset_pin)
>> + return -EOPNOTSUPP;
>> +
>> + if (min_uA > LM51772_ILIM_MAX_UA || max_uA < LM51772_ILIM_MIN_UA)
>> + return -EINVAL;
>> +
>> + if (max_uA > LM51772_ILIM_MAX_UA)
>> + max_uA = LM51772_ILIM_MAX_UA;
>> +
>> + sel = LM51772_UA_TO_ILIM_SEL(priv->sense_res_micro_ohms, max_uA);
>> + sel = clamp_val(sel, LM51772_ILIM_THR_MIN, LM51772_ILIM_THR_MAX);
>> +
>> + /* Check if the selected value is still within the allowed range after computation. */\n> + current_uA = LM51772_ILIM_SEL_TO_UA(priv->sense_res_micro_ohms, sel);
>> + if (current_uA < min_uA)
>> + return -EINVAL;
>
> [Severity: High]
> Can clamping sel up to LM51772_ILIM_THR_MIN cause
> lm51772_regulator_set_current_limit() to program a current limit higher than
> max_uA when a smaller sense resistor is used?
>
> For example, if shunt-resistor-micro-ohms is 5000 (where LM51772_ILIM_THR_MIN
> 0x0A corresponds to 1000000 uA) and the caller requests min_uA = 500000 and
> max_uA = 600000, LM51772_UA_TO_ILIM_SEL() computes sel = 6, which clamp_val()
> raises to 0x0A. Recomputing current_uA gives 1000000 uA, and because only
> current_uA < min_uA is checked (without checking current_uA > max_uA), the
> driver programs 1.0 A into LM51772_REG_ILIM_THRESHOLD, exceeding max_uA.
>
> Also, LM51772_ILIM_MIN_UA and LM51772_ILIM_MAX_UA hardcode the current range
> for a 10 mOhm resistor rather than scaling with priv->sense_res_micro_ohms,
> which rejects valid selectors when a different sense resistor is configured.
>
> [ ... ]
>
Yes. I should use a linear range array to describe which value this register can
take. Clamping was lazy and wrong.
>> +static int lm51772_irq_map_event(int irq, struct regulator_irq_data *rid, unsigned long *dev_mask)
>> +{
>> + struct regmap *regmap = rid->data;
>> + unsigned int status, reg, errors;
>> + bool dev_off;
>> + size_t bit;
>> + int ret;
>> +
>> + ret = regmap_read(regmap, LM51772_REG_STATUS_BYTE, &status);
>> + if (ret)
>> + return REGULATOR_FAILED_RETRY;
>> +
>> + errors = lm51772_status2err(status);
>> + rid->opaque = status;
>> +
>> + dev_off = (status & LM51772_STAT_OFF) != 0;
>> +
>> + ret = regmap_read(regmap, LM51772_REG_CLEAR_FAULTS, ®);
>> + if (ret)
>> + return REGULATOR_FAILED_RETRY;
>> +
>> + for (size_t i = 0; i < rid->num_states; i++) {
>> + rid->states[i].errors = errors & rid->states[i].possible_errs;
>> + rid->states[i].notifs = 0;
>> +
>> + for_each_set_bit(bit, &rid->states[i].errors, BITS_PER_TYPE(rid->states[i].errors))
>> + rid->states[i].notifs |= regulator_err2notif(BIT(bit));
>> +
>> + if (dev_off)
>> + rid->states[i].notifs |= REGULATOR_EVENT_DISABLE;
>> +
>> + if (rid->states[i].errors || dev_off)
>> + *dev_mask |= BIT(i);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int lm51772_irq_renable(struct regulator_irq_data *rid)
>> +{
>> + struct regmap *regmap = rid->data;
>> + unsigned int reg;
>> + int ret;
>> +
>> + ret = regmap_read(regmap, LM51772_REG_STATUS_BYTE, ®);
>> + if (ret)
>> + return REGULATOR_FAILED_RETRY;
>> +
>> + if (rid->opaque && rid->opaque == reg) {
>> + ret = regmap_read(regmap, LM51772_REG_CLEAR_FAULTS, ®);
>> + if (ret)
>> + return REGULATOR_FAILED_RETRY;
>> +
>> + return REGULATOR_ERROR_ON;
>> + }
>> +
>> + rid->opaque = 0;
>> +
>> + return REGULATOR_ERROR_CLEARED;
>> +}
>
> [Severity: High]
> Are LM51772_STAT_OFF and rid->opaque handled as intended in
> lm51772_irq_map_event() and lm51772_irq_renable()?
>
> In lm51772_irq_map_event(), dev_off reflects LM51772_STAT_OFF for the main
> buck-boost converter (vout), including when vout is simply disabled in
> software. However, the loop applies dev_off to all regulators in
> rid->num_states, which sends REGULATOR_EVENT_DISABLE to the auxiliary vcc1
> LDO (and to vout on warning interrupts while vout is software-disabled).
>
> In lm51772_irq_renable(), comparing the raw status byte with
> rid->opaque == reg has two issues:
>
> 1. If an interrupt occurs when status is only LM51772_STAT_OFF (for example,
> an INPUT/VIN_UV event while vout is off), reading LM51772_REG_CLEAR_FAULTS
> does not clear LM51772_STAT_OFF, so rid->opaque == reg stays true and
> lm51772_irq_renable() returns REGULATOR_ERROR_ON in an infinite polling loop.
>
This is the intended behavior.
renable is only called when the pin act as nFLT. When the unit is off, the OFF
bit will be set, until the unit is enabled again.
In the mean time, the IRQ have to be disabled, so REGULATOR_ERROR_ON must be
returned. This is not an infinite loop if may be turned on again, otherwise,
this is a intended infinite loop.
> 2. If multiple status bits were set and one bit changes while other fault
> bits remain active in reg, rid->opaque == reg evaluates to false, skipping
> LM51772_REG_CLEAR_FAULTS and prematurely returning REGULATOR_ERROR_CLEARED.
>
Yes and this is the intended behavior.
If a bit change we want to have an irq and map event again.
LM51772_REG_CLEAR_FAULTS will be called.
>> +
>> +static int lm51772_vout_of_parse(struct device_node *np, const struct regulator_desc *desc,
>> + struct regulator_config *cfg)
>> +{
>> + u32 sense_res_micro_ohms, active_discharge_strength, pcm_window_low;
>> + struct lm51772_priv *priv = cfg->driver_data;
>> + int ret;
>
> [ ... ]
>
>> + priv->iset_pin = of_property_read_bool(np, "ti,iset-pin");
>
> [Severity: Medium]
> What happens to priv->iset_pin if the optional regulators/vout DT node is
> absent?
>
> In lm51772_reg_defaults, LM51772_REG_MFR_SPECIFIC_D9 defaults to 0x2C, which
> has LM51772_SEL_ISET_PIN (BIT(5)) set to 1 (selecting the external ISET pin).
> However, priv->iset_pin is zero-initialized to false in lm51772_probe() and
> LM51772_SEL_ISET_PIN is only updated in lm51772_vout_of_parse().
>
> If the vout child node is omitted (or on non-DT instantiation via
> lm51772_id), regulator_of_get_init_data() does not invoke
> lm51772_vout_of_parse(), leaving LM51772_SEL_ISET_PIN set to 1 in hardware
> while priv->iset_pin is false in software. As a result,
> lm51772_regulator_get_current_limit() and
> lm51772_regulator_set_current_limit() will read and write the inactive
> LM51772_REG_ILIM_THRESHOLD register instead of returning -EOPNOTSUPP (or
> clearing LM51772_SEL_ISET_PIN).
>
> [ ... ]
>
Yes.
>> +static int lm51772_setup_irq(struct device *dev, struct regmap *regmap, int irq,
>> + struct regulator_dev *rdev[LM51772_N_REGS], bool nint)
>> +{
>> + int common_errs = REGULATOR_ERROR_OVER_TEMP | REGULATOR_ERROR_OVER_TEMP_WARN |
>> + REGULATOR_ERROR_FAIL;
>> + int per_rdev_errs[] = {
>> + REGULATOR_ERROR_OVER_CURRENT | REGULATOR_ERROR_OVER_VOLTAGE_WARN,
>> + 0,
>> + };
>> + struct regulator_irq_desc irq_desc = {
>> + .name = "lm51772-irq",
>> + .data = regmap,
>> + .map_event = lm51772_irq_map_event,
>> + };
>> + void *retp;
>> +
>> + if (!nint) {
>> + irq_desc.renable = lm51772_irq_renable;
>> + irq_desc.irq_off_ms = 1000;
>> + irq_desc.reread_ms = 200;
>> + }
>> +
>> + retp = devm_regulator_irq_helper(dev, &irq_desc, irq, 0, common_errs, per_rdev_errs, rdev,
>> + LM51772_N_REGS);
>> + if (IS_ERR(retp))
>> + return PTR_ERR(retp);
>> +
>> + return regmap_update_bits(regmap, LM51772_REG_MFR_SPECIFIC_D1, LM51772_EN_NINT,
>> + FIELD_PREP(LM51772_EN_NINT, nint));
>> +}
>
> [Severity: Medium]
> When nint is true, how do cached error flags in rdev->cached_err get cleared?
>
> Because common_errs and per_rdev_errs are passed to
> devm_regulator_irq_helper(), init_rdev_errors() sets rdev->use_cached_err =
> true, and regulator_notifier_isr() latches any reported errors via
> rdev_flag_err(). However, rdev_clear_err() is only called from
> regulator_notifier_isr_work(), which is only scheduled when
> irq_desc.irq_off_ms is non-zero. With irq_off_ms left at 0 when nint is true,
> _regulator_get_error_flags() will permanently OR the latched rdev->cached_err
> flags into every subsequent regulator_get_error_flags() call even after the
> fault clears.
>
Will be fixed by keeping irq_off_ms, but set the renable callback to NULL when
nint is true.
> Also, should LM51772_EN_NINT be configured via regmap_update_bits() before
> calling devm_regulator_irq_helper() so the IRQ handler is not active while
> the pin is still in its default nFLT mode?
Would be better, yes.
--
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/4] regulator: dt-bindings: Add TI LM51772 support
2026-10-27 9:39 ` [PATCH v2 1/4] regulator: dt-bindings: Add TI LM51772 support Paul Louvel
@ 2026-10-29 8:41 ` Krzysztof Kozlowski
2026-10-29 9:05 ` Paul Louvel
0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2026-10-29 8:41 UTC (permalink / raw)
To: Paul Louvel
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-kernel, devicetree, Thomas Petazzoni,
Herve Codina, Miquel Raynal
On Tue, Oct 27, 2026 at 10:39:23AM +0100, Paul Louvel wrote:
> + regulator-temp-warn-kelvin:
> + enum: [0, 1, 368, 383, 398, 413]
> +
> + regulator-ramp-delay:
> + enum: [40000, 20000, 1000, 500]
> +
> + regulator-min-microvolt:
> + minimum: 1000000
> + maximum: 48000000
> +
> + regulator-max-microvolt:
> + minimum: 1000000
> + maximum: 48000000
> +
> + regulator-min-microamp:
> + minimum: 500000
> + maximum: 7000000
> +
> + regulator-max-microamp:
> + minimum: 500000
> + maximum: 7000000
> +
> + unevaluatedProperties: false
Place this line after description, that's the convention for child nodes
for readability.
> +
> + vcc1:
> + type: object
> + $ref: regulator.yaml#
> + description: |
> + LDO regulator for auxiliary use.
> + It can used to power, for example, a PD controller that will drive this PMIC.
> +
> + properties:
> + regulator-min-microvolt:
> + minimum: 5000000
> + maximum: 5000000
> +
> + regulator-max-microvolt:
> + minimum: 5000000
> + maximum: 5000000
> +
> + unevaluatedProperties: false
Place this line after description, that's the convention for child nodes
for readability.
> +
> + additionalProperties: false
Place this line after description (for relevant block), that's the
convention for child nodes for readability.
> +
> + ti,sync-pin:
> + description: |
> + Selects the function of the SYNC pin, used when several converters
> + operate in parallel.
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [input-rising, input-falling, output-rising, output-falling]
> + default: input-rising
> +
> + ti,ivp-thres-microvolt:
Use either existing over-voltage-threshold-microvolt from battery schema
or ti,ovp-microvolt from the other TI binding.
> + description: |
> + Input Overvoltage Protection and Regulation Threshold.
> + It shall avoid any damage to the device in case the current flows from the
> + output to the input and the input source cannot sink current.
> + This threshold is used for IVP (Input Voltage Protection) and IVR (Input
> + Voltage Regulation).
> + oneOf:
> + - multipleOf: 125000
> + minimum: 4750000
> + maximum: 23250000
> + - multipleOf: 250000
> + minimum: 23500000
> + maximum: 50000000
> + default: 50000000
> +
> + ti,ivp:
> + description: Enable the IVP feature of the regulator.
> + type: boolean
> +
> + ti,ivr:
> + description: Enable the IVR feature of the regulator.
> + type: boolean
Why wouldn't you want to enable these two always?
> +
> + ti,nint:
> + description: |
> + Configures the nFLT/nINT pin to act as an interrupt pin.
> +
> + nFLT mode: The pin remains low until the cause of the status change ceases.
> + In this mode use an level irq trigger.
> +
> + nINT mode: The pin is asserted low to signal a status change.
> + In this mode use an edge irq trigger.
> + type: boolean
> +
> + ti,cable-drop-compensation:
> + description: |
> + The cable drop compensation feature helps to keep the output voltage at the
> + nominal value over a wide range of load current without the need for
> + additional remote sensing. The cable drop compensation measures the
> + current and offsets the output voltage proportionally to the measured
> + current.
> + type: boolean
> +
> +dependencies:
> + ti,ivr: ["ti,ivp"]
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/4] regulator: dt-bindings: Add TI LM51772 support
2026-10-29 8:41 ` Krzysztof Kozlowski
@ 2026-10-29 9:05 ` Paul Louvel
0 siblings, 0 replies; 10+ messages in thread
From: Paul Louvel @ 2026-10-29 9:05 UTC (permalink / raw)
To: Krzysztof Kozlowski, Paul Louvel
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-kernel, devicetree, Thomas Petazzoni,
Herve Codina, Miquel Raynal
Hi Krzysztof,
On Thu Oct 29, 2026 at 9:41 AM CET, Krzysztof Kozlowski wrote:
> On Tue, Oct 27, 2026 at 10:39:23AM +0100, Paul Louvel wrote:
>> + regulator-temp-warn-kelvin:
>> + enum: [0, 1, 368, 383, 398, 413]
>> +
>> + regulator-ramp-delay:
>> + enum: [40000, 20000, 1000, 500]
>> +
>> + regulator-min-microvolt:
>> + minimum: 1000000
>> + maximum: 48000000
>> +
>> + regulator-max-microvolt:
>> + minimum: 1000000
>> + maximum: 48000000
>> +
>> + regulator-min-microamp:
>> + minimum: 500000
>> + maximum: 7000000
>> +
>> + regulator-max-microamp:
>> + minimum: 500000
>> + maximum: 7000000
>> +
>> + unevaluatedProperties: false
>
> Place this line after description, that's the convention for child nodes
> for readability.
>
>> +
>> + vcc1:
>> + type: object
>> + $ref: regulator.yaml#
>> + description: |
>> + LDO regulator for auxiliary use.
>> + It can used to power, for example, a PD controller that will drive this PMIC.
>> +
>> + properties:
>> + regulator-min-microvolt:
>> + minimum: 5000000
>> + maximum: 5000000
>> +
>> + regulator-max-microvolt:
>> + minimum: 5000000
>> + maximum: 5000000
>> +
>> + unevaluatedProperties: false
>
> Place this line after description, that's the convention for child nodes
> for readability.
>
>> +
>> + additionalProperties: false
>
> Place this line after description (for relevant block), that's the
> convention for child nodes for readability.
>
>> +
>> + ti,sync-pin:
>> + description: |
>> + Selects the function of the SYNC pin, used when several converters
>> + operate in parallel.
>> + $ref: /schemas/types.yaml#/definitions/string
>> + enum: [input-rising, input-falling, output-rising, output-falling]
>> + default: input-rising
>> +
>> + ti,ivp-thres-microvolt:
>
> Use either existing over-voltage-threshold-microvolt from battery schema
> or ti,ovp-microvolt from the other TI binding.
IVP is not the same as OVP.
>
>> + description: |
>> + Input Overvoltage Protection and Regulation Threshold.
>> + It shall avoid any damage to the device in case the current flows from the
>> + output to the input and the input source cannot sink current.
>> + This threshold is used for IVP (Input Voltage Protection) and IVR (Input
>> + Voltage Regulation).
>> + oneOf:
>> + - multipleOf: 125000
>> + minimum: 4750000
>> + maximum: 23250000
>> + - multipleOf: 250000
>> + minimum: 23500000
>> + maximum: 50000000
>> + default: 50000000
>> +
>> + ti,ivp:
>> + description: Enable the IVP feature of the regulator.
>> + type: boolean
>> +
>> + ti,ivr:
>> + description: Enable the IVR feature of the regulator.
>> + type: boolean
>
> Why wouldn't you want to enable these two always?
>
By default, these are off when the device is powered on and these features exist
only for reverse-current situation.
Also if IVP and IVR is on, only the regulation feature is enabled, not the
protection feature. This is a requirement from the datasheet. It is not ideal
and I should not reflect that requirement in the DT.
It should be ti,ivp or ti,ivr but not both because it is confusing.
Or:
ti,input-voltage-mode:
enum: [protection, regulation]
...
>> +
>> + ti,nint:
>> + description: |
>> + Configures the nFLT/nINT pin to act as an interrupt pin.
>> +
>> + nFLT mode: The pin remains low until the cause of the status change ceases.
>> + In this mode use an level irq trigger.
>> +
>> + nINT mode: The pin is asserted low to signal a status change.
>> + In this mode use an edge irq trigger.
>> + type: boolean
>> +
>> + ti,cable-drop-compensation:
>> + description: |
>> + The cable drop compensation feature helps to keep the output voltage at the
>> + nominal value over a wide range of load current without the need for
>> + additional remote sensing. The cable drop compensation measures the
>> + current and offsets the output voltage proportionally to the measured
>> + current.
>> + type: boolean
>> +
>> +dependencies:
>> + ti,ivr: ["ti,ivp"]
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>
> Best regards,
> Krzysztof
Thanks.
Best regards,
--
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-10-29 9:05 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27 9:39 [PATCH v2 0/4] Add TI LM51772 buck-boost voltage regulator Paul Louvel
2026-10-27 9:39 ` [PATCH v2 1/4] regulator: dt-bindings: Add TI LM51772 support Paul Louvel
2026-10-29 8:41 ` Krzysztof Kozlowski
2026-10-29 9:05 ` Paul Louvel
2026-10-27 9:39 ` [PATCH v2 2/4] regulator: Support regulators where the voltage selector spans two registers Paul Louvel
2026-10-27 9:39 ` [PATCH v2 3/4] regulator: core: Call set_over_voltage_protection() if OVP limit is fixed after setting new voltage Paul Louvel
2026-10-27 9:50 ` sashiko-bot
2026-10-27 9:39 ` [PATCH v2 4/4] regulator: Add TI LM51772 support Paul Louvel
2026-10-27 10:01 ` sashiko-bot
2026-10-27 11:36 ` Paul Louvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox