Skip to contentSkip to conversation

ARM: OMAP2+: Fix Wake-up power domain power status

PATCH RESEND Superseded Jon Hunter 4 participants 6 messages Jun 29, 2012 – Jul 4, 2012

Subsystems (3)
Arm (32-bit) Arm sub-architectures OMAP2+ support
lore
Share & tools
lore

Jon Hunter

author
#0 lore raw

PATCH RESEND 1/1 ARM: OMAP2+: Fix Wake-up power domain power status

Note: Re-sending with updated kernel doc.

The wake-up power domain is an alway-on power domain and so this power domain does not have a power state status (PM_PWSTST_xxx) register that indicates the current state. However, during the registering of the wake-up power domain the state of the domain is queried by calling pwrdm_read_pwrst(). This actually tries to read a register that does not exist and returns a value of 0 that indicates that the current state is OFF. The OFF state count of the wake-up power domain is then set to 1 and the current state to OFF. Both of which are incorrect.

To fix this, if a power domain only supports the ON state, do not attempt to read the power state status register and simply return ON as the current power state.

This is based upon Tony's current linux-omap master branch.

Testing:
- Boot tested on OMAP4460 panda.
- Boot tested on OMAP3430 beagle and validated CORE RET still working (using Paul's 32k timer patch [1]).

[1] http://marc.info/?l=linux-omap&m=134000053229888&w=2

Signed-off-by Jon Hunter <[email protected]>
---
 arch/arm/mach-omap2/powerdomain.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/mach-omap2/powerdomain.c +5 -1
index eefe179..69b36e1 100644
@@ -526,7 +526,8 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
  *
  * Return the powerdomain @pwrdm's current power state.	Returns -EINVAL
  * if the powerdomain pointer is null or returns the current power state
- * upon success.
+ * upon success. Note that if the power domain only supports the ON state
+ * then just return ON as the current state.
  */
 int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 {

@@ -535,6 +536,9 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 	if (!pwrdm)
 		return -EINVAL;
 
+	if (pwrdm->pwrsts == PWRSTS_ON)
+		return PWRDM_POWER_ON;
+
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_pwrst)
 		ret = arch_pwrdm->pwrdm_read_pwrst(pwrdm);
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH RESEND 1/1 ARM: OMAP2+: Fix Wake-up power domain power status

+ Paul, Rajendra,

Quoted 44 lines · Jon Hunter

Note: Re-sending with updated kernel doc.

The wake-up power domain is an alway-on power domain and so this power domain does not have a power state status (PM_PWSTST_xxx) register that indicates the current state. However, during the registering of the wake-up power domain the state of the domain is queried by calling pwrdm_read_pwrst(). This actually tries to read a register that does not exist and returns a value of 0 that indicates that the current state is OFF. The OFF state count of the wake-up power domain is then set to 1 and the current state to OFF. Both of which are incorrect.

To fix this, if a power domain only supports the ON state, do not attempt to read the power state status register and simply return ON as the current power state.

This is based upon Tony's current linux-omap master branch.

Testing:
- Boot tested on OMAP4460 panda.
- Boot tested on OMAP3430 beagle and validated CORE RET still working (using ?Paul's 32k timer patch [1]).

[1] http://marc.info/?l=linux-omap&m=134000053229888&w=2

Signed-off-by Jon Hunter <[email protected]>

---
?arch/arm/mach-omap2/powerdomain.c | ? ?6 +++++-
?1 file changed, 5 insertions(+), 1 deletion(-)

arch/arm/mach-omap2/powerdomain.c +0 -0
index eefe179..69b36e1 100644
@@ -526,7 +526,8 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)

?*
?* Return the powerdomain @pwrdm's current power state. ? ? ? ?Returns -EINVAL ?* if the powerdomain pointer is null or returns the current power state
- * upon success.
+ * upon success. Note that if the power domain only supports the ON state
+ * then just return ON as the current state.
?*/
?int pwrdm_read_pwrst(struct powerdomain *pwrdm)
?{

arch/arm/mach-omap2/powerdomain.c int pwrdm_read_pwrst(struct powerdomain *pwrdm) · line 541

Quoted 6 lines
arch/arm/mach-omap2/powerdomain.c +3 -0
@@ -535,6 +536,9 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 ? ? ? ?if (!pwrdm)
 ? ? ? ? ? ? ? ?return -EINVAL;
 
+ ? ? ? if (pwrdm->pwrsts == PWRSTS_ON)
+ ? ? ? ? ? ? ? return PWRDM_POWER_ON;
+

The patch as such is correct but just wondering whether we should have some flag rather than above check.

Regards
Santosh

author
#2 lore raw

PATCH RESEND 1/1 ARM: OMAP2+: Fix Wake-up power domain power status

Quoted 50 lines · Santosh

+ Paul, Rajendra,

Quoted 47 lines · Jon Hunter

Note: Re-sending with updated kernel doc.

The wake-up power domain is an alway-on power domain and so this power domain does not have a power state status (PM_PWSTST_xxx) register that indicates the current state. However, during the registering of the wake-up power domain the state of the domain is queried by calling pwrdm_read_pwrst(). This actually tries to read a register that does not exist and returns a value of 0 that indicates that the current state is OFF. The OFF state count of the wake-up power domain is then set to 1 and the current state to OFF. Both of which are incorrect.

To fix this, if a power domain only supports the ON state, do not attempt to read the power state status register and simply return ON as the current power state.

This is based upon Tony's current linux-omap master branch.

Testing:
- Boot tested on OMAP4460 panda.
- Boot tested on OMAP3430 beagle and validated CORE RET still working (using Paul's 32k timer patch [1]).

[1] http://marc.info/?l=linux-omap&m=134000053229888&w=2

Signed-off-by Jon Hunter <[email protected]>
---
 arch/arm/mach-omap2/powerdomain.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/mach-omap2/powerdomain.c +2 -1
index eefe179..69b36e1 100644
@@ -526,7 +526,8 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 *
 * Return the powerdomain @pwrdm's current power state.        Returns -EINVAL
 * if the powerdomain pointer is null or returns the current power state
- * upon success.
+ * upon success. Note that if the power domain only supports the ON state
+ * then just return ON as the current state.
 */
 int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 {

@@ -535,6 +536,9 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
       if (!pwrdm)
               return -EINVAL;
Quoted 5 lines

arch/arm/mach-omap2/powerdomain.c int pwrdm_read_pwrst(struct powerdomain *pwrdm) · line 541

Quoted 3 lines
arch/arm/mach-omap2/powerdomain.c +3 -0
+       if (pwrdm->pwrsts == PWRSTS_ON)
+               return PWRDM_POWER_ON;
+

The patch as such is correct but just wondering whether we should have some flag rather than above check.

I was wondering that too. I opted not to add a flag because there is only one such power domain that needs it. However, I can add a flag if it is preferred.

Cheers
Jon

#3 lore raw

PATCH RESEND 1/1 ARM: OMAP2+: Fix Wake-up power domain power status

Quoted 52 lines · Jon Hunter
Quoted 50 lines · Santosh

+ Paul, Rajendra,

Quoted 47 lines · Jon Hunter

Note: Re-sending with updated kernel doc.

The wake-up power domain is an alway-on power domain and so this power domain does not have a power state status (PM_PWSTST_xxx) register that indicates the current state. However, during the registering of the wake-up power domain the state of the domain is queried by calling pwrdm_read_pwrst(). This actually tries to read a register that does not exist and returns a value of 0 that indicates that the current state is OFF. The OFF state count of the wake-up power domain is then set to 1 and the current state to OFF. Both of which are incorrect.

To fix this, if a power domain only supports the ON state, do not attempt to read the power state status register and simply return ON as the current power state.

This is based upon Tony's current linux-omap master branch.

Testing:
- Boot tested on OMAP4460 panda.
- Boot tested on OMAP3430 beagle and validated CORE RET still working (using Paul's 32k timer patch [1]).

[1] http://marc.info/?l=linux-omap&m=134000053229888&w=2

Signed-off-by Jon Hunter <[email protected]>
---
 arch/arm/mach-omap2/powerdomain.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/mach-omap2/powerdomain.c +2 -1
index eefe179..69b36e1 100644
@@ -526,7 +526,8 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 *
 * Return the powerdomain @pwrdm's current power state.        Returns -EINVAL
 * if the powerdomain pointer is null or returns the current power state
- * upon success.
+ * upon success. Note that if the power domain only supports the ON state
+ * then just return ON as the current state.
 */
 int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 {

@@ -535,6 +536,9 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
       if (!pwrdm)
               return -EINVAL;
Quoted 9 lines
Quoted 5 lines

arch/arm/mach-omap2/powerdomain.c int pwrdm_read_pwrst(struct powerdomain *pwrdm) · line 541

Quoted 3 lines
arch/arm/mach-omap2/powerdomain.c +3 -0
+       if (pwrdm->pwrsts == PWRSTS_ON)
+               return PWRDM_POWER_ON;
+

The patch as such is correct but just wondering whether we should have some flag rather than above check.

I was wondering that too. I opted not to add a flag because there is only one such power domain that needs it.

I agree with you.

Acked-by Santosh Shilimkar <[email protected]>
#4 lore raw

PATCH RESEND 1/1 ARM: OMAP2+: Fix Wake-up power domain power status

Thanks everyone, queued.

- Paul

#5 lore raw

PATCH RESEND 1/1 ARM: OMAP2+: Fix Wake-up power domain power status

In reply to Jon Hunter
Quoted 17 lines · Jon Hunter

Note: Re-sending with updated kernel doc.

The wake-up power domain is an alway-on power domain and so this power domain does not have a power state status (PM_PWSTST_xxx) register that indicates the current state. However, during the registering of the wake-up power domain the state of the domain is queried by calling pwrdm_read_pwrst(). This actually tries to read a register that does not exist and returns a value of 0 that indicates that the current state is OFF. The OFF state count of the wake-up power domain is then set to 1 and the current state to OFF. Both of which are incorrect.

To fix this, if a power domain only supports the ON state, do not attempt to read the power state status register and simply return ON as the current power state.

This is based upon Tony's current linux-omap master branch.

Quoted 8 lines

Testing:
- Boot tested on OMAP4460 panda.
- Boot tested on OMAP3430 beagle and validated CORE RET still working (using Paul's 32k timer patch [1]).

[1] http://marc.info/?l=linux-omap&m=134000053229888&w=2

Signed-off-by Jon Hunter <[email protected]>
Acked-by Kevin Hilman <[email protected]>
Quoted 27 lines
---
 arch/arm/mach-omap2/powerdomain.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/mach-omap2/powerdomain.c +5 -1
index eefe179..69b36e1 100644
@@ -526,7 +526,8 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
  *
  * Return the powerdomain @pwrdm's current power state.	Returns -EINVAL
  * if the powerdomain pointer is null or returns the current power state
- * upon success.
+ * upon success. Note that if the power domain only supports the ON state
+ * then just return ON as the current state.
  */
 int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 {

@@ -535,6 +536,9 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 	if (!pwrdm)
 		return -EINVAL;
 
+	if (pwrdm->pwrsts == PWRSTS_ON)
+		return PWRDM_POWER_ON;
+
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_pwrst)
 		ret = arch_pwrdm->pwrdm_read_pwrst(pwrdm);