ARM: SAMSUNG: Gracefully exit on suspend failure

PATCH Superseded Abhilash Kesavan 5 participants 8 messages Jan 9, 2013 – Jan 25, 2013

Subsystems (4)

Arm (32-bit) Arm sub-architectures Arm Samsung Exynos Cpuidle driver - Arm Exynos

lore

Share & tools

lore

Abhilash Kesavan

author

2013-01-09 11:12 UTC #0 lore raw

PATCH 1/1 ARM: SAMSUNG: Gracefully exit on suspend failure

As per the Exynos5250 User Manual:

When there are pending interrupt events, WFI/WFE instruction are ignored. To cancel the power-down sequence follow these steps:
1) Disable system power-down through CENTRAL_SEQ_CONFIGURATION and CENTRAL_SEQ_CONFIGURATION_DMC registers
2) Clear WAKEUP_STAT register
3) Enable interrupt service routine for CPU

Code for early wakeup for exynos already exists; remove the panic on suspend failure, clear the wakeup state register and return error from cpu_suspend to indicate a failed suspend (to a user daemon).

Older Samsung SoCs have similar panics and I have removed them all. Haven't touched the S3C2410 sleep code.

Signed-off-by Abhilash Kesavan <[email protected]>

---
 arch/arm/mach-exynos/pm.c          |    6 ++++--
 arch/arm/mach-s3c24xx/pm-s3c2412.c |    3 ++-
 arch/arm/mach-s3c24xx/pm-s3c2416.c |    3 ++-
 arch/arm/mach-s3c64xx/pm.c         |    3 ++-
 arch/arm/mach-s5p64x0/pm.c         |    4 ++--
 arch/arm/mach-s5pv210/pm.c         |    4 ++--
 arch/arm/plat-samsung/pm.c         |    5 ++++-
 7 files changed, 18 insertions(+), 10 deletions(-)

arch/arm/mach-exynos/pm.c +4 -2

index f459afd..55df38cd 100644
@@ -92,8 +92,8 @@ static int exynos_cpu_suspend(unsigned long arg)
 	/* issue the standby signal into the pm unit. */
 	cpu_do_idle();
 
-	/* we should never get past here */
-	panic("sleep resumed to originator?");
+	pr_err("Failed to suspend the system\n");
+	return -EBUSY;
 }
 
 static void exynos_pm_prepare(void)

@@ -283,6 +283,8 @@ static void exynos_pm_resume(void)
 	if (!(tmp & S5P_CENTRAL_LOWPWR_CFG)) {
 		tmp |= S5P_CENTRAL_LOWPWR_CFG;
 		__raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
+		/* clear the wakeup state register */
+		__raw_writel(0x0, S5P_WAKEUP_STAT);
 		/* No need to perform below restore code */
 		goto early_wakeup;
 	}

arch/arm/mach-s3c24xx/pm-s3c2412.c +2 -1

index 206765c..a7bc30d 100644
@@ -48,7 +48,8 @@ static int s3c2412_cpu_suspend(unsigned long arg)
 
 	s3c2412_sleep_enter();
 
-	panic("sleep resumed to originator?");
+	pr_err("Failed to suspend the system\n");
+	return -EBUSY;
 }
 
 static void s3c2412_pm_prepare(void)

arch/arm/mach-s3c24xx/pm-s3c2416.c +2 -1

index 1bd4817..97a8d70 100644
@@ -34,7 +34,8 @@ static int s3c2416_cpu_suspend(unsigned long arg)
 
 	s3c2412_sleep_enter();
 
-	panic("sleep resumed to originator?");
+	pr_err("Failed to suspend the system\n");
+	return -EBUSY;
 }
 
 static void s3c2416_pm_prepare(void)

arch/arm/mach-s3c64xx/pm.c +2 -1

index f6b13a2..4d1accc 100644
@@ -297,7 +297,8 @@ static int s3c64xx_cpu_suspend(unsigned long arg)
 
 	/* we should never get past here */
 
-	panic("sleep resumed to originator?");
+	pr_err("Failed to suspend the system\n");
+	return -EBUSY;
 }
 
 /* mapping of interrupts to parts of the wakeup mask */

arch/arm/mach-s5p64x0/pm.c +2 -2

index 9cba18b..710651d 100644
@@ -103,8 +103,8 @@ static int s5p64x0_cpu_suspend(unsigned long arg)
 	    "mcr p15, 0, %0, c7, c10, 4\n\t"
 	    "mcr p15, 0, %0, c7, c0, 4" : : "r" (tmp));
 
