[PATCH v2 0/5] cpuidle: exynos: add coupled cpuidle support for Exynos3250

[PATCH v2 0/5] cpuidle: exynos: add coupled cpuidle support for Exynos3250

[Bartlomiej Zolnierkiewicz]

Hi,

The following patchset adds coupled cpuidle support for Exynos3250
to an existing cpuidle-exynos driver.  As a result it enables AFTR
mode to be used by default on Exynos3250 without the need to hot
unplug CPU1 first.

The patchset depends on:
- for-next branch (commit: 77105c882ba6) of linux-samsung.git
  kernel tree
- "[PATCH] ARM: EXYNOS: Fix failed second suspend on Exynos4"
  (https://lkml.org/lkml/2015/2/3/481)
- "[PATCH v3 0/4] ARM: EXYNOS: cpuidle: add AFTR mode support for
  Exynos3250"
  (https://lkml.org/lkml/2015/3/18/282)

Changes since v1:
- rebased on top of for-next branch (commit: 77105c882ba6) of
  linux-samsung.git kernel tree

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


Bartlomiej Zolnierkiewicz (5):
  ARM: EXYNOS: fix exynos_boot_secondary() return value on timeout
  ARM: EXYNOS: make exynos_core_restart() less verbose
  ARM: EXYNOS: add exynos_set_boot_addr() helper
  ARM: EXYNOS: add exynos_get_boot_addr() helper
  cpuidle: exynos: add coupled cpuidle support for Exynos3250

 arch/arm/include/asm/firmware.h |  4 ++
 arch/arm/mach-exynos/common.h   |  4 +-
 arch/arm/mach-exynos/exynos.c   |  3 +-
 arch/arm/mach-exynos/firmware.c | 18 +++++++++
 arch/arm/mach-exynos/platsmp.c  | 86 ++++++++++++++++++++++++++++-------------
 arch/arm/mach-exynos/pm.c       | 51 ++++++++++++++++++++----
 6 files changed, 129 insertions(+), 37 deletions(-)

-- 
1.8.2.3

[PATCH v2 1/5] ARM: EXYNOS: fix exynos_boot_secondary() return value on timeout

[Bartlomiej Zolnierkiewicz]

exynos_boot_secondary() can erroneously return 0 or -ENOSYS even
when waiting on pen_release being set to -1 timeouts.  Fix it by
adjusting ret variable value to -ETIMEDOUT when necessary.

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 arch/arm/mach-exynos/platsmp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index a825bca..48f950d 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -337,6 +337,9 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
 		udelay(10);
 	}
 
+	if (pen_release != -1)
+		ret = -ETIMEDOUT;
+
 	/*
 	 * now the secondary core is starting up let it run its
 	 * calibrations, then wait for it to finish
-- 
1.8.2.3

[PATCH v2 2/5] ARM: EXYNOS: make exynos_core_restart() less verbose

[Bartlomiej Zolnierkiewicz]

There is a kernel message about secondary CPU bootup when
exynos_core_restart() is called through CPU hotplug code-path
(the only exynos_core_restart() user currently) so there is no
need for an extra info on Exynos3250 SoC about software reset.
This also prepares exynos_core_restart() to be re-used in
coupled cpuidle code-path in the future.

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 arch/arm/mach-exynos/platsmp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 48f950d..6f7a1c7 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -210,7 +210,6 @@ static void exynos_core_restart(u32 core_id)
 	val |= S5P_CORE_WAKEUP_FROM_LOCAL_CFG;
 	pmu_raw_writel(val, EXYNOS_ARM_CORE_STATUS(core_id));
 
-	pr_info("CPU%u: Software reset\n", core_id);
 	pmu_raw_writel(EXYNOS_CORE_PO_RESET(core_id), EXYNOS_SWRESET);
 }
 
-- 
1.8.2.3

[PATCH v2 3/5] ARM: EXYNOS: add exynos_set_boot_addr() helper

[Bartlomiej Zolnierkiewicz]

Add exynos_set_boot_addr() helper and covert existing code
(exynos_boot_secondary() and exynos_smp_prepare_cpus()) to
use it.

There should be no functional changes caused by this patch.

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 arch/arm/mach-exynos/platsmp.c | 53 +++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 6f7a1c7..b7cfcdc 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -247,6 +247,31 @@ static void exynos_secondary_init(unsigned int cpu)
 	spin_unlock(&boot_lock);
 }
 
+static int exynos_set_boot_addr(u32 core_id, unsigned long boot_addr)
+{
+	int ret;
+
+	/*
+	 * Try to set boot address using firmware first
+	 * and fall back to boot register if it fails.
+	 */
+	ret = call_firmware_op(set_cpu_boot_addr, core_id, boot_addr);
+	if (ret && ret != -ENOSYS)
+		goto fail;
+	if (ret == -ENOSYS) {
+		void __iomem *boot_reg = cpu_boot_reg(core_id);
+
+		if (IS_ERR(boot_reg)) {
+			ret = PTR_ERR(boot_reg);
+			goto fail;
+		}
+		__raw_writel(boot_addr, boot_reg);
+		ret = 0;
+	}
+fail:
+	return ret;
+}
+
 static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
 	unsigned long timeout;
