[PATCH 1/4] AM3517 : support for suspend/resume

[Kevin Hilman]

Hi Abhilash,

"Koyamangalath, Abhilash" <abhilash.kv at ti.com> writes:

> Hi
>
> On Wed, Aug 31, 2011 at 4:28 AM, Hilman, Kevin wrote:
>>
>> Abhilash K V <abhilash.kv at ti.com> writes:
>>
>>> 1. Patch to disable dynamic sleep (as it is not supported
>>>    on AM35xx).
>>> 2. Imported the unique suspend/resume sequence for AM3517,
>>>    contained in the new file arch/arm/mach-omap2/sleep3517.S.
>>> 3. Added omap3517_ to symbol-names in sleep3517.S which are common
>>>    with sleep34xx.S, and added appropriate checks.
>>>
>>> There are still 3 caveats:
>>>
>>> 1. If "no_console_suspend" is enabled (via boot-args), the device
>>>    doesnot resume but simply hangs.
>>> 2. Every second and subsequent attempt to suspend/resume prints this slow-path
>>>    WARNING (for both uart1 and uart2), while resuming :
>>>    [   70.943939] omap_hwmod: uart1: idle state can only be entered from
>>>    enabled state
>>> 3. Wakeup using the TSC2004 touch-screen controller is not supported.
>>>
>>> Signed-off-by: Ranjith Lohithakshan <ranjithl at ti.com>
>>> Reviewed-by: Vaibhav Hiremath <hvaibhav at ti.com>
>>> Signed-off-by: Abhilash K V <abhilash.kv at ti.com>
>>
>> In addition to Russell's comments about using the latest code from
>> mainline, I have some comments below.
> [Abhilash K V] I have reworked the patch against the tip (as suggested by
> Russell).
> And I've incorporated all of Kevin's comments too.

Great, thanks!

> There is one "known" issue left which needs to be closed before I can submit v2 of this patch.
> With no_console_suspend, suspend to RAM hangs right now on AM3517, after
> the message:
>           Disabling non-boot CPUs ...
> There is no error message or dump.
> I found that this crash is happening in a call to pr_warning(), from _omap_device_deactivate().
> The same code does not produce this issue on omap34xx due to this snippet from omap_sram_idle() :
>         /* PER */
>         if (per_next_state < PWRDM_POWER_ON) {
>                 per_going_off = (per_next_state == PWRDM_POWER_OFF) ? 1 : 0;
>                 omap_uart_prepare_idle(2);
>                 omap_uart_prepare_idle(3);
>                 omap2_gpio_prepare_for_idle(per_going_off);
>                 if (per_next_state == PWRDM_POWER_OFF)
>                                 omap3_per_save_context();
>         }
>         /* CORE */
>         if (core_next_state < PWRDM_POWER_ON) {
>                 omap_uart_prepare_idle(0);
>                 omap_uart_prepare_idle(1);
>                 if (core_next_state == PWRDM_POWER_OFF) {
>                         omap3_core_save_context();
>                         omap3_cm_save_context();
>                 }
>         }
> This happens in preparation to the suspend operation (I,e. the WFI).
> As seen here, on 34xx the sequence in which the uarts are disabled is 2, 3, 0 and 1.The console-uart, which is uart-1 here (starting from uart-0) is disabled last.

> For AM3517 EVM, the console-uart is uart-2 and this ought to be
> disabled at the last to prevent this crash from occurring.

There are several other OMAP3 platforms (n900, Beagle, etc.) where the
UART console is also UART2, so console ordering is not the problem.

The fact that that pr_warning is making it to the console suggests that
the console is not locked.  In the idle path, we take the console lock
(using console_trylock(), just above the code you showed above.)

But during suspend, there was an assumption (by me[2]) that the console
would always be locked in the suspend path.  During no_console_suspend,
it appears that is not the case.

Can you try the patch below[1] to see if that fixes your problem?  I
think it should.

Kevin


[1]