"Basak, Partha" <p-bas...@ti.com> writes:
[...]
>> > In the idle path (interrupt disabled context), PM runtime APIs cannot
>> > be used as they are not mutex-free functions. Hence omap_device APIs
>> > are used in the idle and resume after idle path.
>>
>> This needs much more fleshing out.
>>
>> Exactly what mutexes are causing the problems here. As pointed out in
>> previous discussions, the ones in the PM runtime core should not be a
>> problem in this path. Therefore, I'll assume the problems are coming
>> from the mutexes when the device code (mach-omap2/gpio.c) calls into the
>> hwmod layer. More on this in comments on the next patch.
>>
>
> Sorry, this has not been documented correctly. The issue has more to
> do unconditional enabling of interrupts. We have received a patch from
> you on using pm_runtime functions in Idle path. We will try on GPIO
> and revert back.
OK
>
>> > To summarize,
>> > 1. pm_runtime_get_sync() for any gpio bank is called when one of the
>> gpios
>> > is requested on the bank, in which, no other gpio is being used (when
>> > mod_usage becomes non-zero)
>> > 2. omap_device_enable() is called during gpio resume after idle, only
>> > if the particular bank is being used (if mod_usage is non-zero)
>>
>> context is saved/restored in the idle path, but...
>>
>> > 3. pm_runtime_put_sync() is called when the last used gpio in that
>> > gpio bank is freed (when mod_usage becomes zero)
>>
>> in this path, the bank is now runtime suspended, but context has not
>> been saved for it. That should be fine, since this bank is no longer
>> used, but now let's assume there was an off-mode transition and context
>> is lost. Then, gpio_request() is called which will trigger
>> a pm_runtime_get_sync() and gpio_bank_runtime_resume() will be called.
>>
>> In this case, it's not terribly clear that runtime_resume is doing sane
>> things if context has just been lost. Seems like runtime_resume should
>> be a nop in this case since any re-init will be be done in gpio_request().
>
> Runtime_suspend/resume for GPIO is not doing any save/restore
> context. In that sense, they are NOP. Context save/restore is taken
> care of only in the Idle path based on target power state and last
> power state respectively.
OK, I didn't explain the problem I'm suspecting very well. Imagine this
sequence of events:
- mod_usage becomes zero
- pm_runtime_put_sync()
- gpio_bank_runtime_suspend() [ no context is saved ]
[ off-mode transition, context is lost]
- gpio_request()
- pm_runtime_get_sync()
- gpio_bank_runtime_resume()
In this path, no context is saved, and no context is restored, which is
what I would expect, since there's no need to save context if nobody is
using that gpio bank anymore. However, gpio_bank_runtime_resume() is
doing lots of reads/writes and read-modify-writes on GPIO bank registers
that may have undefined contents after a context loss.
The point is that the GPIO register twiddling in
gpio_bank_runtime_resume() does not seem to be needed if there are no
users of that GPIO bank.
[...]
>> > static void omap3_enable_io_chain(void)
>> > {
>> > int timeout = 0;
>> > @@ -395,15 +385,17 @@ void omap_sram_idle(void)
>> > /* PER */
>> > if (per_next_state < PWRDM_POWER_ON) {
>> > omap_uart_prepare_idle(2);
>> > - omap2_gpio_prepare_for_idle(per_next_state);
>> > if (per_next_state == PWRDM_POWER_OFF) {
>> > if (core_next_state == PWRDM_POWER_ON) {
>> > per_next_state = PWRDM_POWER_RET;
>> > pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
>> > per_state_modified = 1;
>> > - } else
>> > - omap3_per_save_context();
>> > + }
>> > }
>> > + if (per_next_state == PWRDM_POWER_OFF)
>> > + omap2_gpio_prepare_for_idle(true);
>> > + else
>> > + omap2_gpio_prepare_for_idle(false);
>>
>> Why is this better than passing the next power state?
>
> This would keep the GPIO function omap2_gpio_prepare_for_idle agnostic of
> Power state definition dependencies.
>
And why is this better?
Personally, I think the GPIO code should be told about the powerdomain
state so it can make it's own decision about whether or not to save
context.
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
