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 :) -- 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
