For the cros-ec-pwm, "disabled" is the same as "duty cycle == 0", and is
not possible to program a duty cycle while the device is disabled. However,
the PWM API allows us to configure the "duty cycle" while the device is
"disabled". But now, pwm_get_state() is returning the real hardware state
instead of the last applied state, and this change of behavior, broke
the display on my rk3399-gru-kevin and doesn't turn on anymore.

Commit 01ccf903edd6 ("pwm: Let pwm_get_state() return the last implemented
state") introduced this change of behavior. And, assuming that this is
the right to do, workaround this problem for the cros-ec-pwm driver by
reverting the mentioned commit at the lowlevel driver.

With that patch applied pwm_get_state() will return only the programmed
hardware duty cycle value if the PWM is enabled. When is disabled, will
return the last applied duty_cycle value instead. That's not ideal, but
definetely is better than don't implement .get_state().

Fixes: 01ccf903edd6 ("pwm: Let pwm_get_state() return the last implemented 
state")
Signed-off-by: Enric Balletbo i Serra <enric.balle...@collabora.com>
---

 drivers/pwm/pwm-cros-ec.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 89497448d217..f750a3cf0c6c 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -18,11 +18,13 @@
  * @dev: Device node
  * @ec: Pointer to EC device
  * @chip: PWM controller chip
+ * @state: Holds the last state applied
  */
 struct cros_ec_pwm_device {
        struct device *dev;
        struct cros_ec_device *ec;
        struct pwm_chip chip;
+       struct pwm_state state;
 };
 
 static inline struct cros_ec_pwm_device *pwm_to_cros_ec_pwm(struct pwm_chip *c)
@@ -102,6 +104,9 @@ static int cros_ec_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
        if (state->period != EC_PWM_MAX_DUTY)
                return -EINVAL;
 
+       /* Store the new state */
+       ec_pwm->state = *state;
+
        /*
         * EC doesn't separate the concept of duty cycle and enabled, but
         * kernel does. Translate.
@@ -117,17 +122,28 @@ static void cros_ec_pwm_get_state(struct pwm_chip *chip, 
struct pwm_device *pwm,
        struct cros_ec_pwm_device *ec_pwm = pwm_to_cros_ec_pwm(chip);
        int ret;
 
-       ret = cros_ec_pwm_get_duty(ec_pwm->ec, pwm->hwpwm);
-       if (ret < 0) {
-               dev_err(chip->dev, "error getting initial duty: %d\n", ret);
-               return;
+       /*
+        * As there is no way for this hardware to separate the concept of
+        * duty cycle and enabled, but the PWM API does, let return the last
+        * applied state when the PWM is disabled and only return the real
+        * hardware value when the PWM is enabled. Otherwise, a user of this
+        * driver, can get confused because won't be able to program a duty
+        * cycle while the PWM is disabled.
+        */
+       state->enabled = ec_pwm->state.enabled;
+       if (state->enabled) {
+               ret = cros_ec_pwm_get_duty(ec_pwm->ec, pwm->hwpwm);
+               if (ret < 0) {
+                       dev_err(chip->dev, "error getting initial duty: %d\n",
+                               ret);
+                       return;
+               }
+               state->duty_cycle = ret;
+       } else {
+               state->duty_cycle = ec_pwm->state.duty_cycle;
        }
 
-       state->enabled = (ret > 0);
        state->period = EC_PWM_MAX_DUTY;
-
-       /* Note that "disabled" and "duty cycle == 0" are treated the same */
-       state->duty_cycle = ret;
 }
 
 static struct pwm_device *
-- 
2.20.1

Reply via email to