Skip to contentSkip to conversation

ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value

PATCH New James Morse 6 participants 7 messages Apr 26, 2016 – May 4, 2016 CPU idle time management framework

lore
Share & tools
lore

James Morse

author
#0 lore raw

PATCH 1/1 ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value

arm_cpuidle_suspend() may return -EOPNOTSUPP, or any value returned by the cpu_ops/cpuidle_ops suspend call. arm_enter_idle_state() doesn't update 'ret' with this value, meaning we always signal success to cpuidle_enter_state(), causing it to update the usage counters as if we succeeded.

Fixes 191de17aa3c1 ("ARM64: cpuidle: Replace cpu_suspend by the common ARM/ARM64 function")
Signed-off-by James Morse <[email protected]>
---
 drivers/cpuidle/cpuidle-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpuidle/cpuidle-arm.c +1 -1
index 545069d5fdfb..e342565e8715 100644
@@ -50,7 +50,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
 		 * call the CPU ops suspend protocol with idle index as a
 		 * parameter.
 		 */
-		arm_cpuidle_suspend(idx);
+		ret = arm_cpuidle_suspend(idx);
 
 		cpu_pm_exit();
 	}
Signature · 1 line
2.8.0.rc3
#1 lore raw

PATCH 1/1 ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value

Quoted 11 lines · James Morse

arm_cpuidle_suspend() may return -EOPNOTSUPP, or any value returned by the cpu_ops/cpuidle_ops suspend call. arm_enter_idle_state() doesn't update 'ret' with this value, meaning we always signal success to cpuidle_enter_state(), causing it to update the usage counters as if we succeeded.

Fixes 191de17aa3c1 ("ARM64: cpuidle: Replace cpu_suspend by the common ARM/ARM64 function")
Signed-off-by James Morse <[email protected]>
---
 drivers/cpuidle/cpuidle-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by Lorenzo Pieralisi <[email protected]>
Quoted 17 lines
drivers/cpuidle/cpuidle-arm.c +1 -1
index 545069d5fdfb..e342565e8715 100644
@@ -50,7 +50,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
 		 * call the CPU ops suspend protocol with idle index as a
 		 * parameter.
 		 */
-		arm_cpuidle_suspend(idx);
+		ret = arm_cpuidle_suspend(idx);
 
 		cpu_pm_exit();
 	}
Signature · 1 line
2.8.0.rc3
maintainer
#2 lore raw

PATCH 1/1 ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value

Quoted 27 lines · Lorenzo Pieralisi
Quoted 11 lines · James Morse

arm_cpuidle_suspend() may return -EOPNOTSUPP, or any value returned by the cpu_ops/cpuidle_ops suspend call. arm_enter_idle_state() doesn't update 'ret' with this value, meaning we always signal success to cpuidle_enter_state(), causing it to update the usage counters as if we succeeded.

Fixes 191de17aa3c1 ("ARM64: cpuidle: Replace cpu_suspend by the common ARM/ARM64 function")
Signed-off-by James Morse <[email protected]>
---
 drivers/cpuidle/cpuidle-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by Lorenzo Pieralisi <[email protected]>
Quoted 12 lines
drivers/cpuidle/cpuidle-arm.c +1 -1
index 545069d5fdfb..e342565e8715 100644
@@ -50,7 +50,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
               * call the CPU ops suspend protocol with idle index as a
               * parameter.
               */
-             arm_cpuidle_suspend(idx);
+             ret = arm_cpuidle_suspend(idx);
Quoted 3 lines

drivers/cpuidle/cpuidle-arm.c static int arm_enter_idle_state(struct cpuidle_device *dev,

Quoted 3 lines
drivers/cpuidle/cpuidle-arm.c +0 -1
              cpu_pm_exit();
      }
--

OK

Should that go into -stable?

#3 lore raw

PATCH 1/1 ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value

[+ Lina]

Quoted 29 lines · Rafael J. Wysocki
Quoted 27 lines · Lorenzo Pieralisi
Quoted 11 lines · James Morse

arm_cpuidle_suspend() may return -EOPNOTSUPP, or any value returned by the cpu_ops/cpuidle_ops suspend call. arm_enter_idle_state() doesn't update 'ret' with this value, meaning we always signal success to cpuidle_enter_state(), causing it to update the usage counters as if we succeeded.

Fixes 191de17aa3c1 ("ARM64: cpuidle: Replace cpu_suspend by the common ARM/ARM64 function")
Signed-off-by James Morse <[email protected]>
---
 drivers/cpuidle/cpuidle-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by Lorenzo Pieralisi <[email protected]>
Quoted 12 lines
drivers/cpuidle/cpuidle-arm.c +1 -1
index 545069d5fdfb..e342565e8715 100644
@@ -50,7 +50,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
               * call the CPU ops suspend protocol with idle index as a
               * parameter.
               */
