Skip to content

Commit 8c193f4

Browse files
Uwe Kleine-Königthierryreding
authored andcommitted
pwm: tegra: Optimize period calculation
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 <[email protected]> Signed-off-by: Thierry Reding <[email protected]>
1 parent 615f4e8 commit 8c193f4

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

drivers/pwm/pwm-tegra.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
9999
int duty_ns, int period_ns)
100100
{
101101
struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
102-
unsigned long long c = duty_ns, hz;
102+
unsigned long long c = duty_ns;
103103
unsigned long rate, required_clk_rate;
104104
u32 val = 0;
105105
int err;
@@ -156,11 +156,9 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
156156
pc->clk_rate = clk_get_rate(pc->clk);
157157
}
158158

159-
rate = pc->clk_rate >> PWM_DUTY_WIDTH;
160-
161159
/* Consider precision in PWM_SCALE_WIDTH rate calculation */
162-
hz = DIV_ROUND_CLOSEST_ULL(100ULL * NSEC_PER_SEC, period_ns);
163-
rate = DIV_ROUND_CLOSEST_ULL(100ULL * rate, hz);
160+
rate = mul_u64_u64_div_u64(pc->clk_rate, period_ns,
161+
(u64)NSEC_PER_SEC << PWM_DUTY_WIDTH);
164162

165163
/*
166164
* Since the actual PWM divider is the register's frequency divider
@@ -169,6 +167,8 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
169167
*/
170168
if (rate > 0)
171169
rate--;
170+
else
171+
return -EINVAL;
172172

173173
/*
174174
* Make sure that the rate will fit in the register's frequency

0 commit comments

Comments
 (0)