On 27 July 2018 at 11:26, J, KEERTHY <j-keerthy@xxxxxx> wrote: > > > 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!? Now we have 3 ways to compensate the suspend time, non-stop clocksource ----> persistent clock ----> RTC device. So you have no non-stop clocksource in your system, you can use RTC instead like Grygorii suggested. -- Baolin Wang Best Regards -- 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
