From: khilman@ti•com (Kevin Hilman)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 1/4] AM3517 : support for suspend/resume
Date: Tue, 13 Sep 2011 11:24:05 -0700 [thread overview]
Message-ID: <8739g0b8sa.fsf@ti.com> (raw)
In-Reply-To: <FCCFB4CDC6E5564B9182F639FC356087037B3D8823@dbde02.ent.ti.com> (Abhilash Koyamangalath's message of "Tue, 13 Sep 2011 17:01:01 +0530")
Hi Abhilash,
"Koyamangalath, Abhilash" <abhilash.kv@ti•com> writes:
> Hi
>
> On Wed, Aug 31, 2011 at 4:28 AM, Hilman, Kevin wrote:
>>
>> Abhilash K V <abhilash.kv@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@ti•com>
>>> Reviewed-by: Vaibhav Hiremath <hvaibhav@ti•com>
>>> Signed-off-by: Abhilash K V <abhilash.kv@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]
next prev parent reply other threads:[~2011-09-13 18:24 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-19 11:55 [PATCH 0/4] AM3517: Adding support for suspend/resume Abhilash K V
2011-08-19 11:55 ` [PATCH 1/4] AM3517 : " Abhilash K V
2011-08-19 11:55 ` [PATCH 2/4] AM3517: Add armv7-a flag for sleepam3517.S Abhilash K V
2011-08-19 11:55 ` [PATCH 3/4] pm34xx: Warning FIX related to ambiguous else loop Abhilash K V
2011-08-19 11:55 ` [PATCH 4/4] AM3517: Fix suspend-resume sequence Abhilash K V
2011-08-19 19:53 ` Russell King - ARM Linux
2011-08-30 23:00 ` [PATCH 3/4] pm34xx: Warning FIX related to ambiguous else loop Kevin Hilman
2011-08-19 19:56 ` [PATCH 1/4] AM3517 : support for suspend/resume Russell King - ARM Linux
2011-08-30 22:58 ` Kevin Hilman
2011-09-13 11:31 ` Koyamangalath, Abhilash
2011-09-13 18:24 ` Kevin Hilman [this message]
2011-09-14 13:00 ` Koyamangalath, Abhilash
2011-09-15 17:09 ` Kevin Hilman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8739g0b8sa.fsf@ti.com \
--to=khilman@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox