Re: [PATCH 2/2] regulator: qcom-rpmh: Add support to read regulator settings

[Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>]

Hi Mark,

On Mon, Jun 23, 2025 at 10:34 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Jun 23, 2025 at 10:13:41PM +0530, Kamal Wadhwa wrote:
>
> > To address this issue, enhance the `get_voltage_sel()`,
> > `get_mode()`, and `is_enabled()` callbacks to read the regulator
> > settings directly from the RPMH hardware using the `rpmh_read()`
>
> Two things here.  One is that my understanding was that at least some of
> the firmwares simply do not provide read functionality - this new code
> will turn that into an error if it's the case.  The other is that
> there's an expectation that the read operations will return the value
> that was configured by the host, we might get confused if that's not the
> case.  I'm not sure if there's paths that are currently implemented
> that'd have issues, but it's a concern.

This change should not violate the 2 things you have highlighted.

To elaborate, the regulator status will be read by APPS as ON if the rail was
requested ON by APPS, before reaching kernel stage.  And will *not* be
seen as ON if it’s voted from some other subsystem DSP.

One important note here (about an exception to the above statement),
Internally all rails are ‘assigned' to a subsystem. Even rails shared between 2
or more subsystems are 'internally assigned’ to only one of those subsystems.

Boot loader code initializes the RPMh votes of the 'assigned' subsystem to match
the physical status of each regulator after the power-on sequence completes.
This ensures correct RPMh regulator parameter aggregation when subsystems
issue their own votes.

So, if a rail which is internally assigned to the APPS subsystem, gets
turned ON by
PMIC HW ( power ON sequence), in such cases too,  the rail may be seen
ON from APPS side. This is the exception.

But this exception only applies if default ON rails is ‘internally assigned’ to
APPS subsystem. And will *not* happen if the rail was assigned to some other
subsystem DSP (and was turned ON from PMIC HW). In such a case status
will still be seen as OFF from APPS (as expected), even though they may actually
be ON.


>
> For the enable there's a separate status callback that should be
> implemented, and you could bootstrap the state.  For the voltage
> readback it's a range that's configured so it should be fine to just do
> this I think, though I'd need to go double check the code for keeping
> multiple supplies tied within a range.

Sure, I will bootstrap the state(ON/OFF), and move the read logic to
get_status() op.