All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jean Pihet <jean.pihet@newoldbits.com>
To: linux-omap@vger.kernel.org, paul@pwsan.com,
	linux-arm-kernel@lists.infradead.org, khilman@ti.com,
	Rajendra Nayak <rnayak@ti.com>,
	Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Nishanth Menon <nm@ti.com>
Cc: Jean Pihet <j-pihet@ti.com>
Subject: [PATCH 3/8] ARM: OMAP2+: PM: add a lock to protect the powerdomains next state
Date: Wed, 15 Aug 2012 12:02:43 +0200	[thread overview]
Message-ID: <1345024968-28951-4-git-send-email-j-pihet@ti.com> (raw)
In-Reply-To: <1345024968-28951-1-git-send-email-j-pihet@ti.com>

pwrdm_set_next_fpwrst and pwrdm_read_next_fpwrst are intented
to be the only API to program and request the next state of a power
domain.
This patch protects the power domain next state settings and structs
from concurrent accesses by the use of a lock.

A spinlock is used since pwrdm_set_next_fpwrst and
pwrdm_read_next_fpwrst can be called from atomic context
(.i.e) either from cpuidle path or suspend path.

[ambresh@ti.com: reported the atomic context issue and suggested
to replace the mutex with a spinlock]

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Reported-by: Ambresh K <ambresh@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |   36 +++++++++++++++++++++++++++++++-----
 arch/arm/mach-omap2/powerdomain.h |    3 +++
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 3a1f56c..a8f7a81 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -102,6 +102,7 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
 	INIT_LIST_HEAD(&pwrdm->voltdm_node);
 	voltdm_add_pwrdm(voltdm, pwrdm);
 
+	spin_lock_init(&pwrdm->lock);
 	list_add(&pwrdm->node, &pwrdm_list);
 
 	/* Initialize the powerdomain's state counter */