@@ -306,22 +331,9 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
 
 		boot_addr = virt_to_phys(exynos4_secondary_startup);
 
-		/*
-		 * Try to set boot address using firmware first
-		 * and fall back to boot register if it fails.
-		 */
-		ret = call_firmware_op(set_cpu_boot_addr, core_id, boot_addr);
-		if (ret && ret != -ENOSYS)
+		ret = exynos_set_boot_addr(core_id, boot_addr);
+		if (ret)
 			goto fail;
-		if (ret == -ENOSYS) {
-			void __iomem *boot_reg = cpu_boot_reg(core_id);
-
-			if (IS_ERR(boot_reg)) {
-				ret = PTR_ERR(boot_reg);
-				goto fail;
-			}
-			__raw_writel(boot_addr, boot_reg);
-		}
 
 		call_firmware_op(cpu_boot, core_id);
 
@@ -409,16 +421,9 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
 		core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
 		boot_addr = virt_to_phys(exynos4_secondary_startup);
 
-		ret = call_firmware_op(set_cpu_boot_addr, core_id, boot_addr);
-		if (ret && ret != -ENOSYS)
+		ret = exynos_set_boot_addr(core_id, boot_addr);
+		if (ret)
 			break;
-		if (ret == -ENOSYS) {
-			void __iomem *boot_reg = cpu_boot_reg(core_id);
-
-			if (IS_ERR(boot_reg))
-				break;
-			__raw_writel(boot_addr, boot_reg);
-		}
 	}
 }
 
-- 
1.8.2.3

[PATCH v2 4/5] ARM: EXYNOS: add exynos_get_boot_addr() helper

[Bartlomiej Zolnierkiewicz]

Add get_cpu_boot_addr() firmware operation and then
exynos_get_boot_addr() helper.

This is a preparation for adding coupled cpuidle support
for Exynos3250 SoC.

There should be no functional changes caused by this patch.

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 arch/arm/include/asm/firmware.h |  4 ++++
 arch/arm/mach-exynos/firmware.c | 17 +++++++++++++++++
 arch/arm/mach-exynos/platsmp.c  | 25 +++++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
index 89aefe1..34c1d96 100644
--- a/arch/arm/include/asm/firmware.h
+++ b/arch/arm/include/asm/firmware.h
@@ -34,6 +34,10 @@ struct firmware_ops {
 	 */
 	int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr);
 	/*
+	 * Gets boot address of specified physical CPU
+	 */
+	int (*get_cpu_boot_addr)(int cpu, unsigned long *boot_addr);
+	/*
 	 * Boots specified physical CPU
 	 */
 	int (*cpu_boot)(int cpu);
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index f236877..c9e4481 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -104,6 +104,22 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
 	return 0;
 }
 
