[PATCH] pwm: Assume a disabled PWM to emit a constant inactive output

[PATCH] pwm: Assume a disabled PWM to emit a constant inactive output

[Uwe Kleine-König]

Some PWM hardwares (e.g. MC33XS2410) cannot implement a zero duty cycle
but can instead disable the hardware which also results in a constant
inactive output.

There are some checks (enabled with CONFIG_PWM_DEBUG) to help
implementing a driver without violating the normal rounding rules. Make
them less strict to let above described hardware pass without warning.

Reported-by: Dimitri Fedrau <dima.fedrau@gmail.com>
Link: https://lore.kernel.org/r/20241103205215.GA509903@debian
Fixes: 3ad1f3a33286 ("pwm: Implement some checks for lowlevel drivers")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
Hello,

in the context of discussing the implementation of an MC33XS2410 driver
Dimitri reported this (unjustified) warning. With this patch the
reported warning should go away when applying the optimisation I
suggested.

Best regards
Uwe

 drivers/pwm/core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 6e752e148b98..210368099a06 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -75,7 +75,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 &&
+	if (state->enabled && s2.enabled &&
 	    last->polarity == state->polarity &&
 	    last->period > s2.period &&
 	    last->period <= state->period)
@@ -83,7 +83,11 @@ static void pwm_apply_debug(struct pwm_device *pwm,
 			 ".apply didn't pick the best available period (requested: %llu, applied: %llu, possible: %llu)\n",
 			 state->period, s2.period, last->period);
 
-	if (state->enabled && state->period < s2.period)
+	/*
+	 * 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)
 		dev_warn(pwmchip_parent(chip),
 			 ".apply is supposed to round down period (requested: %llu, applied: %llu)\n",
 			 state->period, s2.period);
@@ -99,7 +103,7 @@ static void pwm_apply_debug(struct pwm_device *pwm,
 			 s2.duty_cycle, s2.period,
 			 last->duty_cycle, last->period);
 
-	if (state->enabled && state->duty_cycle < s2.duty_cycle)
+	if (state->enabled && s2.enabled && 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,

base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc
-- 
2.45.2

Re: [PATCH] pwm: Assume a disabled PWM to emit a constant inactive output

[Dimitri Fedrau]

Am Tue, Nov 05, 2024 at 04:35:22PM +0100 schrieb Uwe Kleine-König:
> Some PWM hardwares (e.g. MC33XS2410) cannot implement a zero duty cycle
> but can instead disable the hardware which also results in a constant
> inactive output.
> 
> There are some checks (enabled with CONFIG_PWM_DEBUG) to help
> implementing a driver without violating the normal rounding rules. Make
> them less strict to let above described hardware pass without warning.
> 
> Reported-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> Link: https://lore.kernel.org/r/20241103205215.GA509903@debian
> Fixes: 3ad1f3a33286 ("pwm: Implement some checks for lowlevel drivers")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> Hello,
> 
> in the context of discussing the implementation of an MC33XS2410 driver
> Dimitri reported this (unjustified) warning. With this patch the
> reported warning should go away when applying the optimisation I
> suggested.
> 
> Best regards
> Uwe
> 
>  drivers/pwm/core.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 6e752e148b98..210368099a06 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -75,7 +75,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 &&
> +	if (state->enabled && s2.enabled &&
>  	    last->polarity == state->polarity &&
>  	    last->period > s2.period &&
>  	    last->period <= state->period)
> @@ -83,7 +83,11 @@ static void pwm_apply_debug(struct pwm_device *pwm,
>  			 ".apply didn't pick the best available period (requested: %llu, applied: %llu, possible: %llu)\n",
>  			 state->period, s2.period, last->period);
>  
> -	if (state->enabled && state->period < s2.period)
> +	/*
> +	 * 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)
>  		dev_warn(pwmchip_parent(chip),
>  			 ".apply is supposed to round down period (requested: %llu, applied: %llu)\n",
>  			 state->period, s2.period);
> @@ -99,7 +103,7 @@ static void pwm_apply_debug(struct pwm_device *pwm,
>  			 s2.duty_cycle, s2.period,
>  			 last->duty_cycle, last->period);
>  
> -	if (state->enabled && state->duty_cycle < s2.duty_cycle)
> +	if (state->enabled && s2.enabled && 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,
> 
> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc
> -- 
> 2.45.2
> 
Hello Uwe,

works for me. Unjustified reported warnings are gone.
Thanks for the patch.

Reviewed-by: Dimitri Fedrau <dima.fedrau@gmail.com>
Tested-by: Dimitri Fedrau <dima.fedrau@gmail.com>

Best regards,
Dimitri

Re: [PATCH] pwm: Assume a disabled PWM to emit a constant inactive output

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

Hello,

On Tue, Nov 05, 2024 at 04:35:22PM +0100, Uwe Kleine-König wrote:
> Some PWM hardwares (e.g. MC33XS2410) cannot implement a zero duty cycle
> but can instead disable the hardware which also results in a constant
> inactive output.
> 
> There are some checks (enabled with CONFIG_PWM_DEBUG) to help
> implementing a driver without violating the normal rounding rules. Make
> them less strict to let above described hardware pass without warning.
> 
> Reported-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> Link: https://lore.kernel.org/r/20241103205215.GA509903@debian
> Fixes: 3ad1f3a33286 ("pwm: Implement some checks for lowlevel drivers")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next
with Dimitri's tags.

Thanks
Uwe


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]