| Message ID | 20180207014117.62611-2-dbasehore@chromium.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 07/02/18 01:41, Derek Basehore wrote: > If cpu_cluster_pm_enter() fails, cpu_pm_exit() should be called. This > will put the CPU in the correct state to resume from the failure. > > Signed-off-by: Derek Basehore <dbasehore@chromium.org> > --- > kernel/cpu_pm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/cpu_pm.c b/kernel/cpu_pm.c > index 67b02e138a47..03bcc0751a51 100644 > --- a/kernel/cpu_pm.c > +++ b/kernel/cpu_pm.c > @@ -186,6 +186,9 @@ static int cpu_pm_suspend(void) > return ret; > > ret = cpu_cluster_pm_enter(); > + if (ret) > + cpu_pm_exit(); > + > return ret; > } > > It is unclear to me why we need this patch as part of the ITS series. I probably fixes something for you, but I don't see the connection with the other patches. Thanks, M.
Hi Marc, On Wed, Feb 07, 2018 at 08:57:27AM +0000, Marc Zyngier wrote: > On 07/02/18 01:41, Derek Basehore wrote: > > If cpu_cluster_pm_enter() fails, cpu_pm_exit() should be called. This > > will put the CPU in the correct state to resume from the failure. > > > > Signed-off-by: Derek Basehore <dbasehore@chromium.org> > > --- > > kernel/cpu_pm.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/kernel/cpu_pm.c b/kernel/cpu_pm.c > > index 67b02e138a47..03bcc0751a51 100644 > > --- a/kernel/cpu_pm.c > > +++ b/kernel/cpu_pm.c > > @@ -186,6 +186,9 @@ static int cpu_pm_suspend(void) > > return ret; > > > > ret = cpu_cluster_pm_enter(); > > + if (ret) > > + cpu_pm_exit(); > > + > > return ret; > > } > > > > > > It is unclear to me why we need this patch as part of the ITS series. I > probably fixes something for you, but I don't see the connection with > the other patches. Ths bug was noticed (by inspection) along with earlier versions of this series, when Derek was still adding new cpu_pm callbacks, and new failure modes within the existing callbacks. It's a proper fix to my knowledge, but I believe it no longer has any particular relevance to this series, since we're not really touching cpu_pm in this series any more. Brian
On Wed, 07 Feb 2018 22:01:56 +0000, Brian Norris wrote: Hi Brian, > Hi Marc, > > On Wed, Feb 07, 2018 at 08:57:27AM +0000, Marc Zyngier wrote: > > On 07/02/18 01:41, Derek Basehore wrote: > > > If cpu_cluster_pm_enter() fails, cpu_pm_exit() should be called. This > > > will put the CPU in the correct state to resume from the failure. > > > > > > Signed-off-by: Derek Basehore <dbasehore@chromium.org> > > > --- > > > kernel/cpu_pm.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/kernel/cpu_pm.c b/kernel/cpu_pm.c > > > index 67b02e138a47..03bcc0751a51 100644 > > > --- a/kernel/cpu_pm.c > > > +++ b/kernel/cpu_pm.c > > > @@ -186,6 +186,9 @@ static int cpu_pm_suspend(void) > > > return ret; > > > > > > ret = cpu_cluster_pm_enter(); > > > + if (ret) > > > + cpu_pm_exit(); > > > + > > > return ret; > > > } > > > > > > > > > > It is unclear to me why we need this patch as part of the ITS series. I > > probably fixes something for you, but I don't see the connection with > > the other patches. > > Ths bug was noticed (by inspection) along with earlier versions of this > series, when Derek was still adding new cpu_pm callbacks, and new > failure modes within the existing callbacks. It's a proper fix to my > knowledge, but I believe it no longer has any particular relevance to > this series, since we're not really touching cpu_pm in this series any > more. I don't doubt that this is a proper fix, but it has a better chance of being noticed on its own, rather than buried together with a now unrelated series. I'd suggest that when you or Derek respin this series, you don't include this patch, but instead post it on its own for review. Thanks, M.
diff --git a/kernel/cpu_pm.c b/kernel/cpu_pm.c index 67b02e138a47..03bcc0751a51 100644 --- a/kernel/cpu_pm.c +++ b/kernel/cpu_pm.c @@ -186,6 +186,9 @@ static int cpu_pm_suspend(void) return ret; ret = cpu_cluster_pm_enter(); + if (ret) + cpu_pm_exit(); + return ret; }
If cpu_cluster_pm_enter() fails, cpu_pm_exit() should be called. This will put the CPU in the correct state to resume from the failure. Signed-off-by: Derek Basehore <dbasehore@chromium.org> --- kernel/cpu_pm.c | 3 +++ 1 file changed, 3 insertions(+)