lkml.org 
[lkml]   [2019]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] pwm: cros-ec: Let cros_ec_pwm_get_state() return the last applied state
Date
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.balletbo@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
\
 
 \ /
  Last update: 2019-10-08 12:54    [W:0.099 / U:0.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site