* [PATCH 1/2] pwm: Disable PWM_DEBUG check for disabled states
2025-07-08 17:17 [PATCH 0/2] pwm: Improvements for PWM_DEBUG checks Uwe Kleine-König
@ 2025-07-08 17:18 ` Uwe Kleine-König
2025-07-08 17:18 ` [PATCH 2/2] pwm: Check actual period and duty_cycle for ignored polarity test Uwe Kleine-König
2025-08-01 9:31 ` [PATCH 0/2] pwm: Improvements for PWM_DEBUG checks Uwe Kleine-König
2 siblings, 0 replies; 4+ messages in thread
From: Uwe Kleine-König @ 2025-07-08 17:18 UTC (permalink / raw)
To: linux-pwm
When a PWM is requested to be disabled, the result is unspecified, the only
intention is to save some power. So skip all checks in this case.
All but two checks already only triggered for states with .enabled = true.
The first resulted in some false positive diagnostics, the other checked
for a condition that depending on hardware might not be implementable.
Similar if the lowlevel driver disabled the hardware this might be a valid
reaction and with .enabled = false all other state parameters are
unreliable, so skip further tests in this case, too.
All later usages of .enabled can be assumed to yield true, and so several
if conditions can be simplified.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
drivers/pwm/core.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 0d66376a83ec..d9d9badf7a8e 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -496,6 +496,13 @@ static void pwm_apply_debug(struct pwm_device *pwm,
if (!chip->ops->get_state)
return;
+ /*
+ * If a disabled PWM was requested the result is unspecified, so nothing
+ * to check.
+ */
+ if (!state->enabled)
+ return;
+
/*
* *state was just applied. Read out the hardware state and do some
* checks.
@@ -507,16 +514,23 @@ static void pwm_apply_debug(struct pwm_device *pwm,
/* If that failed there isn't much to debug */
return;
+ /*
+ * If the PWM was disabled that's maybe strange but there is nothing
+ * that can be sensibly checked then. So return early.
+ */
+ if (!s1.enabled)
+ return;
+
/*
* The lowlevel driver either ignored .polarity (which is a bug) or as
* best effort inverted .polarity and fixed .duty_cycle respectively.
* Undo this inversion and fixup for further tests.
*/
- if (s1.enabled && s1.polarity != state->polarity) {
+ if (s1.polarity != state->polarity) {
s2.polarity = state->polarity;
s2.duty_cycle = s1.period - s1.duty_cycle;
s2.period = s1.period;
- s2.enabled = s1.enabled;
+ s2.enabled = true;
} else {
s2 = s1;
}
@@ -525,8 +539,7 @@ static void pwm_apply_debug(struct pwm_device *pwm,
state->duty_cycle < state->period)
dev_warn(pwmchip_parent(chip), ".apply ignored .polarity\n");
- if (state->enabled && s2.enabled &&
- last->polarity == state->polarity &&
+ if (last->polarity == state->polarity &&
last->period > s2.period &&
last->period <= state->period)
dev_warn(pwmchip_parent(chip),
@@ -537,13 +550,12 @@ static void pwm_apply_debug(struct pwm_device *pwm,
* Rounding period up is fine only if duty_cycle is 0 then, because a
* flat line doesn't have a characteristic period.
*/
- if (state->enabled && s2.enabled && state->period < s2.period && s2.duty_cycle)
+ if (state->period < s2.period && s2.duty_cycle)
dev_warn(pwmchip_parent(chip),
".apply is supposed to round down period (requested: %llu, applied: %llu)\n",
state->period, s2.period);
- if (state->enabled &&
- last->polarity == state->polarity &&
+ if (last->polarity == state->polarity &&
last->period == s2.period &&
last->duty_cycle > s2.duty_cycle &&
last->duty_cycle <= state->duty_cycle)
@@ -553,16 +565,12 @@ static void pwm_apply_debug(struct pwm_device *pwm,
s2.duty_cycle, s2.period,
last->duty_cycle, last->period);
- if (state->enabled && s2.enabled && state->duty_cycle < s2.duty_cycle)
+ if (state->duty_cycle < s2.duty_cycle)
dev_warn(pwmchip_parent(chip),
".apply is supposed to round down duty_cycle (requested: %llu/%llu, applied: %llu/%llu)\n",
state->duty_cycle, state->period,
s2.duty_cycle, s2.period);
- if (!state->enabled && s2.enabled && s2.duty_cycle > 0)
- dev_warn(pwmchip_parent(chip),
- "requested disabled, but yielded enabled with duty > 0\n");
-
/* reapply the state that the driver reported being configured. */
err = chip->ops->apply(chip, pwm, &s1);
trace_pwm_apply(pwm, &s1, err);
--
2.49.0
^ permalink raw reply related [flat|nested] 4+ messages in thread