From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752683AbeDQIVw (ORCPT ); Tue, 17 Apr 2018 04:21:52 -0400 Received: from lelnx193.ext.ti.com ([198.47.27.77]:53427 "EHLO lelnx193.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751248AbeDQITh (ORCPT ); Tue, 17 Apr 2018 04:19:37 -0400 Subject: Re: [PATCH v2 1/3] Input: ti_am335x_tsc - Mark IRQ as wakeup capable To: Dmitry Torokhov CC: "Strashko, Grygorii" , "linux-input@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-omap@vger.kernel.org" , Tony Lindgren References: <20180414095153.32060-1-vigneshr@ti.com> <20180414095153.32060-2-vigneshr@ti.com> <20180416174542.GB77055@dtor-ws> From: Vignesh R Message-ID: <92f270e1-e72b-9ccd-ce9f-d11120786941@ti.com> Date: Tue, 17 Apr 2018 13:50:18 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180416174542.GB77055@dtor-ws> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Monday 16 April 2018 11:15 PM, Dmitry Torokhov wrote: > On Sat, Apr 14, 2018 at 03:21:51PM +0530, Vignesh R wrote: >> On AM335x, ti_am335x_tsc can wake up the system from suspend, mark the >> IRQ as wakeup capable, so that device irq is not disabled during system >> suspend. >> >> Signed-off-by: Vignesh R >> --- >> >> v2: No changes >> >>  drivers/input/touchscreen/ti_am335x_tsc.c | 9 +++++++++ >>  1 file changed, 9 insertions(+) >> >> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c >> index f1043ae71dcc..810e05c9c4f5 100644 >> --- a/drivers/input/touchscreen/ti_am335x_tsc.c >> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c >> @@ -27,6 +27,7 @@ >>  #include >>  #include >>  #include >> +#include >>  >>  #include >>  >> @@ -432,6 +433,12 @@ static int titsc_probe(struct platform_device *pdev) >>                goto err_free_mem; >>        } >>  >> +     if (device_may_wakeup(tscadc_dev->dev)) { >> +             err = dev_pm_set_wake_irq(tscadc_dev->dev, ts_dev->irq); > > Hmm, most of the drivers simply use enable_irq_wake()/disable_irq_wake() > in suspend/resume paths dev_pm_*_wake_irq() function are alternative to above [1]: For most drivers, we should be able to drop the following boilerplate code from runtime_suspend and runtime_resume functions: ... device_init_wakeup(dev, true); ... if (device_may_wakeup(dev)) enable_irq_wake(irq); ... if (device_may_wakeup(dev)) disable_irq_wake(irq); ... device_init_wakeup(dev, false); ... We can replace it with just the following init and exit time code: ... device_init_wakeup(dev, true); dev_pm_set_wake_irq(dev, irq); ... dev_pm_clear_wake_irq(dev); device_init_wakeup(dev, false); [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/base/power/wakeirq.c?id=4990d4fe327b9d9a7a3be7103a82699406fdde69 I see device_may_wakeup() check is not required. Will drop that in next version. > and use dev_pm_set_wake_irq() only for dedicated > and distinct wake interrupts. Why do we not follow the same pattern > here? Thats dev_pm_*_dedicated_wake_irq() Thanks for the review! -- Regards Vignesh