diff mbox series

pwm: sun4i: Remove erroneous else branch

Message ID 20201216174309.1604658-1-thierry.reding@gmail.com
State Accepted
Commit 6eefb79d6f5bc4086bd02c76f1072dd4a8d9d9f6
Headers show
Series pwm: sun4i: Remove erroneous else branch | expand

Commit Message

Thierry Reding Dec. 16, 2020, 5:43 p.m. UTC
Commit d3817a647059 ("pwm: sun4i: Remove redundant needs_delay") changed
the logic of an else branch so that the PWM_EN and PWM_CLK_GATING bits
are now cleared if the PWM is to be disabled, whereas previously the
condition was always false, and hence the branch never got executed.

This code is reported causing backlight issues on boards based on the
Allwinner A20 SoC. Fix this by removing the else branch, which restores
the behaviour prior to the offending commit.

Note that the PWM_EN and PWM_CLK_GATING bits still get cleared later in
sun4i_pwm_apply() if the PWM is to be disabled.

Fixes: d3817a647059 ("pwm: sun4i: Remove redundant needs_delay")
Reported-by: Taras Galchenko <tpgalchenko@gmail.com>
Suggested-by: Taras Galchenko <tpgalchenko@gmail.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
---
 drivers/pwm/pwm-sun4i.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Uwe Kleine-König Dec. 17, 2020, 11:31 a.m. UTC | #1
On Wed, Dec 16, 2020 at 06:43:09PM +0100, Thierry Reding wrote:
> Commit d3817a647059 ("pwm: sun4i: Remove redundant needs_delay") changed
> the logic of an else branch so that the PWM_EN and PWM_CLK_GATING bits
> are now cleared if the PWM is to be disabled, whereas previously the
> condition was always false, and hence the branch never got executed.
> 
> This code is reported causing backlight issues on boards based on the
> Allwinner A20 SoC. Fix this by removing the else branch, which restores
> the behaviour prior to the offending commit.
> 
> Note that the PWM_EN and PWM_CLK_GATING bits still get cleared later in
> sun4i_pwm_apply() if the PWM is to be disabled.
> 
> Fixes: d3817a647059 ("pwm: sun4i: Remove redundant needs_delay")
> Reported-by: Taras Galchenko <tpgalchenko@gmail.com>

This report didn't happen on the linux-pwm list, or did I miss it?

What is the reported problem? I would expect a short time where the
backlight goes off completely (or on depending on polarity?) when only a
small continuous change is expected.

> Suggested-by: Taras Galchenko <tpgalchenko@gmail.com>
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>

Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index cc1eb0818648..ce5c4fc8da6f 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -294,12 +294,8 @@  static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	ctrl |= BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
 
-	if (state->enabled) {
+	if (state->enabled)
 		ctrl |= BIT_CH(PWM_EN, pwm->hwpwm);
-	} else {
-		ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm);
-		ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
-	}
 
 	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);