[PATCH 0/3] regulator: mp886x: add MP8864 support

[PATCH 0/3] regulator: mp886x: add MP8864 support

[Jisheng Zhang]

The MP8864 is a 4 A, 21 V synchronous step-down converter.  Its
register layout and voltage transition handling are compatible with the
MP8867, but its selectable switching frequencies are 600 kHz, 850 kHz,
1.1 MHz and 1.6 MHz.

Add the compatible and chip-specific switching-frequency table. 

patch1 fixes the vsel_mask
patch2 document the mp8864 dt-binding
patch3 add the mp8864 support to the driver

Jisheng Zhang (3):
  regulator: mp886x: fix vsel_mask
  regulator: dt-bindings: mp886x: support mp8864
  regulator: mp886x: add MP8864 support

 .../bindings/regulator/mps,mp886x.yaml        | 16 +++++++++++--
 drivers/regulator/Kconfig                     |  2 +-
 drivers/regulator/mp886x.c                    | 23 +++++++++++++++++--
 3 files changed, 36 insertions(+), 5 deletions(-)

-- 
2.51.0

[PATCH 1/3] regulator: mp886x: fix vsel_mask

[Jisheng Zhang]

The MP886X vsel is 7bits, fix the vsel_mask.

Fixes: 97be82880b61 ("regulator: add support for MP8869 regulator")
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/regulator/mp886x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/mp886x.c b/drivers/regulator/mp886x.c
index e0b62bc02a1e..a10f3a20d413 100644
--- a/drivers/regulator/mp886x.c
+++ b/drivers/regulator/mp886x.c
@@ -267,7 +267,7 @@ static int mp886x_regulator_register(struct mp886x_device_info *di,
 	rdesc->min_uV = 600000;
 	rdesc->uV_step = 10000;
 	rdesc->vsel_reg = MP886X_VSEL;
-	rdesc->vsel_mask = 0x3f;
+	rdesc->vsel_mask = 0x7f;
 	rdesc->ramp_reg = MP886X_SYSCNTLREG1;
 	rdesc->ramp_mask = MP886X_SLEW_MASK;
 	rdesc->ramp_delay_table = di->ci->slew_rates;
-- 
2.51.0

[PATCH 2/3] regulator: dt-bindings: mp886x: support mp8864

[Jisheng Zhang]

The MP8864 is a 4 A, 21 V synchronous step-down converter.  Its
register layout and voltage transition handling are compatible with the
MP8867, but its selectable switching frequencies are 600 kHz, 850 kHz,
1.1 MHz and 1.6 MHz.

Add the compatible and chip-specific switching-frequency table.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Assisted-by: Codex:gpt-5
---
 .../bindings/regulator/mps,mp886x.yaml           | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/mps,mp886x.yaml b/Documentation/devicetree/bindings/regulator/mps,mp886x.yaml
index 374a4f6b1e23..6cf7c1c3c0c3 100644
--- a/Documentation/devicetree/bindings/regulator/mps,mp886x.yaml
+++ b/Documentation/devicetree/bindings/regulator/mps,mp886x.yaml
@@ -4,17 +4,30 @@
 $id: http://devicetree.org/schemas/regulator/mps,mp886x.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Monolithic Power Systems MP8867/MP8869 voltage regulator
+title: Monolithic Power Systems MP8864/MP8867/MP8869 voltage regulator
 
 maintainers:
   - Jisheng Zhang <jszhang@kernel.org>
 
 allOf:
   - $ref: regulator.yaml#
+  - if:
+      properties:
+        compatible:
+          const: mps,mp8864
+    then:
+      properties:
+        mps,switch-frequency-hz:
+          enum: [600000, 850000, 1100000, 1600000]
+    else:
+      properties:
+        mps,switch-frequency-hz:
+          enum: [500000, 750000, 1000000, 1250000, 1500000]
 
 properties:
   compatible:
     enum:
+      - mps,mp8864
       - mps,mp8867
       - mps,mp8869
 
@@ -33,7 +46,6 @@ properties:
 
   mps,switch-frequency-hz:
     description: The valid switch frequency in Hertz.
-    enum: [500000, 750000, 1000000, 1250000, 1500000]
 
 required:
   - compatible
-- 
2.51.0

[PATCH 3/3] regulator: mp886x: add MP8864 support

[Jisheng Zhang]

The MP8864 is a 4 A, 21 V synchronous step-down converter.  Its
register layout and voltage transition handling are compatible with the
MP8867, but its selectable switching frequencies are 600 kHz, 850 kHz,
1.1 MHz and 1.6 MHz.

Add the compatible and chip-specific switching-frequency table.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Assisted-by: Codex:gpt-5
---
 drivers/regulator/Kconfig  |  2 +-
 drivers/regulator/mp886x.c | 21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 89789ac7a786..c8940e2c7e04 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -864,7 +864,7 @@ config REGULATOR_MP8859
 	  module. The module will be named "mp8859".
 
 config REGULATOR_MP886X
-	tristate "MPS MP8869 regulator driver"
+	tristate "MPS MP8864/MP8867/MP8869 regulator driver"
 	depends on I2C && OF
 	select REGMAP_I2C
 	help
diff --git a/drivers/regulator/mp886x.c b/drivers/regulator/mp886x.c
index a10f3a20d413..14f60cdcbbd6 100644
--- a/drivers/regulator/mp886x.c
+++ b/drivers/regulator/mp886x.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 //
-// MP8867/MP8869 regulator driver
+// MP8864/MP8867/MP8869 regulator driver
 //
 // Copyright (C) 2020 Synaptics Incorporated
 //
@@ -229,6 +229,23 @@ static const struct regulator_ops mp8867_regulator_ops = {
 	.set_ramp_delay = regulator_set_ramp_delay_regmap,
 };
 
+static const struct mp886x_cfg_info mp8864_ci = {
+	.rops = &mp8867_regulator_ops,
+	.slew_rates = {
+		64000,
+		32000,
+		16000,
+		8000,
+		4000,
+		2000,
+		1000,
+		500,
+	},
+	.switch_freq = { 600000, 850000, 1100000, 1600000 },
+	.fs_reg = MP886X_SYSCNTLREG1,
+	.fs_shift = 1,
+};
+
 static const struct mp886x_cfg_info mp8867_ci = {
 	.rops = &mp8867_regulator_ops,
 	.slew_rates = {
@@ -341,6 +358,7 @@ static int mp886x_i2c_probe(struct i2c_client *client)
 }
 
 static const struct of_device_id mp886x_dt_ids[] = {
+	{ .compatible = "mps,mp8864", .data = &mp8864_ci },
 	{ .compatible = "mps,mp8867", .data = &mp8867_ci },
 	{ .compatible = "mps,mp8869", .data = &mp8869_ci },
 	{ }
@@ -348,6 +366,7 @@ static const struct of_device_id mp886x_dt_ids[] = {
 MODULE_DEVICE_TABLE(of, mp886x_dt_ids);
 
 static const struct i2c_device_id mp886x_id[] = {
+	{ .name = "mp8864", .driver_data = (kernel_ulong_t)&mp8864_ci },
 	{ .name = "mp8867", .driver_data = (kernel_ulong_t)&mp8867_ci },
 	{ .name = "mp8869", .driver_data = (kernel_ulong_t)&mp8869_ci },
 	{ }
-- 
2.51.0

Re: [PATCH 3/3] regulator: mp886x: add MP8864 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Regulator map failure for scaled feedback voltages exceeding the internal reference limit.
- [Medium] Division by zero in `mp8869_scale` if `mps,fb-voltage-divider` provides 0 for the second resistor value.
--

commit 11c9edb9604a047f4176e93b2eddadc949a9e73b
Author: Jisheng Zhang <jszhang@kernel.org>

regulator: mp886x: add MP8864 support

This commit adds support for the MP8864 synchronous step-down converter to the
mp886x regulator driver. It updates the Kconfig description and adds a new
device tree compatible and specific switching-frequency table.

> diff --git a/drivers/regulator/mp886x.c b/drivers/regulator/mp886x.c
> index a10f3a20d413f..14f60cdcbbd6b 100644
> --- a/drivers/regulator/mp886x.c
> +++ b/drivers/regulator/mp886x.c
[ ... ]
> @@ -229,6 +229,23 @@ static const struct regulator_ops mp8867_regulator_ops = {
>  	.set_ramp_delay = regulator_set_ramp_delay_regmap,
>  };
>  
> +static const struct mp886x_cfg_info mp8864_ci = {
> +	.rops = &mp8867_regulator_ops,
[Severity: High]
This is a pre-existing issue, but by using mp8867_regulator_ops, does the
MP8864 inherit a potential initialization failure for typical step-down
output voltages?

In drivers/regulator/mp886x.c:mp8867_get_voltage_sel(), when fbloop is true,
the driver scales up the internal reference voltage and attempts to map it:

	uv = mp8869_scale(uv, di->r[0], di->r[1]);
	return regulator_map_voltage_linear(rdev, uv, uv);

However, since rdev->desc->min_uV (0.6V) and rdev->desc->uV_step (10mV) are
hardcoded for the unscaled internal reference, the maximum mappable voltage
appears capped at 1.87V. If the scaled voltage is above 1.87V, does
regulator_map_voltage_linear() return -EINVAL and prevent the regulator from
syncing its state?

> +	.slew_rates = {
> +		64000,
> +		32000,
> +		16000,
> +		8000,
> +		4000,
> +		2000,
> +		1000,
> +		500,
> +	},
> +	.switch_freq = { 600000, 850000, 1100000, 1600000 },
> +	.fs_reg = MP886X_SYSCNTLREG1,
> +	.fs_shift = 1,
> +};
> +
[ ... ]
> @@ -341,6 +358,7 @@ static int mp886x_i2c_probe(struct i2c_client *client)
>  }
>  
>  static const struct of_device_id mp886x_dt_ids[] = {
> +	{ .compatible = "mps,mp8864", .data = &mp8864_ci },
[Severity: Medium]
This is a pre-existing issue, but if the device tree for an MP8864 device
includes the "mps,fb-voltage-divider" property with a 0 value for the second
resistor, could this cause a division by zero during boot?

In drivers/regulator/mp886x.c:mp886x_i2c_probe(), the resistor values are
read without validation:

	ret = of_property_read_u32_array(np, "mps,fb-voltage-divider",
					 di->r, 2);

If the initial voltage selector is queried, mp8869_scale() uses di->r[1] as
a divisor:

	u32 tmp = uv * r1 / r2;

>  	{ .compatible = "mps,mp8867", .data = &mp8867_ci },
>  	{ .compatible = "mps,mp8869", .data = &mp8869_ci },
>  	{ }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901043123.5401-1-jszhang@kernel.org?part=3

Re: [PATCH 1/3] regulator: mp886x: fix vsel_mask

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware I2C read/write errors during initialization are silently ignored, resulting in negative error codes stored in unsigned integers and corrupted state logic.
- [Medium] Potential division by zero in `mp8869_scale` due to unvalidated device tree properties.
--

commit e23dcb351cfbd15403625e41999e01844e07121f
Author: Jisheng Zhang <jszhang@kernel.org>

    regulator: mp886x: fix vsel_mask
    
    The MP886X vsel is 7bits, fix the vsel_mask.
    
    Fixes: 97be82880b61 ("regulator: add support for MP8869 regulator")

> diff --git a/drivers/regulator/mp886x.c b/drivers/regulator/mp886x.c
> index e0b62bc02a1e1..a10f3a20d413f 100644
> --- a/drivers/regulator/mp886x.c
> +++ b/drivers/regulator/mp886x.c
> @@ -267,14 +267,14 @@ static int mp886x_regulator_register(struct mp886x_device_info *di,
>  	rdesc->min_uV = 600000;
>  	rdesc->uV_step = 10000;
>  	rdesc->vsel_reg = MP886X_VSEL;
> -	rdesc->vsel_mask = 0x3f;
> +	rdesc->vsel_mask = 0x7f;
>  	rdesc->ramp_reg = MP886X_SYSCNTLREG1;
>  	rdesc->ramp_mask = MP886X_SLEW_MASK;
>  	rdesc->ramp_delay_table = di->ci->slew_rates;
>  	rdesc->n_ramp_values = ARRAY_SIZE(di->ci->slew_rates);
>  	rdesc->owner = THIS_MODULE;
>  
>  	rdev = devm_regulator_register(di->dev, &di->desc, config);
>  	if (IS_ERR(rdev))
>  		return PTR_ERR(rdev);
>  	di->sel = rdesc->ops->get_voltage_sel(rdev);

[Severity: High]
This is a pre-existing issue, but does this code properly handle negative error
codes if I2C communication fails?

If get_voltage_sel() fails and returns -EIO or another negative error code,
it gets implicitly cast and stored in the unsigned int di->sel. The error
is never returned to the caller.

This negative value could later cause issues in mp8867_set_voltage_sel():

drivers/regulator/mp886x.c:mp8867_set_voltage_sel() {
	...
	delta = di->sel - sel;
	if (abs(delta) <= 5)
		ret = regmap_update_bits(rdev->regmap, MP886X_SYSCNTLREG1,
					 MP886X_GO, 0);
	...
}

If di->sel holds an error code like -5 (0xFFFFFFFB), and sel is 0,
delta becomes -5. Since abs(-5) <= 5 is true, the driver incorrectly
writes to the register to clear MP886X_GO, masquerading an I2C error as a
valid small voltage step.

Additionally, hardware initialization errors appear to be ignored elsewhere,
such as in mp886x_set_switch_freq() where the return value of
regmap_update_bits() is not checked. Could the driver end up succeeding in
its probe even if the hardware is completely unresponsive?

[Severity: Medium]
This is also a pre-existing issue, but is there a risk of a division by zero
when reading the divider values from the device tree?

In mp886x_i2c_probe(), the driver reads the mps,fb-voltage-divider array:

drivers/regulator/mp886x.c:mp886x_i2c_probe() {
	...
	ret = of_property_read_u32_array(np, "mps,fb-voltage-divider",
					 di->r, 2);
	if (ret)
		return ret;
	...
}

There does not seem to be a check ensuring that the denominator (di->r[1])
is strictly positive. If the device tree provides a zero value for this second
resistor, wouldn't this cause a kernel panic when mp8869_scale() divides by
r2:

drivers/regulator/mp886x.c:mp8869_scale() {
	...
	u32 tmp = uv * r1 / r2;
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901043123.5401-1-jszhang@kernel.org?part=1

Re: [PATCH 2/3] regulator: dt-bindings: mp886x: support mp8864

[Conor Dooley]

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

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

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