@@ -601,6 +602,22 @@ int pwrdm_pwrst_to_fpwrst(struct powerdomain *pwrdm, u8 pwrst, u8 logic)
 }
 
 /**
+ * _pwrdm_read_next_fpwrst - get next powerdomain functional power state
+ * @pwrdm: struct powerdomain * to get power state
+ *
+ * Return the powerdomain @pwrdm's next functional power state.
+ * Returns -EINVAL if the powerdomain pointer is null or returns
+ * the next power state upon success.
+ */
+int _pwrdm_read_next_fpwrst(struct powerdomain *pwrdm)
+{
+	int next_pwrst = pwrdm_read_next_pwrst(pwrdm);
+	int next_logic = pwrdm_read_logic_retst(pwrdm);
+
+	return pwrdm_pwrst_to_fpwrst(pwrdm, next_pwrst, next_logic);
+}
+
+/**
  * pwrdm_get_achievable_fpwrst() - Provide achievable functional state
  * @pwrdm: struct powerdomain * to set
  * @fpwrst: minimum functional state we would like to hit
@@ -670,6 +687,7 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 	int sleep_switch = -1, ret = 0, hwsup = 0;
 	int new_fpwrst, next_fpwrst, pwrst, logic;
 	u8 curr_pwrst;
+	unsigned long flags;
 
 	if (!pwrdm || IS_ERR(pwrdm)) {
 		pr_debug("%s: invalid params: pwrdm=%p\n", __func__, pwrdm);
@@ -678,13 +696,15 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 
 	pr_debug("%s(%s, fpwrst=%0x)\n", __func__, pwrdm->name, fpwrst);
 
+	spin_lock_irqsave(&pwrdm->lock, flags);
+
 	new_fpwrst = pwrdm_get_achievable_fpwrst(pwrdm, fpwrst);
 	pwrst = pwrdm_fpwrst_to_pwrst(pwrdm, new_fpwrst);
 	logic = pwrdm_fpwrst_to_logic_pwrst(pwrdm, new_fpwrst);
 
-	next_fpwrst = pwrdm_read_next_fpwrst(pwrdm);
+	next_fpwrst = _pwrdm_read_next_fpwrst(pwrdm);
 	if (new_fpwrst == next_fpwrst)
-		return ret;
+		goto out;
 
 	curr_pwrst = pwrdm_read_pwrst(pwrdm);
 	if (curr_pwrst < PWRDM_POWER_ON) {
@@ -732,6 +752,8 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 		break;
 	}
 
+out:
+	spin_unlock_irqrestore(&pwrdm->lock, flags);
 	return ret;
 }
 
@@ -801,10 +823,14 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
  */
 int pwrdm_read_next_fpwrst(struct powerdomain *pwrdm)
 {
-	int next_pwrst = pwrdm_read_next_pwrst(pwrdm);
-	int next_logic = pwrdm_read_logic_retst(pwrdm);
+	int ret;
+	unsigned long flags;
 
-	return pwrdm_pwrst_to_fpwrst(pwrdm, next_pwrst, next_logic);
+	spin_lock_irqsave(&pwrdm->lock, flags);
+	ret = _pwrdm_read_next_fpwrst(pwrdm);
+	spin_unlock_irqrestore(&pwrdm->lock, flags);
+
+	return ret;
 }
 
 /**
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index 45c449d..23b9da9 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -17,6 +17,7 @@
 #include <linux/types.h>
 #include <linux/list.h>
 
+#include <linux/spinlock.h>
 #include <linux/atomic.h>
 
 #include <plat/cpu.h>
@@ -109,6 +110,7 @@ struct powerdomain;
  * @pwrdm_clkdms: Clockdomains in this powerdomain
  * @node: list_head linking all powerdomains
  * @voltdm_node: list_head linking all powerdomains in a voltagedomain
+ * @lock: powerdomain next state registers protection lock
  * @pwrstctrl_offs: (AM33XX only) XXX_PWRSTCTRL reg offset from prcm_offs
  * @pwrstst_offs: (AM33XX only) XXX_PWRSTST reg offset from prcm_offs
  * @logicretstate_mask: (AM33XX only) mask for logic retention bitfield
@@ -142,6 +144,7 @@ struct powerdomain {
 	struct clockdomain *pwrdm_clkdms[PWRDM_MAX_CLKDMS];
 	struct list_head node;
 	struct list_head voltdm_node;
+	spinlock_t lock;
 	int state;
 	unsigned state_counter[PWRDM_MAX_PWRSTS];
 	unsigned ret_logic_off_counter;
-- 
1.7.7.6


WARNING: multiple messages have this Message-ID (diff)
From: jean.pihet@newoldbits.com (Jean Pihet)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/8] ARM: OMAP2+: PM: add a lock to protect the powerdomains next state
Date: Wed, 15 Aug 2012 12:02:43 +0200	[thread overview]
Message-ID: <1345024968-28951-4-git-send-email-j-pihet@ti.com> (raw)
In-Reply-To: <1345024968-28951-1-git-send-email-j-pihet@ti.com>

pwrdm_set_next_fpwrst and pwrdm_read_next_fpwrst are intented
to be the only API to program and request the next state of a power
domain.
This patch protects the power domain next state settings and structs
from concurrent accesses by the use of a lock.

A spinlock is used since pwrdm_set_next_fpwrst and
pwrdm_read_next_fpwrst can be called from atomic context
(.i.e) either from cpuidle path or suspend path.

[ambresh at ti.com: reported the atomic context issue and suggested
to replace the mutex with a spinlock]

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Reported-by: Ambresh K <ambresh@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |   36 +++++++++++++++++++++++++++++++-----
 arch/arm/mach-omap2/powerdomain.h |    3 +++
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 3a1f56c..a8f7a81 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -102,6 +102,7 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
 	INIT_LIST_HEAD(&pwrdm->voltdm_node);
 	voltdm_add_pwrdm(voltdm, pwrdm);
 
+	spin_lock_init(&pwrdm->lock);
 	list_add(&pwrdm->node, &pwrdm_list);
 
 	/* Initialize the powerdomain's state counter */