+static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
+{
+	void __iomem *boot_reg;
+
+	if (!sysram_ns_base_addr)
+		return -ENODEV;
+
+	boot_reg = sysram_ns_base_addr + 0x1c;
+
+	if (soc_is_exynos4412())
+		boot_reg += 4 * cpu;
+
+	*boot_addr = __raw_readl(boot_reg);
+	return 0;
+}
+
 static int exynos_cpu_suspend(unsigned long arg)
 {
 	flush_cache_all();
@@ -138,6 +154,7 @@ static int exynos_resume(void)
 static const struct firmware_ops exynos_firmware_ops = {
 	.do_idle		= IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
 	.set_cpu_boot_addr	= exynos_set_cpu_boot_addr,
+	.get_cpu_boot_addr	= exynos_get_cpu_boot_addr,
 	.cpu_boot		= exynos_cpu_boot,
 	.suspend		= IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
 	.resume			= IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index b7cfcdc..449edd1 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -272,6 +272,31 @@ fail:
 	return ret;
 }
 
+static int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr)
+{
+	int ret;
+
+	/*
+	 * Try to get boot address using firmware first
+	 * and fall back to boot register if it fails.
+	 */
+	ret = call_firmware_op(get_cpu_boot_addr, core_id, boot_addr);
+	if (ret && ret != -ENOSYS)
+		goto fail;
+	if (ret == -ENOSYS) {
+		void __iomem *boot_reg = cpu_boot_reg(core_id);
+
+		if (IS_ERR(boot_reg)) {
+			ret = PTR_ERR(boot_reg);
+			goto fail;
+		}
+		*boot_addr = __raw_readl(boot_reg);
+		ret = 0;
+	}
+fail:
+	return ret;
+}
+
 static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
 	unsigned long timeout;
-- 
1.8.2.3

[PATCH v2 5/5] cpuidle: exynos: add coupled cpuidle support for Exynos3250

[Bartlomiej Zolnierkiewicz]

The following patch adds coupled cpuidle support for Exynos3250 to
an existing cpuidle-exynos driver.  As a result it enables AFTR mode
to be used by default on Exynos3250 without the need to hot unplug
CPU1 first.

The detailed changelog:
- use exynos_[get,set]_boot_addr() in cpuidle-exynos.c and then make
  cpu_boot_reg_base() static
- use exynos_core_restart() in exynos_cpu0_enter_aftr()
- add missing smp_rmb() to exynos_cpu0_enter_aftr() (to make the code
  in-sync with the platform SMP code)
- add call_firmware_op(cpu_boot, 1) to exynos_cpu0_enter_aftr()
- use dsb_sev() instead of IPI wakeup for Exynos3250 in
  exynos_cpu0_enter_aftr()
- add CPU0 vs CPU1 synchronization based on S5P_PMU_SPARE2 register
  for Exynos3250 to cpuidle-exynos.c
- add flush_cache_all() for CPU1/0 before powerdown/AFTR for
  Exynos3250 to exynos_wfi_finisher()/exynos_do_idle()

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 arch/arm/mach-exynos/common.h   |  4 +++-
 arch/arm/mach-exynos/exynos.c   |  3 ++-
 arch/arm/mach-exynos/firmware.c |  1 +
 arch/arm/mach-exynos/platsmp.c  |  8 +++----
 arch/arm/mach-exynos/pm.c       | 51 ++++++++++++++++++++++++++++++++++-------
 5 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 826367f..bc515f0 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -163,7 +163,9 @@ extern void exynos_set_delayed_reset_assertion(bool enable);
 
 extern void s5p_init_cpu(void __iomem *cpuid_addr);
 extern unsigned int samsung_rev(void);
