Re: Suspend broken on linux-next on am437x-gp-evm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/27/2018 8:39 AM, Baolin Wang wrote:
On 27 July 2018 at 11:04, Dave Gerlach <d-gerlach@xxxxxx> wrote:
Hi,
On 07/26/2018 09:50 PM, Baolin Wang wrote:
Hi Keerthy,

On 26 July 2018 at 20:51, J, KEERTHY <j-keerthy@xxxxxx> wrote:


On 7/26/2018 1:45 PM, Baolin Wang wrote:

Hi Keerthy,

On 26 July 2018 at 14:58, J, KEERTHY <j-keerthy@xxxxxx> wrote:



On 7/26/2018 8:04 AM, Baolin Wang wrote:


Hi,

On 26 July 2018 at 09:59, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:


Hi,

On 26 July 2018 at 06:01, Dave Gerlach <d-gerlach@xxxxxx> wrote:


Hi,
On 07/25/2018 08:10 AM, Keerthy wrote:


Hi Baolin,

commit 39232ed5a1793f67b11430c43ed8a9ed6e96c6eb
Author: Baolin Wang <baolin.wang@xxxxxxxxxx>
Date:   Tue Jul 17 15:55:16 2018 +0800

time: Introduce one suspend clocksource to compensate the suspend
time

This patch seems to have broken suspend to memory on AM437x-gp-evm on
linux-next. 4.18-rc6 works well linux-next has this broken on
am437x-gp-evm and i could bisect to the above patch.



Could you point out which clocksource (or clocksource driver) is used
on AM437x-gp-evm?
If you have one non-stop clocksource, maybe you have suspended it when
system suepends, but we should not.
I am not sure what is the problem with your AM437x-gp-evm board, but I
will help to look at your clocksource driver to find the reason.



I did a little investigation, you will use "arm_global_timer" as the
clocksource for timekeeping, right?
But "arm_global_timer" is not a non-stop clocksource, so that means
you will not compensate the suspend time from non-stop clocksource.
Or you have another non-stop clocksource on this board that I missed?
(Sorry, I am not familiar with AM437x-gp-evm)



Baolin,

For AM437X we are using gptimer1 as clocksource which is in always-on


OK, I found it in time-pistachio.c file.

domain. We do idle that during suspend and enable it back during resume.
I tried removing suspend/resume hooks but that did not help either.


I did not see the gptimer supplies suspend/resume interfaces, but it
supplies enable/disable interfaces. That means if the non-stop gptimer
is selected as the suspend clocksource (not the current clocksource
for timekeeping), it will be enabled when system suspends and disabled
when system resumes to save power. But if the non-stop gptimer is also
the current clocksource for timekeeping, it will not be disabled when
system resumes.

So in your system gptimer1 is selected as the clocksource for
timekeeping or not?


Baolin,

Dug a bit more on this. Seems like one is high frequency clocksource
gptimer1:

clocksource: timer1: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns:
79635851949 ns
OMAP clocksource: timer1 at 24000000 Hz

arch/arm/mach-omap2/timer.c

And another is a low frequency 32k:

clocksource: 32k_counter: mask: 0xffffffff max_cycles: 0xffffffff,
max_idle_ns: 58327039986419 ns
OMAP clocksource: 32k_counter at 32768 Hz

drivers/clocksource/timer-ti-32k.c

Boot log: https://pastebin.ubuntu.com/p/FyympH7kK5/

OK. So in your system, timer1 clocksource was selected as the current
clocksource for timekeeping, but timer1 clocksource is not one
non-stop clocksource, so it can be selected as the suspend clocksource
to compensate the suspend time. The 32k_counter clocksource is
non-stop but low resolution, so it will be selected as the suspend
clocksource to compensate the suspend time.

So I don't think my patch will effect the timer1 clocksource which is
not one non-stop clocksource, so could you try to disable the non-stop
32k_counter clocksource to see if the problem is solved or not? But
from my side, I still can not understand why my patch will effect the
32k_counter clocksource, we just use the non-stop clocksource to
compensate the suspend time, maybe could you check again if the
32k_counter clocksource is one __real__ always-on timer?


Yes, it seems 32k_counter being marked non stop is causing our issue, dropping
this flag for it fixes it as Grygorii suggested as well. I'd imagine that module
is powered off in the suspend path before we attempt to read some registers but
we would need to look a little closer to see exactly what's happening. As
Grygorii mentioned I don't think this is a timer we want to use to compensate
suspend time, I am unclear of why it was marked this way in the first place.

OK. Seems you found the reason :)

I am wondering how time compensation is happening at the moment.
gptimer1 is being suspended and seems like 32_counter is also not persistent. How is the time compensation even happening!?


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux