From: Chander Kashyap <k.chander@samsung.com> To: Tomasz Figa <t.figa@samsung.com> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, Tomasz Figa <tomasz.figa@gmail.com>, "linux-samsung-soc@vger.kernel.org" <linux-samsung-soc@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Kukjin Kim <kgene.kim@samsung.com>, Marek Szyprowski <m.szyprowski@samsung.com>, Abhilash Kesavan <kesavan.abhilash@gmail.com>, Chander Kashyap <k.chander@samsung.com> Subject: Re: [PATCH v2 5/6] ARM: EXYNOS: Fix suspend/resume sequences Date: Tue, 15 Jul 2014 23:01:43 +0530 [thread overview] Message-ID: <CADWw7OgQpJh+5V10QfKQyeUk3kV+AiKJ8yk_+jqW9QywgHAqFA@mail.gmail.com> (raw) In-Reply-To: <53C51B57.9040404@samsung.com> On Tue, Jul 15, 2014 at 5:45 PM, Tomasz Figa <t.figa@samsung.com> wrote: > On 15.07.2014 13:19, Bartlomiej Zolnierkiewicz wrote: >> >> Hi, >> >> On Monday, July 14, 2014 11:54:48 AM Tomasz Figa wrote: >>> Hi Kukjin, >>> >>> On 25.06.2014 13:52, Tomasz Figa wrote: >>>> Due to recent consolidation of Exynos suspend and cpuidle code, some >>>> parts of suspend and resume sequences are executed two times, once from >>>> exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it >>>> breaks suspend, at least on Exynos4-based boards. >>>> >>>> This patch fixes the issue by removing exynos_pm_syscore_ops completely >>>> and making the code rely only on CPU PM notifier. >>>> >>>> Tested on Exynos4210-based Trats board. >>>> >>>> Signed-off-by: Tomasz Figa <t.figa@samsung.com> >>>> --- >>>> arch/arm/mach-exynos/pm.c | 23 ++++------------------- >>>> 1 file changed, 4 insertions(+), 19 deletions(-) >>>> >>>> Changes since v1: >>>> - rebased onto Kukjin's fixes branch. >>>> >>>> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c >>>> index 202ca73..f23cc77 100644 >>>> --- a/arch/arm/mach-exynos/pm.c >>>> +++ b/arch/arm/mach-exynos/pm.c >>>> @@ -364,11 +364,6 @@ early_wakeup: >>>> return; >>>> } >>>> >>>> -static struct syscore_ops exynos_pm_syscore_ops = { >>>> - .suspend = exynos_pm_suspend, >>>> - .resume = exynos_pm_resume, >>>> -}; >>>> - >>>> /* >>>> * Suspend Ops >>>> */ >>>> @@ -438,22 +433,13 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self, >>>> >>>> switch (cmd) { >>>> case CPU_PM_ENTER: >>>> - if (cpu == 0) { >>>> - exynos_pm_central_suspend(); >>>> - if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9) >>>> - exynos_cpu_save_register(); >>>> - } >>>> + if (cpu == 0) >>>> + exynos_pm_suspend(); >>>> break; >>>> >>>> case CPU_PM_EXIT: >>>> - if (cpu == 0) { >>>> - if (read_cpuid_part_number() == >>>> - ARM_CPU_PART_CORTEX_A9) { >>>> - scu_enable(S5P_VA_SCU); >>>> - exynos_cpu_restore_register(); >>>> - } >>>> - exynos_pm_central_resume(); >>>> - } >>>> + if (cpu == 0) >>>> + exynos_pm_resume(); >>>> break; >>>> } >>>> >>>> @@ -478,6 +464,5 @@ void __init exynos_pm_init(void) >>>> tmp |= ((0xFF << 8) | (0x1F << 1)); >>>> __raw_writel(tmp, S5P_WAKEUP_MASK); >>>> >>>> - register_syscore_ops(&exynos_pm_syscore_ops); >>>> suspend_set_ops(&exynos_suspend_ops); >>>> } >>>> >>> >>> Please consider this patch for next fixes pull request. Without it >>> suspend/resume is broken for Exynos4 and probably other SoCs. This patch >>> just restores the sequence from before the patch moving things to PM >>> notifier, so I don't think it should need any special treatment. >> >> Your patch fixes the regression and is a step in the good direction but it >> seems that it needs a bit more work: >> >> Your patch adds to cpuidle AFTR code path restoring of exynos_core_save and >> exynos5_sys_save registers without saving them first (restoring is done >> through exynos_pm_resume() which is used by both suspend and cpuidle while >> saving is done through exynos_pm_prepare() which is used only by suspend). > > That's right, unfortunately. Interestingly enough AFTR worked fine on > Exynos4210 with this patch, but still this needs to be fixed. > > Now, in fact, I recall that Chander had some objections to this patch as > well and we decided that he will send another patch to replace mine [1], > but I haven't heard from him since that. Patch for the same has been already posted. Below is the link for the same http://www.spinics.net/lists/arm-kernel/msg343402.html This patch has conflict with Russel's patch, so i will rebase on his tree and resend This patch can be ignored in that case. > > Chander, do you have any progress with this? Keep in mind that we need > this as an rc fix and we already have rc5, so not much time left. If you > don't have time to work on this I can take care of this, proceeding as > we discussed in [1]. > > [1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/29085/focus=33975 > > Best regards, > Tomasz > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: k.chander@samsung.com (Chander Kashyap) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 5/6] ARM: EXYNOS: Fix suspend/resume sequences Date: Tue, 15 Jul 2014 23:01:43 +0530 [thread overview] Message-ID: <CADWw7OgQpJh+5V10QfKQyeUk3kV+AiKJ8yk_+jqW9QywgHAqFA@mail.gmail.com> (raw) In-Reply-To: <53C51B57.9040404@samsung.com> On Tue, Jul 15, 2014 at 5:45 PM, Tomasz Figa <t.figa@samsung.com> wrote: > On 15.07.2014 13:19, Bartlomiej Zolnierkiewicz wrote: >> >> Hi, >> >> On Monday, July 14, 2014 11:54:48 AM Tomasz Figa wrote: >>> Hi Kukjin, >>> >>> On 25.06.2014 13:52, Tomasz Figa wrote: >>>> Due to recent consolidation of Exynos suspend and cpuidle code, some >>>> parts of suspend and resume sequences are executed two times, once from >>>> exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it >>>> breaks suspend, at least on Exynos4-based boards. >>>> >>>> This patch fixes the issue by removing exynos_pm_syscore_ops completely >>>> and making the code rely only on CPU PM notifier. >>>> >>>> Tested on Exynos4210-based Trats board. >>>> >>>> Signed-off-by: Tomasz Figa <t.figa@samsung.com> >>>> --- >>>> arch/arm/mach-exynos/pm.c | 23 ++++------------------- >>>> 1 file changed, 4 insertions(+), 19 deletions(-) >>>> >>>> Changes since v1: >>>> - rebased onto Kukjin's fixes branch. >>>> >>>> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c >>>> index 202ca73..f23cc77 100644 >>>> --- a/arch/arm/mach-exynos/pm.c >>>> +++ b/arch/arm/mach-exynos/pm.c >>>> @@ -364,11 +364,6 @@ early_wakeup: >>>> return; >>>> } >>>> >>>> -static struct syscore_ops exynos_pm_syscore_ops = { >>>> - .suspend = exynos_pm_suspend, >>>> - .resume = exynos_pm_resume, >>>> -}; >>>> - >>>> /* >>>> * Suspend Ops >>>> */ >>>> @@ -438,22 +433,13 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self, >>>> >>>> switch (cmd) { >>>> case CPU_PM_ENTER: >>>> - if (cpu == 0) { >>>> - exynos_pm_central_suspend(); >>>> - if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9) >>>> - exynos_cpu_save_register(); >>>> - } >>>> + if (cpu == 0) >>>> + exynos_pm_suspend(); >>>> break; >>>> >>>> case CPU_PM_EXIT: >>>> - if (cpu == 0) { >>>> - if (read_cpuid_part_number() == >>>> - ARM_CPU_PART_CORTEX_A9) { >>>> - scu_enable(S5P_VA_SCU); >>>> - exynos_cpu_restore_register(); >>>> - } >>>> - exynos_pm_central_resume(); >>>> - } >>>> + if (cpu == 0) >>>> + exynos_pm_resume(); >>>> break; >>>> } >>>> >>>> @@ -478,6 +464,5 @@ void __init exynos_pm_init(void) >>>> tmp |= ((0xFF << 8) | (0x1F << 1)); >>>> __raw_writel(tmp, S5P_WAKEUP_MASK); >>>> >>>> - register_syscore_ops(&exynos_pm_syscore_ops); >>>> suspend_set_ops(&exynos_suspend_ops); >>>> } >>>> >>> >>> Please consider this patch for next fixes pull request. Without it >>> suspend/resume is broken for Exynos4 and probably other SoCs. This patch >>> just restores the sequence from before the patch moving things to PM >>> notifier, so I don't think it should need any special treatment. >> >> Your patch fixes the regression and is a step in the good direction but it >> seems that it needs a bit more work: >> >> Your patch adds to cpuidle AFTR code path restoring of exynos_core_save and >> exynos5_sys_save registers without saving them first (restoring is done >> through exynos_pm_resume() which is used by both suspend and cpuidle while >> saving is done through exynos_pm_prepare() which is used only by suspend). > > That's right, unfortunately. Interestingly enough AFTR worked fine on > Exynos4210 with this patch, but still this needs to be fixed. > > Now, in fact, I recall that Chander had some objections to this patch as > well and we decided that he will send another patch to replace mine [1], > but I haven't heard from him since that. Patch for the same has been already posted. Below is the link for the same http://www.spinics.net/lists/arm-kernel/msg343402.html This patch has conflict with Russel's patch, so i will rebase on his tree and resend This patch can be ignored in that case. > > Chander, do you have any progress with this? Keep in mind that we need > this as an rc fix and we already have rc5, so not much time left. If you > don't have time to work on this I can take care of this, proceeding as > we discussed in [1]. > > [1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/29085/focus=33975 > > Best regards, > Tomasz > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2014-07-15 17:31 UTC|newest] Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-06-24 13:57 [PATCH 0/6] Various Samsung fixes for v3.16-rc3 Tomasz Figa 2014-06-24 13:57 ` Tomasz Figa 2014-06-24 13:57 ` [PATCH 1/6] mmc: sdhci-s3c: Fix local I/O clock gating Tomasz Figa 2014-06-24 13:57 ` Tomasz Figa 2014-07-14 8:05 ` Jaehoon Chung 2014-07-14 8:05 ` Jaehoon Chung 2014-06-24 13:57 ` [PATCH 2/6] ARM: EXYNOS: Fix core ID used by platsmp and hotplug code Tomasz Figa 2014-06-24 13:57 ` Tomasz Figa 2014-06-24 13:57 ` Tomasz Figa 2014-07-08 13:21 ` Kukjin Kim 2014-07-08 13:21 ` Kukjin Kim 2014-07-14 9:50 ` Tomasz Figa 2014-07-14 9:50 ` Tomasz Figa 2014-07-14 9:50 ` Tomasz Figa 2014-07-15 18:00 ` Kukjin Kim 2014-07-15 18:00 ` Kukjin Kim 2014-06-24 13:57 ` [PATCH 3/6] clk: samsung: exynos4: Remove SRC_MASK_ISP gates Tomasz Figa 2014-06-24 13:57 ` Tomasz Figa 2014-06-24 13:57 ` Tomasz Figa 2014-06-25 9:58 ` Daniel Drake 2014-06-25 9:58 ` Daniel Drake 2014-06-25 10:10 ` Tomasz Figa 2014-06-25 10:10 ` Tomasz Figa 2014-06-30 13:50 ` Tomasz Figa 2014-06-30 13:50 ` Tomasz Figa 2014-06-24 13:57 ` [PATCH 4/6] ARM: SAMSUNG: Restore Samsung PM Debug functionality Tomasz Figa 2014-06-24 13:57 ` Tomasz Figa 2014-06-25 10:09 ` Daniel Drake 2014-06-25 10:09 ` Daniel Drake 2014-06-25 11:41 ` Tomasz Figa 2014-06-25 11:41 ` Tomasz Figa 2014-06-25 11:43 ` [PATCH v2 " Tomasz Figa 2014-06-25 11:43 ` Tomasz Figa 2014-06-26 9:24 ` Daniel Drake 2014-06-26 9:24 ` Daniel Drake 2014-07-14 9:52 ` Tomasz Figa 2014-07-14 9:52 ` Tomasz Figa 2014-07-08 13:48 ` [PATCH " Kukjin Kim 2014-07-08 13:48 ` Kukjin Kim 2014-07-08 13:54 ` Tomasz Figa 2014-07-08 13:54 ` Tomasz Figa 2014-07-15 18:02 ` Kukjin Kim 2014-07-15 18:02 ` Kukjin Kim 2014-07-15 23:59 ` Tomasz Figa 2014-07-15 23:59 ` Tomasz Figa 2014-07-21 10:30 ` Tomasz Figa 2014-07-21 10:30 ` Tomasz Figa 2014-06-24 13:57 ` [PATCH 5/6] ARM: EXYNOS: Fix suspend/resume sequencies Tomasz Figa 2014-06-24 13:57 ` Tomasz Figa 2014-06-24 15:33 ` Abhilash Kesavan 2014-06-24 15:33 ` Abhilash Kesavan 2014-07-08 14:02 ` Kukjin Kim 2014-07-08 14:02 ` Kukjin Kim 2014-06-24 13:57 ` [PATCH 6/6] ARM: EXYNOS: Register cpuidle device only on Exynos4210 and 5250 Tomasz Figa 2014-06-24 13:57 ` Tomasz Figa 2014-07-08 14:15 ` Kukjin Kim 2014-07-08 14:15 ` Kukjin Kim 2014-07-08 15:45 ` Bartlomiej Zolnierkiewicz 2014-07-08 15:45 ` Bartlomiej Zolnierkiewicz 2014-07-10 13:30 ` Kukjin Kim 2014-07-10 13:30 ` Kukjin Kim 2014-06-25 11:52 ` [PATCH v2 5/6] ARM: EXYNOS: Fix suspend/resume sequences Tomasz Figa 2014-06-25 11:52 ` Tomasz Figa 2014-07-14 9:54 ` Tomasz Figa 2014-07-14 9:54 ` Tomasz Figa 2014-07-15 11:19 ` Bartlomiej Zolnierkiewicz 2014-07-15 11:19 ` Bartlomiej Zolnierkiewicz 2014-07-15 12:15 ` Tomasz Figa 2014-07-15 12:15 ` Tomasz Figa 2014-07-15 14:24 ` [PATCH v3] " Tomasz Figa 2014-07-15 14:24 ` Tomasz Figa 2014-07-15 14:26 ` Tomasz Figa 2014-07-15 14:26 ` Tomasz Figa 2014-07-17 14:42 ` Tomasz Figa 2014-07-17 14:42 ` Tomasz Figa 2014-07-21 10:23 ` Tomasz Figa 2014-07-21 10:23 ` Tomasz Figa 2014-07-15 17:31 ` Chander Kashyap [this message] 2014-07-15 17:31 ` [PATCH v2 5/6] " Chander Kashyap 2014-07-15 17:31 ` Chander Kashyap 2014-07-01 13:54 ` [PATCH 0/6] Various Samsung fixes for v3.16-rc3 Tomasz Figa 2014-07-01 13:54 ` Tomasz Figa 2014-07-07 23:25 ` Kukjin Kim 2014-07-07 23:25 ` Kukjin Kim
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=CADWw7OgQpJh+5V10QfKQyeUk3kV+AiKJ8yk_+jqW9QywgHAqFA@mail.gmail.com \ --to=k.chander@samsung.com \ --cc=b.zolnierkie@samsung.com \ --cc=kesavan.abhilash@gmail.com \ --cc=kgene.kim@samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=m.szyprowski@samsung.com \ --cc=t.figa@samsung.com \ --cc=tomasz.figa@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.