[PATCHv5 02/10] ARM: OMAP3+: voltage: add support for voltagedomain usecounts

2012-09-25 Thread Tero Kristo
These are updated based on powerdomain usecounts. Also added support
for voltdm-sleep and voltdm-wakeup calls that will be invoked once
voltagedomain enters sleep or wakes up based on usecount numbers. These
will be used for controlling voltage scaling functionality.

Signed-off-by: Tero Kristo t-kri...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Kevin Hilman khil...@ti.com
---
 arch/arm/mach-omap2/powerdomain.c |   17 +-
 arch/arm/mach-omap2/voltage.c |   62 +
 arch/arm/mach-omap2/voltage.h |   13 
 3 files changed, 91 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c 
b/arch/arm/mach-omap2/powerdomain.c
index ba49029..ca54aec 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -1475,10 +1477,16 @@ int pwrdm_state_switch(struct powerdomain *pwrdm)
  */
 void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
 {
+   unsigned long flags;
+
if (!pwrdm)
return;
 
-   atomic_inc(pwrdm-usecount);
+   if (atomic_inc_return(pwrdm-usecount) == 1) {
+   spin_lock_irqsave(pwrdm-lock, flags);
+   voltdm_pwrdm_enable(pwrdm-voltdm.ptr);
+   spin_unlock_irqrestore(pwrdm-lock, flags);
+   }
 }
 
 /**
@@ -1492,12 +1500,19 @@ void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
 void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
 {
int val;
+   unsigned long flags;
 
if (!pwrdm)
return;
 
val = atomic_dec_return(pwrdm-usecount);
 
+   if (!val) {
+   spin_lock_irqsave(pwrdm-lock, flags);
+   voltdm_pwrdm_disable(pwrdm-voltdm.ptr);
+   spin_unlock_irqrestore(pwrdm-lock, flags);
+   }
+
WARN_ON(val  0);
 }
 
diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index 4dc60e8..9eb1a4b 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -340,6 +340,67 @@ int voltdm_add_pwrdm(struct voltagedomain *voltdm, struct 
powerdomain *pwrdm)
 }
 
 /**
+ * voltdm_pwrdm_enable - increase usecount for a voltagedomain
+ * @voltdm: struct voltagedomain * to increase count for
+ *
+ * Increases usecount for a given voltagedomain. If the usecount reaches
+ * 1, the domain is awakened from idle and the function will call the
+ * voltagedomain-wakeup callback for this domain.
+ */
+void voltdm_pwrdm_enable(struct voltagedomain *voltdm)
+{
+   unsigned long flags;
+
+   if (!voltdm)
+   return;
+
+   if (atomic_inc_return(voltdm-usecount) == 1  voltdm-wakeup) {
+   spin_lock_irqsave(voltdm-lock, flags);
+   voltdm-wakeup(voltdm);
+   spin_unlock_irqrestore(voltdm-lock, flags);
+   }
+}
+
+/**
+ * voltdm_pwrdm_disable - decrease usecount for a voltagedomain
+ * @voltdm: struct voltagedomain * to decrease count for
+ *
+ * Decreases the usecount for a given voltagedomain. If the usecount
+ * reaches zero, the domain can idle and the function will call the
+ * voltagedomain-sleep callback, and calculate the overall target
+ * state for the voltagedomain.
+ */
+void voltdm_pwrdm_disable(struct voltagedomain *voltdm)
+{
+   u8 target_state = PWRDM_POWER_OFF;
+   int state;
+   struct powerdomain *pwrdm;
+   int val;
+   unsigned long flags;
+
+   if (!voltdm)
+   return;
+
+   val = atomic_dec_return(voltdm-usecount);
+
+   WARN_ON(val  0);
+
+   if (val == 0) {
+   spin_lock_irqsave(voltdm-lock, flags);
+   /* Determine target state for voltdm */
+   list_for_each_entry(pwrdm, voltdm-pwrdm_list, voltdm_node) {
+   state = pwrdm_read_next_pwrst(pwrdm);
+   if (state  target_state)
+   target_state = state;
+   }
+   voltdm-target_state = target_state;
+   if (voltdm-sleep)
+   voltdm-sleep(voltdm);
+   spin_unlock_irqrestore(voltdm-lock, flags);
+   }
+}
+
+/**
  * voltdm_for_each_pwrdm - call function for each pwrdm in a voltdm
  * @voltdm: struct voltagedomain * to iterate over
  * @fn: callback function *
@@ -402,6 +463,7 @@ static int _voltdm_register(struct voltagedomain *voltdm)
INIT_LIST_HEAD(voltdm-pwrdm_list);
list_add(voltdm-node, voltdm_list);
 
+   spin_lock_init(voltdm-lock);
pr_debug(voltagedomain: registered %s\n, voltdm-name);
 
return 0;
diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h
index 0ac2caf..7f4f99d 100644
--- a/arch/arm/mach-omap2/voltage.h
+++ b/arch/arm/mach-omap2/voltage.h
@@ -15,6 +15,7 @@
 #define __ARCH_ARM_MACH_OMAP2_VOLTAGE_H
 
 #include linux/err.h
+#include linux/spinlock.h
 
 #include plat/voltage.h
 
@@ -56,10 +57,14 @@ struct omap_vfsm_instance {
  * @pwrdm_list: list_head linking all powerdomains in this voltagedomain
  * @vc: 

Re: [PATCHv5 02/10] ARM: OMAP3+: voltage: add support for voltagedomain usecounts

2012-09-25 Thread Russell King - ARM Linux
On Tue, Sep 25, 2012 at 12:32:37PM +0300, Tero Kristo wrote:
 diff --git a/arch/arm/mach-omap2/powerdomain.c 
 b/arch/arm/mach-omap2/powerdomain.c
 index ba49029..ca54aec 100644
 --- a/arch/arm/mach-omap2/powerdomain.c
 +++ b/arch/arm/mach-omap2/powerdomain.c
 @@ -1475,10 +1477,16 @@ int pwrdm_state_switch(struct powerdomain *pwrdm)
   */
  void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
  {
 + unsigned long flags;
 +
   if (!pwrdm)
   return;
  
 - atomic_inc(pwrdm-usecount);
 + if (atomic_inc_return(pwrdm-usecount) == 1) {
 + spin_lock_irqsave(pwrdm-lock, flags);
 + voltdm_pwrdm_enable(pwrdm-voltdm.ptr);
 + spin_unlock_irqrestore(pwrdm-lock, flags);
 + }

This looks like the classic I like atomic types because they have magic
properties brain-deadness.

What would happen to users of this if you had this sequence:

pwrdm-usecount starts off as 1.

Thread0 Thread1
atomic_inc_return() (returns 1)
atomic_inc_return() (returns 2)
starts using stuff in power domain
spin_lock_irqsave()
voltdm_pwrdm_enable()
spin_unlock_irqrestore()

?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv5 02/10] ARM: OMAP3+: voltage: add support for voltagedomain usecounts

