diff options
author | Jeff LaBundy <jeff@labundy.com> | 2021-01-18 22:30:29 -0600 |
---|---|---|
committer | Thierry Reding <thierry.reding@gmail.com> | 2021-02-22 15:18:50 +0100 |
commit | 28208c7b4a2c38ea91b6ee04f6023d3145257f5d (patch) | |
tree | 3b7f44b5dc811fe3931bb4ef54ac18122bcd0166 | |
parent | 72d6b2459dbd539c1369149e501fdc3dc8ddef16 (diff) | |
download | linux-28208c7b4a2c38ea91b6ee04f6023d3145257f5d.tar.bz2 |
pwm: iqs620a: Correct a stale state variable
If duty cycle is first set to a value that is sufficiently high to
enable the output (e.g. 10000 ns) but then lowered to a value that
is quantized to zero (e.g. 1000 ns), the output is disabled as the
device cannot drive a constant zero (as expected).
However if the device is later re-initialized due to watchdog bite,
the output is re-enabled at the next-to-last duty cycle (10000 ns).
This is because the iqs620_pwm->out_en flag unconditionally tracks
state->enabled instead of what was actually written to the device.
To solve this problem, use one state variable that encodes all 257
states of the output (duty_scale) with 0 representing tri-state, 1
representing the minimum available duty cycle and 256 representing
100% duty cycle.
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
-rw-r--r-- | drivers/pwm/pwm-iqs620a.c | 88 |
1 files changed, 37 insertions, 51 deletions
diff --git a/drivers/pwm/pwm-iqs620a.c b/drivers/pwm/pwm-iqs620a.c index 14b18fb4f527..957b972c458b 100644 --- a/drivers/pwm/pwm-iqs620a.c +++ b/drivers/pwm/pwm-iqs620a.c @@ -37,15 +37,32 @@ struct iqs620_pwm_private { struct pwm_chip chip; struct notifier_block notifier; struct mutex lock; - bool out_en; - u8 duty_val; + unsigned int duty_scale; }; +static int iqs620_pwm_init(struct iqs620_pwm_private *iqs620_pwm, + unsigned int duty_scale) +{ + struct iqs62x_core *iqs62x = iqs620_pwm->iqs62x; + int ret; + + if (!duty_scale) + return regmap_update_bits(iqs62x->regmap, IQS620_PWR_SETTINGS, + IQS620_PWR_SETTINGS_PWM_OUT, 0); + + ret = regmap_write(iqs62x->regmap, IQS620_PWM_DUTY_CYCLE, + duty_scale - 1); + if (ret) + return ret; + + return regmap_update_bits(iqs62x->regmap, IQS620_PWR_SETTINGS, + IQS620_PWR_SETTINGS_PWM_OUT, 0xff); +} + static int iqs620_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, const struct pwm_state *state) { struct iqs620_pwm_private *iqs620_pwm; - struct iqs62x_core *iqs62x; unsigned int duty_cycle; unsigned int duty_scale; int ret; @@ -57,7 +74,6 @@ static int iqs620_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, return -EINVAL; iqs620_pwm = container_of(chip, struct iqs620_pwm_private, chip); - iqs62x = iqs620_pwm->iqs62x; /* * The duty cycle generated by the device is calculated as follows: @@ -74,36 +90,15 @@ static int iqs620_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, duty_cycle = min_t(u64, state->duty_cycle, IQS620_PWM_PERIOD_NS); duty_scale = duty_cycle * 256 / IQS620_PWM_PERIOD_NS; - mutex_lock(&iqs620_pwm->lock); - - if (!state->enabled || !duty_scale) { - ret = regmap_update_bits(iqs62x->regmap, IQS620_PWR_SETTINGS, - IQS620_PWR_SETTINGS_PWM_OUT, 0); - if (ret) - goto err_mutex; - } - - if (duty_scale) { - u8 duty_val = duty_scale - 1; + if (!state->enabled) + duty_scale = 0; - ret = regmap_write(iqs62x->regmap, IQS620_PWM_DUTY_CYCLE, - duty_val); - if (ret) - goto err_mutex; - - iqs620_pwm->duty_val = duty_val; - } - - if (state->enabled && duty_scale) { - ret = regmap_update_bits(iqs62x->regmap, IQS620_PWR_SETTINGS, - IQS620_PWR_SETTINGS_PWM_OUT, 0xff); - if (ret) - goto err_mutex; - } + mutex_lock(&iqs620_pwm->lock); - iqs620_pwm->out_en = state->enabled; + ret = iqs620_pwm_init(iqs620_pwm, duty_scale); + if (!ret) + iqs620_pwm->duty_scale = duty_scale; -err_mutex: mutex_unlock(&iqs620_pwm->lock); return ret; @@ -121,12 +116,11 @@ static void iqs620_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, /* * Since the device cannot generate a 0% duty cycle, requests to do so * cause subsequent calls to iqs620_pwm_get_state to report the output - * as disabled with duty cycle equal to that which was in use prior to - * the request. This is not ideal, but is the best compromise based on + * as disabled. This is not ideal, but is the best compromise based on * the capabilities of the device. */ - state->enabled = iqs620_pwm->out_en; - state->duty_cycle = DIV_ROUND_UP((iqs620_pwm->duty_val + 1) * + state->enabled = iqs620_pwm->duty_scale > 0; + state->duty_cycle = DIV_ROUND_UP(iqs620_pwm->duty_scale * IQS620_PWM_PERIOD_NS, 256); mutex_unlock(&iqs620_pwm->lock); @@ -138,7 +132,6 @@ static int iqs620_pwm_notifier(struct notifier_block *notifier, unsigned long event_flags, void *context) { struct iqs620_pwm_private *iqs620_pwm; - struct iqs62x_core *iqs62x; int ret; if (!(event_flags & BIT(IQS62X_EVENT_SYS_RESET))) @@ -146,7 +139,6 @@ static int iqs620_pwm_notifier(struct notifier_block *notifier, iqs620_pwm = container_of(notifier, struct iqs620_pwm_private, notifier); - iqs62x = iqs620_pwm->iqs62x; mutex_lock(&iqs620_pwm->lock); @@ -155,16 +147,8 @@ static int iqs620_pwm_notifier(struct notifier_block *notifier, * of a device reset, so nothing else is printed here unless there is * an additional failure. */ - ret = regmap_write(iqs62x->regmap, IQS620_PWM_DUTY_CYCLE, - iqs620_pwm->duty_val); - if (ret) - goto err_mutex; + ret = iqs620_pwm_init(iqs620_pwm, iqs620_pwm->duty_scale); - ret = regmap_update_bits(iqs62x->regmap, IQS620_PWR_SETTINGS, - IQS620_PWR_SETTINGS_PWM_OUT, - iqs620_pwm->out_en ? 0xff : 0); - -err_mutex: mutex_unlock(&iqs620_pwm->lock); if (ret) { @@ -211,12 +195,14 @@ static int iqs620_pwm_probe(struct platform_device *pdev) ret = regmap_read(iqs62x->regmap, IQS620_PWR_SETTINGS, &val); if (ret) return ret; - iqs620_pwm->out_en = val & IQS620_PWR_SETTINGS_PWM_OUT; - ret = regmap_read(iqs62x->regmap, IQS620_PWM_DUTY_CYCLE, &val); - if (ret) - return ret; - iqs620_pwm->duty_val = val; + if (val & IQS620_PWR_SETTINGS_PWM_OUT) { + ret = regmap_read(iqs62x->regmap, IQS620_PWM_DUTY_CYCLE, &val); + if (ret) + return ret; + + iqs620_pwm->duty_scale = val + 1; + } iqs620_pwm->chip.dev = &pdev->dev; iqs620_pwm->chip.ops = &iqs620_pwm_ops; |