Skip to content

Commit

Permalink
pwm: samsung: Fix output race on disabling
Browse files Browse the repository at this point in the history
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 3bdf878, 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 3bdf878 (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: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
  • Loading branch information
Sjoerd Simons authored and Thierry Reding committed Mar 27, 2015
1 parent 24ccea1 commit 4a1c683
Showing 1 changed file with 31 additions and 1 deletion.
32 changes: 31 additions & 1 deletion drivers/pwm/pwm-samsung.c
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,31 @@ static void pwm_samsung_disable(struct pwm_chip *chip, struct pwm_device *pwm)
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,
int duty_ns, int period_ns)
{
struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
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
Expand All @@ -288,6 +307,7 @@ static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
return 0;

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. */
++tcnt;
Expand Down Expand Up @@ -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(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->tin_ns = tin_ns;
chan->duty_ns = duty_ns;
Expand Down

0 comments on commit 4a1c683

Please sign in to comment.