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

[sashiko-bot@kernel.org]

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