-extern void __iomem *cpu_boot_reg_base(void);
+extern void exynos_core_restart(u32 core_id);
+extern int exynos_set_boot_addr(u32 core_id, unsigned long boot_addr);
+extern int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr);
 
 static inline void pmu_raw_writel(u32 val, u32 offset)
 {
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index ec0312b..c4a2852 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -264,7 +264,8 @@ static void __init exynos_dt_machine_init(void)
 		exynos_sysram_init();
 
 #if defined(CONFIG_SMP) && defined(CONFIG_ARM_EXYNOS_CPUIDLE)
-	if (of_machine_is_compatible("samsung,exynos4210"))
+	if (of_machine_is_compatible("samsung,exynos4210") ||
+	    of_machine_is_compatible("samsung,exynos3250"))
 		exynos_cpuidle.dev.platform_data = &cpuidle_coupled_exynos_data;
 #endif
 	if (of_machine_is_compatible("samsung,exynos4210") ||
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index c9e4481..90db46f 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -49,6 +49,7 @@ static int exynos_do_idle(unsigned long mode)
 			     sysram_ns_base_addr + 0x24);
 		__raw_writel(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
 		if (soc_is_exynos3250()) {
+			flush_cache_all();
 			exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
 				   SMC_POWERSTATE_IDLE, 0);
 			exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 449edd1..58e05a2 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -169,7 +169,7 @@ int exynos_cluster_power_state(int cluster)
 		S5P_CORE_LOCAL_PWR_EN);
 }
 
