[PATCH v3 0/4] ARM: EXYNOS: cpuidle: add AFTR mode support for Exynos3250

[PATCH v3 0/4] ARM: EXYNOS: cpuidle: add AFTR mode support for Exynos3250

[Bartlomiej Zolnierkiewicz]

Hi,

This patch series adds support for AFTR idle mode on boards with
Exynos3250 SoC and allows EXYNOS cpuidle driver usage on these
boards.

It has been tested on Samsung Rinato board (Gear 2).

Depends on:
- for-next branch (commit: 77105c882ba6) of linux-samsung.git
  kernel tree

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

Changes since v1:
- rebased on top of for-next branch (commit: ce275c369a0b) of
  linux-samsung.git kernel tree
- fixed lockup on hotplug by using dsb_sev() instead of IPI in
  exynos_boot_secondary() on Exynos3250

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


Bartlomiej Zolnierkiewicz (4):
  ARM: EXYNOS: fix CPU1 hotplug for AFTR mode on Exynos3250
  ARM: EXYNOS: add code for setting/clearing boot flag
  ARM: EXYNOS: cpuidle: add AFTR mode support for Exynos3250
  ARM: EXYNOS: cpuidle: allow driver usage on Exynos3250 SoC

 arch/arm/mach-exynos/common.h   |  6 ++++++
 arch/arm/mach-exynos/exynos.c   | 26 ++++++++++++++++++++++++++
 arch/arm/mach-exynos/firmware.c |  8 +++++++-
 arch/arm/mach-exynos/platsmp.c  | 23 ++++++++++++++++++++---
 arch/arm/mach-exynos/pm.c       | 12 +++++++++++-
 arch/arm/mach-exynos/regs-pmu.h |  3 +++
 arch/arm/mach-exynos/smc.h      |  9 +++++++++
 7 files changed, 82 insertions(+), 5 deletions(-)

-- 
1.8.2.3

[PATCH v3 1/4] ARM: EXYNOS: fix CPU1 hotplug for AFTR mode on Exynos3250

[Bartlomiej Zolnierkiewicz]

CPU1 hotplug may hang when AFTR is used.  Fix it by:
- setting AUTOWAKEUP_EN bit in ARM_COREx_CONFIGURATION register in
  exynos_cpu_power_up()
- not clearing reserved bits of ARM_COREx_CONFIGURATION register in
  exynos_cpu_power_down()
- waiting while an undocumented register 0x0908 becomes non-zero in
  exynos_core_restart()
