linux-kernel - Re: [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808

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]

Message-ID: <5451872F.4050306@collabora.co.uk>
Date:	Thu, 30 Oct 2014 01:32:47 +0100
From:	Javier Martinez Canillas <javier.martinez@...labora.co.uk>
To:	Doug Anderson <dianders@...omium.org>
CC:	Chris Zhong <zyw@...k-chips.com>,
	Heiko Stübner <heiko@...ech.de>,
	Mike Turquette <mturquette@...aro.org>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Russell King <linux@....linux.org.uk>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Kevin Hilman <khilman@...nel.org>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	Kumar Gala <galak@...eaurora.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Mark Brown <broonie@...aro.org>
Subject: Re: [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808

Hello Doug,

On 10/29/2014 05:51 PM, Doug Anderson wrote:
>>
>> In other words, do you want the core to call rk808_set_suspend_voltage() or
>> rk808_set_suspend_enable()? If the later then you can use Chanwoo's bindings:
>>
>> regulator-state-mem {
>>    regulator-on-in-suspend;
>> };
> 
> The rk808 has a separate register for storing the sleep voltage,
> though.  ...so just using "regulator-on-in-suspend" without any
> changes to the rk808 driver won't get us this "for free".  I'd expect
> to need
> 
>  regulator-state-mem {
>     regulator-on-in-suspend;
>     regulator-suspend-microvolts = <3300000>;
>  };
>

I see, then as you said the voltage has to be defined in the DT for the suspend
state.

As I mentioned in my previous email, Chanwoo's original suspend state series had
a "regulator-volt" DT property that was used to fill the struct regulator_state
.uV field so the core could call the .set_suspend_voltage function handler.

AFAIU Mark wanted the property to be renamed to "rebulator-microvolt" [0] to be
consistent with the rest of the binding and also he was not sure if the suspend
voltage needed be in the runtime voltage range specified by the properties
"regulator-{min,max}-microvolt" since Chanwoo's patch had a check for that [1].

Chanwoo decided to drop setting the suspend voltage in a following version, but
given that there is a use case for this feature, I think that those two issues
could be addressed and added that property to the generic regulator DT binding.

> IMHO it wouldn't be insane to say that if someone has
> "regulator-on-in-suspend" but no suspend voltage defined that it
> should carry forward the runtime voltage.  I actually argued for that
> earlier and Mark Brown said "no" at
> <http://marc.info/?l=linux-kernel&m=141277262106368&w=2>.  More at
> <http://marc.info/?l=linux-kernel&m=141279245027919&w=2>.  It's
> entirely possible I wasn't making myself clear, though.
> 

What I learned from previous Mark's explanations is that there is always a
reason why the regulator API split different operations. So if there is both
a .set_voltage and .set_suspend_voltage, then that is an indication for me
that if someone wants a particular voltage during suspend then it has to be
explicitly set and should not rely on the runtime voltage previously set.

The fact that the rk808 has different registers to configure the runtime
and suspend voltage seems to confirm that the API split makes sense.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/9/4/652
[1]: https://lkml.org/lkml/2014/9/4/651
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/