lm8323 pwm fixes

This patch fixes two problems in LM8323 PWM control. One is that
locking is needed when setting and reading pwm->desired_brightness
asynchronously. The other is that LM8323 may stop a PWM script only
after the current instruction has finished. If it is a long RAMP,
the chip would keep executing the old instruction and new
settings were effectively ignored.

Signed-off-by: Timo Karjalainen <timo.o.karjalainen@nokia.com>
Signed-off-by: Daniel Stone <daniel.stone@nokia.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
index 72bb587eb035adca819363047f527c62baa1c915..342ef6a9290a4a427aed2a2c1aafbcb55d7bbaa8 100644 (file)
--- a/drivers/input/keyboard/lm8323.c
+++ b/drivers/input/keyboard/lm8323.c
@@ -141,6 +141,8 @@ struct lm8323_pwm {
        int                     fade_time;
        int                     brightness;
        int                     desired_brightness;
+       int                     running;
+       struct mutex            lock;
        struct work_struct      work;
        struct led_classdev     cdev;
 };
@@ -384,6 +386,15 @@ static int lm8323_configure(struct lm8323_chip *lm)
        return 0;
 }
 
+static void pwm_done(struct lm8323_pwm *pwm)
+{
+       mutex_lock(&pwm->lock);
+       pwm->running = 0;
+       if (pwm->desired_brightness != pwm->brightness)
+               schedule_work(&pwm->work);
+       mutex_unlock(&pwm->lock);
+}
+
 /*
  * Bottom half: handle the interrupt by posting key events, or dealing with
  * errors appropriately.
@@ -411,12 +422,18 @@ static void lm8323_work(struct work_struct *work)
                                                  "reinitialising\n");
                        lm8323_configure(lm);
                }
-               if (ints & INT_PWM1)
+               if (ints & INT_PWM1) {
                        debug(&lm->client->dev, "pwm1 engine completed\n");
-               if (ints & INT_PWM2)
+                       pwm_done(&lm->pwm1);
+               }
+               if (ints & INT_PWM2) {
                        debug(&lm->client->dev, "pwm2 engine completed\n");
-               if (ints & INT_PWM3)
+                       pwm_done(&lm->pwm2);
+               }
+               if (ints & INT_PWM3) {
                        debug(&lm->client->dev, "pwm3 engine completed\n");
+                       pwm_done(&lm->pwm3);
+               }
        }
 
        mutex_unlock(&lm->lock);
@@ -458,92 +475,80 @@ static void lm8323_write_pwm_one(struct lm8323_pwm *pwm, int pos, u16 cmd)
 
 /*
  * Write a script into a given PWM engine, concluding with PWM_END.
- * If 'keepalive' is specified, the engine will be kept running
- * indefinitely.
+ * If 'kill' is nonzero, the engine will be shut down at the end
+ * of the script, producing a zero output. Otherwise the engine
+ * will be kept running at the final PWM level indefinitely.
  */
