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 <vigneshr@xxxxxx> >> --- >> >> 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 <linux/of.h> >> #include <linux/of_device.h> >> #include <linux/sort.h> >> +#include <linux/pm_wakeirq.h> >> >> #include <linux/mfd/ti_am335x_tscadc.h> >> >> @@ -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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html