-             arm_cpuidle_suspend(idx);
+             ret = arm_cpuidle_suspend(idx);
Quoted 7 lines
Quoted 3 lines

drivers/cpuidle/cpuidle-arm.c static int arm_enter_idle_state(struct cpuidle_device *dev,

Quoted 3 lines
drivers/cpuidle/cpuidle-arm.c +0 -1
              cpu_pm_exit();
      }
--

OK

Should that go into -stable?

Yes for arm64, but I would ask Daniel and Lina please to chime in if they see any issue with this change (in particular in relation to the Qualcomm back-end), it looks like it just slipped through the cracks but let's check with them before merging it and send it to stable kernels to prevent any issue.

Thanks !
Lorenzo

#4 lore raw

PATCH 1/1 ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value

Quoted 38 lines · Lorenzo Pieralisi

[+ Lina]

Quoted 35 lines · Rafael J. Wysocki
Quoted 27 lines · Lorenzo Pieralisi
Quoted 11 lines · James Morse

arm_cpuidle_suspend() may return -EOPNOTSUPP, or any value returned by the cpu_ops/cpuidle_ops suspend call. arm_enter_idle_state() doesn't update 'ret' with this value, meaning we always signal success to cpuidle_enter_state(), causing it to update the usage counters as if we succeeded.

Fixes 191de17aa3c1 ("ARM64: cpuidle: Replace cpu_suspend by the common ARM/ARM64 function")
Signed-off-by James Morse <[email protected]>
---
 drivers/cpuidle/cpuidle-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by Lorenzo Pieralisi <[email protected]>
Quoted 12 lines
drivers/cpuidle/cpuidle-arm.c +1 -1
index 545069d5fdfb..e342565e8715 100644
@@ -50,7 +50,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
               * call the CPU ops suspend protocol with idle index as a
               * parameter.
               */
-             arm_cpuidle_suspend(idx);
+             ret = arm_cpuidle_suspend(idx);
Quoted 3 lines

drivers/cpuidle/cpuidle-arm.c static int arm_enter_idle_state(struct cpuidle_device *dev,

Quoted 3 lines
drivers/cpuidle/cpuidle-arm.c +0 -1
              cpu_pm_exit();
      }
--

OK

Quoted 8 lines
Quoted 1 line

Should that go into -stable?

Yes for arm64, but I would ask Daniel and Lina please to chime in if they see any issue with this change (in particular in relation to the Qualcomm back-end), it looks like it just slipped through the cracks but let's check with them before merging it and send it to stable kernels to prevent any issue.

This patch looks okay to me. In the QCOM backend driver, wee return -1 if the state was not achieved because of a pending interrupt. This was previously ignored, but a quick check doesnt yield an issue returning that backend specific value. Its not a real error per-se and therefore a -1.

Thanks,
Lina

#5 lore raw

PATCH 1/1 ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value

In reply to Lorenzo Pieralisi
Quoted 19 lines · Lorenzo Pieralisi

[+ Lina]

Quoted 16 lines · Rafael J. Wysocki
Quoted 14 lines · Lorenzo Pieralisi
Quoted 11 lines · James Morse

arm_cpuidle_suspend() may return -EOPNOTSUPP, or any value returned by the cpu_ops/cpuidle_ops suspend call. arm_enter_idle_state() doesn't update 'ret' with this value, meaning we always signal success to cpuidle_enter_state(), causing it to update the usage counters as if we succeeded.

Fixes 191de17aa3c1 ("ARM64: cpuidle: Replace cpu_suspend by the common ARM/ARM64 function")
Signed-off-by James Morse <[email protected]>
---
 drivers/cpuidle/cpuidle-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by Lorenzo Pieralisi <[email protected]>
Acked-by Daniel Lezcano <[email protected]>
#6 lore raw

PATCH 1/1 ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value

Quoted 19 lines · Daniel Lezcano
Quoted 18 lines · Lorenzo Pieralisi

[+ Lina]

Quoted 15 lines · Rafael J. Wysocki
Quoted 13 lines · Lorenzo Pieralisi
Quoted 11 lines · James Morse

arm_cpuidle_suspend() may return -EOPNOTSUPP, or any value returned by the cpu_ops/cpuidle_ops suspend call. arm_enter_idle_state() doesn't update 'ret' with this value, meaning we always signal success to cpuidle_enter_state(), causing it to update the usage counters as if we succeeded.

Fixes 191de17aa3c1 ("ARM64: cpuidle: Replace cpu_suspend by the common ARM/ARM64 function")
Signed-off-by James Morse <[email protected]>
---
 drivers/cpuidle/cpuidle-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 3 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Acked-by Lorenzo Pieralisi <[email protected]>
Acked-by Daniel Lezcano <[email protected]>

OK, applied, tagged for -stable (4.1+).

Thanks everybody!