Re: [PATCH v11 06/12] pwm: imx27: Use 64-bit division macro and function

[Guru Das Srinagesh]

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?

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);

Thank you.

Guru Das.