From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Thierry Reding <thierry.reding@gmail.com>,
Lee Jones <lee.jones@linaro.org>
Cc: linux-pwm@vger.kernel.org, kernel@pengutronix.de
Subject: [PATCH v2] pwm: Ensure for legacy drivers that pwm->state stays consistent
Date: Sat, 1 May 2021 18:09:43 +0200 [thread overview]
Message-ID: <20210501160943.108821-1-u.kleine-koenig@pengutronix.de> (raw)
In-Reply-To: <20210411160451.1207799-1-u.kleine-koenig@pengutronix.de>
Without this change it can happen that if changing the polarity succeeded
but changing duty_cycle and period failed pwm->state contains a mixture
between the old and the requested state.
So remember the initial state before starting to modify the configuration
and restore it when one of the required callback fails.
Compared to the previous implementation .disable() (if necessary) is called
earlier to prevent a glitch.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,
just a small optimisation: At the end of pwm_apply_legacy()
state->enabled is known to be true, so simplify
if (state->enabled && !pwm->state.enabled) {
to
if (!pwm->state.enabled) {
Best regards
Uwe
drivers/pwm/core.c | 139 +++++++++++++++++++++++++--------------------
1 file changed, 78 insertions(+), 61 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index c4d5c0667137..57105deafb55 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -535,6 +535,71 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
}
}
+static int pwm_apply_legacy(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ int err;
+ struct pwm_state initial_state = pwm->state;
+
+ if (state->polarity != pwm->state.polarity) {
+ if (!chip->ops->set_polarity) {
+ err = -EINVAL;
+ goto out_err;
+ }
+
+ /*
+ * Changing the polarity of a running PWM is only allowed when
+ * the PWM driver implements ->apply().
+ */
+ if (pwm->state.enabled) {
+ chip->ops->disable(chip, pwm);
+
+ /*
+ * Update pwm->state already here in case
+ * .set_polarity() or another callback depend on that.
+ */
+ pwm->state.enabled = false;
+ }
+
+ err = chip->ops->set_polarity(chip, pwm,
+ state->polarity);
+ if (err)
+ goto out_err;
+
+ pwm->state.polarity = state->polarity;
+ }
+
+ if (!state->enabled) {
+ if (pwm->state.enabled)
+ chip->ops->disable(chip, pwm);
+ return 0;
+ }
+
+ 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)
+ goto out_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)
+ goto out_err;
+ }
+
+ return 0;
+
+out_err:
+ pwm->state = initial_state;
+ return err;
+}
+
/**
* pwm_apply_state() - atomically apply a new state to a PWM device
* @pwm: PWM device
@@ -544,6 +609,8 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
{
struct pwm_chip *chip;
int err;
+ int (*apply)(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state);
if (!pwm || !state || !state->period ||
state->duty_cycle > state->period)
@@ -557,70 +624,20 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
state->enabled == pwm->state.enabled)
return 0;
- if (chip->ops->apply) {
- err = chip->ops->apply(chip, pwm, state);
- if (err)
- return err;
-
- trace_pwm_apply(pwm, state);
-
- pwm->state = *state;
-
- /*
- * only do this after pwm->state was applied as some
- * implementations of .get_state depend on this
- */
- pwm_apply_state_debug(pwm, state);
- } else {
- /*
- * FIXME: restore the initial state in case of error.
- */
- if (state->polarity != pwm->state.polarity) {
- if (!chip->ops->set_polarity)
- return -EINVAL;
-
- /*
- * Changing the polarity of a running PWM is
- * only allowed when the PWM driver implements
- * ->apply().
- */
- if (pwm->state.enabled) {
- chip->ops->disable(chip, pwm);
- pwm->state.enabled = false;
- }
+ apply = chip->ops->apply ?: pwm_apply_legacy;
+ err = apply(chip, pwm, state);
+ if (err)
+ return err;
- err = chip->ops->set_polarity(chip, pwm,
- state->polarity);
- if (err)
- return err;
+ trace_pwm_apply(pwm, state);
- 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;
+ pwm->state = *state;
- pwm->state.duty_cycle = state->duty_cycle;
- pwm->state.period = state->period;
- }
-
- if (state->enabled != pwm->state.enabled) {
- if (state->enabled) {
- err = chip->ops->enable(chip, pwm);
- if (err)
- return err;
- } else {
- chip->ops->disable(chip, pwm);
- }
-
- pwm->state.enabled = state->enabled;
- }
- }
+ /*
+ * only do this after pwm->state was applied as some
+ * implementations of .get_state depend on this
+ */
+ pwm_apply_state_debug(pwm, state);
return 0;
}
--
2.30.2
next prev parent reply other threads:[~2021-05-01 16:09 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-11 16:04 [PATCH] pwm: Ensure for legacy drivers that pwm->state stays consistent Uwe Kleine-König
2021-05-01 16:09 ` Uwe Kleine-König [this message]
2021-06-28 11:46 ` [PATCH v2] " Thierry Reding
2021-06-28 17:24 ` Uwe Kleine-König
2021-06-29 19:44 ` Geert Uytterhoeven
2021-06-30 6:48 ` Uwe Kleine-König
2021-06-30 10:22 ` Geert Uytterhoeven
2021-06-30 16:21 ` Uwe Kleine-König
2021-06-30 17:08 ` Geert Uytterhoeven
2021-06-30 17:18 ` Thierry Reding
2021-06-29 19:52 ` Geert Uytterhoeven
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210501160943.108821-1-u.kleine-koenig@pengutronix.de \
--to=u.kleine-koenig@pengutronix.de \
--cc=kernel@pengutronix.de \
--cc=lee.jones@linaro.org \
--cc=linux-pwm@vger.kernel.org \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).