* Timo Karjalainen <timo.o.karjalainen@xxxxxxxxx> [081002 02:09]: > 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. Yet another driver to go to mainline.. Pushing to l-o. Tony > Signed-off-by: Timo Karjalainen <timo.o.karjalainen@xxxxxxxxx> > Signed-off-by: Daniel Stone <daniel.stone@xxxxxxxxx> > --- > drivers/input/keyboard/lm8323.c | 129 +++++++++++++++++++++------------------ > 1 files changed, 69 insertions(+), 60 deletions(-) > > diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c > index 72bb587..342ef6a 100644 > --- 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@xxxxxxxxx>, Daniel Stone"); > MODULE_DESCRIPTION("LM8323 keypad driver"); > MODULE_LICENSE("GPL"); > > -- > 1.6.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html