Commit 4a1c683c authored by Sjoerd Simons's avatar Sjoerd Simons Committed by Thierry Reding

pwm: samsung: Fix output race on disabling

When disabling the Samsung PWM the output state remains at the level it
was at the end of a PWM cycle. In other words, calling pwm_disable()
when at 100% duty cycle will keep the output active, while at all other
settings the output will go/stay inactive. On top of that the Samsung
PWM settings are double-buffered, which means the new settings only get
applied at the start of a new PWM cycle.

This results in a race if the PWM is at 100% duty cycle and a driver
calls:

  pwm_config(pwm, 0, period);
  pwm_disable(pwm);

In this case the PWMs output will unexpectedly stay active, unless a new
PWM cycle happened to start between the register writes in pwm_config()
and pwm_disable(). As far as I can tell this is a regression introduced
by 3bdf8781, before that a call to pwm_config() would call
pwm_samsung_enable() which, while heavy-handed, made sure the expected
settings were live.

To resolve this, while not re-introducing the issues 3bdf8781 (flickering
as the PWM got reset while in a PWM cycle) fixed, only force an update
of the settings when at 100% duty cycle, which shouldn't have any
noticeable effect on the output but is enough to ensure the behaviour is
as expected on disable.
Signed-off-by: default avatarSjoerd Simons <sjoerd.simons@collabora.co.uk>
Reviewed-by: default avatarJavier Martinez Canillas <javier.martinez@collabora.co.uk>
Acked-by: default avatarLukasz Majewski <l.majewski@samsung.com>
Signed-off-by: default avatarThierry Reding <thierry.reding@gmail.com>
parent 24ccea1c
...@@ -269,12 +269,31 @@ static void pwm_samsung_disable(struct pwm_chip *chip, struct pwm_device *pwm) ...@@ -269,12 +269,31 @@ static void pwm_samsung_disable(struct pwm_chip *chip, struct pwm_device *pwm)
spin_unlock_irqrestore(&samsung_pwm_lock, flags); spin_unlock_irqrestore(&samsung_pwm_lock, flags);
} }
static void pwm_samsung_manual_update(struct samsung_pwm_chip *chip,
struct pwm_device *pwm)
{
unsigned int tcon_chan = to_tcon_channel(pwm->hwpwm);
u32 tcon;
unsigned long flags;
spin_lock_irqsave(&samsung_pwm_lock, flags);
tcon = readl(chip->base + REG_TCON);
tcon |= TCON_MANUALUPDATE(tcon_chan);
writel(tcon, chip->base + REG_TCON);
tcon &= ~TCON_MANUALUPDATE(tcon_chan);
writel(tcon, chip->base + REG_TCON);
spin_unlock_irqrestore(&samsung_pwm_lock, flags);
}
static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm, static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
int duty_ns, int period_ns) int duty_ns, int period_ns)
{ {
struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip); struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm); struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm);
u32 tin_ns = chan->tin_ns, tcnt, tcmp; u32 tin_ns = chan->tin_ns, tcnt, tcmp, oldtcmp;
/* /*
* We currently avoid using 64bit arithmetic by using the * We currently avoid using 64bit arithmetic by using the
...@@ -288,6 +307,7 @@ static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm, ...@@ -288,6 +307,7 @@ static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
return 0; return 0;
tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm)); tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm));
oldtcmp = readl(our_chip->base + REG_TCMPB(pwm->hwpwm));
/* We need tick count for calculation, not last tick. */ /* We need tick count for calculation, not last tick. */
++tcnt; ++tcnt;
...@@ -335,6 +355,16 @@ static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm, ...@@ -335,6 +355,16 @@ static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
writel(tcnt, our_chip->base + REG_TCNTB(pwm->hwpwm)); writel(tcnt, our_chip->base + REG_TCNTB(pwm->hwpwm));
writel(tcmp, our_chip->base + REG_TCMPB(pwm->hwpwm)); writel(tcmp, our_chip->base + REG_TCMPB(pwm->hwpwm));
/*
* In case the PWM is currently at 100% duty cycle, force a manual
* update to prevent the signal staying high if the PWM is disabled
* shortly afer this update (before it autoreloaded the new values).
*/
if (oldtcmp == (u32) -1) {
dev_dbg(our_chip->chip.dev, "Forcing manual update");
pwm_samsung_manual_update(our_chip, pwm);
}
chan->period_ns = period_ns; chan->period_ns = period_ns;
chan->tin_ns = tin_ns; chan->tin_ns = tin_ns;
chan->duty_ns = duty_ns; chan->duty_ns = duty_ns;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment