From 9dd42d019e6399e6e7d9e90759a61ff430625285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 28 Jun 2024 12:35:19 +0200 Subject: [PATCH] pwm: Allow pwm state transitions from an invalid state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While driving a PWM via the sysfs API it's hard to determine the right order of writes to the pseudo files "period" and "duty_cycle": If you want to go from duty_cycle/period = 50/100 to 150/300 you have to write period first (because 150/100 is invalid). If however you start at 400/500 the duty_cycle must be configured first. The rule that works is: If you increase period write period first, otherwise write duty_cycle first. A complication however is that it's usually sensible to configure the polarity before both period and duty_cycle. This can only be done if the current state's duty_cycle and period configuration isn't bogus though. It is still worse (but I think only theoretic) if you have a PWM that only supports inverted polarity and you start with period = 0 and polarity = normal. Then you can change neither period (because polarity = normal is refused) nor polarity (because there is still period = 0). To simplify the corner cases for userspace, let invalid target states pass if the current state is invalid already. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20240628103519.105020-2-u.kleine-koenig@baylibre.com Signed-off-by: Uwe Kleine-König --- drivers/pwm/core.c | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index cf6d91f1a9e6..8acbcf5b6673 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -137,6 +137,25 @@ static void pwm_apply_debug(struct pwm_device *pwm, } } +static bool pwm_state_valid(const struct pwm_state *state) +{ + /* + * For a disabled state all other state description is irrelevant and + * and supposed to be ignored. So also ignore any strange values and + * consider the state ok. + */ + if (state->enabled) + return true; + + if (!state->period) + return false; + + if (state->duty_cycle > state->period) + return false; + + return true; +} + /** * __pwm_apply() - atomically apply a new state to a PWM device * @pwm: PWM device @@ -147,10 +166,26 @@ static int __pwm_apply(struct pwm_device *pwm, const struct pwm_state *state) struct pwm_chip *chip; int err; - if (!pwm || !state || !state->period || - state->duty_cycle > state->period) + if (!pwm || !state) return -EINVAL; + if (!pwm_state_valid(state)) { + /* + * Allow to transition from one invalid state to another. + * This ensures that you can e.g. change the polarity while + * the period is zero. (This happens on stm32 when the hardware + * is in its poweron default state.) This greatly simplifies + * working with the sysfs API where you can only change one + * parameter at a time. + */ + if (!pwm_state_valid(&pwm->state)) { + pwm->state = *state; + return 0; + } + + return -EINVAL; + } + chip = pwm->chip; if (state->period == pwm->state.period &&