linux-kernel - Re: [PATCH 03/14] pwm: rockchip: Fix period and duty

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: <20160607172522.GA130037@google.com>
Date:	Tue, 7 Jun 2016 10:25:22 -0700
From:	Brian Norris <briannorris@...omium.org>
To:	Boris Brezillon <boris.brezillon@...e-electrons.com>
Cc:	Thierry Reding <thierry.reding@...il.com>,
	linux-pwm@...r.kernel.org, Mark Brown <broonie@...nel.org>,
	Liam Girdwood <lgirdwood@...il.com>,
	Heiko Stuebner <heiko@...ech.de>,
	linux-rockchip@...ts.infradead.org,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Milo Kim <milo.kim@...com>,
	Doug Anderson <dianders@...gle.com>,
	Caesar Wang <wxt@...k-chips.com>,
	Stephen Barber <smbarber@...omium.org>,
	Ajit Pal Singh <ajitpal.singh@...com>,
	Srinivas Kandagatla <srinivas.kandagatla@...il.com>,
	Maxime Coquelin <maxime.coquelin@...com>,
	Patrice Chotard <patrice.chotard@...com>, kernel@...inux.com
Subject: Re: [PATCH 03/14] pwm: rockchip: Fix period and duty_cycle
 approximation

On Sat, Jun 04, 2016 at 08:19:55AM +0200, Boris Brezillon wrote:
> On Fri, 3 Jun 2016 13:03:26 -0700
> Brian Norris <briannorris@...omium.org> wrote:
> 
> > On Fri, Jun 03, 2016 at 10:23:01AM +0200, Boris Brezillon wrote:
> > > The current implementation always round down the duty and period
> > > values, while it would be better to round them to the closest integer.  
> > 
> > Agreed. As I noted to you elsewhere, not having this change can cause
> > problems where doing a series of pwm_get_state() / modify /
> > pwm_apply_state() will propagate rounding errors, which will change the
> > period unexpectedly. e.g., I have an expected period of 3.333 us and a
> > clk rate of 112.666667 MHz -- the clock frequency doesn't divide evenly,
> > so the period (stashed in nanoseconds) shrinks when we convert to the
> > register value and back, as follows:
> > 
> >  pwm_apply_state(): register = period * 112666667 / 1000000000;
> >  pwm_get_state(): period = register * 1000000000 / 112666667;
> > 
> > or in other words:
> > 
> >  period = period * 112666667 / 1000000000 * 1000000000 / 112666667;
> > 
> > which yields a sequence like:
> > 
> >  3333 -> 3328
> >  3328 -> 3319
> >  3319 -> 3310
> >  3310 -> 3301
> >  3301 -> 3292
> >  3292 -> ... (etc) ...
> > 
> > With this patch, we'd see instead:
> > 
> >  period = div_round_closest(period * 112666667, 1000000000) * 1000000000 / 112666667;
> > 
> > which yields a stable sequence:
> > 
> >  3333 -> 3337
> >  3337 -> 3337
> >  3337 -> ... (etc) ...
> 
> Woh! Thanks for the detailed explanation. Do you want me to put that in
> a comment explaining why we're using DIV_ROUND_CLOSEST_ULL()?

If you'd like, feel free to add some of this to your v2 description.

Brian