From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751927AbbIORsP (ORCPT ); Tue, 15 Sep 2015 13:48:15 -0400 Received: from foss.arm.com ([217.140.101.70]:52090 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751634AbbIORsN (ORCPT ); Tue, 15 Sep 2015 13:48:13 -0400 Subject: Re: [PATCH v4] pinctrl: mediatek: Implement wake handler and suspend resume To: Dmitry Torokhov References: <1439541486-22203-1-git-send-email-maoguang.meng@mediatek.com> <55DB4609.5040904@arm.com> <1441535972.22230.5.camel@mhfsdcap03> <55EEAA24.6080706@arm.com> <55EF11E6.7030307@arm.com> <1442051454.5661.12.camel@mhfsdcap03> <55F6AC87.4040301@arm.com> Cc: Sudeep Holla , maoguang meng , Hongzhou Yang , Linus Walleij , "linux-kernel@vger.kernel.org" , Daniel Kurtz , "linux-gpio@vger.kernel.org" , "linux-mediatek@lists.infradead.org" , Matthias Brugger , =?UTF-8?B?WWluZ2pvZSBDaGVuICjpmbPoi7HmtLIp?= , "linux-arm-kernel@lists.infradead.org" From: Sudeep Holla Message-ID: <55F859D7.1000803@arm.com> Date: Tue, 15 Sep 2015 18:48:07 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 15/09/15 03:52, Dmitry Torokhov wrote: > On Mon, Sep 14, 2015 at 4:16 AM, Sudeep Holla wrote: [...] >> >> This is wrong assumption in the driver. enable_irq_wake doesn't >> implicitly enable the IRQ. So the disable_irq should be moved to else. >> And the resume patch also needs to be fixed accordingly, otherwise you >> may get unbalanced irq. But this should not be the reason for fixing the >> pinctrl suspend/resume. >> > > Elan driver does not want to enable servicing IRQs, it just wants to > configure them as wakeup sources. Hence the current elan_suspend() is > fine. When system wakes up and the device is resumed and the driver is > ready to service interrupts it will enable IRQ again. > Fair enough. But I am struggling to understand how this fits into existing IRQ infrastructure. Few controllers that don't have wakeup source configuration facility can set IRQCHIP_SKIP_SET_WAKE and just leave the interrupts enabled in suspend path to wake it up. So IMO, the above strategy might not work on such controllers. > IOW enable_irq_wake() and enable_irq() are 2 completely different > calls and it is perfectly fine to disable IRQ and then ebale it as a > wakeup source. I agree that they are entirely different APIs, I am not sure if we can support different interrupt controller with such strategy. Since the irq/pm core handle disabling device IRQs and section "System Wakeup Interrupts, enable_irq_wake() and disable_irq_wake()" in Documentation/power/suspend-and-interrupts.txt gives me different understanding, we can check with tglx on how to handle this. Regards, Sudeep