Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932295AbbHYNy0 (ORCPT ); Tue, 25 Aug 2015 09:54:26 -0400 Received: from foss.arm.com ([217.140.101.70]:41192 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752358AbbHYNyZ (ORCPT ); Tue, 25 Aug 2015 09:54:25 -0400 Message-ID: <55DC738D.8000302@arm.com> Date: Tue, 25 Aug 2015 14:54:21 +0100 From: Sudeep Holla User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Shenwei Wang CC: Sudeep Holla , "shawn.guo@linaro.org" , "tglx@linutronix.de" , "jason@lakedaemon.net" , Huang Anson , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup sources References: <1440443055-7291-1-git-send-email-shenwei.wang@freescale.com> <55DC3452.3070205@arm.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3496 Lines: 88 On 25/08/15 14:38, Shenwei Wang wrote: > > >> -----Original Message----- >> From: Sudeep Holla [mailto:sudeep.holla@arm.com] >> Sent: 2015年8月25日 4:25 >> To: Wang Shenwei-B38339 >> Cc: shawn.guo@linaro.org; tglx@linutronix.de; jason@lakedaemon.net; Sudeep >> Holla; Huang Yongcai-B20788; linux-kernel@vger.kernel.org; >> linux-arm-kernel@lists.infradead.org >> Subject: Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup >> sources >> >> >> >> On 24/08/15 20:04, Shenwei Wang wrote: >>> IMX7D contains a new version of GPC IP block (GPCv2). It has two major >>> functions: power management and wakeup source management. >>> This patch adds a new irqchip driver to manage the interrupt wakeup >>> sources on IMX7D. >> >> Interesting, you mention that this IP block has mainly power management and it >> itself requires save/restore. Is it not in always on domain ? > > Yes, it is in always on domain. > Hmm, then why do you need to save and restore the mask ? >>> When the system is in WFI (wait for interrupt) mode, this GPC block >>> will be the first block on the platform to be activated and signaled. >>> Under normal wait mode during cpu idle, the system can be woke up by >>> +static void gpcv2_wakeup_source_restore(void) { >>> + struct gpcv2_irqchip_data *cd; >>> + void __iomem *reg; >>> + int i; >>> + >>> + cd = imx_gpcv2_instance; >>> + if (!cd) >>> + return; >>> + >>> + for (i = 0; i < IMR_NUM; i++) { >>> + reg = cd->gpc_base + cd->cpu2wakeup + i * 4; >>> + writel_relaxed(cd->saved_irq_mask[i], reg); >> >> Instead of saving all the non-wakeup sources, can't you use raw save/restore of >> these registers and mask all the non-wakeup sources by setting >> MASK_ON_SUSPEND ? > > I can't catch what you mean. Can you show me an example? > What I meant is instead of you tracking all the enabled irqs(both wakeup and non-wakeup) in saved_irq_mask, just set MASK_ON_SUSPEND flag where all the non-wakeup interrupts are masked on suspend and unmasked on resume by the irq core. You can then just save and restore the wakeup irq mask, but as I asked above do you have to do that as you are saying it's in always on domain ? >> Also your interrupt controller seems like has no special way to configure wakeups, >> you are just leaving them enabled. i.e. I see cpu2wakeup used for both >> {un,}masking and wakeup enable. So you can just use IRQCHIP_SKIP_SET_WAKE. >> Correct me if my understanding is wrong. > > In this driver, the Core0 is configured to be the default core to be woke up, not both. You can > configure it to Core1 by changing the cpu2wakeup value. I don't see any reason to > use IRQCHIP_SKIP_SET_WAKE flag. > I don't see this driver doing anything extra apart from keeping the wakeup irqs enabled. i.e. You use the same cpu*wake register to mask/unmask the interrupt as well as set the wakeup source. Since the wakeup interrupt will be enabled by the driver, you just need to mark it as wake-up source and nothing extra in the controller right ? If so, you need to set IRQCHIP_SKIP_SET_WAKE as you are just leaving that irq enabled and not doing any extra configuration to enable it as wakeup source. Please correct if that wrong, but from the code that's what I could infer. Regards, Sudeep -- 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/