linux-kernel - Re: [PATCH] regulator: pwm: Fix regulator ramp delay for continuous mode

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:	Tue, 28 Jun 2016 12:31:19 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Mark Brown <broonie@...nel.org>
Cc:	Boris Brezillon <boris.brezillon@...e-electrons.com>,
	Lee Jones <lee.jones@...aro.org>,
	Brian Norris <briannorris@...omium.org>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	Heiko Stuebner <heiko@...ech.de>,
	Liam Girdwood <lgirdwood@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] regulator: pwm: Fix regulator ramp delay for continuous mode

Mark,

On Tue, Jun 28, 2016 at 11:52 AM, Mark Brown <broonie@...nel.org> wrote:
> On Mon, Jun 27, 2016 at 09:53:11PM -0700, Douglas Anderson wrote:
>
>> Note also that the upper bound of usleep_range probably shouldn't be a
>> full 1 ms longer than the lower bound since I've seen plenty of hardware
>> with a ramp rate of ~5000 uS / uV and for small jumps the total delays
>> are in the tens of uS.  1000 is way too much.  We'll try to be dynamic
>> and use 10%
>
> Surely the upper bound here is just an upper bound and we're essentially
> just saying that "anything over minimum is fine" here?  Though now I
> look at the implementation it seems it's doing something entirely
> unehelpful and actually trying to delay for the longest possible time
> which doesn't seem like what we want or what the usleep_range() API
> would suggest :(

Yeah, you're thinking like someone who wants your computer to perform
fast.  That's just simply the wrong mentality to have (how dare you?).
You need to think about power savings, and a sleeping computer is a
computer that doesn't draw much power.  Thus, you want to sleep as
long as you can.  :-P

In all seriousness, I think the design for usleep_range() is to try to
batch up wakeups to allow longer periods of sleeping and to optimize
power.  This you want to sleep as long as is allowable and then if you
happen to wakeup for some other reason anyway then you process all the
things whose minimum has passed.  IIRC it was trying to get back to
the good old days of only having jiffies where everyone was
synchronized on the tick and you could sleep till the next tick after
all the work was done.  When you think of it this way then sleeping to
the max makes sense.  ...but it also means that you need to be careful
and really not set the max too high.

Of course, in practice I've found that often usleep_range() 99% of the
time sleeps for the max.  That can lead to some very subtle bugs if
your min sleep is not long enough (!).

-Doug