From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755066AbeASJWP (ORCPT ); Fri, 19 Jan 2018 04:22:15 -0500 Received: from foss.arm.com ([217.140.101.70]:36182 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750841AbeASJWK (ORCPT ); Fri, 19 Jan 2018 04:22:10 -0500 Subject: Re: [PATCH 4/8] irqchip/gic-v3: add ability to save/restore GIC/ITS state To: Brian Norris Cc: Derek Basehore , linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, rafael.j.wysocki@intel.com, tglx@linutronix.de, Sudeep Holla References: <20180112212422.148625-1-dbasehore@chromium.org> <20180112212422.148625-5-dbasehore@chromium.org> <86fu79fx3n.wl-marc.zyngier@arm.com> <20180118233243.GA209323@google.com> From: Marc Zyngier Organization: ARM Ltd Message-ID: Date: Fri, 19 Jan 2018 09:22:06 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180118233243.GA209323@google.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 18/01/18 23:33, Brian Norris wrote: > Hi, > > On Sat, Jan 13, 2018 at 06:10:52PM +0000, Marc Zyngier wrote: >> On Fri, 12 Jan 2018 21:24:18 +0000, >> Derek Basehore wrote: >>> >>> Some platforms power off GIC logic in S3, so we need to save/restore >> >> S3 is a not a GIC concept, and is only vaguely mentioned in terms of >> the rk3399 silicon, if grep serves me right. Please expand on what >> state this is exactly. >> >>> state. This adds a DT-binding to save/restore the GICD/GICR/GITS >>> states using the new CPU_PM_SYSTEM_ENTER/EXIT CPU PM states. >> >> DT binding? I can't see any in this patch. >> >>> >>> Change-Id: I1fb2117296373fa67397fdd4a8960077b241462e >> >> It's been mentioned somewhere else in the thread: these tags have no >> purpose in the kernel. Please sanitise your patches before posting them. >> >>> Signed-off-by: Derek Basehore >>> Signed-off-by: Brian Norris >> >> Who is the author of this patch? If that's a joined authorship, please >> use the Co-Developed-by: tag. > > I only did some minimal code shuffling when rebasing and working with > this code in our downstream tree. I probably didn't actually need to > apply my Signed-off-by at the time, but Derek carried it along anyway. > > Derek is the author, and I'd be perfectly fine dropping my S-o-b from > these patches. > >>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c >>> index 9a7a15049903..95d37fb6f458 100644 >>> --- a/drivers/irqchip/irq-gic-v3.c >>> +++ b/drivers/irqchip/irq-gic-v3.c [...] >>> + if (IS_ERR(gicr_ctx)) { >>> + err = PTR_ERR(gicr_ctx); >>> + goto out_free_gicd_ctx; >>> + } >>> + } >> >> You really want to kill the box because something went wrong in your >> save area allocation? It doesn't feel quite right. > > Isn't that what all drivers (including irqchip drivers) do on failed > allocations? What else would we do? Pretend that we can limp along and > just b0rk the system when it suspends? It would certainly give the user a chance to diagnostic the problem (which is otherwise pretty hard if the system doesn't boot). We kill the system if we cannot continue. In this case, we can. So why not try it? Thanks, M. -- Jazz is not dead. It just smells funny...