The change fixes a problem, if duty_ns is too small in comparison to period_ns (as a valid corner case duty_ns is 0 ns), then due to PWM_DUTY() macro applied on a value the result is overflowed over 8 bits, and instead of the highest bitfield duty cycle value 0xff the invalid duty cycle bitfield value 0x00 is written. For reference the LPC32xx spec defines PWMx_DUTY bitfield description is this way and it seems to be correct: [Low]/[High] = [PWM_DUTY]/[256-PWM_DUTY], where 0 < PWM_DUTY <= 255. In addition according to my oscilloscope measurements LPC32xx PWM is "tristate" in sense that it produces a wave with floating min/max voltage levels for different duty cycle values, for corner cases: PWM_DUTY == 0x01 => signal is in range from -1.05v to 0v .... PWM_DUTY == 0x80 => signal is in range from -0.75v to +0.75v .... PWM_DUTY == 0xff => signal is in range from 0v to +1.05v PWM_DUTY == 0x00 => signal is around 0v, PWM is off Due to this peculiarity on very long period ranges (less than 1KHz) and odd pre-divider values PWM generated wave does not remind a clock shape signal, but rather a heartbit shape signal with positive and negative peaks, so I would recommend to use high-speed HCLK clock as a PWM parent clock and avoid using RTC clock as a parent. The change corrects PWM output in corner cases and prevents any possible overflows in calculation of values for PWM_DUTY and PWM_RELOADV bitfields, thus helper macro definitions may be removed. Signed-off-by: Vladimir Zapolskiy <vz@xxxxxxxxx> --- Changes from v1 to v2: - none drivers/pwm/pwm-lpc32xx.c | 53 +++++++++++++++++------------------------------ 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c index 63468a8..294a68f 100644 --- a/drivers/pwm/pwm-lpc32xx.c +++ b/drivers/pwm/pwm-lpc32xx.c @@ -24,9 +24,7 @@ struct lpc32xx_pwm_chip { void __iomem *base; }; -#define PWM_ENABLE (1 << 31) -#define PWM_RELOADV(x) (((x) & 0xFF) << 8) -#define PWM_DUTY(x) ((x) & 0xFF) +#define PWM_ENABLE BIT(31) #define to_lpc32xx_pwm_chip(_chip) \ container_of(_chip, struct lpc32xx_pwm_chip, chip) @@ -38,40 +36,27 @@ static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, unsigned long long c; int period_cycles, duty_cycles; u32 val; - - c = clk_get_rate(lpc32xx->clk) / 256; - c = c * period_ns; - do_div(c, NSEC_PER_SEC); - - /* Handle high and low extremes */ - if (c == 0) - c = 1; - if (c > 255) - c = 0; /* 0 set division by 256 */ - period_cycles = c; - - /* The duty-cycle value is as follows: - * - * DUTY-CYCLE HIGH LEVEL - * 1 99.9% - * 25 90.0% - * 128 50.0% - * 220 10.0% - * 255 0.1% - * 0 0.0% - * - * In other words, the register value is duty-cycle % 256 with - * duty-cycle in the range 1-256. - */ - c = 256 * duty_ns; - do_div(c, period_ns); - if (c > 255) - c = 255; - duty_cycles = 256 - c; + c = clk_get_rate(lpc32xx->clk); + + /* The highest acceptable divisor is 256, which is represented by 0 */ + period_cycles = div64_u64(c * period_ns, + (unsigned long long)NSEC_PER_SEC * 256); + if (!period_cycles) + period_cycles = 1; + if (period_cycles > 255) + period_cycles = 0; + + /* Compute 256 x #duty/period value and care for corner cases */ + duty_cycles = div64_u64((unsigned long long)(period_ns - duty_ns) * 256, + period_ns); + if (!duty_cycles) + duty_cycles = 1; + if (duty_cycles > 255) + duty_cycles = 255; val = readl(lpc32xx->base + (pwm->hwpwm << 2)); val &= ~0xFFFF; - val |= PWM_RELOADV(period_cycles) | PWM_DUTY(duty_cycles); + val |= (period_cycles << 8) | duty_cycles; writel(val, lpc32xx->base + (pwm->hwpwm << 2)); return 0; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html