linux-kernel - Re: [PATCH v2] 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:	Thu, 7 Jul 2016 09:30:57 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Laxman Dewangan <ldewangan@...dia.com>
Cc:	Mark Brown <broonie@...nel.org>,
	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>,
	Thierry Reding <thierry.reding@...il.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	afrid@...dia.com
Subject: Re: [PATCH v2] regulator: pwm: Fix regulator ramp delay for
 continuous mode

Hi,

On Thu, Jul 7, 2016 at 1:36 AM, Laxman Dewangan <ldewangan@...dia.com> wrote:
>
> On Thursday 07 July 2016 12:12 AM, Douglas Anderson wrote:
>>
>> The original commit adding support for continuous voltage mode didn't
>> handle the regulator ramp delay properly.  It treated the delay as a
>> fixed delay in uS despite the property being defined as uV / uS.  Let's
>> adjust it.  Luckily there appear to be no users of this ramp delay for
>> PWM regulators (as per grepping through device trees in linuxnext).
>>
>> 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%.
>>
>> NOTE: This commit doesn't add support for regulator-enable-ramp-delay.
>> That could be done in a future patch when someone has a user of that
>> featre.
>>
>> Though this patch is shows as "fixing" a bug, there are no actual known
>> users of continuous mode PWM regulator w/ ramp delay in mainline and so
>> this likely won't have any effect on anyone unless they are working
>> out-of-tree with private patches.  For anyone in this state, it is
>> highly encouraged to also pick Boris Brezillon's WIP patches to get
>> yourself a reliable and glitch-free regulator.
>>
>> Fixes: 4773be185a0f ("regulator: pwm-regulator: Add support for
>> continuous-voltage")
>> Signed-off-by: Douglas Anderson <dianders@...omium.org>
>
>
>
> Looks fine here.
> Acked-by: Laxman Dewangan <ldewangan@...dia.com>
>
> BTW, for some PWM regulator, the settling time for voltage change is same
> for any steps.

In that case we should probably add a new PWM regulator property and
not abuse the existing one.  Maybe you use "pwm-regulator-settle-us"
or something?  ...and actually the right thing is probably to
implement 'regulator-ramp-delay' as doing several small steps in that
case.  So if you've got:

* settle time: 10 us
* ramp delay = 5000 uV / us
* voltage change of 200000 uV

In that case you'd probably want to break your 200 mV voltage change
into a few steps?  So you could do bump by 50mV, delay 10us, bump by
50mV, delay 10us, bump by 50mV, delay by 10us.

The final delay would be the same as with my patch applied but you'd
get there more smoothly.

-Doug