From: Anand Moon <linux.amoon@gmail.com>
To: Lukasz Majewski <l.majewski@samsung.com>,
Eduardo Valentin <edubezval@gmail.com>,
Sjoerd Simons <sjoerd.simons@collabora.co.uk>,
Russell King <linux@arm.linux.org.uk>,
Kukjin Kim <kgene@kernel.org>
Cc: devicetree@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
linux-pwm@vger.kernel.org, Anand Moon <linux.amoon@gmail.com>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: [PATCH 5/6] pwm: samsung: Fix output race on disabling
Date: Fri, 27 Mar 2015 03:09:14 +1030 [thread overview]
Message-ID: <1427387955-5129-6-git-send-email-linux.amoon@gmail.com> (raw)
In-Reply-To: <1427387955-5129-1-git-send-email-linux.amoon@gmail.com>
From: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
When disabling the samsung PWM the output state remains at the level it
was in the end of a pwm cycle. In other words, calling pwm_disable when
at 100% duty will keep the output active, while at all other setting 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 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 _config and
_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). Only force an update of the
settings when at 100% duty, which shouldn't have a 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>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
drivers/pwm/pwm-samsung.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 3e9b583..649f6c4 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)
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
@@ -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;
@@ -335,6 +355,15 @@ 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, force a manual update
+ * to prevent the signal staying high in 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;
--
1.9.1
WARNING: multiple messages have this Message-ID (diff)
From: linux.amoon@gmail.com (Anand Moon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 5/6] pwm: samsung: Fix output race on disabling
Date: Fri, 27 Mar 2015 03:09:14 +1030 [thread overview]
Message-ID: <1427387955-5129-6-git-send-email-linux.amoon@gmail.com> (raw)
In-Reply-To: <1427387955-5129-1-git-send-email-linux.amoon@gmail.com>
From: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
When disabling the samsung PWM the output state remains at the level it
was in the end of a pwm cycle. In other words, calling pwm_disable when
at 100% duty will keep the output active, while at all other setting 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 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 _config and
_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). Only force an update of the
settings when at 100% duty, which shouldn't have a 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>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
drivers/pwm/pwm-samsung.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 3e9b583..649f6c4 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)
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
@@ -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;
@@ -335,6 +355,15 @@ 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, force a manual update
+ * to prevent the signal staying high in 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;
--
1.9.1
WARNING: multiple messages have this Message-ID (diff)
From: Anand Moon <linux.amoon@gmail.com>
To: Lukasz Majewski <l.majewski@samsung.com>,
Eduardo Valentin <edubezval@gmail.com>,
Sjoerd Simons <sjoerd.simons@collabora.co.uk>,
Russell King <linux@arm.linux.org.uk>,
Kukjin Kim <kgene@kernel.org>
Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pwm@vger.kernel.org, Anand Moon <linux.amoon@gmail.com>
Subject: [PATCH 5/6] pwm: samsung: Fix output race on disabling
Date: Fri, 27 Mar 2015 03:09:14 +1030 [thread overview]
Message-ID: <1427387955-5129-6-git-send-email-linux.amoon@gmail.com> (raw)
In-Reply-To: <1427387955-5129-1-git-send-email-linux.amoon@gmail.com>
From: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
When disabling the samsung PWM the output state remains at the level it
was in the end of a pwm cycle. In other words, calling pwm_disable when
at 100% duty will keep the output active, while at all other setting 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 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 _config and
_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). Only force an update of the
settings when at 100% duty, which shouldn't have a 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>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
drivers/pwm/pwm-samsung.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 3e9b583..649f6c4 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)
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
@@ -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;
@@ -335,6 +355,15 @@ 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, force a manual update
+ * to prevent the signal staying high in 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;
--
1.9.1
next prev parent reply other threads:[~2015-03-26 16:39 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-26 16:39 Exynos5422 odroidxu3 pwm-fan control using thermal sensors Anand Moon
2015-03-26 16:39 ` Anand Moon
2015-03-26 16:39 ` [PATCH 1/6] ARM: dts :exynos5422-odroidxu3 Add pwm-fan node to the Odroid-XU3 board Anand Moon
2015-03-26 16:39 ` Anand Moon
2015-04-08 7:46 ` Lukasz Majewski
2015-04-08 7:46 ` Lukasz Majewski
2015-03-26 16:39 ` [PATCH 2/6] ARM: dts exynos5420 update the cooling cells for core cpu0 Anand Moon
2015-03-26 16:39 ` Anand Moon
2015-04-08 7:47 ` Lukasz Majewski
2015-04-08 7:47 ` Lukasz Majewski
2015-03-26 16:39 ` [PATCH 3/6] ARM:dts exynos5422 Update the thermal sensor for tmu_cpu0 Anand Moon
2015-03-26 16:39 ` Anand Moon
2015-04-08 8:02 ` Lukasz Majewski
2015-04-08 8:02 ` Lukasz Majewski
2015-04-08 8:02 ` Lukasz Majewski
2015-04-08 9:12 ` Anand Moon
2015-04-08 9:12 ` Anand Moon
2015-04-08 9:12 ` Anand Moon
2015-04-08 9:30 ` Lukasz Majewski
2015-04-08 9:30 ` Lukasz Majewski
2015-04-08 9:44 ` Anand Moon
2015-04-08 9:44 ` Anand Moon
2015-03-26 16:39 ` [PATCH 4/6] ARM: dts: OdroidXU3: Enable TMU at Exynos5422 base Anand Moon
2015-03-26 16:39 ` Anand Moon
2015-04-08 8:03 ` Lukasz Majewski
2015-04-08 8:03 ` Lukasz Majewski
2015-03-26 16:39 ` Anand Moon [this message]
2015-03-26 16:39 ` [PATCH 5/6] pwm: samsung: Fix output race on disabling Anand Moon
2015-03-26 16:39 ` Anand Moon
2015-04-08 8:28 ` Lukasz Majewski
2015-04-08 8:28 ` Lukasz Majewski
2015-04-08 8:42 ` Sjoerd Simons
2015-04-08 8:42 ` Sjoerd Simons
2015-04-08 8:53 ` Anand Moon
2015-04-08 8:53 ` Anand Moon
2015-03-26 16:39 ` [PATCH 6/6] hwmon: pwm-fan: Update the duty cycle inorder to control the pwm-fan Anand Moon
2015-03-26 16:39 ` Anand Moon
2015-04-08 8:44 ` Lukasz Majewski
2015-04-08 8:44 ` Lukasz Majewski
2015-04-08 13:11 ` Guenter Roeck
2015-04-08 13:11 ` Guenter Roeck
2015-04-08 15:32 ` Guenter Roeck
2015-04-08 15:32 ` Guenter Roeck
2015-04-08 16:02 ` Anand Moon
2015-04-08 16:02 ` Anand Moon
2015-04-08 16:53 ` Guenter Roeck
2015-04-08 16:53 ` Guenter Roeck
2015-04-08 17:49 ` Anand Moon
2015-04-08 17:49 ` Anand Moon
2015-04-10 11:28 ` Anand Moon
2015-04-10 11:28 ` Anand Moon
2015-04-10 12:00 ` Sjoerd Simons
2015-04-10 12:00 ` Sjoerd Simons
2015-04-10 12:00 ` Sjoerd Simons
[not found] ` <1428667201.22057.20.camel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2015-04-10 12:59 ` Anand Moon
2015-04-10 12:59 ` Anand Moon
2015-04-10 12:59 ` Anand Moon
2015-04-10 13:09 ` Guenter Roeck
2015-04-10 13:09 ` Guenter Roeck
2015-04-10 13:17 ` Anand Moon
2015-04-10 13:17 ` Anand Moon
2015-04-10 13:30 ` Sjoerd Simons
2015-04-10 13:30 ` Sjoerd Simons
2015-04-10 13:30 ` Sjoerd Simons
2015-04-10 13:58 ` Thierry Reding
2015-04-10 13:58 ` Thierry Reding
2015-04-10 17:25 ` Ben Gamari
2015-04-10 17:25 ` Ben Gamari
2015-04-10 17:25 ` Ben Gamari
2015-04-10 17:58 ` Guenter Roeck
2015-04-10 17:58 ` Guenter Roeck
2015-04-02 10:02 ` Exynos5422 odroidxu3 pwm-fan control using thermal sensors Markus Reichl
2015-04-02 10:02 ` Markus Reichl
2015-04-02 10:06 ` Markus Reichl
2015-04-02 10:06 ` Markus Reichl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1427387955-5129-6-git-send-email-linux.amoon@gmail.com \
--to=linux.amoon@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=edubezval@gmail.com \
--cc=kgene@kernel.org \
--cc=l.majewski@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=sjoerd.simons@collabora.co.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.