- using dsb_sev() instead of IPI in exynos_boot_secondary() on
  Exynos3250

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-exynos/platsmp.c  | 23 ++++++++++++++++++++---
 arch/arm/mach-exynos/regs-pmu.h |  2 ++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index d2e9f12..ebd135b 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -126,6 +126,8 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
  */
 void exynos_cpu_power_down(int cpu)
 {
+	u32 core_conf;
+
 	if (cpu == 0 && (soc_is_exynos5420() || soc_is_exynos5800())) {
 		/*
 		 * Bypass power down for CPU0 during suspend. Check for
@@ -137,7 +139,10 @@ void exynos_cpu_power_down(int cpu)
 		if (!(val & S5P_CORE_LOCAL_PWR_EN))
 			return;
 	}
-	pmu_raw_writel(0, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
+
+	core_conf = pmu_raw_readl(EXYNOS_ARM_CORE_CONFIGURATION(cpu));
+	core_conf &= ~S5P_CORE_LOCAL_PWR_EN;
+	pmu_raw_writel(core_conf, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
 }
 
 /**
@@ -148,7 +153,12 @@ void exynos_cpu_power_down(int cpu)
  */
 void exynos_cpu_power_up(int cpu)
 {
-	pmu_raw_writel(S5P_CORE_LOCAL_PWR_EN,
+	u32 core_conf = S5P_CORE_LOCAL_PWR_EN;
+
+	if (soc_is_exynos3250())
+		core_conf |= S5P_CORE_AUTOWAKEUP_EN;
+
+	pmu_raw_writel(core_conf,
 			EXYNOS_ARM_CORE_CONFIGURATION(cpu));
 }
 
@@ -226,6 +236,10 @@ static void exynos_core_restart(u32 core_id)
 	if (!of_machine_is_compatible("samsung,exynos3250"))
 		return;
 
+	while (!pmu_raw_readl(S5P_PMU_SPARE2))
+		udelay(10);
+	udelay(10);
+
 	val = pmu_raw_readl(EXYNOS_ARM_CORE_STATUS(core_id));
 	val |= S5P_CORE_WAKEUP_FROM_LOCAL_CFG;
 	pmu_raw_writel(val, EXYNOS_ARM_CORE_STATUS(core_id));
@@ -346,7 +360,10 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
 
 		call_firmware_op(cpu_boot, core_id);
 
-		arch_send_wakeup_ipi_mask(cpumask_of(cpu));
+		if (soc_is_exynos3250())
+			dsb_sev();
+		else
+			arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 
 		if (pen_release == -1)
 			break;
diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
index eb461e1..84ddce1 100644
--- a/arch/arm/mach-exynos/regs-pmu.h
+++ b/arch/arm/mach-exynos/regs-pmu.h
@@ -49,6 +49,7 @@
 #define S5P_INFORM5				0x0814
 #define S5P_INFORM6				0x0818
 #define S5P_INFORM7				0x081C
+#define S5P_PMU_SPARE2				0x0908
 #define S5P_PMU_SPARE3				0x090C
 
 #define EXYNOS_IROM_DATA2			0x0988
@@ -182,6 +183,7 @@
 
 #define S5P_CORE_LOCAL_PWR_EN			0x3
 #define S5P_CORE_WAKEUP_FROM_LOCAL_CFG		(0x3 << 8)
+#define S5P_CORE_AUTOWAKEUP_EN			(1 << 31)
 
 /* Only for EXYNOS4210 */
 #define S5P_CMU_CLKSTOP_LCD1_LOWPWR	0x1154
-- 
1.8.2.3

[PATCH v3 1/4] ARM: EXYNOS: fix CPU1 hotplug for AFTR mode on Exynos3250

[Krzysztof Kozlowski]

2015-03-18 13:51 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> CPU1 hotplug may hang when AFTR is used.  Fix it by:
> - setting AUTOWAKEUP_EN bit in ARM_COREx_CONFIGURATION register in
>   exynos_cpu_power_up()
> - not clearing reserved bits of ARM_COREx_CONFIGURATION register in
>   exynos_cpu_power_down()
> - waiting while an undocumented register 0x0908 becomes non-zero in
>   exynos_core_restart()
> - using dsb_sev() instead of IPI in exynos_boot_secondary() on
>   Exynos3250
>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/mach-exynos/platsmp.c  | 23 ++++++++++++++++++++---
>  arch/arm/mach-exynos/regs-pmu.h |  2 ++
>  2 files changed, 22 insertions(+), 3 deletions(-)


Looks good (except one nit below) and this also fixes hotplug issues
during resume from S2R:
$ echo mem > /sys/power/state
[  156.517266] Disabling non-boot CPUs ...
[  156.517781] IRQ18 no longer affine to CPU1
[  156.518043] CPU1: shutdown
[  156.544718] Enabling non-boot CPUs ...
[  156.554925] CPU1: Software reset
[  158.552631] CPU1: failed to come online
[  158.552753] Error taking CPU1 up: -5

Reviewed and tested on Rinato (Gear 2/Exynos 3250) board:

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

One comment below...

>
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index d2e9f12..ebd135b 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -126,6 +126,8 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
>   */
>  void exynos_cpu_power_down(int cpu)
>  {
> +       u32 core_conf;
> +
>         if (cpu == 0 && (soc_is_exynos5420() || soc_is_exynos5800())) {
>                 /*
>                  * Bypass power down for CPU0 during suspend. Check for
> @@ -137,7 +139,10 @@ void exynos_cpu_power_down(int cpu)
>                 if (!(val & S5P_CORE_LOCAL_PWR_EN))
>                         return;
>         }
> -       pmu_raw_writel(0, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> +
> +       core_conf = pmu_raw_readl(EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> +       core_conf &= ~S5P_CORE_LOCAL_PWR_EN;
> +       pmu_raw_writel(core_conf, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
>  }
>
>  /**
> @@ -148,7 +153,12 @@ void exynos_cpu_power_down(int cpu)
>   */
>  void exynos_cpu_power_up(int cpu)
>  {
> -       pmu_raw_writel(S5P_CORE_LOCAL_PWR_EN,
> +       u32 core_conf = S5P_CORE_LOCAL_PWR_EN;
> +
> +       if (soc_is_exynos3250())
> +               core_conf |= S5P_CORE_AUTOWAKEUP_EN;
> +
> +       pmu_raw_writel(core_conf,
>                         EXYNOS_ARM_CORE_CONFIGURATION(cpu));
>  }
>
> @@ -226,6 +236,10 @@ static void exynos_core_restart(u32 core_id)
>         if (!of_machine_is_compatible("samsung,exynos3250"))
>                 return;
>
> +       while (!pmu_raw_readl(S5P_PMU_SPARE2))
> +               udelay(10);
> +       udelay(10);

We really need to start documenting this. Please add short description
why this SPARE2 check is here and who uses it. Without documenting
this behavior future generations won't be able to debug this stuff.
Imagine replacing sboot with uboot by someone...

Best regards,
Krzysztof

[PATCH v3 1/4] ARM: EXYNOS: fix CPU1 hotplug for AFTR mode on Exynos3250

[Bartlomiej Zolnierkiewicz]

Hi,

On Wednesday, March 18, 2015 02:10:31 PM Krzysztof Kozlowski wrote:
> 2015-03-18 13:51 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> > CPU1 hotplug may hang when AFTR is used.  Fix it by:
> > - setting AUTOWAKEUP_EN bit in ARM_COREx_CONFIGURATION register in
> >   exynos_cpu_power_up()
> > - not clearing reserved bits of ARM_COREx_CONFIGURATION register in
> >   exynos_cpu_power_down()
> > - waiting while an undocumented register 0x0908 becomes non-zero in
> >   exynos_core_restart()
> > - using dsb_sev() instead of IPI in exynos_boot_secondary() on
> >   Exynos3250
> >
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  arch/arm/mach-exynos/platsmp.c  | 23 ++++++++++++++++++++---
> >  arch/arm/mach-exynos/regs-pmu.h |  2 ++
> >  2 files changed, 22 insertions(+), 3 deletions(-)
> 
> 
> Looks good (except one nit below) and this also fixes hotplug issues
> during resume from S2R:
> $ echo mem > /sys/power/state
> [  156.517266] Disabling non-boot CPUs ...
> [  156.517781] IRQ18 no longer affine to CPU1
> [  156.518043] CPU1: shutdown
> [  156.544718] Enabling non-boot CPUs ...
> [  156.554925] CPU1: Software reset
> [  158.552631] CPU1: failed to come online
> [  158.552753] Error taking CPU1 up: -5
> 
> Reviewed and tested on Rinato (Gear 2/Exynos 3250) board:
> 
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Thank you!

> One comment below...
> 
> >
> > diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> > index d2e9f12..ebd135b 100644
> > --- a/arch/arm/mach-exynos/platsmp.c
> > +++ b/arch/arm/mach-exynos/platsmp.c
> > @@ -126,6 +126,8 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
> >   */
> >  void exynos_cpu_power_down(int cpu)
> >  {
> > +       u32 core_conf;
> > +
> >         if (cpu == 0 && (soc_is_exynos5420() || soc_is_exynos5800())) {
> >                 /*
> >                  * Bypass power down for CPU0 during suspend. Check for
> > @@ -137,7 +139,10 @@ void exynos_cpu_power_down(int cpu)
> >                 if (!(val & S5P_CORE_LOCAL_PWR_EN))
> >                         return;
> >         }
> > -       pmu_raw_writel(0, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> > +
> > +       core_conf = pmu_raw_readl(EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> > +       core_conf &= ~S5P_CORE_LOCAL_PWR_EN;
> > +       pmu_raw_writel(core_conf, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> >  }
> >
> >  /**
> > @@ -148,7 +153,12 @@ void exynos_cpu_power_down(int cpu)
> >   */
> >  void exynos_cpu_power_up(int cpu)
> >  {
> > -       pmu_raw_writel(S5P_CORE_LOCAL_PWR_EN,
> > +       u32 core_conf = S5P_CORE_LOCAL_PWR_EN;
> > +
> > +       if (soc_is_exynos3250())
> > +               core_conf |= S5P_CORE_AUTOWAKEUP_EN;
> > +
> > +       pmu_raw_writel(core_conf,
> >                         EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> >  }
> >
> > @@ -226,6 +236,10 @@ static void exynos_core_restart(u32 core_id)
> >         if (!of_machine_is_compatible("samsung,exynos3250"))
> >                 return;
> >
> > +       while (!pmu_raw_readl(S5P_PMU_SPARE2))
> > +               udelay(10);
> > +       udelay(10);
> 
> We really need to start documenting this. Please add short description
> why this SPARE2 check is here and who uses it. Without documenting
> this behavior future generations won't be able to debug this stuff.
> Imagine replacing sboot with uboot by someone...

I've already planned to do this for this code and for coupled cpuidle
use of SPARE2 as well.  However I would really prefer to do it in
an incremental patch if there are no other issues with this patchset.

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

[PATCH v3 1/4] ARM: EXYNOS: fix CPU1 hotplug for AFTR mode on Exynos3250

[Krzysztof Kozlowski]

On ?ro, 2015-03-18 at 14:23 +0100, Bartlomiej Zolnierkiewicz wrote:
> Hi,
> 
> On Wednesday, March 18, 2015 02:10:31 PM Krzysztof Kozlowski wrote:
> > 2015-03-18 13:51 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> > >
> > > +       while (!pmu_raw_readl(S5P_PMU_SPARE2))
> > > +               udelay(10);
> > > +       udelay(10);
> > 
> > We really need to start documenting this. Please add short description
> > why this SPARE2 check is here and who uses it. Without documenting
> > this behavior future generations won't be able to debug this stuff.
> > Imagine replacing sboot with uboot by someone...
> 
> I've already planned to do this for this code and for coupled cpuidle
> use of SPARE2 as well.  However I would really prefer to do it in
> an incremental patch if there are no other issues with this patchset.

OK, please do so in incremental patch. Usage of various memory regions
of sysram also should be documented. In patch 2 you add usage of 0x28 +
4*cpu. The various sysrams regions are spread over different files...

Best regards,
Krzysztof

[PATCH v3 1/4] ARM: EXYNOS: fix CPU1 hotplug for AFTR mode on Exynos3250

[Bartlomiej Zolnierkiewicz]

On Wednesday, March 18, 2015 02:32:25 PM Krzysztof Kozlowski wrote:
> On ?ro, 2015-03-18 at 14:23 +0100, Bartlomiej Zolnierkiewicz wrote:
> > Hi,
> > 
> > On Wednesday, March 18, 2015 02:10:31 PM Krzysztof Kozlowski wrote:
> > > 2015-03-18 13:51 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> > > >
> > > > +       while (!pmu_raw_readl(S5P_PMU_SPARE2))
> > > > +               udelay(10);
> > > > +       udelay(10);
> > > 
> > > We really need to start documenting this. Please add short description
> > > why this SPARE2 check is here and who uses it. Without documenting
> > > this behavior future generations won't be able to debug this stuff.
> > > Imagine replacing sboot with uboot by someone...
> > 
> > I've already planned to do this for this code and for coupled cpuidle
> > use of SPARE2 as well.  However I would really prefer to do it in
> > an incremental patch if there are no other issues with this patchset.
> 
> OK, please do so in incremental patch. Usage of various memory regions

OK.

> of sysram also should be documented. In patch 2 you add usage of 0x28 +
> 4*cpu. The various sysrams regions are spread over different files...

I completely agree that there should be some Documentation file with
the SYSRAM layout.  I'll do it later unless someone beats me to it (please
also add this to our internal TODO list of open issues, thanks!).

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

[PATCH v3 2/4] ARM: EXYNOS: add code for setting/clearing boot flag

[Bartlomiej Zolnierkiewicz]

This code is needed for cpuidle (W-)AFTR mode support on Exynos3250.

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-exynos/common.h |  6 ++++++
 arch/arm/mach-exynos/exynos.c | 25 +++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index f70eca7..87bf1f3 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -119,6 +119,12 @@ extern void __iomem *sysram_base_addr;
 extern void __iomem *pmu_base_addr;
 void exynos_sysram_init(void);
 
+/* CPU BOOT mode flag */
+#define C2_STATE	(1 << 3)
+
+void exynos_set_boot_flag(unsigned int cpu, unsigned int mode);
+void exynos_clear_boot_flag(unsigned int cpu, unsigned int mode);
+
 enum {
 	FW_DO_IDLE_SLEEP,
 	FW_DO_IDLE_AFTR,
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 4031a96..90ed2e5 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -117,6 +117,31 @@ static void __init exynos_init_late(void)
 	exynos_pm_init();
 }
 
+#define REG_CPU_STATE_ADDR	(sysram_ns_base_addr + 0x28)
+#define BOOT_MODE_MASK		0x1f
+
+void exynos_set_boot_flag(unsigned int cpu, unsigned int mode)
+{
+	unsigned int tmp;
+
+	tmp = __raw_readl(REG_CPU_STATE_ADDR + cpu * 4);
+
+	if (mode & BOOT_MODE_MASK)
+		tmp &= ~BOOT_MODE_MASK;
+
+	tmp |= mode;
+	__raw_writel(tmp, REG_CPU_STATE_ADDR + cpu * 4);
+}
+
+void exynos_clear_boot_flag(unsigned int cpu, unsigned int mode)
+{
+	unsigned int tmp;
+
+	tmp = __raw_readl(REG_CPU_STATE_ADDR + cpu * 4);
+	tmp &= ~mode;
+	__raw_writel(tmp, REG_CPU_STATE_ADDR + cpu * 4);
+}
+
 static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
 					int depth, void *data)
 {
-- 
1.8.2.3

[PATCH v3 2/4] ARM: EXYNOS: add code for setting/clearing boot flag

[Krzysztof Kozlowski]

2015-03-18 13:51 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> This code is needed for cpuidle (W-)AFTR mode support on Exynos3250.
>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/mach-exynos/common.h |  6 ++++++
>  arch/arm/mach-exynos/exynos.c | 25 +++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index f70eca7..87bf1f3 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -119,6 +119,12 @@ extern void __iomem *sysram_base_addr;
>  extern void __iomem *pmu_base_addr;
>  void exynos_sysram_init(void);
>
> +/* CPU BOOT mode flag */
> +#define C2_STATE       (1 << 3)

Is this "C2_STATE" like ACPI C2 state? It looks specific to Exynos3250
boot loader so maybe describe its real purpose?

> +
> +void exynos_set_boot_flag(unsigned int cpu, unsigned int mode);
> +void exynos_clear_boot_flag(unsigned int cpu, unsigned int mode);
> +
>  enum {
>         FW_DO_IDLE_SLEEP,
>         FW_DO_IDLE_AFTR,
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 4031a96..90ed2e5 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -117,6 +117,31 @@ static void __init exynos_init_late(void)
>         exynos_pm_init();
>  }
>
> +#define REG_CPU_STATE_ADDR     (sysram_ns_base_addr + 0x28)
> +#define BOOT_MODE_MASK         0x1f
> +
> +void exynos_set_boot_flag(unsigned int cpu, unsigned int mode)
> +{
> +       unsigned int tmp;
> +
> +       tmp = __raw_readl(REG_CPU_STATE_ADDR + cpu * 4);
> +
> +       if (mode & BOOT_MODE_MASK)
> +               tmp &= ~BOOT_MODE_MASK;
> +
> +       tmp |= mode;
> +       __raw_writel(tmp, REG_CPU_STATE_ADDR + cpu * 4);
> +}
> +
> +void exynos_clear_boot_flag(unsigned int cpu, unsigned int mode)
> +{
> +       unsigned int tmp;
> +
> +       tmp = __raw_readl(REG_CPU_STATE_ADDR + cpu * 4);
> +       tmp &= ~mode;
> +       __raw_writel(tmp, REG_CPU_STATE_ADDR + cpu * 4);
> +}
> +

Shouldn't these to functions be put in firmware.c? The
exynos_set_cpu_boot_addr() is there already. It would be consistent to
have them in one place.

Best regards,
Krzysztof

[PATCH v3 2/4] ARM: EXYNOS: add code for setting/clearing boot flag

[Bartlomiej Zolnierkiewicz]

On Wednesday, March 18, 2015 02:33:54 PM Krzysztof Kozlowski wrote:
> 2015-03-18 13:51 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> > This code is needed for cpuidle (W-)AFTR mode support on Exynos3250.
> >
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  arch/arm/mach-exynos/common.h |  6 ++++++
> >  arch/arm/mach-exynos/exynos.c | 25 +++++++++++++++++++++++++
> >  2 files changed, 31 insertions(+)
> >
> > diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> > index f70eca7..87bf1f3 100644
> > --- a/arch/arm/mach-exynos/common.h
> > +++ b/arch/arm/mach-exynos/common.h
> > @@ -119,6 +119,12 @@ extern void __iomem *sysram_base_addr;
> >  extern void __iomem *pmu_base_addr;
> >  void exynos_sysram_init(void);
> >
> > +/* CPU BOOT mode flag */
> > +#define C2_STATE       (1 << 3)
> 
> Is this "C2_STATE" like ACPI C2 state? It looks specific to Exynos3250
> boot loader so maybe describe its real purpose?

According to my knowledge it is not like ACPI C2 but my knowledge is
limited since this state is not documented anywhere.  I can add a comment
about it being currently limited to Exynos3250 though.

> > +
> > +void exynos_set_boot_flag(unsigned int cpu, unsigned int mode);
> > +void exynos_clear_boot_flag(unsigned int cpu, unsigned int mode);
> > +
> >  enum {
> >         FW_DO_IDLE_SLEEP,
> >         FW_DO_IDLE_AFTR,
> > diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> > index 4031a96..90ed2e5 100644
> > --- a/arch/arm/mach-exynos/exynos.c
> > +++ b/arch/arm/mach-exynos/exynos.c
> > @@ -117,6 +117,31 @@ static void __init exynos_init_late(void)
> >         exynos_pm_init();
> >  }
> >
> > +#define REG_CPU_STATE_ADDR     (sysram_ns_base_addr + 0x28)
> > +#define BOOT_MODE_MASK         0x1f
> > +
> > +void exynos_set_boot_flag(unsigned int cpu, unsigned int mode)
> > +{
> > +       unsigned int tmp;
> > +
> > +       tmp = __raw_readl(REG_CPU_STATE_ADDR + cpu * 4);
> > +
> > +       if (mode & BOOT_MODE_MASK)
> > +               tmp &= ~BOOT_MODE_MASK;
> > +
> > +       tmp |= mode;
> > +       __raw_writel(tmp, REG_CPU_STATE_ADDR + cpu * 4);
> > +}
> > +
> > +void exynos_clear_boot_flag(unsigned int cpu, unsigned int mode)
> > +{
> > +       unsigned int tmp;
> > +
> > +       tmp = __raw_readl(REG_CPU_STATE_ADDR + cpu * 4);
> > +       tmp &= ~mode;
> > +       __raw_writel(tmp, REG_CPU_STATE_ADDR + cpu * 4);
> > +}
> > +
> 
> Shouldn't these to functions be put in firmware.c? The
> exynos_set_cpu_boot_addr() is there already. It would be consistent to
> have them in one place.

I can move it to firmware.c if this is desired.

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

[PATCH v3 3/4] ARM: EXYNOS: cpuidle: add AFTR mode support for Exynos3250

[Bartlomiej Zolnierkiewicz]

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-exynos/firmware.c |  8 +++++++-
 arch/arm/mach-exynos/pm.c       | 12 +++++++++++-
 arch/arm/mach-exynos/regs-pmu.h |  1 +
 arch/arm/mach-exynos/smc.h      |  9 +++++++++
 4 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index 4791a3c..f236877 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -48,7 +48,13 @@ static int exynos_do_idle(unsigned long mode)
 		__raw_writel(virt_to_phys(exynos_cpu_resume_ns),
 			     sysram_ns_base_addr + 0x24);
 		__raw_writel(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
-		exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
+		if (soc_is_exynos3250()) {
+			exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
+				   SMC_POWERSTATE_IDLE, 0);
+			exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
+				   SMC_POWERSTATE_IDLE, 0);
+		} else
+			exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
 		break;
 	case FW_DO_IDLE_SLEEP:
 		exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 5685250..cc75ab4 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -127,6 +127,8 @@ int exynos_pm_central_resume(void)
 static void exynos_set_wakeupmask(long mask)
 {
 	pmu_raw_writel(mask, S5P_WAKEUP_MASK);
+	if (soc_is_exynos3250())
+		pmu_raw_writel(0x0, S5P_WAKEUP_MASK2);
 }
 
 static void exynos_cpu_set_boot_vector(long flags)
@@ -140,7 +142,7 @@ static int exynos_aftr_finisher(unsigned long flags)
 {
 	int ret;
 
-	exynos_set_wakeupmask(0x0000ff3e);
+	exynos_set_wakeupmask(soc_is_exynos3250() ? 0x40003ffe : 0x0000ff3e);
 	/* Set value of power down register for aftr mode */
 	exynos_sys_powerdown_conf(SYS_AFTR);
 
@@ -157,8 +159,13 @@ static int exynos_aftr_finisher(unsigned long flags)
 
 void exynos_enter_aftr(void)
 {
+	unsigned int cpuid = smp_processor_id();
+
 	cpu_pm_enter();
 
+	if (soc_is_exynos3250())
+		exynos_set_boot_flag(cpuid, C2_STATE);
+
 	exynos_pm_central_suspend();
 
 	if (of_machine_is_compatible("samsung,exynos4212") ||
@@ -178,6 +185,9 @@ void exynos_enter_aftr(void)
 
 	exynos_pm_central_resume();
 
+	if (soc_is_exynos3250())
+		exynos_clear_boot_flag(cpuid, C2_STATE);
+
 	cpu_pm_exit();
 }
 
diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
index 84ddce1..b761433 100644
--- a/arch/arm/mach-exynos/regs-pmu.h
+++ b/arch/arm/mach-exynos/regs-pmu.h
@@ -43,6 +43,7 @@
 #define S5P_WAKEUP_STAT				0x0600
 #define S5P_EINT_WAKEUP_MASK			0x0604
 #define S5P_WAKEUP_MASK				0x0608
+#define S5P_WAKEUP_MASK2				0x0614
 
 #define S5P_INFORM0				0x0800
 #define S5P_INFORM1				0x0804
diff --git a/arch/arm/mach-exynos/smc.h b/arch/arm/mach-exynos/smc.h
index f7b82f9..27a976d 100644
--- a/arch/arm/mach-exynos/smc.h
+++ b/arch/arm/mach-exynos/smc.h
@@ -17,6 +17,8 @@
 #define SMC_CMD_SLEEP		(-3)
 #define SMC_CMD_CPU1BOOT	(-4)
 #define SMC_CMD_CPU0AFTR	(-5)
+#define SMC_CMD_SAVE           	(-6)
+#define SMC_CMD_SHUTDOWN       	(-7)
 /* For CP15 Access */
 #define SMC_CMD_C15RESUME	(-11)
 /* For L2 Cache Access */
@@ -32,4 +34,11 @@ extern void exynos_smc(u32 cmd, u32 arg1, u32 arg2, u32 arg3);
 
 #endif /* __ASSEMBLY__ */
 
+/* op type for SMC_CMD_SAVE and SMC_CMD_SHUTDOWN */
+#define OP_TYPE_CORE            0x0
+#define OP_TYPE_CLUSTER         0x1
+
+/* Power State required for SMC_CMD_SAVE and SMC_CMD_SHUTDOWN */
+#define SMC_POWERSTATE_IDLE     0x1
+
 #endif
-- 
1.8.2.3

[PATCH v3 3/4] ARM: EXYNOS: cpuidle: add AFTR mode support for Exynos3250

[Krzysztof Kozlowski]

2015-03-18 13:51 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:

The patchset itself looks good... but it's missing commit message.
What benefits does the AFTR bring?

Best regards,
Krzysztof

[PATCH v3 3/4] ARM: EXYNOS: cpuidle: add AFTR mode support for Exynos3250

[Bartlomiej Zolnierkiewicz]

On Wednesday, March 18, 2015 02:38:26 PM Krzysztof Kozlowski wrote:
> 2015-03-18 13:51 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> 
> The patchset itself looks good... but it's missing commit message.
> What benefits does the AFTR bring?

AFTR support brings reduced energy consumption and is a prerequisite
for more advanced W-AFTR/LPA power saving modes.  AFTR has been already
supported on other Exynos SoCs for few years so there is really no need
to explain its purpose with every new SoC support addition.

[ Moreover you know this all really well since we've worked together on
  many Exynos Power Management issues :). ]

I understand that you are complaining about skimpy commit message and
I can certainly improve it (however personally I think that it will not
bring much benefit because the patch is rather straightforward one).

Please also note that while I appreciate review comments please try to
make them more substantial and bring them earlier (this patchset hasn't
really changed since v1 which was posted in October 2014).

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

[PATCH v3 4/4] ARM: EXYNOS: cpuidle: allow driver usage on Exynos3250 SoC

[Bartlomiej Zolnierkiewicz]

Register cpuidle platform device on Exynos3250 SoC allowing EXYNOS
cpuidle driver usage on this SoC.

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

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 90ed2e5..204f0e6 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -244,6 +244,7 @@ static void __init exynos_dt_machine_init(void)
 	    of_machine_is_compatible("samsung,exynos4212") ||
 	    (of_machine_is_compatible("samsung,exynos4412") &&
 	     of_machine_is_compatible("samsung,trats2")) ||
+	    of_machine_is_compatible("samsung,exynos3250") ||
 	    of_machine_is_compatible("samsung,exynos5250"))
 		platform_device_register(&exynos_cpuidle);
 
-- 
1.8.2.3

[PATCH v3 4/4] ARM: EXYNOS: cpuidle: allow driver usage on Exynos3250 SoC

[Krzysztof Kozlowski]

2015-03-18 13:51 GMT+01:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>:
> Register cpuidle platform device on Exynos3250 SoC allowing EXYNOS
> cpuidle driver usage on this SoC.
>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/mach-exynos/exynos.c | 1 +
>  1 file changed, 1 insertion(+)

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

Best regards,
Krzysztof