Re: [FIX][RESEND PATCH v3] ARM: EXYNOS: Fix suspend/resume sequences

[Olof Johansson]

On Fri, Aug 01, 2014 at 11:18:53PM +0900, Kukjin Kim wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > 
> > Hi,
> > 
> Hi,
> 
> > On Friday, August 01, 2014 10:51:37 AM Tomasz Figa wrote:
> > > From: Tomasz Figa <t.figa@xxxxxxxxxxx>
> > >
> > > 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. In addition, simple
> > > core power down from a cpuidle driver could, in case of CPU 0 could
> > > result in calling functions that are specific to suspend and deeper idle
> > > states.
> > >
> > > This patch fixes the issue by moving those operations outside the CPU PM
> > > notifier into suspend and AFTR code paths. This leads to a bit of code
> > > duplication, but allows additional code simplification, so in the end
> > > more code is removed than added.
> > >
> > > Fixes: 85f9f90808b4 ("ARM: EXYNOS: Use the cpu_pm notifier for pm")
> > > Cc: Kukjin Kim <kgene.kim@xxxxxxxxxxx>
> > > Cc: Arnd Bergmann <arnd@xxxxxxxx>
> > > Cc: Olof Johansson <olof@xxxxxxxxx>
> > > Cc: arm@xxxxxxxxxx
> > > Signed-off-by: Tomasz Figa <t.figa@xxxxxxxxxxx>
> > > ---
> > >  arch/arm/mach-exynos/pm.c        | 164 ++++++++++++++++++---------------------
> > >  drivers/cpuidle/cpuidle-exynos.c |  25 +-----
> > >  2 files changed, 80 insertions(+), 109 deletions(-)
> > >
> > > This is resend of a regression fix sent more than two weeks ago (July, 15;
> > > [1]), without any response from respective maintainers. It fixes
> > > a significant regression breaking suspend/resume on Exynos4-based systems.
> > >
> > > I know we already have -rc7, but this patch in its first iteration was
> > > sent more than a month ago (June, 24; [2]) and it changed only for
> > > convenience of authors of further patches, to either avoid conflicts ([3])
> > > or reduce the need of changes to support more functionality later (as
> > > discussed in [4]). However it should have had priority over clean-up or
> > > functional patches, which was apparently not the case.
> > 
> > This is not quite right, your initial version needed to be changed also
> > because of potential problems (https://lkml.org/lkml/2014/7/15/206).
> > 
> Yeah...hmm...maybe it should be updated in 3.17...
> 
> > However the current version (from two weeks ago) is fine and IMO still
> > should be merged as a _regression_ fix for v3.16 (despite not being
> > a tiny patch).
> > 
> Agreed. OK.
> 
> > Also FWIW:
> > 
> > Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx>
> > 
> Acked-by: Kukjin Kim <kgene.kim@xxxxxxxxxxx>
> 
> @Olof,
> 
> It's true this is a regression for v3.16 and I couldn't take this before
> because of some dependencies...so I'd like to ask you to take for v3.16 even
> though it's late and big...

It's really, really late in the release cycle to pick this up now. I'm also
travelling right now and have limited email access to handle fall-out. :(

I'll apply this to the fixes branch for 3.17 and mark it for stable instead.

Kukjin, seems like you're not testing suspend/resume yourself, since you
haven't known about the regression and the fix. Can you please make sure that
you have appropriate test coverage? Thanks!


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/