Messages in this thread
 From Arnd Bergmann <> Date Fri, 3 Apr 2020 21:13:18 +0200 Subject Re: [PATCH v11 06/12] pwm: imx27: Use 64-bit division macro and function
`On Fri, Apr 3, 2020 at 7:37 PM Guru Das Srinagesh <gurus@codeaurora.org> wrote:>> On Thu, Apr 02, 2020 at 11:16:22PM +0200, Arnd Bergmann wrote:> > This looks correct, but very expensive, and you don't really have to> > go this far, given that c1 is guaranteed to be a 32-bit number, and> > you divide by a constant in the end.> >> > Why not do something like> >> > #define SHIFT 41 /* arbitrarily picked, not too big, not too small */> > #define MUL 2199 /* 2^SHIFT / NSEC_PER_SEC */> > period_cycles = clk_get_rate(imx->clk_per) * ((state->period * MUL) >> SHIFT);>> I have two concerns with this:>> 1. This actually results in the division by 1000010575.5125057 instead>    of NSECS_PER_SEC whereas both the existing as well as the proposed logic>    divide exactly by NSECS_PER_SEC.> 2. What method shall be used to pick the SHIFT value? How is this to be>    chosen?I picked one that would result in a fairly accurate rounding and did notseem too likely to overflow.> Also, this seems sort of similar to my initial attempt at this> problem, where period was being pre-divided prior to the multiplication,> which was (rightly) NACKed.>>         c *= div_u64(state->period, 1000000000);Ah, right, I did make the same mistake here. What I actually indendedwas to do two separate shifts that add up to 41. I suppose there is also stillthe fundamental problem that without bounds checking the calculation canalways overflow for large inputs.       Arnd`

Last update: 2020-04-03 21:14    [W:0.077 / U:1.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site