* [PATCH] ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value
@ 2016-04-26 11:15 James Morse
2016-04-26 11:31 ` Lorenzo Pieralisi
0 siblings, 1 reply; 7+ messages in thread
From: James Morse @ 2016-04-26 11:15 UTC (permalink / raw)
To: linux-arm-kernel
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 <james.morse@arm.com>
---
drivers/cpuidle/cpuidle-arm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
index 545069d5fdfb..e342565e8715 100644
--- a/drivers/cpuidle/cpuidle-arm.c
+++ b/drivers/cpuidle/cpuidle-arm.c
@@ -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();
}
--
2.8.0.rc3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH] ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value 2016-04-26 11:15 [PATCH] ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value James Morse @ 2016-04-26 11:31 ` Lorenzo Pieralisi 2016-04-26 19:05 ` Rafael J. Wysocki 0 siblings, 1 reply; 7+ messages in thread From: Lorenzo Pieralisi @ 2016-04-26 11:31 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 26, 2016 at 12:15:01PM +0100, James Morse wrote: > 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 <james.morse@arm.com> > --- > drivers/cpuidle/cpuidle-arm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c > index 545069d5fdfb..e342565e8715 100644 > --- a/drivers/cpuidle/cpuidle-arm.c > +++ b/drivers/cpuidle/cpuidle-arm.c > @@ -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(); > } > -- > 2.8.0.rc3 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value 2016-04-26 11:31 ` Lorenzo Pieralisi @ 2016-04-26 19:05 ` Rafael J. Wysocki 2016-04-27 9:14 ` Lorenzo Pieralisi 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2016-04-26 19:05 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 26, 2016 at 1:31 PM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Tue, Apr 26, 2016 at 12:15:01PM +0100, James Morse wrote: >> 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 <james.morse@arm.com> >> --- >> drivers/cpuidle/cpuidle-arm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > >> >> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c >> index 545069d5fdfb..e342565e8715 100644 >> --- a/drivers/cpuidle/cpuidle-arm.c >> +++ b/drivers/cpuidle/cpuidle-arm.c >> @@ -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(); >> } >> -- OK Should that go into -stable? ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value 2016-04-26 19:05 ` Rafael J. Wysocki @ 2016-04-27 9:14 ` Lorenzo Pieralisi 2016-04-27 15:04 ` Lina Iyer 2016-04-28 8:20 ` Daniel Lezcano 0 siblings, 2 replies; 7+ messages in thread From: Lorenzo Pieralisi @ 2016-04-27 9:14 UTC (permalink / raw) To: linux-arm-kernel [+ Lina] On Tue, Apr 26, 2016 at 09:05:57PM +0200, Rafael J. Wysocki wrote: > On Tue, Apr 26, 2016 at 1:31 PM, Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: > > On Tue, Apr 26, 2016 at 12:15:01PM +0100, James Morse wrote: > >> 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 <james.morse@arm.com> > >> --- > >> drivers/cpuidle/cpuidle-arm.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > >> > >> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c > >> index 545069d5fdfb..e342565e8715 100644 > >> --- a/drivers/cpuidle/cpuidle-arm.c > >> +++ b/drivers/cpuidle/cpuidle-arm.c > >> @@ -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(); > >> } > >> -- > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value 2016-04-27 9:14 ` Lorenzo Pieralisi @ 2016-04-27 15:04 ` Lina Iyer 2016-04-28 8:20 ` Daniel Lezcano 1 sibling, 0 replies; 7+ messages in thread From: Lina Iyer @ 2016-04-27 15:04 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 27 2016 at 03:14 -0600, Lorenzo Pieralisi wrote: >[+ Lina] > >On Tue, Apr 26, 2016 at 09:05:57PM +0200, Rafael J. Wysocki wrote: >> On Tue, Apr 26, 2016 at 1:31 PM, Lorenzo Pieralisi >> <lorenzo.pieralisi@arm.com> wrote: >> > On Tue, Apr 26, 2016 at 12:15:01PM +0100, James Morse wrote: >> >> 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 <james.morse@arm.com> >> >> --- >> >> drivers/cpuidle/cpuidle-arm.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >> > >> >> >> >> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c >> >> index 545069d5fdfb..e342565e8715 100644 >> >> --- a/drivers/cpuidle/cpuidle-arm.c >> >> +++ b/drivers/cpuidle/cpuidle-arm.c >> >> @@ -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(); >> >> } >> >> -- >> >> 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. > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value 2016-04-27 9:14 ` Lorenzo Pieralisi 2016-04-27 15:04 ` Lina Iyer @ 2016-04-28 8:20 ` Daniel Lezcano 2016-05-04 21:20 ` Rafael J. Wysocki 1 sibling, 1 reply; 7+ messages in thread From: Daniel Lezcano @ 2016-04-28 8:20 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 27, 2016 at 10:14:34AM +0100, Lorenzo Pieralisi wrote: > [+ Lina] > > On Tue, Apr 26, 2016 at 09:05:57PM +0200, Rafael J. Wysocki wrote: > > On Tue, Apr 26, 2016 at 1:31 PM, Lorenzo Pieralisi > > <lorenzo.pieralisi@arm.com> wrote: > > > On Tue, Apr 26, 2016 at 12:15:01PM +0100, James Morse wrote: > > >> 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 <james.morse@arm.com> > > >> --- > > >> drivers/cpuidle/cpuidle-arm.c | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value 2016-04-28 8:20 ` Daniel Lezcano @ 2016-05-04 21:20 ` Rafael J. Wysocki 0 siblings, 0 replies; 7+ messages in thread From: Rafael J. Wysocki @ 2016-05-04 21:20 UTC (permalink / raw) To: linux-arm-kernel On Thursday, April 28, 2016 10:20:23 AM Daniel Lezcano wrote: > On Wed, Apr 27, 2016 at 10:14:34AM +0100, Lorenzo Pieralisi wrote: > > [+ Lina] > > > > On Tue, Apr 26, 2016 at 09:05:57PM +0200, Rafael J. Wysocki wrote: > > > On Tue, Apr 26, 2016 at 1:31 PM, Lorenzo Pieralisi > > > <lorenzo.pieralisi@arm.com> wrote: > > > > On Tue, Apr 26, 2016 at 12:15:01PM +0100, James Morse wrote: > > > >> 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 <james.morse@arm.com> > > > >> --- > > > >> drivers/cpuidle/cpuidle-arm.c | 2 +- > > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org> OK, applied, tagged for -stable (4.1+). Thanks everybody! ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-05-04 21:20 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-26 11:15 [PATCH] ARM: cpuidle: Pass on arm_cpuidle_suspend()'s return value James Morse 2016-04-26 11:31 ` Lorenzo Pieralisi 2016-04-26 19:05 ` Rafael J. Wysocki 2016-04-27 9:14 ` Lorenzo Pieralisi 2016-04-27 15:04 ` Lina Iyer 2016-04-28 8:20 ` Daniel Lezcano 2016-05-04 21:20 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).