Re: [PATCH] gpio: omap-gpio: add support for pm_runtime autosuspend

[Santosh Shilimkar <santosh.shilimkar@ti.com>]

On Wednesday 31 October 2012 04:07 PM, Kevin Hilman wrote:
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>
> [...]
>
>> Just to summaries, there are 3 things we are talking here.
>
> Santosh, thanks for the summary.  You are right on.
>
>> 1. Delaying the idle with a timeout which $subject patch is
>> trying to do to reduce latency for interrupts. That itself
>> is reasonable.
>
> I agree, this is reasonable.   IMO, adding autosuspend should be done as
> an isolated patch and kept separate from any other cleanup or changes.
>
> Also, as Jon mentioned, I think this should be done with a default
> timeout of zero so that current behaviour is unchanged.  Tim can then
> set the timeout from userspace for his usecase and everyone is happy.
>
Default 0 sounds good to me as well.

>> 2. Removal of the bank "mod_usage" which is also clubbed
>> in $subject path. Ofcourse that break the current driver
>> for idle. So that change needs to be made with better thought
>> and in a separate patch. This is doable.
>
> I had this discussion with Charu/Tarun during the last round of cleanup
> because I didn't like this mod_usage either.  The alternative is to do a
> 'get' for every GPIO in the bank since runtime PM is doing the
> usecounting already.  As Santosh said, this is fine for suspend, but for
> idle it means calling 'put' for every enabled GPIO in the bank instead
> of just forcing things with a single 'put.'
>
Exactly. The oherhead becomes too much when if you need to call put like 
6 Banks * 32 GPIOS times.
Actually even though there are 32 GPIOs in a bank, from PM point of
view, it only needs to care about a bank level control. Because
clocks and register set is per bank. And we model a GPIO device
per bank as well.

>> 3. Removing omap_gpio_[prepare/resume]_for_idle() with soome thing
>> better. For this one though, so far I have not come across a good
>> solution. Ideas/Solution is welcome !!
>
> I agree that the prepare/resume idle hooks a are ugly and should be
> removed, but this needs quite a bit more thought.  In particular,
> the 'remove triggering' stuff that is done needs pretty close
> examination.
>
> I'm not saying it can't be done, but patches to do this should be
> separate, well described and well tested, especially with off-mode.
>
Couldn't agree more. Thanks for confirming the points.

Regards
santosh