-void __iomem *cpu_boot_reg_base(void)
+static void __iomem *cpu_boot_reg_base(void)
 {
 	if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
 		return pmu_base_addr + S5P_INFORM5;
@@ -195,7 +195,7 @@ static inline void __iomem *cpu_boot_reg(int cpu)
  *
  * Currently this is needed only when booting secondary CPU on Exynos3250.
  */
-static void exynos_core_restart(u32 core_id)
+void exynos_core_restart(u32 core_id)
 {
 	u32 val;
 
@@ -247,7 +247,7 @@ static void exynos_secondary_init(unsigned int cpu)
 	spin_unlock(&boot_lock);
 }
 
-static int exynos_set_boot_addr(u32 core_id, unsigned long boot_addr)
+int exynos_set_boot_addr(u32 core_id, unsigned long boot_addr)
 {
 	int ret;
 
@@ -272,7 +272,7 @@ fail:
 	return ret;
 }
 
-static int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr)
+int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr)
 {
 	int ret;
 
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index cc75ab4..9c1506b 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -22,6 +22,7 @@
 #include <asm/firmware.h>
 #include <asm/smp_scu.h>
 #include <asm/suspend.h>
+#include <asm/cacheflush.h>
 
 #include <mach/map.h>
 
@@ -209,6 +210,8 @@ static int exynos_cpu0_enter_aftr(void)
 		 * sequence, let's wait for one of these to happen
 		 */
 		while (exynos_cpu_power_state(1)) {
+			unsigned long boot_addr;
+
 			/*
 			 * The other cpu may skip idle and boot back
 			 * up again
@@ -221,7 +224,11 @@ static int exynos_cpu0_enter_aftr(void)
 			 * boot back up again, getting stuck in the
 			 * boot rom code
 			 */
-			if (__raw_readl(cpu_boot_reg_base()) == 0)
+			ret = exynos_get_boot_addr(1, &boot_addr);
+			if (ret)
+				goto fail;
+			ret = -1;
+			if (boot_addr == 0)
 				goto abort;
 
 			cpu_relax();
@@ -233,11 +240,14 @@ static int exynos_cpu0_enter_aftr(void)
 
 abort:
 	if (cpu_online(1)) {
+		unsigned long boot_addr = virt_to_phys(exynos_cpu_resume);
+
 		/*
 		 * Set the boot vector to something non-zero
 		 */
-		__raw_writel(virt_to_phys(exynos_cpu_resume),
-			     cpu_boot_reg_base());
+		ret = exynos_set_boot_addr(1, boot_addr);
+		if (ret)
+			goto fail;
 		dsb();
 
 		/*
@@ -247,22 +257,42 @@ abort:
 		while (exynos_cpu_power_state(1) != S5P_CORE_LOCAL_PWR_EN)
 			cpu_relax();
 
+		if (soc_is_exynos3250()) {
+			while (!pmu_raw_readl(S5P_PMU_SPARE2) &&
+			       !atomic_read(&cpu1_wakeup))
+				cpu_relax();
+
+			if (!atomic_read(&cpu1_wakeup))
+				exynos_core_restart(1);
+		}
+
 		while (!atomic_read(&cpu1_wakeup)) {
+			smp_rmb();
+
 			/*
 			 * Poke cpu1 out of the boot rom
 			 */
-			__raw_writel(virt_to_phys(exynos_cpu_resume),
-				     cpu_boot_reg_base());
 
-			arch_send_wakeup_ipi_mask(cpumask_of(1));
+			ret = exynos_set_boot_addr(1, boot_addr);
+			if (ret)
+				goto fail;
+
+			call_firmware_op(cpu_boot, 1);
+
+			if (soc_is_exynos3250())
+				dsb_sev();
+			else
+				arch_send_wakeup_ipi_mask(cpumask_of(1));
 		}
 	}
-
+fail:
 	return ret;
 }
 
 static int exynos_wfi_finisher(unsigned long flags)
 {
+	if (soc_is_exynos3250())
+		flush_cache_all();
 	cpu_do_idle();
 
 	return -1;
@@ -283,6 +313,9 @@ static int exynos_cpu1_powerdown(void)
 	 */
 	exynos_cpu_power_down(1);
 
+	if (soc_is_exynos3250())
+		pmu_raw_writel(0, S5P_PMU_SPARE2);
+
 	ret = cpu_suspend(0, exynos_wfi_finisher);
 
 	cpu_pm_exit();
@@ -299,7 +332,9 @@ cpu1_aborted:
 
 static void exynos_pre_enter_aftr(void)
 {
-	__raw_writel(virt_to_phys(exynos_cpu_resume), cpu_boot_reg_base());
+	unsigned long boot_addr = virt_to_phys(exynos_cpu_resume);
+
+	(void)exynos_set_boot_addr(1, boot_addr);
 }
 
 static void exynos_post_enter_aftr(void)
-- 
1.8.2.3

Re: [PATCH v2 4/5] ARM: EXYNOS: add exynos_get_boot_addr() helper

[Krzysztof Kozlowski]

2015-03-18 14:09 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> Add get_cpu_boot_addr() firmware operation and then
> exynos_get_boot_addr() helper.
>
> This is a preparation for adding coupled cpuidle support
> for Exynos3250 SoC.
>
> There should be no functional changes caused by this patch.
>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  arch/arm/include/asm/firmware.h |  4 ++++
>  arch/arm/mach-exynos/firmware.c | 17 +++++++++++++++++
>  arch/arm/mach-exynos/platsmp.c  | 25 +++++++++++++++++++++++++
>  3 files changed, 46 insertions(+)
>
> diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
> index 89aefe1..34c1d96 100644
> --- a/arch/arm/include/asm/firmware.h
> +++ b/arch/arm/include/asm/firmware.h
> @@ -34,6 +34,10 @@ struct firmware_ops {
>          */
>         int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr);
>         /*
> +        * Gets boot address of specified physical CPU
> +        */
> +       int (*get_cpu_boot_addr)(int cpu, unsigned long *boot_addr);
> +       /*
>          * Boots specified physical CPU
>          */
>         int (*cpu_boot)(int cpu);
> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> index f236877..c9e4481 100644
> --- a/arch/arm/mach-exynos/firmware.c
> +++ b/arch/arm/mach-exynos/firmware.c
> @@ -104,6 +104,22 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>         return 0;
>  }
>
> +static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
> +{
> +       void __iomem *boot_reg;
> +
> +       if (!sysram_ns_base_addr)
> +               return -ENODEV;
> +
> +       boot_reg = sysram_ns_base_addr + 0x1c;
> +
> +       if (soc_is_exynos4412())
> +               boot_reg += 4 * cpu;
> +
> +       *boot_addr = __raw_readl(boot_reg);
> +       return 0;
> +}
> +
>  static int exynos_cpu_suspend(unsigned long arg)
>  {
>         flush_cache_all();
> @@ -138,6 +154,7 @@ static int exynos_resume(void)
>  static const struct firmware_ops exynos_firmware_ops = {
>         .do_idle                = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
>         .set_cpu_boot_addr      = exynos_set_cpu_boot_addr,
> +       .get_cpu_boot_addr      = exynos_get_cpu_boot_addr,
>         .cpu_boot               = exynos_cpu_boot,
>         .suspend                = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
>         .resume                 = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index b7cfcdc..449edd1 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -272,6 +272,31 @@ fail:
>         return ret;
>  }
>
> +static int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr)
> +{
> +       int ret;
> +
> +       /*
> +        * Try to get boot address using firmware first
> +        * and fall back to boot register if it fails.
> +        */
> +       ret = call_firmware_op(get_cpu_boot_addr, core_id, boot_addr);
> +       if (ret && ret != -ENOSYS)
> +               goto fail;

The exynos_get_cpu_boot_addr() returns ENODEV. Do you really want to
check for ENOSYS here?

> +       if (ret == -ENOSYS) {
> +               void __iomem *boot_reg = cpu_boot_reg(core_id);
> +
> +               if (IS_ERR(boot_reg)) {
> +                       ret = PTR_ERR(boot_reg);
> +                       goto fail;
> +               }

Re: [PATCH v2 3/5] ARM: EXYNOS: add exynos_set_boot_addr() helper

[Krzysztof Kozlowski]

2015-03-18 14:09 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> Add exynos_set_boot_addr() helper and covert existing code
> (exynos_boot_secondary() and exynos_smp_prepare_cpus()) to
> use it.
>
> There should be no functional changes caused by this patch.
>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  arch/arm/mach-exynos/platsmp.c | 53 +++++++++++++++++++++++-------------------
>  1 file changed, 29 insertions(+), 24 deletions(-)

Looks good.
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH v2 4/5] ARM: EXYNOS: add exynos_get_boot_addr() helper

[Bartlomiej Zolnierkiewicz]

On Wednesday, March 18, 2015 02:43:49 PM Krzysztof Kozlowski wrote:
> 2015-03-18 14:09 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> > Add get_cpu_boot_addr() firmware operation and then
> > exynos_get_boot_addr() helper.
> >
> > This is a preparation for adding coupled cpuidle support
> > for Exynos3250 SoC.
> >
> > There should be no functional changes caused by this patch.
> >
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  arch/arm/include/asm/firmware.h |  4 ++++
> >  arch/arm/mach-exynos/firmware.c | 17 +++++++++++++++++
> >  arch/arm/mach-exynos/platsmp.c  | 25 +++++++++++++++++++++++++
> >  3 files changed, 46 insertions(+)
> >
> > diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
> > index 89aefe1..34c1d96 100644
> > --- a/arch/arm/include/asm/firmware.h
> > +++ b/arch/arm/include/asm/firmware.h
> > @@ -34,6 +34,10 @@ struct firmware_ops {
> >          */
> >         int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr);
> >         /*
> > +        * Gets boot address of specified physical CPU
> > +        */
> > +       int (*get_cpu_boot_addr)(int cpu, unsigned long *boot_addr);
> > +       /*
> >          * Boots specified physical CPU
> >          */
> >         int (*cpu_boot)(int cpu);
> > diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> > index f236877..c9e4481 100644
> > --- a/arch/arm/mach-exynos/firmware.c
> > +++ b/arch/arm/mach-exynos/firmware.c
> > @@ -104,6 +104,22 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> >         return 0;
> >  }
> >
> > +static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
> > +{
> > +       void __iomem *boot_reg;
> > +
> > +       if (!sysram_ns_base_addr)
> > +               return -ENODEV;
> > +
> > +       boot_reg = sysram_ns_base_addr + 0x1c;
> > +
> > +       if (soc_is_exynos4412())
> > +               boot_reg += 4 * cpu;
> > +
> > +       *boot_addr = __raw_readl(boot_reg);
> > +       return 0;
> > +}
> > +
> >  static int exynos_cpu_suspend(unsigned long arg)
> >  {
> >         flush_cache_all();
> > @@ -138,6 +154,7 @@ static int exynos_resume(void)
> >  static const struct firmware_ops exynos_firmware_ops = {
> >         .do_idle                = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
> >         .set_cpu_boot_addr      = exynos_set_cpu_boot_addr,
> > +       .get_cpu_boot_addr      = exynos_get_cpu_boot_addr,
> >         .cpu_boot               = exynos_cpu_boot,
> >         .suspend                = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
> >         .resume                 = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
> > diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> > index b7cfcdc..449edd1 100644
> > --- a/arch/arm/mach-exynos/platsmp.c
> > +++ b/arch/arm/mach-exynos/platsmp.c
> > @@ -272,6 +272,31 @@ fail:
> >         return ret;
> >  }
> >
> > +static int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr)
> > +{
> > +       int ret;
> > +
> > +       /*
> > +        * Try to get boot address using firmware first
> > +        * and fall back to boot register if it fails.
> > +        */
> > +       ret = call_firmware_op(get_cpu_boot_addr, core_id, boot_addr);
> > +       if (ret && ret != -ENOSYS)
> > +               goto fail;
> 
> The exynos_get_cpu_boot_addr() returns ENODEV. Do you really want to
> check for ENOSYS here?

Yes.  When secure firmware is not present on the board the call_firmware_op()
itself returns -ENOSYS (exynos_get_cpu_boot_addr() is not called in this
case) and it needs to be handled in a special way.

> > +       if (ret == -ENOSYS) {
> > +               void __iomem *boot_reg = cpu_boot_reg(core_id);
> > +
> > +               if (IS_ERR(boot_reg)) {
> > +                       ret = PTR_ERR(boot_reg);
> > +                       goto fail;
> > +               }

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH v2 4/5] ARM: EXYNOS: add exynos_get_boot_addr() helper

[Krzysztof Kozlowski]

On śro, 2015-03-18 at 16:18 +0100, Bartlomiej Zolnierkiewicz wrote:
> On Wednesday, March 18, 2015 02:43:49 PM Krzysztof Kozlowski wrote:
> > 2015-03-18 14:09 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> > > Add get_cpu_boot_addr() firmware operation and then
> > > exynos_get_boot_addr() helper.
> > >
> > > This is a preparation for adding coupled cpuidle support
> > > for Exynos3250 SoC.
> > >
> > > There should be no functional changes caused by this patch.
> > >
> > > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > > ---
> > >  arch/arm/include/asm/firmware.h |  4 ++++
> > >  arch/arm/mach-exynos/firmware.c | 17 +++++++++++++++++
> > >  arch/arm/mach-exynos/platsmp.c  | 25 +++++++++++++++++++++++++
> > >  3 files changed, 46 insertions(+)
> > >
> > > diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
> > > index 89aefe1..34c1d96 100644
> > > --- a/arch/arm/include/asm/firmware.h
> > > +++ b/arch/arm/include/asm/firmware.h
> > > @@ -34,6 +34,10 @@ struct firmware_ops {
> > >          */
> > >         int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr);
> > >         /*
> > > +        * Gets boot address of specified physical CPU
> > > +        */
> > > +       int (*get_cpu_boot_addr)(int cpu, unsigned long *boot_addr);
> > > +       /*
> > >          * Boots specified physical CPU
> > >          */
> > >         int (*cpu_boot)(int cpu);
> > > diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> > > index f236877..c9e4481 100644
> > > --- a/arch/arm/mach-exynos/firmware.c
> > > +++ b/arch/arm/mach-exynos/firmware.c
> > > @@ -104,6 +104,22 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> > >         return 0;
> > >  }
> > >
> > > +static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
> > > +{
> > > +       void __iomem *boot_reg;
> > > +
> > > +       if (!sysram_ns_base_addr)
> > > +               return -ENODEV;
> > > +
> > > +       boot_reg = sysram_ns_base_addr + 0x1c;
> > > +
> > > +       if (soc_is_exynos4412())
> > > +               boot_reg += 4 * cpu;
> > > +
> > > +       *boot_addr = __raw_readl(boot_reg);
> > > +       return 0;
> > > +}
> > > +
> > >  static int exynos_cpu_suspend(unsigned long arg)
> > >  {
> > >         flush_cache_all();
> > > @@ -138,6 +154,7 @@ static int exynos_resume(void)
> > >  static const struct firmware_ops exynos_firmware_ops = {
> > >         .do_idle                = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
> > >         .set_cpu_boot_addr      = exynos_set_cpu_boot_addr,
> > > +       .get_cpu_boot_addr      = exynos_get_cpu_boot_addr,
> > >         .cpu_boot               = exynos_cpu_boot,
> > >         .suspend                = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
> > >         .resume                 = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
> > > diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> > > index b7cfcdc..449edd1 100644
> > > --- a/arch/arm/mach-exynos/platsmp.c
> > > +++ b/arch/arm/mach-exynos/platsmp.c
> > > @@ -272,6 +272,31 @@ fail:
> > >         return ret;
> > >  }
> > >
> > > +static int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr)
> > > +{
> > > +       int ret;
> > > +
> > > +       /*
> > > +        * Try to get boot address using firmware first
> > > +        * and fall back to boot register if it fails.
> > > +        */
> > > +       ret = call_firmware_op(get_cpu_boot_addr, core_id, boot_addr);
> > > +       if (ret && ret != -ENOSYS)
> > > +               goto fail;
> > 
> > The exynos_get_cpu_boot_addr() returns ENODEV. Do you really want to
> > check for ENOSYS here?
> 
> Yes.  When secure firmware is not present on the board the call_firmware_op()
> itself returns -ENOSYS (exynos_get_cpu_boot_addr() is not called in this
> case) and it needs to be handled in a special way.

Right, my mistake. Rest looks good:

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH v2 0/5] cpuidle: exynos: add coupled cpuidle support for Exynos3250