-	/* we should never get past here */
-	panic("sleep resumed to originator?");
+	pr_err("Failed to suspend the system\n");
+	return -EBUSY;
 }
 
 /* mapping of interrupts to parts of the wakeup mask */

arch/arm/mach-s5pv210/pm.c +2 -2

index 736bfb1..c584f49 100644
@@ -104,8 +104,8 @@ static int s5pv210_cpu_suspend(unsigned long arg)
 	    "mcr p15, 0, %0, c7, c10, 4\n\t"
 	    "wfi" : : "r" (tmp));
 
-	/* we should never get past here */
-	panic("sleep resumed to originator?");
+	pr_err("Failed to suspend the system\n");
+	return -EBUSY;
 }
 
 static void s5pv210_pm_prepare(void)

arch/arm/plat-samsung/pm.c +4 -1

index 1507028..aa4b06d 100644
@@ -243,6 +243,7 @@ int (*pm_cpu_sleep)(unsigned long);
 
 static int s3c_pm_enter(suspend_state_t state)
 {
+	int ret;
 	/* ensure the debug is initialised (if enabled) */
 
 	s3c_pm_debug_init();

@@ -300,7 +301,9 @@ static int s3c_pm_enter(suspend_state_t state)
 	 * we resume as it saves its own register state and restores it
 	 * during the resume.  */
 
-	cpu_suspend(0, pm_cpu_sleep);
+	ret = cpu_suspend(0, pm_cpu_sleep);
+	if (ret)
+		return ret;
 
 	/* restore the system state */

Signature · 1 line

1.7.9.5

Kyungmin Park

2013-01-09 19:46 UTC #1 lore raw

PATCH 1/1 ARM: SAMSUNG: Gracefully exit on suspend failure

Hi,
I wodner do you see the real case it's happened? It it's occured, does it better to find out the root cause?

Thank you,
Kyungmin Park

Quoted 155 lines · Abhilash Kesavan

As per the Exynos5250 User Manual:

Older Samsung SoCs have similar panics and I have removed them all. Haven't touched the S3C2410 sleep code.

Signed-off-by Abhilash Kesavan <[email protected]>

---
 arch/arm/mach-exynos/pm.c          |    6 ++++--
 arch/arm/mach-s3c24xx/pm-s3c2412.c |    3 ++-
 arch/arm/mach-s3c24xx/pm-s3c2416.c |    3 ++-
 arch/arm/mach-s3c64xx/pm.c         |    3 ++-
 arch/arm/mach-s5p64x0/pm.c         |    4 ++--
 arch/arm/mach-s5pv210/pm.c         |    4 ++--
 arch/arm/plat-samsung/pm.c         |    5 ++++-
 7 files changed, 18 insertions(+), 10 deletions(-)

arch/arm/mach-exynos/pm.c +4 -2

index f459afd..55df38cd 100644
@@ -92,8 +92,8 @@ static int exynos_cpu_suspend(unsigned long arg)
        /* issue the standby signal into the pm unit. */
        cpu_do_idle();
 
-       /* we should never get past here */
-       panic("sleep resumed to originator?");
+       pr_err("Failed to suspend the system\n");
+       return -EBUSY;
 }
 
 static void exynos_pm_prepare(void)

@@ -283,6 +283,8 @@ static void exynos_pm_resume(void)
        if (!(tmp & S5P_CENTRAL_LOWPWR_CFG)) {
                tmp |= S5P_CENTRAL_LOWPWR_CFG;
                __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
+               /* clear the wakeup state register */
+               __raw_writel(0x0, S5P_WAKEUP_STAT);
                /* No need to perform below restore code */
                goto early_wakeup;
        }

arch/arm/mach-s3c24xx/pm-s3c2412.c +2 -1

index 206765c..a7bc30d 100644
@@ -48,7 +48,8 @@ static int s3c2412_cpu_suspend(unsigned long arg)
 
        s3c2412_sleep_enter();
 
-       panic("sleep resumed to originator?");
+       pr_err("Failed to suspend the system\n");
+       return -EBUSY;
 }
 
 static void s3c2412_pm_prepare(void)

arch/arm/mach-s3c24xx/pm-s3c2416.c +2 -1

index 1bd4817..97a8d70 100644
@@ -34,7 +34,8 @@ static int s3c2416_cpu_suspend(unsigned long arg)
 
        s3c2412_sleep_enter();
 
