Hello,
during review of my previous pwm-ab8500 patch I learned that there is a
(somewhat) publically available reference manual. Reading in it showed
that .apply() is still more broken that I assumed by just reading the
code.
This series first fixes .apply() to not be off by a factor of ~3000 and
then adds a .get_state() callback.
Note this is only compile tested as I don't have the hardware.
Also note this breaks all consumers that relied on the previously broken
behaviour.
Uwe Kleine-König (2):
pwm: ab8500: Fix calculation of duty and period
pwm: ab8500: Implement .get_state()
drivers/pwm/pwm-ab8500.c | 112 +++++++++++++++++++++++++++++++++++----
1 file changed, 103 insertions(+), 9 deletions(-)
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
--
2.39.0
The registers are readable, so it's possible to implement the
.get_state() callback for this PWM.
Signed-off-by: Uwe Kleine-König <[email protected]>
---
drivers/pwm/pwm-ab8500.c | 43 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/drivers/pwm/pwm-ab8500.c b/drivers/pwm/pwm-ab8500.c
index c5239eef3b8f..507ff0d5f7bd 100644
--- a/drivers/pwm/pwm-ab8500.c
+++ b/drivers/pwm/pwm-ab8500.c
@@ -136,8 +136,51 @@ static int ab8500_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
return ret;
}
+static int ab8500_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ u8 ctrl7, lower_val, higher_val;
+ int ret;
+ struct ab8500_pwm_chip *ab8500 = ab8500_pwm_from_chip(chip);
+ unsigned int div, duty_steps;
+
+ ret = abx500_get_register_interruptible(chip->dev, AB8500_MISC,
+ AB8500_PWM_OUT_CTRL7_REG,
+ &ctrl7);
+ if (ret)
+ return ret;
+
+ state->polarity = PWM_POLARITY_NORMAL;
+
+ if (!(ctrl7 & 1 << ab8500->hwid)) {
+ state->enabled = false;
+ return 0;
+ }
+
+ ret = abx500_get_register_interruptible(chip->dev, AB8500_MISC,
+ AB8500_PWM_OUT_CTRL1_REG + (ab8500->hwid * 2),
+ &lower_val);
+ if (ret)
+ return ret;
+
+ ret = abx500_get_register_interruptible(chip->dev, AB8500_MISC,
+ AB8500_PWM_OUT_CTRL2_REG + (ab8500->hwid * 2),
+ &higher_val);
+ if (ret)
+ return ret;
+
+ div = 32 - ((higher_val & 0xf0) >> 4);
+ duty_steps = ((higher_val & 3) << 8 | lower_val) + 1;
+
+ state->period = DIV64_U64_ROUND_UP((u64)div << 10, AB8500_PWM_CLKRATE);
+ state->duty_cycle = DIV64_U64_ROUND_UP((u64)div * duty_steps, AB8500_PWM_CLKRATE);
+
+ return 0;
+}
+
static const struct pwm_ops ab8500_pwm_ops = {
.apply = ab8500_pwm_apply,
+ .get_state = ab8500_pwm_get_state,
.owner = THIS_MODULE,
};
--
2.39.0
Hi Uwe,
On Wed, Jan 18, 2023 at 4:48 PM Uwe Kleine-König
<[email protected]> wrote:
> during review of my previous pwm-ab8500 patch I learned that there is a
> (somewhat) publically available reference manual. Reading in it showed
> that .apply() is still more broken that I assumed by just reading the
> code.
>
> This series first fixes .apply() to not be off by a factor of ~3000 and
> then adds a .get_state() callback.
>
> Note this is only compile tested as I don't have the hardware.
> Also note this breaks all consumers that relied on the previously broken
> behaviour.
I looked over the patch and I can't find anything to comment on
it just seems well researched and correct so:
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Wed, 18 Jan 2023 16:48:15 +0100, Uwe Kleine-König wrote:
> during review of my previous pwm-ab8500 patch I learned that there is a
> (somewhat) publically available reference manual. Reading in it showed
> that .apply() is still more broken that I assumed by just reading the
> code.
>
> This series first fixes .apply() to not be off by a factor of ~3000 and
> then adds a .get_state() callback.
>
> [...]
Applied, thanks!
[1/2] pwm: ab8500: Fix calculation of duty and period
commit: 486dd4e846814016443abfcbfee0b8b3f3b35330
[2/2] pwm: ab8500: Implement .get_state()
commit: 327437884e9a752ccbf759cbab641439ca708f5b
Best regards,
--
Thierry Reding <[email protected]>