Re: [PATCH v4 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Kamal Wadhwa]

On Tue, Apr 21, 2026 at 10:16:48PM +0100, Mark Brown wrote:
> On Mon, Apr 20, 2026 at 09:13:40PM +0530, Kamal Wadhwa wrote:
> > Currently, during regulator registration, regulator framework sends an
> > unnecessary `min-microvolts` request for the rpmh-regulator device. This
> > happens because in current design, we do not have a way to readback the
> > voltage settings that was set during the bootloader stage.
> 
> > +static int _rpmh_regulator_vrm_get_voltage(struct regulator_dev *rdev, int *uV)
> > +{
> > +	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
> > +	struct tcs_cmd cmd = {
> > +		.addr = vreg->addr + RPMH_REGULATOR_REG_VRM_VOLTAGE,
> > +	};
> > +	int min_uV = rdev->constraints->min_uV;
> > +	int max_uV = rdev->constraints->max_uV;
> > +	int ret, _uV = 0;
> > +
> > +	ret = rpmh_regulator_read_data(vreg, &cmd);
> > +	if (!ret)
> > +		_uV = (cmd.data & RPMH_REGULATOR_VOLTAGE_MASK) * 1000;
> > +	else
> > +		dev_err(vreg->dev, "failed to read VOLTAGE ret = %d\n", ret);
> > +
> > +	if (!_uV || (_uV >= min_uV && _uV <= max_uV))
> > +		*uV = _uV;
> > +	else
> > +		dev_err(vreg->dev, "read voltage %d is out-of-range[%d:%d]\n",
> > +						_uV, min_uV, max_uV);
> > +
> > +	return ret;
> > +}
> 
> Why are we constraining the reported voltage?  The hardware may have a
> value that's outside of our constraints, we should report that if it's
> the case.

This will help with backward compatibility.

To elaborate, till now for all chipsets using RPMH regulator driver, the boot
time voltage setting would always be overwritten by `min-microvolts` by the
framework. However, i found during the development of this patch that on some
chipsets like SM8550, SM8660 etc for one or two rails there exists some
misalignments between the bootloader voltage setting and the APPS side min/max
voltage limits(subset of RPMH firmware min/max) where the voltage value is
set lower or higher from the bootloader side compared to the DT min/max, which
caused the regulator registeration failure and all the sibling regulators under
the same pmic will also fail to register, even though issue was with only one
regulator.

Ideally this should be fixed from the bootloader or RPMH firmware side, but
since that may not be possible to do retrospectively for older chipsets using
older bootloader/RPMH firmware images, i thought to add this as an error print
then to cause a probe failure.

Note - After printing the error, the code will flow the same way as it would
have if the read capabilty was not there i.e regulator framework would proceed
to use `min-microvolts` as initial setting. So for the older targets this
doesnt break anything while making sure to report this as an error print so it
can be caught and fixed for newer chipsets.