| Message ID | 20210531044608.1006024-3-roman.beranek@prusa3d.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | pwm: sun4i: only wait 2 cycles prior to disabling | expand |
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 --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);
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(-)