-       panic("sleep resumed to originator?");
+       pr_err("Failed to suspend the system\n");
+       return -EBUSY;
 }
 
 static void s3c2416_pm_prepare(void)

arch/arm/mach-s3c64xx/pm.c +2 -1

index f6b13a2..4d1accc 100644
@@ -297,7 +297,8 @@ static int s3c64xx_cpu_suspend(unsigned long arg)
 
        /* we should never get past here */
 
-       panic("sleep resumed to originator?");
+       pr_err("Failed to suspend the system\n");
+       return -EBUSY;
 }
 
 /* mapping of interrupts to parts of the wakeup mask */

arch/arm/mach-s5p64x0/pm.c +2 -2

index 9cba18b..710651d 100644
@@ -103,8 +103,8 @@ static int s5p64x0_cpu_suspend(unsigned long arg)
            "mcr p15, 0, %0, c7, c10, 4\n\t"
            "mcr p15, 0, %0, c7, c0, 4" : : "r" (tmp));
 
-       /* we should never get past here */
-       panic("sleep resumed to originator?");
+       pr_err("Failed to suspend the system\n");
+       return -EBUSY;
 }
 
 /* mapping of interrupts to parts of the wakeup mask */

arch/arm/mach-s5pv210/pm.c +2 -2

index 736bfb1..c584f49 100644
@@ -104,8 +104,8 @@ static int s5pv210_cpu_suspend(unsigned long arg)
            "mcr p15, 0, %0, c7, c10, 4\n\t"
            "wfi" : : "r" (tmp));
 
-       /* we should never get past here */
-       panic("sleep resumed to originator?");
+       pr_err("Failed to suspend the system\n");
+       return -EBUSY;
 }
 
 static void s5pv210_pm_prepare(void)

arch/arm/plat-samsung/pm.c +4 -2

index 1507028..aa4b06d 100644
@@ -243,6 +243,7 @@ int (*pm_cpu_sleep)(unsigned long);
 
 static int s3c_pm_enter(suspend_state_t state)
 {
+       int ret;
        /* ensure the debug is initialised (if enabled) */
 
        s3c_pm_debug_init();

@@ -300,7 +301,9 @@ static int s3c_pm_enter(suspend_state_t state)
         * we resume as it saves its own register state and restores it
         * during the resume.  */
 
-       cpu_suspend(0, pm_cpu_sleep);
+       ret = cpu_suspend(0, pm_cpu_sleep);
+       if (ret)
+               return ret;
 
        /* restore the system state */
 
--

1.7.9.5

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Russell King - ARM Linux

2013-01-09 19:52 UTC #2 lore raw

PATCH 1/1 ARM: SAMSUNG: Gracefully exit on suspend failure

Quoted 3 lines · Kyungmin Park

Hi,
I wodner do you see the real case it's happened? It it's occured, does it better to find out the root cause?

No. It's better that if it does happen, the system remains functional so you _can_ debug why it happened. If you cause the system to panic then you have to find some other way to get information out of it.

Abhilash Kesavan

2013-01-18 13:10 UTC #3 lore raw

PATCH 1/1 ARM: SAMSUNG: Gracefully exit on suspend failure

In reply to Kyungmin Park

Hi,

Quoted 4 lines · Kyungmin Park

Hi,
I wodner do you see the real case it's happened? It it's occured, does it better to find out the root cause?

We have seen infrequent suspend failures on the Exynos5. This patch helps us obtain
failure statistics over a period of time and continue debugging the issue.

Quoted 2 lines

Thank you,
Kyungmin Park

Abhilash

Kukjin Kim

2013-01-10 00:32 UTC #4 lore raw

PATCH 1/1 ARM: SAMSUNG: Gracefully exit on suspend failure

In reply to Abhilash Kesavan

Quoted 34 lines · Abhilash Kesavan

As per the Exynos5250 User Manual:
When there are pending interrupt events, WFI/WFE instruction are ignored. To cancel the power-down sequence follow these steps:
1) Disable system power-down through CENTRAL_SEQ_CONFIGURATION and CENTRAL_SEQ_CONFIGURATION_DMC registers
2) Clear WAKEUP_STAT register
3) Enable interrupt service routine for CPU
Code for early wakeup for exynos already exists; remove the panic on suspend failure, clear the wakeup state register and return error from cpu_suspend to indicate a failed suspend (to a user daemon).
Older Samsung SoCs have similar panics and I have removed them all. Haven't touched the S3C2410 sleep code.
Signed-off-by Abhilash Kesavan <[email protected]>
---
 arch/arm/mach-exynos/pm.c          |    6 ++++--
 arch/arm/mach-s3c24xx/pm-s3c2412.c |    3 ++-
 arch/arm/mach-s3c24xx/pm-s3c2416.c |    3 ++-
 arch/arm/mach-s3c64xx/pm.c         |    3 ++-
 arch/arm/mach-s5p64x0/pm.c         |    4 ++--
 arch/arm/mach-s5pv210/pm.c         |    4 ++--
 arch/arm/plat-samsung/pm.c         |    5 ++++-
 7 files changed, 18 insertions(+), 10 deletions(-)
