Re: [PATCH 5/5] ARM: dts: Add initial regulator mode on exynos Peach boards

[Mark Brown <broonie@kernel.org>]

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

On Wed, Oct 08, 2014 at 03:44:07PM +0200, Javier Martinez Canillas wrote:

> The regulator core now has support to choose a default initial
> operating mode for regulators from DT. Set the initial opmode
> for the max77802 PMIC regulators with the same modes that are
> used in the downstream ChromeOS kernel, in order to allow the
> system to lower power at suspend time.

The stated goal of this change doesn't appear to correspond to what it
actually does.  It's saying that we want to be able to set the regulator
into low power modes on suspend which is a sensible and useful thing to
want to do (especially for regulators which don't have physical suspend
modes which we currently make any effort to handle) but what the change
actually does is cause us to set the default state of supplies on boot.

The device tree should describe what it's trying to achieve, otherwise
even if things happen to work right now it's going to be vulnerable to
being broken by future kernel changes which take what it's saying at
face value.

>  			buck2_reg: BUCK2 {
> @@ -201,6 +203,7 @@
>  				regulator-always-on;
>  				regulator-boot-on;
>  				regulator-ramp-delay = <12500>;
> +				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
>  			};

This appears to set the supply which is labelled as VDD_ARM into standby
mode by default (from a first glance the change appears to set all
supplies into standby mode) and of course we currently have no way of
changing the mode at runtime in DT systems.  Are you *really* sure this
is a good idea of which an electrical engineer would approve, CPU cores
are typically one of the most demanding workloads available for a
regulator?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]