- using arch_send_wakeup_ipi_mask() instead of dsb_sev()
(this matches CPU hotplug code in arch/arm/mach-exynos/platsmp.c)
I am curious. You experienced very rare hangs after running the tests a
few hours, right ? Is the SEV replaced by the IPI solving the issue ? If
yes, how did you catch it ?
Rare hangs showed up after about 30-40 minutes of testing with the attached
app and script (running of "./cpuidle_state1_test.sh script 2 500" has never
completed on the umodified driver).
The problem turned out to be in the following loop waiting for CPU1 to get
stuck in the BOOT ROM:
/*
* Wait for cpu1 to get stuck in the boot rom
*/
while ((__raw_readl(BOOT_VECTOR) != 0) &&
!atomic_read(&cpu1_wakeup))
cpu_relax();
[ Removal of the loop fixed the problem. ]
Using the SEV instead of the IPI was not a issue but it was changed to
match the existing Exynos platform code (exynos_boot_secondary() in
arch/arm/mach-exynos/platsmp.c) and as preparation for Exynos4412 (quad
core) support.
+#ifdef CONFIG_ARM_EXYNOS_CPUIDLE
+ if (of_machine_is_compatible("samsung,exynos4210"))
+ exynos_cpuidle.dev.platform_data = &cpuidle_coupled_exynos_data;
+#endif
You should not add those #ifdef.
Without those #ifdef I get:
LD init/built-in.o
arch/arm/mach-exynos/built-in.o: In function `exynos_dt_machine_init':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/exynos.c:334: undefined reference to `cpuidle_coupled_exynos_data'
make: *** [vmlinux] Error 1
when CONFIG_EXYNOS_CPU_SUSPEND is disabled.
if (of_machine_is_compatible("samsung,exynos4210") ||
of_machine_is_compatible("samsung,exynos4212") ||
(of_machine_is_compatible("samsung,exynos4412") &&
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
- exynos_enter_aftr = (void *)(pdev->dev.platform_data);
+ if (of_machine_is_compatible("samsung,exynos4210")) {
+ exynos_cpuidle_pdata = pdev->dev.platform_data;
+
+ exynos_idle_driver.states[1].enter =
+ exynos_enter_coupled_lowpower;
+ exynos_idle_driver.states[1].exit_latency = 5000;
+ exynos_idle_driver.states[1].target_residency = 10000;
+ exynos_idle_driver.states[1].flags |= CPUIDLE_FLAG_COUPLED |
+ CPUIDLE_FLAG_TIMER_STOP;
I tried to remove those dynamic state allocation everywhere in the
different drivers. I would prefer to have another cpuidle_driver to be
registered with its states instead of overwriting the existing idle state.
struct cpuidle_driver exynos4210_idle_driver = {
.name = "exynos4210_idle",
.owner = THIS_MODULE,
.states = {
[0] = ARM_CPUIDLE_WFI_STATE,
[1] = {
.enter = exynos_enter_coupled_lowpower,
.exit_latency = 5000,
.target_residency = 10000,
.flags = CPUIDLE_FLAG_TIME_VALID |
CPUIDLE_FLAG_COUPLED |
CPUIDLE_FLAG_TIMER_STOP,
.name = "C1",
.desc = "ARM power down",
},
}
};
and then:
if (of_machine_is_compatible("samsung,exynos4210")) {
...
ret = cpuidle_register(&exynos4210_idle_driver,
cpu_online_mask);
...
}
...
OK, I will fix it but (if you are OK with it) I will make the code use
"exynos_coupled" naming instead of "exynos4210" one to not have to change
it later.
If we can reuse this mechanism, which I believe it is possible to, for
4420 and 5250. Then we will be able to refactor this out again.
I plan to add support for Exynos3250 next as it should be the simplest
(it is also dual core) and I need it for other reasons anyway. Exynos4412
(quad core) support requires more work but should also be doable.
When it comes to Exynos5250 I was thinking about disabling normal AFTR
mode support for it as according to my testing (on Arndale board) it has
never worked (at least in upstream kernels, I don't know about Linaro or
internal ones).