exynos4: cpuidle does never enter AFTR state with CONFIG_THUMB2_KERNEL=y

Bug #1171382 reported by Daniel Lezcano
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Linaro PMWG Kernel
Confirmed
High
Chander Mohan Kashyap

Bug Description

Despite the cpuidle framework choose the state1 for exynos which is the AFTR state, the cpu_suspend returns always true, meaning the AFTR shutdown sequence has been aborted and the cpu0 was never off.

How to reproduce ?

Go to the tag: tracking-mainline-linux-linaro-core-3.7-20130103.0

Add a trace in the cpuidle driver:

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index cff0595..73b3d17 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -113,7 +113,8 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
        __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);

        cpu_pm_enter();
- cpu_suspend(0, idle_finisher);
+ if (!cpu_suspend(0, idle_finisher))
+ printk(KERN_ERR "cpu_suspend succeed\n");

 #ifdef CONFIG_SMP
        scu_enable(S5P_VA_SCU);

Offline cpu1 and wait for this trace, it will never come.

Amit Kucheria (amitk)
Changed in linaro-power-kernel:
importance: Undecided → High
assignee: nobody → Chander Mohan Kashyap (chander-kashyap)
Revision history for this message
Chander Mohan Kashyap (chander-kashyap) wrote :

It is working as per expectations. cpu_suspend never returns in successful scenario. To test it please do the following:
1:
Apply the following patch: By this patch we can see the actual numbers of AFTER state.

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index cff0595..3bcf7c2 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -85,9 +85,11 @@ static void restore_cpu_arch_register(void)
        return;
 }

+static int counter;
 static int idle_finisher(unsigned long flags)
 {
        cpu_do_idle();
+ counter --;
        return 1;
 }

@@ -112,6 +114,7 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
        tmp &= ~S5P_CENTRAL_LOWPWR_CFG;
        __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);

+ counter ++;
        cpu_pm_enter();
        cpu_suspend(0, idle_finisher);

@@ -119,7 +122,7 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
        scu_enable(S5P_VA_SCU);
 #endif
        cpu_pm_exit();
-
+ printk("\n Actual Counter = %d \n", counter);
        restore_cpu_arch_register();

        /*

2:
Applying this patch we can confirm that AFTR works, as system will hang when it enters the AFTR state.

--- a/arch/arm/plat-samsung/s5p-sleep.S
+++ b/arch/arm/plat-samsung/s5p-sleep.S
@@ -71,6 +71,7 @@ ENTRY(s3c_cpu_resume)
        str r2, [r1, #L2X0_CTRL]
 resume_l2on:
 #endif
+ b .
        b cpu_resume
 ENDPROC(s3c_cpu_resume)
 #ifdef CONFIG_CACHE_L2X0

Revision history for this message
Daniel Lezcano (daniel-lezcano) wrote :

The kernel does not enter AFTR mode if it was compiled with CONFIG_THUMB2_KERNEL=y
Without this option, it enters AFTR successfully.

Revision history for this message
Amit Kucheria (amitk) wrote : Re: [Linaro-pm-wg] [Bug 1171382] Re: exynos4: cpuidle does never enter AFTR state

Tixy, Any theories?

On Mon, Apr 22, 2013 at 6:45 PM, Daniel Lezcano
<email address hidden> wrote:
> The kernel does not enter AFTR mode if it was compiled with CONFIG_THUMB2_KERNEL=y
> Without this option, it enters AFTR successfully.

Revision history for this message
Daniel Lezcano (daniel-lezcano) wrote :

Changed title to add the information above

summary: - exynos4: cpuidle does never enter AFTR state
+ exynos4: cpuidle does never enter AFTR state with CONFIG_THUMB2_KERNEL=y
Revision history for this message
Tixy (Jon Medhurst) (tixy) wrote :

On Mon, 2013-04-22 at 18:56 +0530, Amit Kucheria wrote:
> Tixy, Any theories?

Theories:

1. The value of s3c_cpu_resume will have bit 0 set to indicate Thumb
code and the bootloader doesn't cope with that, e.g. when looking for
the magic 0x2bedf00d value before that address.

2. The bootloader calls s3c_cpu_resume in ARM mode, not thumb mode. Not
likely, because in ARMv7 I believe most (all?) methods of branching to
an address now switch from ARM to Thumb mode if bit zero is set. If
s3c_cpu_resume is getting called in ARM mode you could try pinching the
code from the boot entrypoint 'stext' from arch/arm/kernel/head.S. I.e.
replace "ENTRY(s3c_cpu_resume)" in s5p-sleep.S with:

 .arm
ENTRY(s3c_cpu_resume)
 THUMB( adr r9, BSYM(1f) ) @ Kernel is always entered in ARM.
 THUMB( bx r9 ) @ If this is a Thumb-2 kernel,
 THUMB( .thumb ) @ switch to Thumb now.
 THUMB(1:

The above will also be needed if theory 1. is the problem and you work
around it by clearing bit zero of the address.

3. Another (unlikely) theory is that the "b ." in the code in bug
comment #1 may not do what we expect, you could try:

1:
 b 1b

--
Tixy

Revision history for this message
Chander Mohan Kashyap (chander-kashyap) wrote :

Hi Daniel,
Apply this patch to yours tag version. It is alreay in current linaro linux.

diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
index c3f825b..d9712fe 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -99,13 +99,7 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
                if (cpu == 1)
                        __raw_writel(0, S5P_ARM_CORE1_CONFIGURATION);

- /*
- * here's the WFI
- */
- asm(".word 0xe320f003\n"
- :
- :
- : "memory", "cc");
+ wfi();

                if (pen_release == cpu_logical_map(cpu)) {
                        /*
--

Revision history for this message
Chander Mohan Kashyap (chander-kashyap) wrote :

exynos4 cpuidle on current linux-linaro fails. Add following patches to fix.

Revision history for this message
Chander Mohan Kashyap (chander-kashyap) wrote :
Changed in linaro-power-kernel:
status: New → Confirmed
Revision history for this message
Daniel Lezcano (daniel-lezcano) wrote :

The encoded value for WFI has been changed to the wfi instruction.

Adding link to https://cards.linaro.org/browse/PMWG-451 for the record.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers