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. Regards, Dave -- 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
