From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751190AbeBGWKp (ORCPT ); Wed, 7 Feb 2018 17:10:45 -0500 Received: from foss.arm.com ([217.140.101.70]:56806 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750745AbeBGWKn (ORCPT ); Wed, 7 Feb 2018 17:10:43 -0500 Date: Wed, 07 Feb 2018 22:10:33 +0000 Message-ID: <86bmh0a1t2.wl-marc.zyngier@arm.com> From: Marc Zyngier To: Brian Norris Cc: Derek Basehore , linux-kernel@vger.kernel.org, Soby.Mathew@arm.com, sudeep.holla@arm.com, devicetree@vger.kernel.org, robh+dt@kernel.org, mark.rutland@arm.com, linux-pm@vger.kernel.org, rafael.j.wysocki@intel.com, tglx@linutronix.de Subject: Re: [PATCH v5 1/4] cpu_pm: add syscore_suspend error handling In-Reply-To: <20180207220155.GA106856@ban.mtv.corp.google.com> References: <20180207014117.62611-1-dbasehore@chromium.org> <20180207014117.62611-2-dbasehore@chromium.org> <04ebd1d9-b45b-df87-d59e-1a3df97f3371@arm.com> <20180207220155.GA106856@ban.mtv.corp.google.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL/10.8 EasyPG/1.0.0 Emacs/25.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Organization: ARM Ltd MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > > --- > > > 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. -- Jazz is not dead, it just smell funny.