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

[Mark Brown <broonie@kernel.org>]

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

On Thu, May 17, 2018 at 01:48:41PM -0700, David Collins wrote:

> The RPMh hardware is configured by the boot loader.  The configuration
> does reflect reality; however, it cannot handle all configurations at
> initialization time.  Specific headroom management typically comes up in
> modem usecases for RF supplies that are sensitive to noise.  This feature

...

> If you really don't like having this feature present in the Linux RPMh
> regulator driver, then I'd be ok removing it.  It is not required for
> SDM845 which the driver is initially targeting.

It's certainly going to be easier to review it separately.

> >> In the case of XOB managed LDO regulators, the LDOs physically can be
> >> configured to different voltages by the bootloader.  However, the RPMh
> >> interface provides no mechanism for the application processor to read or
> >> change that voltage.  Therefore, we need a way to specify such voltages in
> >> a board specific (as opposed to driver specific) manner (i.e. device tree).

> > Is the kernel somehow prevented from varying these voltages?

> Yes.  Physically, there exists no RPMh register to read or write the
> voltage of LDOs managed via XOB.  Additionally, the kernel running on the
> application processor is blocked from configuring the voltage via a direct
> SPMI writes by access permissions that crash the system when violated.

*sigh*  Please provide feedback on the problems this (and everything
else) is causing to the team working on the firmware.  The number of
issues with this interface compared to anything else we've seen is
really noticable, I see what it's trying to do with providing something
like the regulator API is doing but there's quite a lot of missing steps
in it which cause no end of problems for general purpose software
written on top of it.

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