[Bartlomiej Zolnierkiewicz]

On Wednesday, March 18, 2015 02:09:52 PM Bartlomiej Zolnierkiewicz wrote:
> Hi,
> 
> The following patchset adds coupled cpuidle support for Exynos3250
> to an existing cpuidle-exynos driver.  As a result it enables AFTR
> mode to be used by default on Exynos3250 without the need to hot
> unplug CPU1 first.
> 
> The patchset depends on:
> - for-next branch (commit: 77105c882ba6) of linux-samsung.git
>   kernel tree
> - "[PATCH] ARM: EXYNOS: Fix failed second suspend on Exynos4"
>   (https://lkml.org/lkml/2015/2/3/481)
> - "[PATCH v3 0/4] ARM: EXYNOS: cpuidle: add AFTR mode support for
>   Exynos3250"
>   (https://lkml.org/lkml/2015/3/18/282)

This patchset also applies fine on top of v4 of the above patchset:
"[PATCH v4 0/4] ARM: EXYNOS: cpuidle: add AFTR mode support for Exynos3250"
(https://lkml.org/lkml/2015/3/18/505).

> Changes since v1:
> - rebased on top of for-next branch (commit: 77105c882ba6) of
>   linux-samsung.git kernel tree
> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
> 
> Bartlomiej Zolnierkiewicz (5):
>   ARM: EXYNOS: fix exynos_boot_secondary() return value on timeout
>   ARM: EXYNOS: make exynos_core_restart() less verbose
>   ARM: EXYNOS: add exynos_set_boot_addr() helper
>   ARM: EXYNOS: add exynos_get_boot_addr() helper
>   cpuidle: exynos: add coupled cpuidle support for Exynos3250
> 
>  arch/arm/include/asm/firmware.h |  4 ++
>  arch/arm/mach-exynos/common.h   |  4 +-
>  arch/arm/mach-exynos/exynos.c   |  3 +-
>  arch/arm/mach-exynos/firmware.c | 18 +++++++++
>  arch/arm/mach-exynos/platsmp.c  | 86 ++++++++++++++++++++++++++++-------------
>  arch/arm/mach-exynos/pm.c       | 51 ++++++++++++++++++++----
>  6 files changed, 129 insertions(+), 37 deletions(-)

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics