Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752619AbbHEJ7Q (ORCPT ); Wed, 5 Aug 2015 05:59:16 -0400 Received: from foss.arm.com ([217.140.101.70]:33216 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752469AbbHEJ7N (ORCPT ); Wed, 5 Aug 2015 05:59:13 -0400 Message-ID: <55C1DE6E.6020405@arm.com> Date: Wed, 05 Aug 2015 10:59:10 +0100 From: Marc Zyngier Organization: ARM Ltd User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.7.0 MIME-Version: 1.0 To: Anson Huang , "rmk+kernel@arm.linux.org.uk" CC: Anson Huang , "linux-kernel@vger.kernel.org" , "tglx@linutronix.de" , "jason@lakedaemon.net" Subject: Re: [PATCH] irqchip/gic: restore global interrupts group settings in distributor References: <1438792778-7586-1-git-send-email-b20788@freescale.com> <55C1D383.2060202@arm.com> <20150805091751.GA9359@shlinux1.ap.freescale.net> In-Reply-To: <20150805091751.GA9359@shlinux1.ap.freescale.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3610 Lines: 93 On 05/08/15 10:17, Anson Huang wrote: > On Wed, Aug 05, 2015 at 10:12:35AM +0100, Marc Zyngier wrote: >> Hi Anson, >> >> On 05/08/15 17:39, Anson Huang wrote: >>> In GIC's distributor initializtion, all global interrupts >>> are set to group 1, however, after suspend/resume with >>> ARM/GIC power off/on, distributor does NOT restore >>> these global interrupts group setting, it will cause >>> system fail to resume. >>> >>> This patch adds global interrupts group setting restore >>> for distributor. >>> >>> Signed-off-by: Anson Huang >>> --- >>> drivers/irqchip/irq-gic.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c >>> index a530d9a..c8fa6ee 100644 >>> --- a/drivers/irqchip/irq-gic.c >>> +++ b/drivers/irqchip/irq-gic.c >>> @@ -532,6 +532,16 @@ static void gic_dist_restore(unsigned int gic_nr) >>> writel_relaxed(gic_data[gic_nr].saved_spi_enable[i], >>> dist_base + GIC_DIST_ENABLE_SET + i * 4); >>> >>> + writel_relaxed(GICD_ENABLE_GRP1, dist_base + GIC_DIST_CTRL); >>> + >>> + /* >>> + * Optionally set all global interrupts to be group 1. >>> + */ >>> + if (readl_relaxed(dist_base + GIC_DIST_CTRL) & GICD_ENABLE_GRP1) { >>> + for (i = 32; i < gic_irqs; i += 32) >>> + writel_relaxed(0xffffffff, dist_base + GIC_DIST_IGROUP + i * 4 / 32); >>> + } >>> + >>> writel_relaxed(GICD_ENABLE | GICD_ENABLE_GRP1, dist_base + GIC_DIST_CTRL); >>> } >>> >>> >> >> I'm afraid you'll have to explain a few more things here. >> >> For GICv1/v2, we exclusively use Group0 interrupts when booted in secure >> mode (i.e. we don't use FIQ yet, but RMK and Daniel Thompson have >> patches for that). When booted in non-secure, the group configuration is >> not accessible (it is secure only). >> >> So the first case is not applicable yet, and the second one is not >> possible. Which side are you on? >> >> Thanks, >> >> M. > > Hi, Marc > I may NOT know the history of enabling secure/non-secure of GIC very well, will spend > some time look into it later, during the upstream of our i.MX6UL's suepnd/resume, > I found patch(19bcd001 ARM: cobble together FIQ backtracing) break kernel resume function. After > looking into the gic driver, I saw the dist_inti set all global interrupts to GROUP 1(non-secure), but in > dist_restore, it does NOT restore those settings, after restoring them, suspend/resume works. Right, this is the code from Russell and Daniel I was talking about. As it says in the commit message: Cobble the FIQ backtracing together, some of this patch is based on some of Daniel Thompson's work. Experimental, and not for submission yet. So this patch doesn't apply to mainline. > Don't we need to make this GIC distributor settings after resume aligned with before suspend? > Before suspend, global interrupts are set to non-secure, but after resume, it is by default set to > secure mode, while cpu controller signal secure interrupts to FIQn, so that is incorrect, not sure > if my understanding is correct, please correct me if I am wrong. You're correct that something has to be done. But this should be done as part of Russell and Daniel FIQ series. Russell, are you willing to carry that patch (or something similar) as part of your series? Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/