[PATCH] gpio/omap: fix off-mode bug: clear debounce clock enable mask on disable

[Grazvydas Ignotas]

On Wed, Oct 24, 2012 at 3:49 PM, Santosh Shilimkar
<santosh.shilimkar at ti.com> wrote:
> On Wednesday 24 October 2012 05:32 PM, Grazvydas Ignotas wrote:
>>
>> On Tue, Oct 23, 2012 at 9:09 PM, Kevin Hilman
>> <khilman at deeprootsystems.com> wrote:
>>>
>>> From: Kevin Hilman <khilman at ti.com>
>>>
>>> When debounce clocks are disabled, ensure that the banks
>>> dbck_enable_mask is cleared also.  Otherwise, context restore on
>>> subsequent off-mode transition will restore previous value from the
>>> shadow copies (bank->context.debounce*) leading to mismatch state
>>> between driver state and hardware state.
>>
>>
>> This doesn't look right to me, aren't you effectively disabling
>> debounce forever here? _gpio_dbck_disable is called from
>> omap_gpio_runtime_suspend() and nothing will ever restore
>> dbck_enable_mask back to what it was set by _set_gpio_debounce and
>> debounce functionality is lost.
>>
> As per commit log, the issue seen with request->debounce->free()
> sequence and hence on free, debounce state needs to be cleared.
> Next gpio_set_debounce() should set the debounce mask right so
> the patch seems to be right.
>
>
>>>
>>> This was discovered when board code was doing
>>>
>>>    gpio_request_one()
>>>    gpio_set_debounce()
>>>    gpio_free()
>>>
>>> which was leaving the GPIO debounce settings in a confused state.
>>> Then, enabling off mode causing bogus state to be restored, leaving
>>> GPIO debounce enabled which then prevented the CORE powerdomain from
>>> transitioning.
>>>
>
> But there is one more case which might break because of this patch.
> As part of idle, we might end up with below without gpio_free()
> omap2_gpio_prepare_for_idle()
>  ->pm_runtime_put_sync_suspend()
>   -> omap_gpio_runtime_suspend()
>    ->_gpio_dbck_disable()
>
> And last call will clear the debounce mask state which will lost and
> hence the debounce clock won't be enabled on resume.
>
> I let Kevin comment whether this is the valid case or not.

That's exactly what I had in mind - we lose debounce functionality on
first runtime suspend.

-- 
Gražvydas