2012-09-25 Thread Tero Kristo
On Tue, 2012-09-25 at 10:41 +0100, Russell King - ARM Linux wrote:
 On Tue, Sep 25, 2012 at 12:32:37PM +0300, Tero Kristo wrote:
  diff --git a/arch/arm/mach-omap2/powerdomain.c 
  b/arch/arm/mach-omap2/powerdomain.c
  index ba49029..ca54aec 100644
  --- a/arch/arm/mach-omap2/powerdomain.c
  +++ b/arch/arm/mach-omap2/powerdomain.c
  @@ -1475,10 +1477,16 @@ int pwrdm_state_switch(struct powerdomain *pwrdm)
*/
   void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
   {
  +   unsigned long flags;
  +
  if (!pwrdm)
  return;
   
  -   atomic_inc(pwrdm-usecount);
  +   if (atomic_inc_return(pwrdm-usecount) == 1) {
  +   spin_lock_irqsave(pwrdm-lock, flags);
  +   voltdm_pwrdm_enable(pwrdm-voltdm.ptr);
  +   spin_unlock_irqrestore(pwrdm-lock, flags);
  +   }
 
 This looks like the classic I like atomic types because they have magic
 properties brain-deadness.

Hi Russell,

Thats a good catch, I was actually thinking about this sequence myself
also, but decided to leave it as is here due to similarity with the
existing code in mach-omap2/clockdomain.c, see e.g.
_clkdm_clk_hwmod_enable. Maybe those parts should be fixed also...?

 
 What would happen to users of this if you had this sequence:
 
 pwrdm-usecount starts off as 1.
 
 Thread0   Thread1
 atomic_inc_return() (returns 1)
   atomic_inc_return() (returns 2)
   starts using stuff in power domain
 spin_lock_irqsave()
 voltdm_pwrdm_enable()
 spin_unlock_irqrestore()
 
 ?

That as such wouldn't break anything, as the callback isn't doing
anything too critical, but yes, for the sequencing of events it is bad.
The alternate implementation I was thinking was to drop the atomic_t and
just use an int for the usecount, and protect the usecount also with the
spinlock. However, there might be some performance issues if this is
done (but I think it is actually better than having some rather
mysterious bugs instead.)

-Tero


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html