Re: [PATCH] gpio_keys, twl4030-pwrbutton: stay awake for 1sec on resume

[Lukas Maerdian <lukas@goldelico.com>]

Hi all!

On 28.06.2014 22:04 UTC+0200, Pavel Machek wrote:
>> And the msec parameter is described as:
>>
>> @msec: Anticipated event processing time (in milliseconds).
>>
>> Isn't calling pm_wakeup_event() with a non-zero msec the standard
>> method to handle this situation? And it is used in other drivers. E.g. in
>> _mmc_detect_change() or hub_suspend().
> 
>  * Notify the PM core of a wakeup event whose source is @ws that will
>    take                    
>  * approximately @msec milliseconds to be processed by the kernel.  If
>    @ws is                 
>  * not active, activate it.  If @msec is nonzero, set up the @ws'
>    timer to                    
>  * execute pm_wakeup_timer_fn() in future.                                                    
> 
> Will take @msec milliseconds to be processed by the _kernel_. Yes, USB
> probing takes a lot of time in kernel. But you are using this
> parameter to wait for userspace...

Well, it's not exactly waiting for userspace: The kernel goes to
suspend, before even being fully resumed.

In any case, 0 msec (i.e. nothing) seems to be insufficient, even for
just the in kernel processing. And I think that's exactly the root cause
of this race condition between the device drivers and the autosleep
module. Of course this only materializes if CONFIG_PM_AUTOSLEEP and
CONFIG_PM_WAKELOCKS are enabled, which is rarely used up to now, I guess.

I think we either need some way of signaling that the kernel has fully
finished resuming, before autosleep sets the system to suspend state
again, or we need to set appropriate delays in the individual device
drivers, to give them enough time to process the resume event.

As the pm_wakeup_event() call is already in place, I assume, that
setting appropriate processing times for each individual driver was the
intended way to go...

I've CCed Neil Brown, who inserted the pm_wakeup_even() call with a
0msec argument in both of the discussed drivers, so maybe he can shed
some light in this discussion?

Best regards,
  Lukas