2013-03-19 16:52:21

by Doug Anderson

[permalink] [raw]
Subject: [PATCH 0/2] These two patches to s3c_pm_arch_prepare_irqs() were part of the work

to make suspend/resume reliable on the ARM Chromebook
(exynos5250-snow).

A few more details:
- The first patch is not strictly needed but was a nice cleanup. Our
understanding was that EINT0 was originally turned on for exynos
evt0 silicon and not needed for evt1.
- The second patch is more important and (also) more obvious. The
function was modifying the S5P_WAKEUP_MASK register and then
clobbering its own modifications.

For some history, see:
- https://gerrit.chromium.org/gerrit/31337
- https://gerrit.chromium.org/gerrit/31341


Jonathan Kliegman (2):
arm: exynos: Remove hardcode wakeup unmask for EINT_0
arm: exynos: Clear ENABLE_WAKEUP_SW bit when entering suspend

arch/arm/mach-exynos/include/mach/pm-core.h | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

--
1.8.1.3


2013-03-19 16:52:22

by Doug Anderson

[permalink] [raw]
Subject: [PATCH 2/2] arm: exynos: Clear ENABLE_WAKEUP_SW bit when entering suspend

From: Jonathan Kliegman <[email protected]>

Setting this bit to 0 causes the system to wait until suspended
to use the wakeup masks. With it being set high previously,
masked interrupts were being received and processed before the
EINT_WAKEUP_MASK was configured.

Signed-off-by: Jonathan Kliegman <[email protected]>
Signed-off-by: Doug Anderson <[email protected]>
Reviewed-by: Doug Anderson <[email protected]>
---
arch/arm/mach-exynos/include/mach/pm-core.h | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/arm/mach-exynos/include/mach/pm-core.h b/arch/arm/mach-exynos/include/mach/pm-core.h
index 9d8da51e3..7dbbfec 100644
--- a/arch/arm/mach-exynos/include/mach/pm-core.h
+++ b/arch/arm/mach-exynos/include/mach/pm-core.h
@@ -27,13 +27,8 @@ static inline void s3c_pm_debug_init_uart(void)

static inline void s3c_pm_arch_prepare_irqs(void)
{
- unsigned int tmp;
- tmp = __raw_readl(S5P_WAKEUP_MASK);
- tmp &= ~(1 << 31);
- __raw_writel(tmp, S5P_WAKEUP_MASK);
-
- __raw_writel(s3c_irqwake_intmask, S5P_WAKEUP_MASK);
__raw_writel(s3c_irqwake_eintmask, S5P_EINT_WAKEUP_MASK);
+ __raw_writel(s3c_irqwake_intmask & ~(1 << 31), S5P_WAKEUP_MASK);
}

static inline void s3c_pm_arch_stop_clocks(void)
--
1.8.1.3

2013-03-19 16:52:46

by Doug Anderson

[permalink] [raw]
Subject: [PATCH 1/2] arm: exynos: Remove hardcode wakeup unmask for EINT_0

From: Jonathan Kliegman <[email protected]>

For legacy reasons EINT_0 was being forced on for all
exynos systems as a wake interrupt. For boards that need
EINT_0 they should probably enable it with enable_irq_wake

Signed-off-by: Jonathan Kliegman <[email protected]>
Signed-off-by: Doug Anderson <[email protected]>
Reviewed-by: Doug Anderson <[email protected]>
Reviewed-by: Thomas Abraham <[email protected]>
---
arch/arm/mach-exynos/include/mach/pm-core.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-exynos/include/mach/pm-core.h b/arch/arm/mach-exynos/include/mach/pm-core.h
index a67ecfa..9d8da51e3 100644
--- a/arch/arm/mach-exynos/include/mach/pm-core.h
+++ b/arch/arm/mach-exynos/include/mach/pm-core.h
@@ -33,7 +33,7 @@ static inline void s3c_pm_arch_prepare_irqs(void)
__raw_writel(tmp, S5P_WAKEUP_MASK);

__raw_writel(s3c_irqwake_intmask, S5P_WAKEUP_MASK);
- __raw_writel(s3c_irqwake_eintmask & 0xFFFFFFFE, S5P_EINT_WAKEUP_MASK);
+ __raw_writel(s3c_irqwake_eintmask, S5P_EINT_WAKEUP_MASK);
}

static inline void s3c_pm_arch_stop_clocks(void)
--
1.8.1.3

2013-04-03 02:03:08

by Kukjin Kim

[permalink] [raw]
Subject: RE: [PATCH 0/2] These two patches to s3c_pm_arch_prepare_irqs() were part of the work

Doug Anderson wrote:
>
> to make suspend/resume reliable on the ARM Chromebook
> (exynos5250-snow).
>
> A few more details:
> - The first patch is not strictly needed but was a nice cleanup. Our
> understanding was that EINT0 was originally turned on for exynos
> evt0 silicon and not needed for evt1.

Looks good to me, applied, thanks.

> - The second patch is more important and (also) more obvious. The
> function was modifying the S5P_WAKEUP_MASK register and then
> clobbering its own modifications.
>
Applied with 1st one, BTW, do you want to send this for stable tree?

- Kukjin

> For some history, see:
> - https://gerrit.chromium.org/gerrit/31337
> - https://gerrit.chromium.org/gerrit/31341
>
>
> Jonathan Kliegman (2):
> arm: exynos: Remove hardcode wakeup unmask for EINT_0
> arm: exynos: Clear ENABLE_WAKEUP_SW bit when entering suspend
>
> arch/arm/mach-exynos/include/mach/pm-core.h | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> --
> 1.8.1.3

2013-04-03 02:16:41

by Kukjin Kim

[permalink] [raw]
Subject: RE: [PATCH 0/2] These two patches to s3c_pm_arch_prepare_irqs() were part of the work

Kukjin Kim wrote:
>
> Doug Anderson wrote:
> >
> > to make suspend/resume reliable on the ARM Chromebook
> > (exynos5250-snow).
> >
> > A few more details:
> > - The first patch is not strictly needed but was a nice cleanup. Our
> > understanding was that EINT0 was originally turned on for exynos
> > evt0 silicon and not needed for evt1.
>
> Looks good to me, applied, thanks.
>
> > - The second patch is more important and (also) more obvious. The
> > function was modifying the S5P_WAKEUP_MASK register and then
> > clobbering its own modifications.
> >
> Applied with 1st one, BTW, do you want to send this for stable tree?
>
One more note, just now I discussed Jaecheol Lee about the bit,
ENABLE_WAKEUP_SW, as the patch fixed, it should be cleared but used to be
set s3c_irqwake_intmask. Let me check again, then if any updates I'll let
you know.

Thanks.

- Kukjin

2013-04-03 17:21:01

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH 0/2] These two patches to s3c_pm_arch_prepare_irqs() were part of the work

Kukjin,

On Tue, Apr 2, 2013 at 7:16 PM, Kukjin Kim <[email protected]> wrote:
>> Applied with 1st one, BTW, do you want to send this for stable tree?

I don't have any need for it to be in stable tree. The ARM Chromebook
hasn't reached critical functionality on any released/upstram Linux
versions so it doesn't make much sense to backport fixes. If someone
else wants it in stable (and can confirm that it helps them) then I
certainly wouldn't object!


> One more note, just now I discussed Jaecheol Lee about the bit,
> ENABLE_WAKEUP_SW, as the patch fixed, it should be cleared but used to be
> set s3c_irqwake_intmask. Let me check again, then if any updates I'll let
> you know.

OK, thanks. If there is a reason that ENABLE_WAKEUP_SW needs to be
set then it would be good to understand that case. :)

-Doug