On Mon, Apr 25, 2022 at 03:22:44PM +0200, Uwe Kleine-König wrote: > Dividing by the result of a division looses precision because the result is > rounded twice. E.g. with clk_rate = 48000000 and period = 32760033 the > following numbers result: > > rate = pc->clk_rate >> PWM_DUTY_WIDTH = 187500 > hz = DIV_ROUND_CLOSEST_ULL(100ULL * NSEC_PER_SEC, period_ns) = 3052 > rate = DIV_ROUND_CLOSEST_ULL(100ULL * rate, hz) = 6144 > > The exact result would be 6142.5061875 and (apart from rounding) this is > found by using a single division. As a side effect is also a tad > cheaper to calculate. > > Also using clk_rate >> PWM_DUTY_WIDTH looses precision. Consider for > example clk_rate = 47999999 and period = 106667: > > mul_u64_u64_div_u64(pc->clk_rate >> PWM_DUTY_WIDTH, period_ns, > NSEC_PER_SEC) = 19 > > mul_u64_u64_div_u64(pc->clk_rate, period_ns, > NSEC_PER_SEC << PWM_DUTY_WIDTH) = 20 > > (The exact result is 20.000062083332033.) > > With this optimizations also switch from round-closest to round-down for > the period calculation. Given that the calculations were non-optimal for > quite some time now with variations in both directions which nobody > reported as a problem, this is the opportunity to align the driver's > behavior to the requirements of new drivers. This has several upsides: > > - Implementation is easier as there are no round-nearest variants of > mul_u64_u64_div_u64(). > - Requests for too small periods are now consistently refused. This was > kind of arbitrary before, where period_ns < min_period_ns was > refused, but in some cases min_period_ns isn't actually implementable > and then values between min_period_ns and the actual minimum were > rounded up to the actual minimum. > > Note that the duty_cycle calculation isn't using the usual round-down > approach yet. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> > --- > Hello, > > changes since (implicit) v1: Updated changelog to explain why rate = 0 > is refused now. > > Best regards > Uwe > > drivers/pwm/pwm-tegra.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Applied, thanks. Thierry
Attachment:
signature.asc
Description: PGP signature