diff mbox series

[2/6] pwm: sun4i: disable EN bit prior to the delay

Message ID 20210531044608.1006024-3-roman.beranek@prusa3d.com (mailing list archive)
State New, archived
Headers show
Series pwm: sun4i: only wait 2 cycles prior to disabling | expand

Commit Message

Roman Beranek May 31, 2021, 4:46 a.m. UTC
The reason why we wait before gating the clock is to allow for the PWM
to finish its cycle and stop. But it won't stop unless the EN bit is
disabled.

Signed-off-by: Roman Beranek <roman.beranek@prusa3d.com>
---
 drivers/pwm/pwm-sun4i.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Uwe Kleine-König June 7, 2021, 8:07 a.m. UTC | #1
On Mon, May 31, 2021 at 06:46:04AM +0200, Roman Beranek wrote:
> The reason why we wait before gating the clock is to allow for the PWM
> to finish its cycle and stop. But it won't stop unless the EN bit is
> disabled.
> 
> Signed-off-by: Roman Beranek <roman.beranek@prusa3d.com>
> ---
>  drivers/pwm/pwm-sun4i.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 3721b9894cf6..2777abe66f79 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -303,6 +303,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  
>  	if (state->enabled)
>  		ctrl |= BIT_CH(PWM_EN, pwm->hwpwm);
> +	else
> +		ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm);

Catching the case !state->enabled even earlier would make sense.
Otherwise you might see a needless glitch after

	pwm_apply_state(mypwm, { .period = A, .duty_cycle = B, .enabled = true });
	pwm_apply_state(mypwm, { .period = C, .duty_cycle = D, .enabled = false });

which might make C+D visible on the PWM before disabling.

>  
>  	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
>  
> @@ -325,7 +327,6 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  	spin_lock(&sun4i_pwm->ctrl_lock);
>  	ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
>  	ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
> -	ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm);
>  	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
>  	spin_unlock(&sun4i_pwm->ctrl_lock);

So the comment

  /* We need a full period to elapse before disabling the channel. */

is wrong?

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 3721b9894cf6..2777abe66f79 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -303,6 +303,8 @@  static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	if (state->enabled)
 		ctrl |= BIT_CH(PWM_EN, pwm->hwpwm);
+	else
+		ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm);
 
 	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
 
@@ -325,7 +327,6 @@  static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	spin_lock(&sun4i_pwm->ctrl_lock);
 	ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
 	ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
-	ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm);
 	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
 	spin_unlock(&sun4i_pwm->ctrl_lock);