Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

From: Mark Brown <broonie@kernel.org>
To: David Collins <collinsd@codeaurora.org>
Cc: Stephen Boyd <swboyd@chromium.org>,
	lgirdwood@gmail.com, mark.rutland@arm.com, robh+dt@kernel.org,
	linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, rnayak@codeaurora.org,
	ilina@codeaurora.org
Subject: Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver
Date: Tue, 24 Apr 2018 18:41:11 +0100	[thread overview]
Message-ID: <20180424174111.GH22073@sirena.org.uk> (raw)
In-Reply-To: <38f42537-f801-115a-4120-1344a67a0462@codeaurora.org>

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

On Fri, Apr 20, 2018 at 12:28:21PM -0700, David Collins wrote:
> On 04/19/2018 05:08 AM, Mark Brown wrote:

> > This doesn't sound like what the min_dropout_uV constraint is intended
> > to handle - that's there for the regulator driver (not constraints) to
> > indicate how much headroom the regulator needs in the supply voltage in
> > order to provide regulation.  It's not something the regulator uses,
> > it's something that gets fed into voltage requests made on the supply of
> > the regulator which I can't see that the hardware is going to be able to
> > handle unaided.

> RPMh hardware enforces the requested minimum headroom voltage for all
> regulators with a parent.  It has full knowledge of the parent-child
> connections of regulators on the board (as programmed by the bootloader).
> It automatically reconfigures the parent voltage when needed as a result
> of requests changing the voltage of any of its child regulators.

If the hardware has full knowledge of all these constraints and enforces
them transparently then why does the kernel care that it's doing that?
Doesn't it defeat the point of it doing all this stuff if we have to
know about it?

> > Ideally future versions of the RPM will have improved interfaces,
> > there's a bunch of problems like this :(

> Do you have a preference for qcom,regulator-initial-microvolt vs a generic
> framework supported regulator-initial-microvolt property for configuring a
> specific voltage at registration time?  We'll need to have support for one
> or the other in order for the qcom_rpmh-regulator driver to be functional.

This is basically specific to Qualcomm, I can't off hand think of any
other devices with similar issues.

> > Yes, constraints that specify a single voltage are done by setting min
> > and max to the same value.  fixed_uV is *only* for regulators that have
> > a physically fixed voltage.

> XOB managed regulators physically cannot change voltage.  Therefore, do
> you agree that it is reasonable to use fixed_uV for them?  Note that I
> removed init_data->constraints.apply_uV manipulation in version 2 of this
> patch.

If these regulators can't change voltage then surely we know what
voltage they have without needing it to be specified in DT?

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