Re: [PATCH v2 3/3] Input: ti_am335x_tsc - Prevent system suspend when TSC is in use

[Vignesh R]

On Monday 16 April 2018 11:31 PM, Dmitry Torokhov wrote:
> On Sat, Apr 14, 2018 at 03:21:53PM +0530, Vignesh R wrote:
>> From: Grygorii Strashko <grygorii.strashko@xxxxxx>
>> 
>> Prevent system suspend while user has finger on touch screen,
>> because TSC is wakeup source and suspending device while in use will
>> result in failure to disable the module.
>> This patch uses pm_stay_awake() and pm_relax() APIs to prevent and
>> resume system suspend as required.
> 
> This looks like common behavior for all touchscreens and many other
> input devices, but other systems seem to cope without having to add
> pm_stay_awake() and pm_relax(). I wonder why your system requires it and
> whether we can generalize this somehow.
> 

Not sure if other touch drivers with IP level wakeup, have been tested
in the same way as above scenario. But, if user has finger on
touchscreen, its better not to enter suspend until user stops
interacting with wakeup source.
I see pm_stay_awake()/pm_relax() pair as the only way to stop system
suspend when wakeup source is active. Also, one other touchscreen
driver(zforce_ts.c) and gpio_keys.c use pm_stay_awake()/pm_relax() pair.


Regards
Vignesh
>> 
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@xxxxxx>
>> Signed-off-by: Vignesh R <vigneshr@xxxxxx>
>> ---
>> 
>> v2: No changes.
>> 
>>Â drivers/input/touchscreen/ti_am335x_tsc.c | 3 +++
>>Â 1 file changed, 3 insertions(+)
>> 
>> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
>> index dcd9db768169..43b22e071842 100644
>> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
>> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
>> @@ -275,6 +275,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
>>ÂÂÂÂÂÂÂ if (status & IRQENB_HW_PEN) {
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ts_dev->pen_down = true;
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ irqclr |= IRQENB_HW_PEN;
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ pm_stay_awake(ts_dev->mfd_tscadc->dev);
>>ÂÂÂÂÂÂÂ }
>>Â 
>>ÂÂÂÂÂÂÂ if (status & IRQENB_PENUP) {
>> @@ -284,6 +285,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ input_report_key(input_dev, BTN_TOUCH, 0);
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ input_report_abs(input_dev, ABS_PRESSURE, 0);
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ input_sync(input_dev);
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ pm_relax(ts_dev->mfd_tscadc->dev);
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ } else {
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ts_dev->pen_down = true;
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ }
>> @@ -524,6 +526,7 @@ static int __maybe_unused titsc_resume(struct device *dev)
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ titsc_writel(ts_dev, REG_IRQWAKEUP,
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ 0x00);
>>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ pm_relax(ts_dev->mfd_tscadc->dev);
>>ÂÂÂÂÂÂÂ }
>>ÂÂÂÂÂÂÂ titsc_step_config(ts_dev);
>>ÂÂÂÂÂÂÂ titsc_writel(ts_dev, REG_FIFO0THR,
>> -- 
>> 2.17.0
>> 
> 
> Thanks.
> 
> -- 
> Dmitry