arch/arm/mach-exynos/pm.c +0 -0
index f459afd..55df38cd 100644

arch/arm/mach-exynos/pm.c static int exynos_cpu_suspend(unsigned long arg) · line 96

Quoted 7 lines

arch/arm/mach-exynos/pm.c +2 -2

@@ -92,8 +92,8 @@ static int exynos_cpu_suspend(unsigned long arg)
 	/* issue the standby signal into the pm unit. */
 	cpu_do_idle();
 
-	/* we should never get past here */
-	panic("sleep resumed to originator?");
+	pr_err("Failed to suspend the system\n");
+	return -EBUSY;

This can be happened under non-error situation, so this should be:

+ pr_info("sleep resumed to originator?");
+ return 1; /* abort suspend */

[...]

- Kukjin

Abhilash Kesavan

2013-01-18 13:10 UTC #5 lore raw

PATCH 1/1 ARM: SAMSUNG: Gracefully exit on suspend failure

Hi,

Review

Quoted 9 lines

arch/arm/mach-exynos/pm.c static int exynos_cpu_suspend(unsigned long arg) · line 96
Quoted 3 lines
arch/arm/mach-exynos/pm.c +2 -1
-     panic("sleep resumed to originator?");
+     pr_err("Failed to suspend the system\n");
+     return -EBUSY;
This can be happened under non-error situation, so this should be:
(quoted diff) +2 -0
+       pr_info("sleep resumed to originator?");
+       return 1; /* abort suspend */

Will re-post with suggested changes.

Abhilash

Abhilash Kesavan

author

2013-01-18 13:18 UTC #6 lore raw

PATCH v2 1/1 ARM: SAMSUNG: Gracefully exit on suspend failure

In reply to Abhilash Kesavan

As per the Exynos5250 User Manual:
When there are pending interrupt events, WFI/WFE instruction are ignored. To cancel the power-down sequence follow these steps:
1) Disable system power-down using CENTRAL_SEQ_CONFIGURATION register
2) Clear WAKEUP_STAT register
3) Enable interrupt service routine for CPU

Code for early wakeup for exynos already exists. Remove the panic on suspend failure, clear the wakeup state register and return 1 from cpu_suspend to indicate a failed suspend (to a user daemon).

Older Samsung SoCs have similar panics and I have removed them all. Haven't touched the S3C2410 sleep code.

Signed-off-by Abhilash Kesavan <[email protected]>

---
Changes since v1:
* Returned 1 instead of a specific error
* Used pr_info rather than pr_err

 arch/arm/mach-exynos/pm.c          |    6 ++++--
 arch/arm/mach-s3c24xx/pm-s3c2412.c |    3 ++-
 arch/arm/mach-s3c24xx/pm-s3c2416.c |    3 ++-
 arch/arm/mach-s3c64xx/pm.c         |    3 ++-
 arch/arm/mach-s5p64x0/pm.c         |    4 ++--
 arch/arm/mach-s5pv210/pm.c         |    4 ++--
 arch/arm/plat-samsung/pm.c         |    5 ++++-
 7 files changed, 18 insertions(+), 10 deletions(-)

arch/arm/mach-exynos/pm.c +4 -2

index f459afd..e3faaa81 100644
@@ -92,8 +92,8 @@ static int exynos_cpu_suspend(unsigned long arg)
 	/* issue the standby signal into the pm unit. */
 	cpu_do_idle();
 
-	/* we should never get past here */
-	panic("sleep resumed to originator?");
+	pr_info("Failed to suspend the system\n");
+	return 1; /* Aborting suspend */
 }
 
 static void exynos_pm_prepare(void)

@@ -283,6 +283,8 @@ static void exynos_pm_resume(void)
 	if (!(tmp & S5P_CENTRAL_LOWPWR_CFG)) {
 		tmp |= S5P_CENTRAL_LOWPWR_CFG;
 		__raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
+		/* clear the wakeup state register */
+		__raw_writel(0x0, S5P_WAKEUP_STAT);
 		/* No need to perform below restore code */
 		goto early_wakeup;
 	}

