pwm: Prevent a glitch for legacy drivers
authorUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Thu, 1 Jul 2021 07:29:26 +0000 (09:29 +0200)
committerThierry Reding <thierry.reding@gmail.com>
Wed, 17 Nov 2021 16:09:22 +0000 (17:09 +0100)
If a running PWM is reconfigured to disabled calling the ->config()
callback before disabling the hardware might result in a glitch where
the (maybe) new period and duty_cycle are visible on the output before
disabling the hardware.

So handle disabling before calling ->config(). Also exit early in this case
which is possible because period and duty_cycle don't matter for disabled PWMs.
In return however ->config has to be called even if state->period ==
pwm->state.period && state->duty_cycle != pwm->state.duty_cycle because setting
these might have been skipped in the previous call.

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

index c4bbe12..dedf38a 100644 (file)
@@ -555,26 +555,33 @@ static int pwm_apply_legacy(struct pwm_chip *chip, struct pwm_device *pwm,
                pwm->state.polarity = state->polarity;
        }
 
-       if (state->period != pwm->state.period ||
-           state->duty_cycle != pwm->state.duty_cycle) {
-               err = chip->ops->config(pwm->chip, pwm,
-                                       state->duty_cycle,
-                                       state->period);
-               if (err)
-                       return err;
+       if (!state->enabled) {
+               if (pwm->state.enabled)
+                       chip->ops->disable(chip, pwm);
 
-               pwm->state.period = state->period;
-               pwm->state.duty_cycle = state->duty_cycle;
+               return 0;
        }
 
-       if (state->enabled != pwm->state.enabled) {
-               if (!pwm->state.enabled) {
-                       err = chip->ops->enable(chip, pwm);
-                       if (err)
-                               return err;
-               } else {
-                       chip->ops->disable(chip, pwm);
-               }
+       /*
+        * We cannot skip calling ->config even if state->period ==
+        * pwm->state.period && state->duty_cycle == pwm->state.duty_cycle
+        * because we might have exited early in the last call to
+        * pwm_apply_state because of !state->enabled and so the two values in
+        * pwm->state might not be configured in hardware.
+        */
+       err = chip->ops->config(pwm->chip, pwm,
+                               state->duty_cycle,
+                               state->period);
+       if (err)
+               return err;
+
+       pwm->state.period = state->period;
+       pwm->state.duty_cycle = state->duty_cycle;
+
+       if (!pwm->state.enabled) {
+               err = chip->ops->enable(chip, pwm);
+               if (err)
+                       return err;
        }
 
        return 0;