-static void lm8323_write_pwm(struct lm8323_pwm *pwm, int keepalive,
-                            int len, ...)
+static void lm8323_write_pwm(struct lm8323_pwm *pwm, int kill,
+                            int len, const u16 *cmds)
 {
        struct lm8323_chip *lm = pwm_to_lm8323(pwm);
-       int i, cmd;
-       va_list ap;
-
-       /*
-        * If there are any scripts running at the moment, terminate them
-        * and make sure the duty cycle is as if it finished.
-        */
-       lm8323_write(lm, 2, LM8323_CMD_STOP_PWM, pwm->id);
-
-       va_start(ap, len);
-       for (i = 0; i < len; i++) {
-               cmd = va_arg(ap, int);
-               lm8323_write_pwm_one(pwm, i, cmd);
-       }
-       va_end(ap);
+       int i;
 
-       /* Wait for a trigger from any channel. This keeps the engine alive. */
-       if (keepalive)
-               lm8323_write_pwm_one(pwm, i++, PWM_WAIT_TRIG(0xe));
-       else
-               lm8323_write_pwm_one(pwm, i++, PWM_END(1));
+       for (i = 0; i < len; i++)
+               lm8323_write_pwm_one(pwm, i, cmds[i]);
 
+       lm8323_write_pwm_one(pwm, i++, PWM_END(kill));
        lm8323_write(lm, 2, LM8323_CMD_START_PWM, pwm->id);
+       pwm->running = 1;
 }
 
 static void lm8323_pwm_work(struct work_struct *work)
 {
        struct lm8323_pwm *pwm = work_to_pwm(work);
-       int div, perstep, steps, hz, direction, keepalive;
+       int div512, perstep, steps, hz, up, kill;
+       u16 pwm_cmds[3];
+       int num_cmds = 0;
+
+       mutex_lock(&pwm->lock);
 
-       /* Do nothing if we're already at the requested level. */
-       if (pwm->desired_brightness == pwm->brightness)
+       /*
+        * Do nothing if we're already at the requested level,
+        * or previous setting is not yet complete. In the latter
+        * case we will be called again when the previous PWM script
+        * finishes.
+        */
+       if (pwm->running || pwm->desired_brightness == pwm->brightness) {
+               mutex_unlock(&pwm->lock);
                return;
+       }
 
-       keepalive = (pwm->desired_brightness > 0);
-       direction = (pwm->desired_brightness > pwm->brightness);
+       kill = (pwm->desired_brightness == 0);
+       up = (pwm->desired_brightness > pwm->brightness);
        steps = abs(pwm->desired_brightness - pwm->brightness);
 
        /*
         * Convert time (in ms) into a divisor (512 or 16 on a refclk of
         * 32768Hz), and number of ticks per step.
         */
-       if ((pwm->fade_time / steps) > (32768 / 512))
-               div = 512;
-       else
-               div = 16;
+       if ((pwm->fade_time / steps) > (32768 / 512)) {
+               div512 = 1;
+               hz = 32768 / 512;
+       }
+       else {
+               div512 = 0;
+               hz = 32768 / 16;
+       }
 
-       hz = 32768 / div;
-       if (pwm->fade_time < ((steps * 1000) / hz))
-               perstep = 1;
-       else
-               perstep = (hz * pwm->fade_time) / (steps * 1000);
+       perstep = (hz * pwm->fade_time) / (steps * 1000);
 
        if (perstep == 0)
                perstep = 1;
        else if (perstep > 63)
                perstep = 63;
 
-       if (steps > 252) {
-               lm8323_write_pwm(pwm, keepalive, 3,
-                                PWM_RAMP((div == 512), perstep, 126,
-                                         direction),
-                                PWM_RAMP((div == 512), perstep, 126,
-                                         direction),
-                                PWM_RAMP((div == 512), perstep, steps - 252,
-                                         direction));
-       } else if (steps > 126) {
-               lm8323_write_pwm(pwm, keepalive, 2,
-                                PWM_RAMP((div == 512), perstep, 126,
-                                         direction),
-                                PWM_RAMP((div == 512), perstep, steps - 126,
-                                         direction));
-       } else {
-               lm8323_write_pwm(pwm, keepalive, 1,
-                                PWM_RAMP((div == 512), perstep, steps,
-                                         direction));
+       while (steps) {
+               int s;
+
+               s = min(126, steps);
+               pwm_cmds[num_cmds++] = PWM_RAMP(div512, perstep, s, up);
+               steps -= s;
        }
 
+       lm8323_write_pwm(pwm, kill, num_cmds, pwm_cmds);
+
        pwm->brightness = pwm->desired_brightness;
+       mutex_unlock(&pwm->lock);
 }
 
 static void lm8323_pwm_set_brightness(struct led_classdev *led_cdev,
@@ -552,7 +557,9 @@ static void lm8323_pwm_set_brightness(struct led_classdev *led_cdev,
        struct lm8323_pwm *pwm = cdev_to_pwm(led_cdev);
        struct lm8323_chip *lm = pwm_to_lm8323(pwm);
 
+       mutex_lock(&pwm->lock);
        pwm->desired_brightness = brightness;
+       mutex_unlock(&pwm->lock);
 
        if (in_interrupt()) {
                schedule_work(&pwm->work);
@@ -620,6 +627,8 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
        pwm->fade_time = 0;
        pwm->brightness = 0;
        pwm->desired_brightness = 0;
+       pwm->running = 0;
+       mutex_init(&pwm->lock);
        if (name) {
                pwm->cdev.name = name;
                pwm->cdev.brightness_set = lm8323_pwm_set_brightness;
@@ -917,7 +926,7 @@ static void __exit lm8323_exit(void)
        i2c_del_driver(&lm8323_i2c_driver);
 }
 
-MODULE_AUTHOR("Daniel Stone");
+MODULE_AUTHOR("Timo O. Karjalainen <timo.o.karjalainen@nokia.com>, Daniel Stone");
 MODULE_DESCRIPTION("LM8323 keypad driver");
 MODULE_LICENSE("GPL");