Date:   Thu, 15 Apr 2021 08:48:54 +0200
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Clemens Gruber <clemens.gruber@...ruber.com>
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:45:32PM +0200, Clemens Gruber wrote:
> 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?

No, I don't think so for several reasons. First, just rounding down is
easier (and keeping lowlevel drivers rules and implementation easy is
IMHO a good goal). The second reason is that round-nearest is a bit
ambigous because round to the nearest frequency is slightly different to
round to the nearest period length. So to actually implement (or use)
it correctly, people have to grasp that difference. Compared to that
rounding down the period length corresponds 1:1 to rounding up
frequency. That's easy.

For the third reason I have to backup a bit: I intend to introduce a
function pwm_round_rate that predicts what pwm_apply_rate will actually
implement. Of course it must have the same rounding rules. This allows
to implement efficient search for consumers that e.g. prefer
round-nearest time, or round-nearest frequency. I'm convinced that
searching the optimal request to make is easier if round_rate uses
round-down and not round-nearest.

All three reasons boil down to "the math for round-down is just simpler
(for implementers and for users) than with round-nearest".

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists