diff options
| author | Sjoerd Simons <sjoerd.simons@collabora.co.uk> | 2015-03-05 03:14:03 -0500 |
|---|---|---|
| committer | Thierry Reding <thierry.reding@gmail.com> | 2015-03-27 08:06:47 -0400 |
| commit | 4a1c683c98e4e2997c79258bffbb9be4af4fba83 (patch) | |
| tree | 305543fa7a9731704c71d52e62be572d71355618 | |
| parent | 24ccea1ce6717b91bb1e71b12cfd956f8d32dcf3 (diff) | |
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 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>
| -rw-r--r-- | drivers/pwm/pwm-samsung.c | 32 |
1 files changed, 31 insertions, 1 deletions
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c index 3e9b5835a4af..ff201e1b9219 100644 --- a/drivers/pwm/pwm-samsung.c +++ b/drivers/pwm/pwm-samsung.c | |||
| @@ -269,12 +269,31 @@ static void pwm_samsung_disable(struct pwm_chip *chip, struct pwm_device *pwm) | |||
| 269 | spin_unlock_irqrestore(&samsung_pwm_lock, flags); | 269 | spin_unlock_irqrestore(&samsung_pwm_lock, flags); |
| 270 | } | 270 | } |
| 271 | 271 | ||
| 272 | static void pwm_samsung_manual_update(struct samsung_pwm_chip *chip, | ||
| 273 | struct pwm_device *pwm) | ||
| 274 | { | ||
| 275 | unsigned int tcon_chan = to_tcon_channel(pwm->hwpwm); | ||
| 276 | u32 tcon; | ||
| 277 | unsigned long flags; | ||
| 278 | |||
| 279 | spin_lock_irqsave(&samsung_pwm_lock, flags); | ||
| 280 | |||
| 281 | tcon = readl(chip->base + REG_TCON); | ||
| 282 | tcon |= TCON_MANUALUPDATE(tcon_chan); | ||
| 283 | writel(tcon, chip->base + REG_TCON); | ||
| 284 | |||
| 285 | tcon &= ~TCON_MANUALUPDATE(tcon_chan); | ||
| 286 | writel(tcon, chip->base + REG_TCON); | ||
| 287 | |||
| 288 | spin_unlock_irqrestore(&samsung_pwm_lock, flags); | ||
| 289 | } | ||
| 290 | |||
| 272 | static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm, | 291 | static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm, |
| 273 | int duty_ns, int period_ns) | 292 | int duty_ns, int period_ns) |
| 274 | { | 293 | { |
| 275 | struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip); | 294 | struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip); |
| 276 | struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm); | 295 | struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm); |
| 277 | u32 tin_ns = chan->tin_ns, tcnt, tcmp; | 296 | u32 tin_ns = chan->tin_ns, tcnt, tcmp, oldtcmp; |
| 278 | 297 | ||
| 279 | /* | 298 | /* |
| 280 | * We currently avoid using 64bit arithmetic by using the | 299 | * 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 | return 0; | 307 | return 0; |
| 289 | 308 | ||
| 290 | tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm)); | 309 | tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm)); |
| 310 | oldtcmp = readl(our_chip->base + REG_TCMPB(pwm->hwpwm)); | ||
| 291 | 311 | ||
| 292 | /* We need tick count for calculation, not last tick. */ | 312 | /* We need tick count for calculation, not last tick. */ |
| 293 | ++tcnt; | 313 | ++tcnt; |
| @@ -335,6 +355,16 @@ static int pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm, | |||
| 335 | writel(tcnt, our_chip->base + REG_TCNTB(pwm->hwpwm)); | 355 | writel(tcnt, our_chip->base + REG_TCNTB(pwm->hwpwm)); |
| 336 | writel(tcmp, our_chip->base + REG_TCMPB(pwm->hwpwm)); | 356 | writel(tcmp, our_chip->base + REG_TCMPB(pwm->hwpwm)); |
| 337 | 357 | ||
| 358 | /* | ||
| 359 | * In case the PWM is currently at 100% duty cycle, force a manual | ||
| 360 | * update to prevent the signal staying high if the PWM is disabled | ||
| 361 | * shortly afer this update (before it autoreloaded the new values). | ||
| 362 | */ | ||
| 363 | if (oldtcmp == (u32) -1) { | ||
| 364 | dev_dbg(our_chip->chip.dev, "Forcing manual update"); | ||
| 365 | pwm_samsung_manual_update(our_chip, pwm); | ||
| 366 | } | ||
| 367 | |||
| 338 | chan->period_ns = period_ns; | 368 | chan->period_ns = period_ns; |
| 339 | chan->tin_ns = tin_ns; | 369 | chan->tin_ns = tin_ns; |
| 340 | chan->duty_ns = duty_ns; | 370 | chan->duty_ns = duty_ns; |