arch/arm/mach-s3c24xx/pm-s3c2412.c +2 -1

index 206765c..0f183ae 100644
@@ -48,7 +48,8 @@ static int s3c2412_cpu_suspend(unsigned long arg)
 
 	s3c2412_sleep_enter();
 
-	panic("sleep resumed to originator?");
+	pr_info("Failed to suspend the system\n");
+	return 1; /* Aborting suspend */
 }
 
 static void s3c2412_pm_prepare(void)

arch/arm/mach-s3c24xx/pm-s3c2416.c +2 -1

index 1bd4817..1a9e8dd 100644
@@ -34,7 +34,8 @@ static int s3c2416_cpu_suspend(unsigned long arg)
 
 	s3c2412_sleep_enter();
 
-	panic("sleep resumed to originator?");
+	pr_info("Failed to suspend the system\n");
+	return 1; /* Aborting suspend */
 }
 
 static void s3c2416_pm_prepare(void)

arch/arm/mach-s3c64xx/pm.c +2 -1

index f6b13a2..1006f3c 100644
@@ -297,7 +297,8 @@ static int s3c64xx_cpu_suspend(unsigned long arg)
 
 	/* we should never get past here */
 
-	panic("sleep resumed to originator?");
+	pr_info("Failed to suspend the system\n");
+	return 1; /* Aborting suspend */
 }
 
 /* mapping of interrupts to parts of the wakeup mask */

arch/arm/mach-s5p64x0/pm.c +2 -2

index 9cba18b..97c2a08a 100644
@@ -103,8 +103,8 @@ static int s5p64x0_cpu_suspend(unsigned long arg)
 	    "mcr p15, 0, %0, c7, c10, 4\n\t"
 	    "mcr p15, 0, %0, c7, c0, 4" : : "r" (tmp));
 
-	/* we should never get past here */
-	panic("sleep resumed to originator?");
+	pr_info("Failed to suspend the system\n");
+	return 1; /* Aborting suspend */
 }
 
 /* mapping of interrupts to parts of the wakeup mask */

arch/arm/mach-s5pv210/pm.c +2 -2

index 736bfb1..2b68a67 100644
@@ -104,8 +104,8 @@ static int s5pv210_cpu_suspend(unsigned long arg)
 	    "mcr p15, 0, %0, c7, c10, 4\n\t"
 	    "wfi" : : "r" (tmp));
 
-	/* we should never get past here */
-	panic("sleep resumed to originator?");
+	pr_info("Failed to suspend the system\n");
+	return 1; /* Aborting suspend */
 }
 
 static void s5pv210_pm_prepare(void)

arch/arm/plat-samsung/pm.c +4 -1

index 1507028..aa4b06d 100644
@@ -243,6 +243,7 @@ int (*pm_cpu_sleep)(unsigned long);
 
 static int s3c_pm_enter(suspend_state_t state)
 {
+	int ret;
 	/* ensure the debug is initialised (if enabled) */
 
 	s3c_pm_debug_init();

@@ -300,7 +301,9 @@ static int s3c_pm_enter(suspend_state_t state)
 	 * we resume as it saves its own register state and restores it
 	 * during the resume.  */
 
-	cpu_suspend(0, pm_cpu_sleep);
+	ret = cpu_suspend(0, pm_cpu_sleep);
+	if (ret)
+		return ret;
 
 	/* restore the system state */

Signature · 1 line

1.7.9.5

Kukjin Kim

2013-01-25 18:40 UTC #7 lore raw

PATCH v2 1/1 ARM: SAMSUNG: Gracefully exit on suspend failure

Quoted 13 lines · Abhilash Kesavan

As per the Exynos5250 User Manual:
When there are pending interrupt events, WFI/WFE instruction are ignored. To cancel the power-down sequence follow these steps:
1) Disable system power-down using CENTRAL_SEQ_CONFIGURATION register
2) Clear WAKEUP_STAT register
3) Enable interrupt service routine for CPU
Code for early wakeup for exynos already exists. Remove the panic on suspend failure, clear the wakeup state register and return 1 from cpu_suspend to indicate a failed suspend (to a user daemon).

Quoted 5 lines

Older Samsung SoCs have similar panics and I have removed them all. Haven't touched the S3C2410 sleep code.
Signed-off-by Abhilash Kesavan <[email protected]>
---

Looks OK to me, applied.

Thanks.

- Kukjin