Re: [PATCH v2 2/3] regulator: pfuze100: add set_suspend_disable for LDO ops

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:
- [Medium] The callback `pfuze100_ldo_set_suspend_disable` does not explici=
tly clear the Low Power mode bit (`VGENxLPWR`), relying on the fragile assu=
mption that the bootloader left it at its reset value of 0. If it is 1, the=
 LDO enters low-power mode instead of powering off.
--

--- Patch [2]: [PATCH v2 2/3] regulator: pfuze100: add set_suspend_disable =
for LDO ops ---
commit b5ef15eff59cfdbe79ee9305f34ea04f79252ea1
Author: Joy Zou <[email protected]>

regulator: pfuze100: add set_suspend_disable for LDO ops

This commit adds a set_suspend_disable callback to pfuze100_ldo_regulator_o=
ps.
This supports the regulator-off-in-suspend DTS property for the VGEN LDO
regulators, allowing unused LDO regulators to be properly disabled during
system suspend.

> diff --git a/drivers/regulator/pfuze100-regulator.c b/drivers/regulator/p=
fuze100-regulator.c
> index 7d56c22b5e400..d3a0e25055248 100644
> --- a/drivers/regulator/pfuze100-regulator.c
> +++ b/drivers/regulator/pfuze100-regulator.c

[ ... ]

> @@ -158,6 +158,25 @@ static int pfuze100_set_ramp_delay(struct regulator_=
dev *rdev, int ramp_delay)
>  	return ret;
>  }
> =20
> +static int pfuze100_ldo_set_suspend_disable(struct regulator_dev *rdev)
> +{
> +	struct pfuze_chip *pfuze100 =3D rdev_get_drvdata(rdev);
> +	int id =3D rdev_get_id(rdev);
> +	struct pfuze_regulator *desc =3D &pfuze100->regulator_descs[id];
> +
> +	/*
> +	 * Set the standby bit so the LDO output is turned off when the PMIC
> +	 * receives a STANDBY event, using the per-regulator stby_reg/stby_mask
> +	 * that describe the standby control for each LDO.
> +	 *
> +	 * The stby_mask only covers the VGENxSTBY bit. The VGENxLPWR stays at
> +	 * its reset value of 0, so the LDO is switched off rather than put
> +	 * into low-power mode.
> +	 */
> +	return regmap_update_bits(pfuze100->regmap, desc->stby_reg,
> +				  desc->stby_mask, desc->stby_mask);

[Severity: Medium]
Will this sequence reliably turn off the regulator if the bootloader has
previously set the VGENxLPWR bit to 1?

Since this only updates the VGENxSTBY bit defined in stby_mask, it relies on
the VGENxLPWR bit remaining at its reset value of 0.

If the bootloader modified this bit, does the LDO enter low-power mode
instead of completely powering off during system suspend?

Could the VGENxLPWR bit be explicitly cleared here to guarantee the regulat=
or
turns off, similar to how it is handled in pfuze_power_off_prepare()?

> +}
> +
>  static const struct regulator_ops pfuze100_ldo_regulator_ops =3D {
>  	.enable =3D regulator_enable_regmap,
>  	.disable =3D regulator_disable_regmap,

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731-b4-regulat=
[email protected]?part=3D2