| Message ID | CANruHrS1D8d_y43xa-2OGduTvAGWfdeY1REVzZ6YJHjhzLg+hw@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sunil, On Fri, Dec 20, 2013 at 3:56 PM, sunil joshi <sjoshi.open@gmail.com> wrote: > Hi Abhilash, > I saw another patch in chrome tree ..by Andrew Bresticker > which may be relevant here .. > > Just wondering if you missed adding this ? or this is not needed ? > You did not face any issue in getting core to suspend ? This has not been added for Exynos5250 in mainline yet. We did notice that the system would fail to suspend on the rare occasion (~1% of the time on exynos5250) when we repeatedly suspended and resumed the system. I have not run s2r stress tests, but have had no issues suspending the system in my limited testing. We can probably let this be for now as the system would resume fine even on a failed suspend. I will do some stress testing and then post if needed. > > ------------------------------------------------------------------------------------------ > commit 95402d816b9f1a05ce633f7ff64b4c939c142482 > Author: Andrew Bresticker <abrestic@chromium.org> > Date: Mon Jul 15 13:14:36 2013 -0700 > > arm: exynos: disable all interrupts on Exynos5420 before suspend > > Disable all interrupts from the GIC before entering suspend on > Exynos5420 as is done on Exynos5250. If interrupts are enabled, we > may receive an interrupt after entering WFI but before the PMU has > suspended the system, causing suspend to fail. > > BUG=chrome-os-partner:20523 > TEST=Run suspend_stress_test on Pit and observe that entering suspend > no longer occasionally fails with the "Failed to suspend the system" > error in exynos_cpu_suspend(). > > Change-Id: Ia2d963191f4e5485beb295b28f128f2ce256f987 > Signed-off-by: Andrew Bresticker <abrestic@chromium.org> > Reviewed-on: https://gerrit.chromium.org/gerrit/61948 > Reviewed-by: Simon Glass <sjg@chromium.org> > Regards, Abhilash
Hi Abhilash, On Fri, Dec 20, 2013 at 5:08 PM, Abhilash Kesavan <kesavan.abhilash@gmail.com> wrote: > Hi Sunil, > > On Fri, Dec 20, 2013 at 3:56 PM, sunil joshi <sjoshi.open@gmail.com> wrote: >> Hi Abhilash, >> I saw another patch in chrome tree ..by Andrew Bresticker >> which may be relevant here .. >> >> Just wondering if you missed adding this ? or this is not needed ? >> You did not face any issue in getting core to suspend ? > > This has not been added for Exynos5250 in mainline yet. We did notice > that the system would fail to suspend on the rare occasion (~1% of the > time on exynos5250) when we repeatedly suspended and resumed the > system. > > I have not run s2r stress tests, but have had no issues suspending the > system in my limited testing. We can probably let this be for now as > the system would resume fine even on a failed suspend. I will do some > stress testing and then post if needed. >> Ok. Sure .. Thanks for details ! >> ------------------------------------------------------------------------------------------ >> commit 95402d816b9f1a05ce633f7ff64b4c939c142482 >> Author: Andrew Bresticker <abrestic@chromium.org> >> Date: Mon Jul 15 13:14:36 2013 -0700 >> >> arm: exynos: disable all interrupts on Exynos5420 before suspend >> >> Disable all interrupts from the GIC before entering suspend on >> Exynos5420 as is done on Exynos5250. If interrupts are enabled, we >> may receive an interrupt after entering WFI but before the PMU has >> suspended the system, causing suspend to fail. >> >> BUG=chrome-os-partner:20523 >> TEST=Run suspend_stress_test on Pit and observe that entering suspend >> no longer occasionally fails with the "Failed to suspend the system" >> error in exynos_cpu_suspend(). >> >> Change-Id: Ia2d963191f4e5485beb295b28f128f2ce256f987 >> Signed-off-by: Andrew Bresticker <abrestic@chromium.org> >> Reviewed-on: https://gerrit.chromium.org/gerrit/61948 >> Reviewed-by: Simon Glass <sjg@chromium.org> >> > > Regards, > Abhilash > _______________________________________________ >>>> linux-arm-kernel mailing list >>>> linux-arm-kernel@lists.infradead.org >>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>> -- >>> 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
Hi, On Friday 20 of December 2013 15:56:38 sunil joshi wrote: > Hi Abhilash, > I saw another patch in chrome tree ..by Andrew Bresticker > which may be relevant here .. > > Just wondering if you missed adding this ? or this is not needed ? > You did not face any issue in getting core to suspend ? > > ------------------------------------------------------------------------------------------ > commit 95402d816b9f1a05ce633f7ff64b4c939c142482 > Author: Andrew Bresticker <abrestic@chromium.org> > Date: Mon Jul 15 13:14:36 2013 -0700 > > arm: exynos: disable all interrupts on Exynos5420 before suspend > > Disable all interrupts from the GIC before entering suspend on > Exynos5420 as is done on Exynos5250. If interrupts are enabled, we > may receive an interrupt after entering WFI but before the PMU has > suspended the system, causing suspend to fail. > > BUG=chrome-os-partner:20523 > TEST=Run suspend_stress_test on Pit and observe that entering suspend > no longer occasionally fails with the "Failed to suspend the system" > error in exynos_cpu_suspend(). A question about this for Chromium and LSI guys: If you find out that there is already a pending interrupt before you enter the sleep mode, isn't it more reasonable to cancel the process ASAP and handle the event instead of entering the sleep just to leave it? I believe this should be both more efficient with respect to power usage and latency, because sleep-wakeup transition takes time and power. Do you have any reason to think the opposite? Best regards, Tomasz
On Fri, Dec 20, 2013 at 3:38 AM, Abhilash Kesavan <kesavan.abhilash@gmail.com> wrote: > Hi Sunil, > > On Fri, Dec 20, 2013 at 3:56 PM, sunil joshi <sjoshi.open@gmail.com> wrote: >> Hi Abhilash, >> I saw another patch in chrome tree ..by Andrew Bresticker >> which may be relevant here .. >> >> Just wondering if you missed adding this ? or this is not needed ? >> You did not face any issue in getting core to suspend ? > > This has not been added for Exynos5250 in mainline yet. We did notice > that the system would fail to suspend on the rare occasion (~1% of the > time on exynos5250) when we repeatedly suspended and resumed the > system. > > I have not run s2r stress tests, but have had no issues suspending the > system in my limited testing. We can probably let this be for now as > the system would resume fine even on a failed suspend. I will do some > stress testing and then post if needed. Hold on, you're claiming that this patch you've posted has only seen very limited testing, and that you are aware of failures? Don't merge known-broken code into the mainline kernel. -Olof
On Friday 20 of December 2013 13:22:06 Olof Johansson wrote: > On Fri, Dec 20, 2013 at 3:38 AM, Abhilash Kesavan > <kesavan.abhilash@gmail.com> wrote: > > Hi Sunil, > > > > On Fri, Dec 20, 2013 at 3:56 PM, sunil joshi <sjoshi.open@gmail.com> wrote: > >> Hi Abhilash, > >> I saw another patch in chrome tree ..by Andrew Bresticker > >> which may be relevant here .. > >> > >> Just wondering if you missed adding this ? or this is not needed ? > >> You did not face any issue in getting core to suspend ? > > > > This has not been added for Exynos5250 in mainline yet. We did notice > > that the system would fail to suspend on the rare occasion (~1% of the > > time on exynos5250) when we repeatedly suspended and resumed the > > system. > > > > I have not run s2r stress tests, but have had no issues suspending the > > system in my limited testing. We can probably let this be for now as > > the system would resume fine even on a failed suspend. I will do some > > stress testing and then post if needed. > > Hold on, you're claiming that this patch you've posted has only seen > very limited testing, and that you are aware of failures? > > Don't merge known-broken code into the mainline kernel. I'm afraid that a "failed suspend" does not really mean a failure here, but let me wait for clarification of things I asked in another part of this thread. Best regards, Tomasz
On Fri, Dec 20, 2013 at 1:23 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > On Friday 20 of December 2013 13:22:06 Olof Johansson wrote: >> On Fri, Dec 20, 2013 at 3:38 AM, Abhilash Kesavan >> <kesavan.abhilash@gmail.com> wrote: >> > Hi Sunil, >> > >> > On Fri, Dec 20, 2013 at 3:56 PM, sunil joshi <sjoshi.open@gmail.com> wrote: >> >> Hi Abhilash, >> >> I saw another patch in chrome tree ..by Andrew Bresticker >> >> which may be relevant here .. >> >> >> >> Just wondering if you missed adding this ? or this is not needed ? >> >> You did not face any issue in getting core to suspend ? >> > >> > This has not been added for Exynos5250 in mainline yet. We did notice >> > that the system would fail to suspend on the rare occasion (~1% of the >> > time on exynos5250) when we repeatedly suspended and resumed the >> > system. >> > >> > I have not run s2r stress tests, but have had no issues suspending the >> > system in my limited testing. We can probably let this be for now as >> > the system would resume fine even on a failed suspend. I will do some >> > stress testing and then post if needed. >> >> Hold on, you're claiming that this patch you've posted has only seen >> very limited testing, and that you are aware of failures? >> >> Don't merge known-broken code into the mainline kernel. > > I'm afraid that a "failed suspend" does not really mean a failure here, > but let me wait for clarification of things I asked in another part of > this thread. Fair enough. Still, I expect code that enters the kernel to actually be tested. We've got way too much code just sitting there in broken state because whomever posted it didn't care about it working at the time, and nobody knows better because you can't actually use the code/driver on any real hardware with a mainline kernel. -Olof
On Friday 20 of December 2013 13:25:06 Olof Johansson wrote: > On Fri, Dec 20, 2013 at 1:23 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > > On Friday 20 of December 2013 13:22:06 Olof Johansson wrote: > >> On Fri, Dec 20, 2013 at 3:38 AM, Abhilash Kesavan > >> <kesavan.abhilash@gmail.com> wrote: > >> > Hi Sunil, > >> > > >> > On Fri, Dec 20, 2013 at 3:56 PM, sunil joshi <sjoshi.open@gmail.com> wrote: > >> >> Hi Abhilash, > >> >> I saw another patch in chrome tree ..by Andrew Bresticker > >> >> which may be relevant here .. > >> >> > >> >> Just wondering if you missed adding this ? or this is not needed ? > >> >> You did not face any issue in getting core to suspend ? > >> > > >> > This has not been added for Exynos5250 in mainline yet. We did notice > >> > that the system would fail to suspend on the rare occasion (~1% of the > >> > time on exynos5250) when we repeatedly suspended and resumed the > >> > system. > >> > > >> > I have not run s2r stress tests, but have had no issues suspending the > >> > system in my limited testing. We can probably let this be for now as > >> > the system would resume fine even on a failed suspend. I will do some > >> > stress testing and then post if needed. > >> > >> Hold on, you're claiming that this patch you've posted has only seen > >> very limited testing, and that you are aware of failures? > >> > >> Don't merge known-broken code into the mainline kernel. > > > > I'm afraid that a "failed suspend" does not really mean a failure here, > > but let me wait for clarification of things I asked in another part of > > this thread. > > Fair enough. > > Still, I expect code that enters the kernel to actually be tested. > We've got way too much code just sitting there in broken state because > whomever posted it didn't care about it working at the time, and > nobody knows better because you can't actually use the code/driver on > any real hardware with a mainline kernel. Agreed. Best regards, Tomasz
On Fri, Dec 20, 2013 at 1:19 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > Hi, > > On Friday 20 of December 2013 15:56:38 sunil joshi wrote: >> Hi Abhilash, >> I saw another patch in chrome tree ..by Andrew Bresticker >> which may be relevant here .. >> >> Just wondering if you missed adding this ? or this is not needed ? >> You did not face any issue in getting core to suspend ? >> >> ------------------------------------------------------------------------------------------ >> commit 95402d816b9f1a05ce633f7ff64b4c939c142482 >> Author: Andrew Bresticker <abrestic@chromium.org> >> Date: Mon Jul 15 13:14:36 2013 -0700 >> >> arm: exynos: disable all interrupts on Exynos5420 before suspend >> >> Disable all interrupts from the GIC before entering suspend on >> Exynos5420 as is done on Exynos5250. If interrupts are enabled, we >> may receive an interrupt after entering WFI but before the PMU has >> suspended the system, causing suspend to fail. >> >> BUG=chrome-os-partner:20523 >> TEST=Run suspend_stress_test on Pit and observe that entering suspend >> no longer occasionally fails with the "Failed to suspend the system" >> error in exynos_cpu_suspend(). > > A question about this for Chromium and LSI guys: > > If you find out that there is already a pending interrupt before you enter > the sleep mode, isn't it more reasonable to cancel the process ASAP and > handle the event instead of entering the sleep just to leave it? > > I believe this should be both more efficient with respect to power usage > and latency, because sleep-wakeup transition takes time and power. > > Do you have any reason to think the opposite? If it's expected to be a rare or very rare event, it's not a given that the added complexity of dealing with the aborted suspend that late is worth it. -Olof
On Friday 20 of December 2013 13:37:36 Olof Johansson wrote: > On Fri, Dec 20, 2013 at 1:19 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > > Hi, > > > > On Friday 20 of December 2013 15:56:38 sunil joshi wrote: > >> Hi Abhilash, > >> I saw another patch in chrome tree ..by Andrew Bresticker > >> which may be relevant here .. > >> > >> Just wondering if you missed adding this ? or this is not needed ? > >> You did not face any issue in getting core to suspend ? > >> > >> ------------------------------------------------------------------------------------------ > >> commit 95402d816b9f1a05ce633f7ff64b4c939c142482 > >> Author: Andrew Bresticker <abrestic@chromium.org> > >> Date: Mon Jul 15 13:14:36 2013 -0700 > >> > >> arm: exynos: disable all interrupts on Exynos5420 before suspend > >> > >> Disable all interrupts from the GIC before entering suspend on > >> Exynos5420 as is done on Exynos5250. If interrupts are enabled, we > >> may receive an interrupt after entering WFI but before the PMU has > >> suspended the system, causing suspend to fail. > >> > >> BUG=chrome-os-partner:20523 > >> TEST=Run suspend_stress_test on Pit and observe that entering suspend > >> no longer occasionally fails with the "Failed to suspend the system" > >> error in exynos_cpu_suspend(). > > > > A question about this for Chromium and LSI guys: > > > > If you find out that there is already a pending interrupt before you enter > > the sleep mode, isn't it more reasonable to cancel the process ASAP and > > handle the event instead of entering the sleep just to leave it? > > > > I believe this should be both more efficient with respect to power usage > > and latency, because sleep-wakeup transition takes time and power. > > > > Do you have any reason to think the opposite? > > If it's expected to be a rare or very rare event, it's not a given > that the added complexity of dealing with the aborted suspend that > late is worth it. I think that the code to support this is already in place, just printing an unfortunate message about "suspend failures". It doesn't add any significant complexity too. I'm not sure about the frequency of such events, though, and any real effect of this or the other behavior in such case and so there is my question about this. Best regards, Tomasz
Tomasz, On Fri, Dec 20, 2013 at 1:19 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > Hi, > > On Friday 20 of December 2013 15:56:38 sunil joshi wrote: >> Hi Abhilash, >> I saw another patch in chrome tree ..by Andrew Bresticker >> which may be relevant here .. >> >> Just wondering if you missed adding this ? or this is not needed ? >> You did not face any issue in getting core to suspend ? >> >> ------------------------------------------------------------------------------------------ >> commit 95402d816b9f1a05ce633f7ff64b4c939c142482 >> Author: Andrew Bresticker <abrestic@chromium.org> >> Date: Mon Jul 15 13:14:36 2013 -0700 >> >> arm: exynos: disable all interrupts on Exynos5420 before suspend >> >> Disable all interrupts from the GIC before entering suspend on >> Exynos5420 as is done on Exynos5250. If interrupts are enabled, we >> may receive an interrupt after entering WFI but before the PMU has >> suspended the system, causing suspend to fail. >> >> BUG=chrome-os-partner:20523 >> TEST=Run suspend_stress_test on Pit and observe that entering suspend >> no longer occasionally fails with the "Failed to suspend the system" >> error in exynos_cpu_suspend(). > > A question about this for Chromium and LSI guys: > > If you find out that there is already a pending interrupt before you enter > the sleep mode, isn't it more reasonable to cancel the process ASAP and > handle the event instead of entering the sleep just to leave it? > > I believe this should be both more efficient with respect to power usage > and latency, because sleep-wakeup transition takes time and power. > > Do you have any reason to think the opposite? I'm not actually sure I know every last detail, but... From what I remember on 5250 there was some type of mystery interrupt that was hitting in the system but that wasn't identifiable as any particular interrupt source. I think that this was an attempt to deal with that with a heavy hammer. I don't think it was a very elegant solution and it would be nice to do better. Ah, here's the original CL: <https://chromium-review.googlesource.com/#/c/34541/>. ...as you can see I wasn't super happy about it at the time but was OK with it going in since it was very late in the Chromebook release cycle and we needed suspend/resume to be reliable. Another fairly questionable CL: <https://chromium-review.googlesource.com/#/c/37991/> It would be super great if we could get suspend/resume reliable upstream without those hacks. -Doug
Hi Olof, On Sat, Dec 21, 2013 at 2:52 AM, Olof Johansson <olof@lixom.net> wrote: > On Fri, Dec 20, 2013 at 3:38 AM, Abhilash Kesavan > <kesavan.abhilash@gmail.com> wrote: >> Hi Sunil, >> >> On Fri, Dec 20, 2013 at 3:56 PM, sunil joshi <sjoshi.open@gmail.com> wrote: >>> Hi Abhilash, >>> I saw another patch in chrome tree ..by Andrew Bresticker >>> which may be relevant here .. >>> >>> Just wondering if you missed adding this ? or this is not needed ? >>> You did not face any issue in getting core to suspend ? >> >> This has not been added for Exynos5250 in mainline yet. We did notice >> that the system would fail to suspend on the rare occasion (~1% of the >> time on exynos5250) when we repeatedly suspended and resumed the >> system. >> >> I have not run s2r stress tests, but have had no issues suspending the >> system in my limited testing. We can probably let this be for now as >> the system would resume fine even on a failed suspend. I will do some >> stress testing and then post if needed. > > Hold on, you're claiming that this patch you've posted has only seen > very limited testing, and that you are aware of failures? > > Don't merge known-broken code into the mainline kernel. I have tested this patch on an 5250-snow and SMDK5420 through 30 cycles without any failure. Yes, I have seen failures on a 3.4 chromium kernel on an exynos5250 during stress testing. Mainline kernel is significantly different from what we are carrying in 3.4 or 3.8 chromium and the reason for me not adding the GIC patch was that it was a hack in the first place. The basic S2R functionality works fine, but if you'd rather have it pass a stress test of say a 1000 cycles then let me do the same and provide you with the results. > > > -Olof
Hi Tomasz, On Sat, Dec 21, 2013 at 3:23 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > On Friday 20 of December 2013 13:37:36 Olof Johansson wrote: >> On Fri, Dec 20, 2013 at 1:19 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: >> > Hi, >> > >> > On Friday 20 of December 2013 15:56:38 sunil joshi wrote: >> >> Hi Abhilash, >> >> I saw another patch in chrome tree ..by Andrew Bresticker >> >> which may be relevant here .. >> >> >> >> Just wondering if you missed adding this ? or this is not needed ? >> >> You did not face any issue in getting core to suspend ? >> >> >> >> ------------------------------------------------------------------------------------------ >> >> commit 95402d816b9f1a05ce633f7ff64b4c939c142482 >> >> Author: Andrew Bresticker <abrestic@chromium.org> >> >> Date: Mon Jul 15 13:14:36 2013 -0700 >> >> >> >> arm: exynos: disable all interrupts on Exynos5420 before suspend >> >> >> >> Disable all interrupts from the GIC before entering suspend on >> >> Exynos5420 as is done on Exynos5250. If interrupts are enabled, we >> >> may receive an interrupt after entering WFI but before the PMU has >> >> suspended the system, causing suspend to fail. >> >> >> >> BUG=chrome-os-partner:20523 >> >> TEST=Run suspend_stress_test on Pit and observe that entering suspend >> >> no longer occasionally fails with the "Failed to suspend the system" >> >> error in exynos_cpu_suspend(). >> > >> > A question about this for Chromium and LSI guys: >> > >> > If you find out that there is already a pending interrupt before you enter >> > the sleep mode, isn't it more reasonable to cancel the process ASAP and >> > handle the event instead of entering the sleep just to leave it? >> > >> > I believe this should be both more efficient with respect to power usage >> > and latency, because sleep-wakeup transition takes time and power. >> > >> > Do you have any reason to think the opposite? >> >> If it's expected to be a rare or very rare event, it's not a given >> that the added complexity of dealing with the aborted suspend that >> late is worth it. > > I think that the code to support this is already in place, just printing > an unfortunate message about "suspend failures". It doesn't add any > significant complexity too. > > I'm not sure about the frequency of such events, though, and any real > effect of this or the other behavior in such case and so there is my > question about this. Adding to what Doug already mentioned in the thread, in the cases where we saw cpu_do_idle fail we noticed the following: 1) No wake-up source as having woken up the system (by checking the WAKEUP_STAT) 2) On checking all the EINT Pend registers the values indicated no pending interrupts. The UM says that the PMU may fail to go to suspend when there are pending interrupt events. The patch assumed that we have a non-wakeup interrupt pending causing "wfi" to fail, especially as we saw the failure more often with no_console_suspend. > > Best regards, > Tomasz Abhilash >
Hi, I have observed on an average 2 failures in 100 S2R cycles even with the mainline kernel on SMDK5420. The system resumes fine after an aborted suspend. I have isolated the problem to the MCT_L0 interrupt. On disabling the forwarding of just this interrupt from the distributor there are no failures for over 1000 cycles. Code exists to mask the system timer wake-up and post-resume in a failed case the WAKEUP_STAT register does not show the timer having woken up the system (I am checking this quite early post-resume). On suspending the system, the non-boot cores would free their timer irqs (in the MCT driver) but not the boot core. So, there is a possibility that L0 irq might wake the system if it races with suspend. Is this a possibility ? Please correct me if my understanding is wrong. Abhilash On Sat, Jan 11, 2014 at 1:06 AM, Jonathan Kliegman <kliegs@chromium.org> wrote: > > > > On Fri, Dec 20, 2013 at 8:15 PM, Doug Anderson <dianders@chromium.org> > wrote: >> >> Tomasz, >> >> On Fri, Dec 20, 2013 at 1:19 PM, Tomasz Figa <tomasz.figa@gmail.com> >> wrote: >> > Hi, >> > >> > On Friday 20 of December 2013 15:56:38 sunil joshi wrote: >> >> Hi Abhilash, >> >> I saw another patch in chrome tree ..by Andrew Bresticker >> >> which may be relevant here .. >> >> >> >> Just wondering if you missed adding this ? or this is not needed ? >> >> You did not face any issue in getting core to suspend ? >> >> >> >> >> >> ------------------------------------------------------------------------------------------ >> >> commit 95402d816b9f1a05ce633f7ff64b4c939c142482 >> >> Author: Andrew Bresticker <abrestic@chromium.org> >> >> Date: Mon Jul 15 13:14:36 2013 -0700 >> >> >> >> arm: exynos: disable all interrupts on Exynos5420 before suspend >> >> >> >> Disable all interrupts from the GIC before entering suspend on >> >> Exynos5420 as is done on Exynos5250. If interrupts are enabled, we >> >> may receive an interrupt after entering WFI but before the PMU has >> >> suspended the system, causing suspend to fail. >> >> >> >> BUG=chrome-os-partner:20523 >> >> TEST=Run suspend_stress_test on Pit and observe that entering >> >> suspend >> >> no longer occasionally fails with the "Failed to suspend the >> >> system" >> >> error in exynos_cpu_suspend(). >> > >> > A question about this for Chromium and LSI guys: >> > >> > If you find out that there is already a pending interrupt before you >> > enter >> > the sleep mode, isn't it more reasonable to cancel the process ASAP and >> > handle the event instead of entering the sleep just to leave it? >> > >> > I believe this should be both more efficient with respect to power usage >> > and latency, because sleep-wakeup transition takes time and power. >> > >> > Do you have any reason to think the opposite? >> >> I'm not actually sure I know every last detail, but... >> >> From what I remember on 5250 there was some type of mystery interrupt >> that was hitting in the system but that wasn't identifiable as any >> particular interrupt source. I think that this was an attempt to deal >> with that with a heavy hammer. I don't think it was a very elegant >> solution and it would be nice to do better. >> >> Ah, here's the original CL: >> <https://chromium-review.googlesource.com/#/c/34541/>. ...as you can >> see I wasn't super happy about it at the time but was OK with it going >> in since it was very late in the Chromebook release cycle and we >> needed suspend/resume to be reliable. >> >> Another fairly questionable CL: >> <https://chromium-review.googlesource.com/#/c/37991/> >> >> It would be super great if we could get suspend/resume reliable >> upstream without those hacks. > > My recollection is the first CL (34541 that I put in) didn't actually do > anything itself, but had enough of a change in the timings that it made > behavior better. The second CL (37991) is definitely a hack that covered up > the issue and I don't think anyone was happy with but given the timing it > was the best alternative. > > I did a lot of debugging and don't believe I ever found an actual interrupt > firing that caused this. I'm pretty sure this was related to power settings > and it just failing to enter the suspend state. >> >> >> -Doug > >
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c index 537051d..9fdb8bc 100644 --- a/arch/arm/mach-exynos/pm.c +++ b/arch/arm/mach-exynos/pm.c @@ -126,7 +126,7 @@ static int exynos_cpu_suspend(unsigned long arg) * Disable all interrupts. eints will still be active during * suspend so its ok to mask everything here */ - if (soc_is_exynos5250()) + if (soc_is_exynos5250() || soc_is_exynos5420()) __raw_writel(0x0, S5P_VA_GIC_DIST + GIC_DIST_CTRL); On Tue, Dec 17, 2013 at 8:40 AM, Abhilash Kesavan
Hi Abhilash, I saw another patch in chrome tree ..by Andrew Bresticker which may be relevant here .. Just wondering if you missed adding this ? or this is not needed ? You did not face any issue in getting core to suspend ? ------------------------------------------------------------------------------------------ commit 95402d816b9f1a05ce633f7ff64b4c939c142482 Author: Andrew Bresticker <abrestic@chromium.org> Date: Mon Jul 15 13:14:36 2013 -0700 arm: exynos: disable all interrupts on Exynos5420 before suspend Disable all interrupts from the GIC before entering suspend on Exynos5420 as is done on Exynos5250. If interrupts are enabled, we may receive an interrupt after entering WFI but before the PMU has suspended the system, causing suspend to fail. BUG=chrome-os-partner:20523 TEST=Run suspend_stress_test on Pit and observe that entering suspend no longer occasionally fails with the "Failed to suspend the system" error in exynos_cpu_suspend(). Change-Id: Ia2d963191f4e5485beb295b28f128f2ce256f987 Signed-off-by: Andrew Bresticker <abrestic@chromium.org> Reviewed-on: https://gerrit.chromium.org/gerrit/61948 Reviewed-by: Simon Glass <sjg@chromium.org> <kesavan.abhilash@gmail.com> wrote: > Hi Bartlomiej, > > On Mon, Dec 16, 2013 at 6:18 PM, Bartlomiej Zolnierkiewicz > <b.zolnierkie@samsung.com> wrote: >> >> Hi, >> >> On Monday, December 16, 2013 05:31:10 PM Abhilash Kesavan wrote: >>> Add PMU configuration table for various low power modes - AFTR/LPA/SLEEP. >>> Also, add core s2r support for Exynos5420. >>> >>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> >>> --- >>> This patch depends on "ARM: EXYNOS5: Add PMU settings for exynos5420" >>> http://www.spinics.net/lists/linux-samsung-soc/msg24902.html >> >> [...] >> >>> @@ -253,10 +316,20 @@ static int exynos_pm_suspend(void) >>> >>> /* Setting SEQ_OPTION register */ >>> >>> - tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0); >>> - __raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION); >>> + if (soc_is_exynos5420()) { >>> + cluster_id = (read_cpuid(CPUID_MPIDR) >> 8) & 0xf; >>> + if (!cluster_id) >>> + __raw_writel(EXYNOS5420_ARM_USE_STANDBY_WFI0, >>> + S5P_CENTRAL_SEQ_OPTION); >>> + else >>> + __raw_writel(EXYNOS5420_KFC_USE_STANDBY_WFI0, >>> + S5P_CENTRAL_SEQ_OPTION); >>> + } else if (soc_is_exynos5250()) { >> >> Adding a check here for EXYNOS5250 doesn't look correct >> (the old code behavior for older EXYNOYS SoCs should be >> preserved). > > Yes, will fix this in the next version. > >> >>> + tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0); >>> + __raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION); >>> + } >> >> Best regards, >> -- >> Bartlomiej Zolnierkiewicz >> Samsung R&D Institute Poland >> Samsung Electronics >> >> > > Thanks, > Abhilash >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- > 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