@@ -601,6 +602,22 @@ int pwrdm_pwrst_to_fpwrst(struct powerdomain *pwrdm, u8 pwrst, u8 logic)
 }
 
 /**
+ * _pwrdm_read_next_fpwrst - get next powerdomain functional power state
+ * @pwrdm: struct powerdomain * to get power state
+ *
+ * Return the powerdomain @pwrdm's next functional power state.
+ * Returns -EINVAL if the powerdomain pointer is null or returns
+ * the next power state upon success.
+ */
+int _pwrdm_read_next_fpwrst(struct powerdomain *pwrdm)
+{
+	int next_pwrst = pwrdm_read_next_pwrst(pwrdm);
+	int next_logic = pwrdm_read_logic_retst(pwrdm);
+
+	return pwrdm_pwrst_to_fpwrst(pwrdm, next_pwrst, next_logic);
+}
+
+/**
  * pwrdm_get_achievable_fpwrst() - Provide achievable functional state
  * @pwrdm: struct powerdomain * to set
  * @fpwrst: minimum functional state we would like to hit
@@ -670,6 +687,7 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 	int sleep_switch = -1, ret = 0, hwsup = 0;
 	int new_fpwrst, next_fpwrst, pwrst, logic;
 	u8 curr_pwrst;
+	unsigned long flags;
 
 	if (!pwrdm || IS_ERR(pwrdm)) {
 		pr_debug("%s: invalid params: pwrdm=%p\n", __func__, pwrdm);
@@ -678,13 +696,15 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 
 	pr_debug("%s(%s, fpwrst=%0x)\n", __func__, pwrdm->name, fpwrst);
 
+	spin_lock_irqsave(&pwrdm->lock, flags);
+
 	new_fpwrst = pwrdm_get_achievable_fpwrst(pwrdm, fpwrst);
 	pwrst = pwrdm_fpwrst_to_pwrst(pwrdm, new_fpwrst);
 	logic = pwrdm_fpwrst_to_logic_pwrst(pwrdm, new_fpwrst);
 
-	next_fpwrst = pwrdm_read_next_fpwrst(pwrdm);
+	next_fpwrst = _pwrdm_read_next_fpwrst(pwrdm);
 	if (new_fpwrst == next_fpwrst)
-		return ret;
+		goto out;
 
 	curr_pwrst = pwrdm_read_pwrst(pwrdm);
 	if (curr_pwrst < PWRDM_POWER_ON) {
@@ -732,6 +752,8 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 		break;
 	}
 
+out:
+	spin_unlock_irqrestore(&pwrdm->lock, flags);
 	return ret;
 }
 
@@ -801,10 +823,14 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
  */
 int pwrdm_read_next_fpwrst(struct powerdomain *pwrdm)
 {
-	int next_pwrst = pwrdm_read_next_pwrst(pwrdm);
-	int next_logic = pwrdm_read_logic_retst(pwrdm);
+	int ret;
+	unsigned long flags;
 
-	return pwrdm_pwrst_to_fpwrst(pwrdm, next_pwrst, next_logic);
+	spin_lock_irqsave(&pwrdm->lock, flags);
+	ret = _pwrdm_read_next_fpwrst(pwrdm);
+	spin_unlock_irqrestore(&pwrdm->lock, flags);
+
+	return ret;
 }
 
 /**
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index 45c449d..23b9da9 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -17,6 +17,7 @@
 #include <linux/types.h>
 #include <linux/list.h>
 
+#include <linux/spinlock.h>
 #include <linux/atomic.h>
 
 #include <plat/cpu.h>
@@ -109,6 +110,7 @@ struct powerdomain;
  * @pwrdm_clkdms: Clockdomains in this powerdomain
  * @node: list_head linking all powerdomains
  * @voltdm_node: list_head linking all powerdomains in a voltagedomain
+ * @lock: powerdomain next state registers protection lock
  * @pwrstctrl_offs: (AM33XX only) XXX_PWRSTCTRL reg offset from prcm_offs
  * @pwrstst_offs: (AM33XX only) XXX_PWRSTST reg offset from prcm_offs
  * @logicretstate_mask: (AM33XX only) mask for logic retention bitfield
@@ -142,6 +144,7 @@ struct powerdomain {
 	struct clockdomain *pwrdm_clkdms[PWRDM_MAX_CLKDMS];
 	struct list_head node;
 	struct list_head voltdm_node;
+	spinlock_t lock;
 	int state;
 	unsigned state_counter[PWRDM_MAX_PWRSTS];
 	unsigned ret_logic_off_counter;
-- 
1.7.7.6

  parent reply	other threads:[~2012-08-15 10:03 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-15 10:02 [PATCH v5 0/8] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
2012-08-15 10:02 ` Jean Pihet
2012-08-15 10:02 ` [PATCH 1/8] ARM: OMAP2+: PM: introduce " Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 2/8] ARM: OMAP2+: PM: introduce power domains achievable " Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` Jean Pihet [this message]
2012-08-15 10:02   ` [PATCH 3/8] ARM: OMAP2+: PM: add a lock to protect the powerdomains next state Jean Pihet
2012-08-15 10:02 ` [PATCH 4/8] ARM: OMAP2+: PM: use the functional power states API Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 5/8] ARM: OMAP2+: PM: use power domain functional state in stats counters Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 6/8] ARM: OMAP2+: PM debug: trace the functional power domains states Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 7/8] ARM: OMAP2+: powerdomain: add error logs Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 8/8] ARM: OMAP2+: PM: reorganize the powerdomain API in public and private parts Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 17:05 ` [PATCH v5 0/8] ARM: OMAP2+: PM: introduce the power domains functional states Shilimkar, Santosh
2012-08-15 17:05   ` Shilimkar, Santosh
2012-08-16  0:48   ` Paul Walmsley
2012-08-16  0:48     ` Paul Walmsley
2012-08-16  5:50     ` Shilimkar, Santosh
2012-08-16  5:50       ` Shilimkar, Santosh
2012-09-10 15:09       ` Tero Kristo
2012-09-10 15:09         ` Tero Kristo
2012-09-11  7:50         ` Pihet-XID, Jean
2012-09-11  7:50           ` Pihet-XID, Jean

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=1345024968-28951-4-git-send-email-j-pihet@ti.com \
    --to=jean.pihet@newoldbits.com \
    --cc=j-pihet@ti.com \
    --cc=khilman@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=paul@pwsan.com \
    --cc=rnayak@ti.com \
    --cc=santosh.shilimkar@ti.com \
    /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.