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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 8 Oct 2014 15:56:27 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Javier Martinez Canillas <javier.martinez@...labora.co.uk>
Cc:	Doug Anderson <dianders@...omium.org>,
	Chanwoo Choi <cw00.choi@...sung.com>,
	Olof Johansson <olof@...om.net>,
	Chris Zhong <zyw@...k-chips.com>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	Abhilash Kesavan <kesavan.abhilash@...il.com>,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH 5/5] ARM: dts: Add initial regulator mode on exynos Peach
 boards

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?

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)