linux-kernel - Re: [PATCH v8 1/8] pwm: pca9685: Switch to atomic API

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: <YHdGXG3PbsmicK7U@workstation.tuxnet>
Date:   Wed, 14 Apr 2021 21:45:32 +0200
From:   Clemens Gruber <clemens.gruber@...ruber.com>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     linux-pwm@...r.kernel.org,
        Thierry Reding <thierry.reding@...il.com>,
        Sven Van Asbroeck <TheSven73@...il.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 1/8] pwm: pca9685: Switch to atomic API

On Wed, Apr 14, 2021 at 09:21:31PM +0200, Uwe Kleine-König wrote:
> On Wed, Apr 14, 2021 at 02:09:14PM +0200, Clemens Gruber wrote:
> > Hi Uwe,
> > 
> > On Tue, Apr 13, 2021 at 09:38:18PM +0200, Uwe Kleine-König wrote:
> > > Hello Clemens,
> > > 
> > > On Tue, Apr 13, 2021 at 02:11:38PM +0200, Clemens Gruber wrote:
> > > > On Mon, Apr 12, 2021 at 10:10:19PM +0200, Uwe Kleine-König wrote:
> > > > > On Mon, Apr 12, 2021 at 06:39:28PM +0200, Clemens Gruber wrote:
> > > > > > With your suggested round-down, the example with frequency of 200 Hz
> > > > > > would no longer result in 30 but 29 and that contradicts the datasheet.
> > > > > 
> > > > > Well, with PRESCALE = 30 we get a frequency of 196.88 Hz and with
> > > > > PRESCALE = 29 we get a frequency of 203.45 Hz. So no matter if you pick
> > > > > 29 or 30, you don't get 200 Hz. And which of the two possible values is
> > > > > the better one depends on the consumer, no matter what rounding
> > > > > algorithm the data sheet suggests. Also note that the math here contains
> > > > > surprises you don't expect at first. For example, what PRESCALE value
> > > > > would you pick to get 284 Hz? [If my mail was a video, I'd suggest to
> > > > > press Space now to pause and let you think first :-)] The data sheet's
> > > > > formula suggests:
> > > > > 
> > > > > 	round(25 MHz / (4096 * 284)) - 1 = 20
> > > > > 
> > > > > The resulting frequency when picking PRESCALE = 20 is 290.644 Hz (so an
> > > > > error of 6.644 Hz). If instead you pick PRESCALE = 21 you get 277.433 Hz
> > > > > (error = 6.567 Hz), so 21 is the better choice.
> > > > > 
> > > > > Exercise for the reader:
> > > > >  What is the correct formula to really determine the PRESCALE value that
> > > > >  yields the best approximation (i.e. minimizing
> > > > >  abs(real_freq - target_freq)) for a given target_freq?
> > > 
> > > I wonder if you tried this.
> > 
> > We could calculate both round-up and round-down and decide which one is
> > closer to "real freq" (even though that is not the actual frequency but
> > just our backwards-calculated frequency).
> 
> Yeah, the backwards-calculated frequency is the best assumption we
> have.
> 
> > But I can't give you a formula with minimized abs(real_freq-target_freq)
> > Is it a different round point than 0.5 and maybe relative to f ?
> > 
> > Please enlighten us :-)
> 
> Sorry, I cannot. I spend ~20 min today after lunch with pencil and
> paper, but without success. I was aware that it isn't trivial and this
> is the main reason I established round-down as default for new drivers
> instead of round-nearest.

Oh, I thought you already solved it. I tried too for a while but was
unsuccessful. Not trivial indeed!

But regarding you establishing round-down: Wouldn't it be even better if
the driver did what I suggested above, namely calculate backwards from
both the rounded-up as well as the rounded-down prescale value and then
write the one with the smallest abs(f_target - f_real) to the register?

Clemens