pwm: ab8500: Fix calculation of duty and period

After a check of the manual it becomes obvious that the calculations in
.apply() are totally bogus:

FreqPWMOutx was always written as zero, so the period was fixed at
3413333.33 ns. However state->period wasn't checked at all.
The lower 10 bits of duty_cycle were just used as DutyPWMOutx. So if
a duty cycle of 512 ns (or 1536 ns) was requested, it actually
programmed 1710000 ns. Other values were wrong by the same factor.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>

1 files changed, 60 insertions, 9 deletions

diff --git a/drivers/pwm/pwm-ab8500.c b/drivers/pwm/pwm-ab8500.c
index ad37bc46f272..c5239eef3b8f 100644
--- a/drivers/pwm/pwm-ab8500.c
+++ b/drivers/pwm/pwm-ab8500.c
@@ -3,6 +3,7 @@
  * Copyright (C) ST-Ericsson SA 2010
  *
  * Author: Arun R Murthy <arun.murthy@stericsson.com>
+ * Datasheet: https://web.archive.org/web/20130614115108/http://www.stericsson.com/developers/CD00291561_UM1031_AB8500_user_manual-rev5_CTDS_public.pdf
  */
 #include <linux/err.h>
 #include <linux/platform_device.h>
@@ -20,6 +21,8 @@
 #define AB8500_PWM_OUT_CTRL2_REG	0x61
 #define AB8500_PWM_OUT_CTRL7_REG	0x66
 
+#define AB8500_PWM_CLKRATE 9600000
+
 struct ab8500_pwm_chip {
 	struct pwm_chip chip;
 	unsigned int hwid;
@@ -35,13 +38,60 @@ static int ab8500_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 {
 	int ret;
 	u8 reg;
-	unsigned int higher_val, lower_val;
+	u8 higher_val, lower_val;
+	unsigned int duty_steps, div;
 	struct ab8500_pwm_chip *ab8500 = ab8500_pwm_from_chip(chip);
 
 	if (state->polarity != PWM_POLARITY_NORMAL)
 		return -EINVAL;
 
-	if (!state->enabled) {
+	if (state->enabled) {
+		/*
+		 * A time quantum is
+		 *   q = (32 - FreqPWMOutx[3:0]) / AB8500_PWM_CLKRATE
+		 * The period is always 1024 q, duty_cycle is between 1q and 1024q.
+		 *
+		 * FreqPWMOutx[3:0] | output frequency | output frequency | 1024q = period
+		 *                  | (from manual)    |   (1 / 1024q)    | = 1 / freq
+		 * -----------------+------------------+------------------+--------------
+		 *      b0000       |      293 Hz      |  292.968750 Hz   | 3413333.33 ns
+		 *      b0001       |      302 Hz      |  302.419355 Hz   | 3306666.66 ns
+		 *      b0010       |      312 Hz      |  312.500000 Hz   | 3200000    ns
+		 *      b0011       |      323 Hz      |  323.275862 Hz   | 3093333.33 ns
+		 *      b0100       |      334 Hz      |  334.821429 Hz   | 2986666.66 ns
+		 *      b0101       |      347 Hz      |  347.222222 Hz   | 2880000    ns
+		 *      b0110       |      360 Hz      |  360.576923 Hz   | 2773333.33 ns
+		 *      b0111       |      375 Hz      |  375.000000 Hz   | 2666666.66 ns
+		 *      b1000       |      390 Hz      |  390.625000 Hz   | 2560000    ns
+		 *      b1001       |      407 Hz      |  407.608696 Hz   | 2453333.33 ns
+		 *      b1010       |      426 Hz      |  426.136364 Hz   | 2346666.66 ns
+		 *      b1011       |      446 Hz      |  446.428571 Hz   | 2240000    ns
+		 *      b1100       |      468 Hz      |  468.750000 Hz   | 2133333.33 ns
+		 *      b1101       |      493 Hz      |  493.421053 Hz   | 2026666.66 ns
+		 *      b1110       |      520 Hz      |  520.833333 Hz   | 1920000    ns
+		 *      b1111       |      551 Hz      |  551.470588 Hz   | 1813333.33 ns
+		 *
+		 *
+		 * AB8500_PWM_CLKRATE is a multiple of 1024, so the division by
+		 * 1024 can be done in this factor without loss of precision.
+		 */
+		div = min_t(u64, mul_u64_u64_div_u64(state->period,
+						     AB8500_PWM_CLKRATE >> 10,
+						     NSEC_PER_SEC), 32); /* 32 - FreqPWMOutx[3:0] */
+		if (div <= 16)
+			/* requested period < 3413333.33 */
+			return -EINVAL;
+
+		duty_steps = max_t(u64, mul_u64_u64_div_u64(state->duty_cycle,
+							    AB8500_PWM_CLKRATE,
+							    (u64)NSEC_PER_SEC * div), 1024);
+	}
+
+	/*
+	 * The hardware doesn't support duty_steps = 0 explicitly, but emits low
+	 * when disabled.
+	 */
+	if (!state->enabled || duty_steps == 0) {
 		ret = abx500_mask_and_set_register_interruptible(chip->dev,
 					AB8500_MISC, AB8500_PWM_OUT_CTRL7_REG,
 					1 << ab8500->hwid, 0);
@@ -53,28 +103,29 @@ static int ab8500_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	}
 
 	/*
-	 * get the first 8 bits that are be written to
+	 * The lower 8 bits of duty_steps is written to ...
 	 * AB8500_PWM_OUT_CTRL1_REG[0:7]
 	 */
-	lower_val = state->duty_cycle & 0x00FF;
+	lower_val = (duty_steps - 1) & 0x00ff;
 	/*
-	 * get bits [9:10] that are to be written to
-	 * AB8500_PWM_OUT_CTRL2_REG[0:1]
+	 * The two remaining high bits to
+	 * AB8500_PWM_OUT_CTRL2_REG[0:1]; together with FreqPWMOutx.
 	 */
-	higher_val = ((state->duty_cycle & 0x0300) >> 8);
+	higher_val = ((duty_steps - 1) & 0x0300) >> 8 | (32 - div) << 4;
 
 	reg = AB8500_PWM_OUT_CTRL1_REG + (ab8500->hwid * 2);
 
 	ret = abx500_set_register_interruptible(chip->dev, AB8500_MISC,
-			reg, (u8)lower_val);
+			reg, lower_val);
 	if (ret < 0)
 		return ret;
 
 	ret = abx500_set_register_interruptible(chip->dev, AB8500_MISC,
-			(reg + 1), (u8)higher_val);
+			(reg + 1), higher_val);
 	if (ret < 0)
 		return ret;
 
+	/* enable */
 	ret = abx500_mask_and_set_register_interruptible(chip->dev,
 				AB8500_MISC, AB8500_PWM_OUT_CTRL7_REG,
 				1 << ab8500->hwid, 1 << ab8500->hwid);