[PATCH 0/4] add cpuidle support for Exynos5420

[PATCH 0/4] add cpuidle support for Exynos5420

[Chander Kashyap]

Exynos5420 is a big-little Soc from Samsung. It has 4 A15 and 4 A7 cores.

This patchset adds cpuidle support for Exynos5420 SoC based on
generic big.little cpuidle driver.

Tested on SMDK5420.

This patch set depends on:
	1. [PATCH 0/5] MCPM backend for Exynos5420
	   http://www.spinics.net/lists/arm-kernel/msg321666.html

	2. [PATCH v4] arm: exynos: generalize power register address calculation
	   http://www.spinics.net/lists/arm-kernel/msg324024.html
		
Chander Kashyap (4):
  cpuidle: config: Add SOC_EXYNOS5420 entry to select
    cpuidle-big-little driver
  driver: cpuidle: cpuidle-big-little: init driver for Exynos5420
  exynos: cpuidle: do not allow cpuidle registration for Exynos5420
  mcpm: exynos: populate suspend and powered_up callbacks

 arch/arm/mach-exynos/cpuidle.c       |    3 ++
 arch/arm/mach-exynos/mcpm-exynos.c   |   53 ++++++++++++++++++++++++++++++++++
 drivers/cpuidle/Kconfig.arm          |    2 +-
 drivers/cpuidle/cpuidle-big_little.c |    3 +-
 4 files changed, 59 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH 1/4] cpuidle: config: Add SOC_EXYNOS5420 entry to select cpuidle-big-little driver

[Chander Kashyap]

Exynos5420 is a big-little SoC from Samsung. It has 4 A15 and 4 A7 cores.
In order to use generic cpuidle-big-little driver, this patch adds Exynos5420
specific check to initialize generic cpuidle driver.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
 drivers/cpuidle/Kconfig.arm |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 97ccc31..5244d87 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -4,7 +4,7 @@
 
 config ARM_BIG_LITTLE_CPUIDLE
 	bool "Support for ARM big.LITTLE processors"
-	depends on ARCH_VEXPRESS_TC2_PM
+	depends on ARCH_VEXPRESS_TC2_PM || SOC_EXYNOS5420
 	select ARM_CPU_SUSPEND
 	select CPU_IDLE_MULTIPLE_DRIVERS
 	help
-- 
1.7.9.5

[PATCH 1/4] cpuidle: config: Add SOC_EXYNOS5420 entry to select cpuidle-big-little driver

[Daniel Lezcano]

On 04/21/2014 01:49 PM, Chander Kashyap wrote:
> Exynos5420 is a big-little SoC from Samsung. It has 4 A15 and 4 A7 cores.
> In order to use generic cpuidle-big-little driver, this patch adds Exynos5420
> specific check to initialize generic cpuidle driver.
>
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
> ---
>   drivers/cpuidle/Kconfig.arm |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> index 97ccc31..5244d87 100644
> --- a/drivers/cpuidle/Kconfig.arm
> +++ b/drivers/cpuidle/Kconfig.arm
> @@ -4,7 +4,7 @@
>
>   config ARM_BIG_LITTLE_CPUIDLE
>   	bool "Support for ARM big.LITTLE processors"
> -	depends on ARCH_VEXPRESS_TC2_PM
> +	depends on ARCH_VEXPRESS_TC2_PM || SOC_EXYNOS5420

For the sake of consistency, I would prefer:

depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS

and let the current code (and future platform driver) to handle the 
loading of the driver.

>   	select ARM_CPU_SUSPEND
>   	select CPU_IDLE_MULTIPLE_DRIVERS
>   	help
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 1/4] cpuidle: config: Add SOC_EXYNOS5420 entry to select cpuidle-big-little driver

[Chander Kashyap]

Hi Daniel,

On 22 April 2014 16:12, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> On 04/21/2014 01:49 PM, Chander Kashyap wrote:
>>
>> Exynos5420 is a big-little SoC from Samsung. It has 4 A15 and 4 A7 cores.
>> In order to use generic cpuidle-big-little driver, this patch adds
>> Exynos5420
>> specific check to initialize generic cpuidle driver.
>>
>> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
>> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
>> ---
>>   drivers/cpuidle/Kconfig.arm |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
>> index 97ccc31..5244d87 100644
>> --- a/drivers/cpuidle/Kconfig.arm
>> +++ b/drivers/cpuidle/Kconfig.arm
>> @@ -4,7 +4,7 @@
>>
>>   config ARM_BIG_LITTLE_CPUIDLE
>>         bool "Support for ARM big.LITTLE processors"
>> -       depends on ARCH_VEXPRESS_TC2_PM
>> +       depends on ARCH_VEXPRESS_TC2_PM || SOC_EXYNOS5420
>
>
> For the sake of consistency, I would prefer:
>
> depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS

Yes i will change it.

Thanks
>
> and let the current code (and future platform driver) to handle the loading
> of the driver.
>
>
>>         select ARM_CPU_SUSPEND
>>         select CPU_IDLE_MULTIPLE_DRIVERS
>>         help
>>
>
>
> --
>  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>



-- 
with warm regards,
Chander Kashyap

[PATCH 2/4] driver: cpuidle: cpuidle-big-little: init driver for Exynos5420

[Chander Kashyap]

Add "samsung,exynos5420" compatible string to initialize generic
big-little cpuidle driver for Exynos5420.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.org>
---
 drivers/cpuidle/cpuidle-big_little.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
index b45fc62..d0fac53 100644
--- a/drivers/cpuidle/cpuidle-big_little.c
+++ b/drivers/cpuidle/cpuidle-big_little.c
@@ -170,7 +170,8 @@ static int __init bl_idle_init(void)
 	/*
 	 * Initialize the driver just for a compliant set of machines
 	 */
-	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7"))
+	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7") &&
+		(!of_machine_is_compatible("samsung,exynos5420")))
 		return -ENODEV;
 	/*
 	 * For now the differentiation between little and big cores
-- 
1.7.9.5

[PATCH 2/4] driver: cpuidle: cpuidle-big-little: init driver for Exynos5420

[Daniel Lezcano]

On 04/21/2014 01:49 PM, Chander Kashyap wrote:
> Add "samsung,exynos5420" compatible string to initialize generic
> big-little cpuidle driver for Exynos5420.
>
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Chander Kashyap <k.chander@samsung.org>
> ---

To be migrated to platform_driver but until that:

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

>   drivers/cpuidle/cpuidle-big_little.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
> index b45fc62..d0fac53 100644
> --- a/drivers/cpuidle/cpuidle-big_little.c
> +++ b/drivers/cpuidle/cpuidle-big_little.c
> @@ -170,7 +170,8 @@ static int __init bl_idle_init(void)
>   	/*
>   	 * Initialize the driver just for a compliant set of machines
>   	 */
> -	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7"))
> +	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7") &&
> +		(!of_machine_is_compatible("samsung,exynos5420")))
>   		return -ENODEV;
>   	/*
>   	 * For now the differentiation between little and big cores
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 2/4] driver: cpuidle: cpuidle-big-little: init driver for Exynos5420

[Daniel Lezcano]

On 04/21/2014 01:49 PM, Chander Kashyap wrote:
> Add "samsung,exynos5420" compatible string to initialize generic
> big-little cpuidle driver for Exynos5420.
>
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Chander Kashyap <k.chander@samsung.org>
> ---
>   drivers/cpuidle/cpuidle-big_little.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
> index b45fc62..d0fac53 100644
> --- a/drivers/cpuidle/cpuidle-big_little.c
> +++ b/drivers/cpuidle/cpuidle-big_little.c
> @@ -170,7 +170,8 @@ static int __init bl_idle_init(void)
>   	/*
>   	 * Initialize the driver just for a compliant set of machines
>   	 */
> -	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7"))
> +	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7") &&
> +		(!of_machine_is_compatible("samsung,exynos5420")))
>   		return -ENODEV;
>   	/*
>   	 * For now the differentiation between little and big cores

BTW, are the latencies the same than the TC2 ?


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 3/4] exynos: cpuidle: do not allow cpuidle registration for Exynos5420

[Chander Kashyap]

Exynos5420 is big.Little Soc. It uses cpuidle-big-litle generic cpuidle driver.
Hence do not allow exynos cpuidle driver registration for Exynos5420.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
 arch/arm/mach-exynos/cpuidle.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index c57cae0..242f75d 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -219,6 +219,9 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
 	int cpu_id, ret;
 	struct cpuidle_device *device;
 
+	if (soc_is_exynos5420())
+		return -ENODEV;
+
 	if (soc_is_exynos5250())
 		exynos5_core_down_clk();
 
-- 
1.7.9.5

[PATCH 3/4] exynos: cpuidle: do not allow cpuidle registration for Exynos5420

[Daniel Lezcano]

On 04/21/2014 01:49 PM, Chander Kashyap wrote:
> Exynos5420 is big.Little Soc. It uses cpuidle-big-litle generic cpuidle driver.
> Hence do not allow exynos cpuidle driver registration for Exynos5420.
>
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
> ---

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

but will conflict with:

http://www.spinics.net/lists/arm-kernel/msg319862.html

>   arch/arm/mach-exynos/cpuidle.c |    3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> index c57cae0..242f75d 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -219,6 +219,9 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
>   	int cpu_id, ret;
>   	struct cpuidle_device *device;
>
> +	if (soc_is_exynos5420())
> +		return -ENODEV;
> +
>   	if (soc_is_exynos5250())
>   		exynos5_core_down_clk();
>
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 4/4] mcpm: exynos: populate suspend and powered_up callbacks

[Chander Kashyap]

In order to support cpuidle through mcpm, suspend and powered-up
callbacks are required in mcpm platform code.
Hence populate the same callbacks.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
 arch/arm/mach-exynos/mcpm-exynos.c |   53 ++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
index 46d4968..16af0bd 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -318,10 +318,63 @@ static int exynos_power_down_finish(unsigned int cpu, unsigned int cluster)
 	return 0; /* success: the CPU is halted */
 }
 
+static void enable_coherency(void)
+{
+	unsigned long v, u;
+
+	asm volatile(
+		"mrc	p15, 0, %0, c1, c0, 1\n"
+		"orr	%0, %0, %2\n"
+		"ldr	%1, [%3]\n"
+		"and	%1, %1, #0\n"
+		"orr	%0, %0, %1\n"
+		"mcr	p15, 0, %0, c1, c0, 1\n"
+		: "=&r" (v), "=&r" (u)
+		: "Ir" (0x40), "Ir" (S5P_INFORM0)
+		: "cc");
+}
+
+void exynos_powered_up(void)
+{
+	unsigned int mpidr, cpu, cluster;
+
+	mpidr = read_cpuid_mpidr();
+	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+	arch_spin_lock(&bl_lock);
+	if (cpu_use_count[cpu][cluster] == 0)
+		cpu_use_count[cpu][cluster] = 1;
+	arch_spin_unlock(&bl_lock);
+}
+
+static void exynos_suspend(u64 residency)
+{
+	unsigned int mpidr, cpunr;
+
+	mpidr = read_cpuid_mpidr();
+	cpunr = enynos_pmu_cpunr(mpidr);
+
+	__raw_writel(virt_to_phys(mcpm_entry_point), REG_ENTRY_ADDR);
+
+	exynos_power_down();
+
+	/*
+	 * Execution reaches here only if cpu did not power down.
+	 * Hence roll back the changes done in exynos_power_down function.
+	*/
+	__raw_writel(EXYNOS_CORE_LOCAL_PWR_EN,
+			EXYNOS_ARM_CORE_CONFIGURATION(cpunr));
+	set_cr(get_cr() | CR_C);
+	enable_coherency();
+}
+
 static const struct mcpm_platform_ops exynos_power_ops = {
 	.power_up		= exynos_power_up,
 	.power_down		= exynos_power_down,
 	.power_down_finish	= exynos_power_down_finish,
+	.suspend		= exynos_suspend,
+	.powered_up		= exynos_powered_up,
 };
 
 static void __init exynos_mcpm_usage_count_init(void)
-- 
1.7.9.5

[PATCH 4/4] mcpm: exynos: populate suspend and powered_up callbacks

[Daniel Lezcano]

On 04/21/2014 01:49 PM, Chander Kashyap wrote:
> In order to support cpuidle through mcpm, suspend and powered-up
> callbacks are required in mcpm platform code.
> Hence populate the same callbacks.
>
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
> ---
>   arch/arm/mach-exynos/mcpm-exynos.c |   53 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 53 insertions(+)
>
> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
> index 46d4968..16af0bd 100644
> --- a/arch/arm/mach-exynos/mcpm-exynos.c
> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
> @@ -318,10 +318,63 @@ static int exynos_power_down_finish(unsigned int cpu, unsigned int cluster)
>   	return 0; /* success: the CPU is halted */
>   }
>
> +static void enable_coherency(void)
> +{
> +	unsigned long v, u;
> +
> +	asm volatile(
> +		"mrc	p15, 0, %0, c1, c0, 1\n"
> +		"orr	%0, %0, %2\n"
> +		"ldr	%1, [%3]\n"
> +		"and	%1, %1, #0\n"
> +		"orr	%0, %0, %1\n"
> +		"mcr	p15, 0, %0, c1, c0, 1\n"
> +		: "=&r" (v), "=&r" (u)
> +		: "Ir" (0x40), "Ir" (S5P_INFORM0)
> +		: "cc");
> +}

Shouldn't this function to be used from hotplug.c also ?

> +
> +void exynos_powered_up(void)
> +{
> +	unsigned int mpidr, cpu, cluster;
> +
> +	mpidr = read_cpuid_mpidr();
> +	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> +	arch_spin_lock(&bl_lock);
> +	if (cpu_use_count[cpu][cluster] == 0)
> +		cpu_use_count[cpu][cluster] = 1;
> +	arch_spin_unlock(&bl_lock);
> +}
> +
> +static void exynos_suspend(u64 residency)
> +{
> +	unsigned int mpidr, cpunr;
> +
> +	mpidr = read_cpuid_mpidr();
> +	cpunr = enynos_pmu_cpunr(mpidr);

*enynos*_pmu_cpunr ?

> +
> +	__raw_writel(virt_to_phys(mcpm_entry_point), REG_ENTRY_ADDR);
> +
> +	exynos_power_down();
> +
> +	/*
> +	 * Execution reaches here only if cpu did not power down.
> +	 * Hence roll back the changes done in exynos_power_down function.
> +	*/
> +	__raw_writel(EXYNOS_CORE_LOCAL_PWR_EN,
> +			EXYNOS_ARM_CORE_CONFIGURATION(cpunr));

Why don't you use the functions defined in the

patch 5/5 arm: exynos: Add MCPM call-back functions

exynos_core_power_control() ?

> +	set_cr(get_cr() | CR_C);
> +	enable_coherency();
> +}
> +
>   static const struct mcpm_platform_ops exynos_power_ops = {
>   	.power_up		= exynos_power_up,
>   	.power_down		= exynos_power_down,
>   	.power_down_finish	= exynos_power_down_finish,
> +	.suspend		= exynos_suspend,
> +	.powered_up		= exynos_powered_up,
>   };
>
>   static void __init exynos_mcpm_usage_count_init(void)
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 4/4] mcpm: exynos: populate suspend and powered_up callbacks

[Chander Kashyap]

On 22 April 2014 16:21, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> On 04/21/2014 01:49 PM, Chander Kashyap wrote:
>>
>> In order to support cpuidle through mcpm, suspend and powered-up
>> callbacks are required in mcpm platform code.
>> Hence populate the same callbacks.
>>
>> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
>> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
>> ---
>>   arch/arm/mach-exynos/mcpm-exynos.c |   53
>> ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>>
>> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c
>> b/arch/arm/mach-exynos/mcpm-exynos.c
>> index 46d4968..16af0bd 100644
>> --- a/arch/arm/mach-exynos/mcpm-exynos.c
>> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
>> @@ -318,10 +318,63 @@ static int exynos_power_down_finish(unsigned int
>> cpu, unsigned int cluster)
>>         return 0; /* success: the CPU is halted */
>>   }
>>
>> +static void enable_coherency(void)
>> +{
>> +       unsigned long v, u;
>> +
>> +       asm volatile(
>> +               "mrc    p15, 0, %0, c1, c0, 1\n"
>> +               "orr    %0, %0, %2\n"
>> +               "ldr    %1, [%3]\n"
>> +               "and    %1, %1, #0\n"
>> +               "orr    %0, %0, %1\n"
>> +               "mcr    p15, 0, %0, c1, c0, 1\n"
>> +               : "=&r" (v), "=&r" (u)
>> +               : "Ir" (0x40), "Ir" (S5P_INFORM0)
>> +               : "cc");
>> +}
>
>
> Shouldn't this function to be used from hotplug.c also ?

Hotplug.c already taking care for this. And anyhow that will go away
for mcpm dependent SoCs

>
>
>> +
>> +void exynos_powered_up(void)
>> +{
>> +       unsigned int mpidr, cpu, cluster;
>> +
>> +       mpidr = read_cpuid_mpidr();
>> +       cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
>> +       cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
>> +
>> +       arch_spin_lock(&bl_lock);
>> +       if (cpu_use_count[cpu][cluster] == 0)
>> +               cpu_use_count[cpu][cluster] = 1;
>> +       arch_spin_unlock(&bl_lock);
>> +}
>> +
>> +static void exynos_suspend(u64 residency)
>> +{
>> +       unsigned int mpidr, cpunr;
>> +
>> +       mpidr = read_cpuid_mpidr();
>> +       cpunr = enynos_pmu_cpunr(mpidr);
>
>
> *enynos*_pmu_cpunr ?

oops, I will fix typo

>
>
>> +
>> +       __raw_writel(virt_to_phys(mcpm_entry_point), REG_ENTRY_ADDR);
>> +
>> +       exynos_power_down();
>> +
>> +       /*
>> +        * Execution reaches here only if cpu did not power down.
>> +        * Hence roll back the changes done in exynos_power_down function.
>> +       */
>> +       __raw_writel(EXYNOS_CORE_LOCAL_PWR_EN,
>> +                       EXYNOS_ARM_CORE_CONFIGURATION(cpunr));
>
>
> Why don't you use the functions defined in the
>
> patch 5/5 arm: exynos: Add MCPM call-back functions

In exynos_core_power_control it powerup the alreay powered down core.
But here i need to simply set this value as core never powered down.

>
> exynos_core_power_control() ?
>
>
>> +       set_cr(get_cr() | CR_C);
>> +       enable_coherency();
>> +}
>> +
>>   static const struct mcpm_platform_ops exynos_power_ops = {
>>         .power_up               = exynos_power_up,
>>         .power_down             = exynos_power_down,
>>         .power_down_finish      = exynos_power_down_finish,
>> +       .suspend                = exynos_suspend,
>> +       .powered_up             = exynos_powered_up,
>>   };
>>
>>   static void __init exynos_mcpm_usage_count_init(void)
>>
>
>
> --
>  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>



-- 
with warm regards,
Chander Kashyap

[Patch v2 0/4] add cpuidle support for Exynos5420

[Chander Kashyap]

Exynos5420 is a big-little Soc from Samsung. It has 4 A15 and 4 A7 cores.

This patchset adds cpuidle support for Exynos5420 SoC based on
generic big.little cpuidle driver.

Tested on SMDK5420.

This patch set depends on:
	1. [PATCH 0/5] MCPM backend for Exynos5420
	   http://www.spinics.net/lists/arm-kernel/msg321666.html

	2. [PATCH v4] arm: exynos: generalize power register address calculation
	   http://www.spinics.net/lists/arm-kernel/msg324024.html
		
Changelog is in respective patches.
Chander Kashyap (4):
  cpuidle: config: Add SOC_EXYNOS5420 entry to select
    cpuidle-big-little driver
  driver: cpuidle: cpuidle-big-little: init driver for Exynos5420
  exynos: cpuidle: do not allow cpuidle registration for Exynos5420
  mcpm: exynos: populate suspend and powered_up callbacks

 arch/arm/mach-exynos/cpuidle.c       |    3 ++
 arch/arm/mach-exynos/mcpm-exynos.c   |   53 ++++++++++++++++++++++++++++++++++
 drivers/cpuidle/Kconfig.arm          |    2 +-
 drivers/cpuidle/cpuidle-big_little.c |    3 +-
 4 files changed, 59 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[Patch v2 1/4] cpuidle: config: Add SOC_EXYNOS5420 entry to select cpuidle-big-little driver

[Chander Kashyap]

Exynos5420 is a big-little SoC from Samsung. It has 4 A15 and 4 A7 cores.
In order to use generic cpuidle-big-little driver, this patch adds Exynos5420
specific check to initialize generic cpuidle driver.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
Changes in v2:
	1. Changed config macro from SOC_EXYNOS5420 to SOC_EXYNOS5420
 drivers/cpuidle/Kconfig.arm |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 97ccc31..d9596e7 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -4,7 +4,7 @@
 
 config ARM_BIG_LITTLE_CPUIDLE
 	bool "Support for ARM big.LITTLE processors"
-	depends on ARCH_VEXPRESS_TC2_PM
+	depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS
 	select ARM_CPU_SUSPEND
 	select CPU_IDLE_MULTIPLE_DRIVERS
 	help
-- 
1.7.9.5

[Patch v2 2/4] driver: cpuidle: cpuidle-big-little: init driver for Exynos5420

[Chander Kashyap]

Add "samsung,exynos5420" compatible string to initialize generic
big-little cpuidle driver for Exynos5420.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/cpuidle/cpuidle-big_little.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
index b45fc62..d0fac53 100644
--- a/drivers/cpuidle/cpuidle-big_little.c
+++ b/drivers/cpuidle/cpuidle-big_little.c
@@ -170,7 +170,8 @@ static int __init bl_idle_init(void)
 	/*
 	 * Initialize the driver just for a compliant set of machines
 	 */
-	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7"))
+	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7") &&
+		(!of_machine_is_compatible("samsung,exynos5420")))
 		return -ENODEV;
 	/*
 	 * For now the differentiation between little and big cores
-- 
1.7.9.5

[Patch v2 2/4] driver: cpuidle: cpuidle-big-little: init driver for Exynos5420

[Lorenzo Pieralisi]

On Wed, Apr 23, 2014 at 10:25:52AM +0100, Chander Kashyap wrote:
> Add "samsung,exynos5420" compatible string to initialize generic
> big-little cpuidle driver for Exynos5420.
> 
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/cpuidle/cpuidle-big_little.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
> index b45fc62..d0fac53 100644
> --- a/drivers/cpuidle/cpuidle-big_little.c
> +++ b/drivers/cpuidle/cpuidle-big_little.c
> @@ -170,7 +170,8 @@ static int __init bl_idle_init(void)
>  	/*
>  	 * Initialize the driver just for a compliant set of machines
>  	 */
> -	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7"))
> +	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7") &&
> +		(!of_machine_is_compatible("samsung,exynos5420")))
>  		return -ENODEV;

We should handle the string matching differently, we can't keep adding
comparisons.

Daniel raised the point already: what about the idle tables (data and
number of states ?). TC2 has just a cluster state, and specific
latencies, which are highly unlikely to be correct for this platform.

Lorenzo

[Patch v2 2/4] driver: cpuidle: cpuidle-big-little: init driver for Exynos5420

[Chander Kashyap]

On 23 April 2014 22:02, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> On Wed, Apr 23, 2014 at 10:25:52AM +0100, Chander Kashyap wrote:
>> Add "samsung,exynos5420" compatible string to initialize generic
>> big-little cpuidle driver for Exynos5420.
>>
>> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
>> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
>> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  drivers/cpuidle/cpuidle-big_little.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
>> index b45fc62..d0fac53 100644
>> --- a/drivers/cpuidle/cpuidle-big_little.c
>> +++ b/drivers/cpuidle/cpuidle-big_little.c
>> @@ -170,7 +170,8 @@ static int __init bl_idle_init(void)
>>       /*
>>        * Initialize the driver just for a compliant set of machines
>>        */
>> -     if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7"))
>> +     if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7") &&
>> +             (!of_machine_is_compatible("samsung,exynos5420")))
>>               return -ENODEV;
>
> We should handle the string matching differently, we can't keep adding
> comparisons.

yes, that's true.


>
> Daniel raised the point already: what about the idle tables (data and
> number of states ?). TC2 has just a cluster state, and specific
> latencies, which are highly unlikely to be correct for this platform.
>

As of now only support for one state i.e. core power down.

As latencies are  concerned, need to fine tune.

Thanks again for the review.

> Lorenzo
>



-- 
with warm regards,
Chander Kashyap

[Patch v2 3/4] exynos: cpuidle: do not allow cpuidle registration for Exynos5420

[Chander Kashyap]

Exynos5420 is big.Little Soc. It uses cpuidle-big-litle generic cpuidle driver.
Hence do not allow exynos cpuidle driver registration for Exynos5420.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/mach-exynos/cpuidle.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index c57cae0..242f75d 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -219,6 +219,9 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
 	int cpu_id, ret;
 	struct cpuidle_device *device;
 
+	if (soc_is_exynos5420())
+		return -ENODEV;
+
 	if (soc_is_exynos5250())
 		exynos5_core_down_clk();
 
-- 
1.7.9.5

[Patch v2 4/4] mcpm: exynos: populate suspend and powered_up callbacks

[Chander Kashyap]

In order to support cpuidle through mcpm, suspend and powered-up
callbacks are required in mcpm platform code.
Hence populate the same callbacks.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
changes in v2:
	1. Fixed typo: enynos_pmu_cpunr to exynos_pmu_cpunr

 arch/arm/mach-exynos/mcpm-exynos.c |   53 ++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
index 6c74c82..d53f597 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -272,10 +272,63 @@ static int exynos_power_down_finish(unsigned int cpu, unsigned int cluster)
 	return 0; /* success: the CPU is halted */
 }
 
+static void enable_coherency(void)
+{
+	unsigned long v, u;
+
+	asm volatile(
+		"mrc	p15, 0, %0, c1, c0, 1\n"
+		"orr	%0, %0, %2\n"
+		"ldr	%1, [%3]\n"
+		"and	%1, %1, #0\n"
+		"orr	%0, %0, %1\n"
+		"mcr	p15, 0, %0, c1, c0, 1\n"
+		: "=&r" (v), "=&r" (u)
+		: "Ir" (0x40), "Ir" (S5P_INFORM0)
+		: "cc");
+}
+
+void exynos_powered_up(void)
+{
+	unsigned int mpidr, cpu, cluster;
+
+	mpidr = read_cpuid_mpidr();
+	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+	arch_spin_lock(&exynos_mcpm_lock);
+	if (cpu_use_count[cpu][cluster] == 0)
+		cpu_use_count[cpu][cluster] = 1;
+	arch_spin_unlock(&exynos_mcpm_lock);
+}
+
+static void exynos_suspend(u64 residency)
+{
+	unsigned int mpidr, cpunr;
+
+	mpidr = read_cpuid_mpidr();
+	cpunr = exynos_pmu_cpunr(mpidr);
+
+	__raw_writel(virt_to_phys(mcpm_entry_point), REG_ENTRY_ADDR);
+
+	exynos_power_down();
+
+	/*
+	 * Execution reaches here only if cpu did not power down.
+	 * Hence roll back the changes done in exynos_power_down function.
+	*/
+	__raw_writel(EXYNOS_CORE_LOCAL_PWR_EN,
+			EXYNOS_ARM_CORE_CONFIGURATION(cpunr));
+	set_cr(get_cr() | CR_C);
+	enable_coherency();
+}
+
 static const struct mcpm_platform_ops exynos_power_ops = {
 	.power_up		= exynos_power_up,
 	.power_down		= exynos_power_down,
 	.power_down_finish	= exynos_power_down_finish,
+	.suspend		= exynos_suspend,
+	.powered_up		= exynos_powered_up,
 };
 
 static void __init exynos_mcpm_usage_count_init(void)
-- 
1.7.9.5

[Patch v2 4/4] mcpm: exynos: populate suspend and powered_up callbacks

[Lorenzo Pieralisi]

[added Nico in CC]

On Wed, Apr 23, 2014 at 10:25:54AM +0100, Chander Kashyap wrote:
> In order to support cpuidle through mcpm, suspend and powered-up
> callbacks are required in mcpm platform code.
> Hence populate the same callbacks.
> 
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
> ---
> changes in v2:
> 	1. Fixed typo: enynos_pmu_cpunr to exynos_pmu_cpunr
> 
>  arch/arm/mach-exynos/mcpm-exynos.c |   53 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
> index 6c74c82..d53f597 100644
> --- a/arch/arm/mach-exynos/mcpm-exynos.c
> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
> @@ -272,10 +272,63 @@ static int exynos_power_down_finish(unsigned int cpu, unsigned int cluster)
>  	return 0; /* success: the CPU is halted */
>  }
>  
> +static void enable_coherency(void)
> +{
> +	unsigned long v, u;
> +
> +	asm volatile(
> +		"mrc	p15, 0, %0, c1, c0, 1\n"
> +		"orr	%0, %0, %2\n"
> +		"ldr	%1, [%3]\n"
> +		"and	%1, %1, #0\n"
> +		"orr	%0, %0, %1\n"
> +		"mcr	p15, 0, %0, c1, c0, 1\n"
> +		: "=&r" (v), "=&r" (u)
> +		: "Ir" (0x40), "Ir" (S5P_INFORM0)
> +		: "cc");
> +}
> +
> +void exynos_powered_up(void)
> +{
> +	unsigned int mpidr, cpu, cluster;
> +
> +	mpidr = read_cpuid_mpidr();
> +	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> +	arch_spin_lock(&exynos_mcpm_lock);
> +	if (cpu_use_count[cpu][cluster] == 0)
> +		cpu_use_count[cpu][cluster] = 1;
> +	arch_spin_unlock(&exynos_mcpm_lock);
> +}
> +
> +static void exynos_suspend(u64 residency)
> +{
> +	unsigned int mpidr, cpunr;
> +
> +	mpidr = read_cpuid_mpidr();
> +	cpunr = exynos_pmu_cpunr(mpidr);
> +
> +	__raw_writel(virt_to_phys(mcpm_entry_point), REG_ENTRY_ADDR);
> +
> +	exynos_power_down();
> +
> +	/*
> +	 * Execution reaches here only if cpu did not power down.
> +	 * Hence roll back the changes done in exynos_power_down function.
> +	*/
> +	__raw_writel(EXYNOS_CORE_LOCAL_PWR_EN,
> +			EXYNOS_ARM_CORE_CONFIGURATION(cpunr));
> +	set_cr(get_cr() | CR_C);
> +	enable_coherency();

This is wrong:

1) MCPM would eventually reboot the CPU in question if the suspend call
   returns (and restore SCTLR and ACTLR in cpu_resume), so there is 0 point
   in doing that here.
2) The core would have executed out of coherency for a "while" so the
   tlbs could be stale and you do not invalidate them. But given (1), (2)
   becomes just informational. The register write must be executed
   though (I guess...). Now, on restoring the SMP bit in cpu_resume
   (errata 799270) I need to verify this is safe and get back to you.

Cheers,
Lorenzo

[Patch v2 4/4] mcpm: exynos: populate suspend and powered_up callbacks

[Chander Kashyap]

On 23 April 2014 21:32, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> [added Nico in CC]
>
> On Wed, Apr 23, 2014 at 10:25:54AM +0100, Chander Kashyap wrote:
>> In order to support cpuidle through mcpm, suspend and powered-up
>> callbacks are required in mcpm platform code.
>> Hence populate the same callbacks.
>>
>> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
>> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
>> ---
>> changes in v2:
>>       1. Fixed typo: enynos_pmu_cpunr to exynos_pmu_cpunr
>>
>>  arch/arm/mach-exynos/mcpm-exynos.c |   53 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>>
>> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
>> index 6c74c82..d53f597 100644
>> --- a/arch/arm/mach-exynos/mcpm-exynos.c
>> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
>> @@ -272,10 +272,63 @@ static int exynos_power_down_finish(unsigned int cpu, unsigned int cluster)
>>       return 0; /* success: the CPU is halted */
>>  }
>>
>> +static void enable_coherency(void)
>> +{
>> +     unsigned long v, u;
>> +
>> +     asm volatile(
>> +             "mrc    p15, 0, %0, c1, c0, 1\n"
>> +             "orr    %0, %0, %2\n"
>> +             "ldr    %1, [%3]\n"
>> +             "and    %1, %1, #0\n"
>> +             "orr    %0, %0, %1\n"
>> +             "mcr    p15, 0, %0, c1, c0, 1\n"
>> +             : "=&r" (v), "=&r" (u)
>> +             : "Ir" (0x40), "Ir" (S5P_INFORM0)
>> +             : "cc");
>> +}
>> +
>> +void exynos_powered_up(void)
>> +{
>> +     unsigned int mpidr, cpu, cluster;
>> +
>> +     mpidr = read_cpuid_mpidr();
>> +     cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
>> +     cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
>> +
>> +     arch_spin_lock(&exynos_mcpm_lock);
>> +     if (cpu_use_count[cpu][cluster] == 0)
>> +             cpu_use_count[cpu][cluster] = 1;
>> +     arch_spin_unlock(&exynos_mcpm_lock);
>> +}
>> +
>> +static void exynos_suspend(u64 residency)
>> +{
>> +     unsigned int mpidr, cpunr;
>> +
>> +     mpidr = read_cpuid_mpidr();
>> +     cpunr = exynos_pmu_cpunr(mpidr);
>> +
>> +     __raw_writel(virt_to_phys(mcpm_entry_point), REG_ENTRY_ADDR);
>> +
>> +     exynos_power_down();
>> +
>> +     /*
>> +      * Execution reaches here only if cpu did not power down.
>> +      * Hence roll back the changes done in exynos_power_down function.
>> +     */
>> +     __raw_writel(EXYNOS_CORE_LOCAL_PWR_EN,
>> +                     EXYNOS_ARM_CORE_CONFIGURATION(cpunr));
>> +     set_cr(get_cr() | CR_C);
>> +     enable_coherency();
>
> This is wrong:
>
> 1) MCPM would eventually reboot the CPU in question if the suspend call
>    returns (and restore SCTLR and ACTLR in cpu_resume), so there is 0 point
>    in doing that here.

Yes i missed that. I will correct it.

> 2) The core would have executed out of coherency for a "while" so the
>    tlbs could be stale and you do not invalidate them. But given (1), (2)
>    becomes just informational. The register write must be executed
>    though (I guess...). Now, on restoring the SMP bit in cpu_resume
>    (errata 799270) I need to verify this is safe and get back to you.

Ok

Thanks Lorenzo.


>
> Cheers,
> Lorenzo
>



-- 
with warm regards,
Chander Kashyap

[Patch v2 0/4] add cpuidle support for Exynos5420

[Rafael J. Wysocki]

On Wednesday, April 23, 2014 02:55:50 PM Chander Kashyap wrote:
> Exynos5420 is a big-little Soc from Samsung. It has 4 A15 and 4 A7 cores.
> 
> This patchset adds cpuidle support for Exynos5420 SoC based on
> generic big.little cpuidle driver.
> 
> Tested on SMDK5420.
> 
> This patch set depends on:
> 	1. [PATCH 0/5] MCPM backend for Exynos5420
> 	   http://www.spinics.net/lists/arm-kernel/msg321666.html
> 
> 	2. [PATCH v4] arm: exynos: generalize power register address calculation
> 	   http://www.spinics.net/lists/arm-kernel/msg324024.html
> 		
> Changelog is in respective patches.
> Chander Kashyap (4):
>   cpuidle: config: Add SOC_EXYNOS5420 entry to select
>     cpuidle-big-little driver
>   driver: cpuidle: cpuidle-big-little: init driver for Exynos5420
>   exynos: cpuidle: do not allow cpuidle registration for Exynos5420
>   mcpm: exynos: populate suspend and powered_up callbacks
> 
>  arch/arm/mach-exynos/cpuidle.c       |    3 ++
>  arch/arm/mach-exynos/mcpm-exynos.c   |   53 ++++++++++++++++++++++++++++++++++
>  drivers/cpuidle/Kconfig.arm          |    2 +-
>  drivers/cpuidle/cpuidle-big_little.c |    3 +-
>  4 files changed, 59 insertions(+), 2 deletions(-)

I'm assuming that the Exynos maintainers will take care of this, correct?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[Patch v2 0/4] add cpuidle support for Exynos5420

[Kukjin Kim]

On 04/23/14 19:18, Rafael J. Wysocki wrote:
> On Wednesday, April 23, 2014 02:55:50 PM Chander Kashyap wrote:
>> Exynos5420 is a big-little Soc from Samsung. It has 4 A15 and 4 A7 cores.
>>
>> This patchset adds cpuidle support for Exynos5420 SoC based on
>> generic big.little cpuidle driver.
>>
>> Tested on SMDK5420.
>>
>> This patch set depends on:
>> 	1. [PATCH 0/5] MCPM backend for Exynos5420
>> 	   http://www.spinics.net/lists/arm-kernel/msg321666.html
>>
>> 	2. [PATCH v4] arm: exynos: generalize power register address calculation
>> 	   http://www.spinics.net/lists/arm-kernel/msg324024.html
>> 		
>> Changelog is in respective patches.
>> Chander Kashyap (4):
>>    cpuidle: config: Add SOC_EXYNOS5420 entry to select
>>      cpuidle-big-little driver
>>    driver: cpuidle: cpuidle-big-little: init driver for Exynos5420
>>    exynos: cpuidle: do not allow cpuidle registration for Exynos5420
>>    mcpm: exynos: populate suspend and powered_up callbacks
>>
>>   arch/arm/mach-exynos/cpuidle.c       |    3 ++
>>   arch/arm/mach-exynos/mcpm-exynos.c   |   53 ++++++++++++++++++++++++++++++++++
>>   drivers/cpuidle/Kconfig.arm          |    2 +-
>>   drivers/cpuidle/cpuidle-big_little.c |    3 +-
>>   4 files changed, 59 insertions(+), 2 deletions(-)
>
> I'm assuming that the Exynos maintainers will take care of this, correct?
>

Yeah, I will if you have any objection :-)

BTW, I need to look at the dependent patches.

- Kukjin

[PATCH v3 0/5] add cpuidle support for Exynos5420

[Chander Kashyap]

Exynos5420 is a big-little Soc from Samsung. It has 4 A15 and 4 A7 cores.

This patchset adds cpuidle support for Exynos5420 SoC based on
generic big.little cpuidle driver.

Tested on SMDK5420.

This patch set depends on:
	1. [PATCH 0/5] MCPM backend for Exynos5420
	   http://www.spinics.net/lists/arm-kernel/msg327923.html

	2. [PATCH] arm: exynos: add generic function to calculate cpu number
	   http://www.spinics.net/lists/linux-samsung-soc/msg29446.html
	   http://www.spinics.net/lists/arm-kernel/msg324024.html
		
Changelog is in respective patches.
Chander Kashyap (5):
  driver: cpuidle-big-little: add of_device_id structure
  cpuidle: config: Add ARCH_EXYNOS entry to select cpuidle-big-little
    driver
  driver: cpuidle: cpuidle-big-little: init driver for Exynos5420
  exynos: cpuidle: do not allow cpuidle registration for Exynos5420
  mcpm: exynos: populate suspend and powered_up callbacks

 arch/arm/mach-exynos/cpuidle.c       |    3 +++
 arch/arm/mach-exynos/mcpm-exynos.c   |   34 ++++++++++++++++++++++++++++++++++
 drivers/cpuidle/Kconfig.arm          |    2 +-
 drivers/cpuidle/cpuidle-big_little.c |   12 +++++++++++-
 4 files changed, 49 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[Patch v3 1/5] driver: cpuidle-big-little: add of_device_id structure

[Chander Kashyap]

This driver will be used by many big.Little Soc's. As of now it does
string matching of hardcoded compatible string to init the driver. This
comparison list will keep on growing with addition of new SoC's.
Hence add of_device_id structure to collect the compatible strings of
SoC's using this driver.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
 drivers/cpuidle/cpuidle-big_little.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
index b45fc62..4cd02bd 100644
--- a/drivers/cpuidle/cpuidle-big_little.c
+++ b/drivers/cpuidle/cpuidle-big_little.c
@@ -163,14 +163,23 @@ static int __init bl_idle_driver_init(struct cpuidle_driver *drv, int cpu_id)
 	return 0;
 }
 
+static const struct of_device_id compatible_machine_match[] = {
+	{ .compatible = "arm,vexpress,v2p-ca15_a7" },
+	{},
+};
+
 static int __init bl_idle_init(void)
 {
 	int ret;
+	struct device_node *root = of_find_node_by_path("/");
+
+	if (!root)
+		return -ENODEV;
 
 	/*
 	 * Initialize the driver just for a compliant set of machines
 	 */
-	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7"))
+	if (!of_match_node(compatible_machine_match, root))
 		return -ENODEV;
 	/*
 	 * For now the differentiation between little and big cores
-- 
1.7.9.5

[Patch v3 2/5] cpuidle: config: Add ARCH_EXYNOS entry to select cpuidle-big-little driver

[Chander Kashyap]

Exynos5420 is a big-little SoC from Samsung. It has 4 A15 and 4 A7 cores.
In order to use generic cpuidle-big-little driver, this patch adds Exynos5420
specific check to initialize generic cpuidle driver.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
Changes in v3: None
Changes in v2:
	1. Changed config macro from SOC_EXYNOS5420 to SOC_EXYNOS5420
 drivers/cpuidle/Kconfig.arm |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 97ccc31..d9596e7 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -4,7 +4,7 @@
 
 config ARM_BIG_LITTLE_CPUIDLE
 	bool "Support for ARM big.LITTLE processors"
-	depends on ARCH_VEXPRESS_TC2_PM
+	depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS
 	select ARM_CPU_SUSPEND
 	select CPU_IDLE_MULTIPLE_DRIVERS
 	help
-- 
1.7.9.5

[Patch v3 2/5] cpuidle: config: Add ARCH_EXYNOS entry to select cpuidle-big-little driver

[Andreas Färber]

Hi,

Am 05.05.2014 10:27, schrieb Chander Kashyap:
> Exynos5420 is a big-little SoC from Samsung. It has 4 A15 and 4 A7 cores.
> In order to use generic cpuidle-big-little driver, this patch adds Exynos5420
> specific check to initialize generic cpuidle driver.
> 
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
> ---
> Changes in v3: None
> Changes in v2:
> 	1. Changed config macro from SOC_EXYNOS5420 to SOC_EXYNOS5420

This is probably a copy&pasto? Anyway, the commit message needs an
update since no Exynos5420 specific check is done below afaics. That
went into 3/5.

Cheers,
Andreas

>  drivers/cpuidle/Kconfig.arm |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> index 97ccc31..d9596e7 100644
> --- a/drivers/cpuidle/Kconfig.arm
> +++ b/drivers/cpuidle/Kconfig.arm
> @@ -4,7 +4,7 @@
>  
>  config ARM_BIG_LITTLE_CPUIDLE
>  	bool "Support for ARM big.LITTLE processors"
> -	depends on ARCH_VEXPRESS_TC2_PM
> +	depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS
>  	select ARM_CPU_SUSPEND
>  	select CPU_IDLE_MULTIPLE_DRIVERS
>  	help
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg

[Patch v3 2/5] cpuidle: config: Add ARCH_EXYNOS entry to select cpuidle-big-little driver

[Chander Kashyap]

Hi Andreas,

On 5 May 2014 14:29, Andreas F?rber <afaerber@suse.de> wrote:
> Hi,
>
> Am 05.05.2014 10:27, schrieb Chander Kashyap:
>> Exynos5420 is a big-little SoC from Samsung. It has 4 A15 and 4 A7 cores.
>> In order to use generic cpuidle-big-little driver, this patch adds Exynos5420
>> specific check to initialize generic cpuidle driver.
>>
>> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
>> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
>> ---
>> Changes in v3: None
>> Changes in v2:
>>       1. Changed config macro from SOC_EXYNOS5420 to SOC_EXYNOS5420
>
> This is probably a copy&pasto? Anyway, the commit message needs an
> update since no Exynos5420 specific check is done below afaics. That
> went into 3/5.

Oops that's write,
Copy paste mistake, and commit message change is required. I will fix it .

Thanks


>
> Cheers,
> Andreas
>
>>  drivers/cpuidle/Kconfig.arm |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
>> index 97ccc31..d9596e7 100644
>> --- a/drivers/cpuidle/Kconfig.arm
>> +++ b/drivers/cpuidle/Kconfig.arm
>> @@ -4,7 +4,7 @@
>>
>>  config ARM_BIG_LITTLE_CPUIDLE
>>       bool "Support for ARM big.LITTLE processors"
>> -     depends on ARCH_VEXPRESS_TC2_PM
>> +     depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS
>>       select ARM_CPU_SUSPEND
>>       select CPU_IDLE_MULTIPLE_DRIVERS
>>       help
>>
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg



-- 
with warm regards,
Chander Kashyap

[PATCH v4 0/5] add cpuidle support for Exynos5420

[Chander Kashyap]

Exynos5420 is a big-little Soc from Samsung. It has 4 A15 and 4 A7 cores.

This patchset adds cpuidle support for Exynos5420 SoC based on
generic big.little cpuidle driver.

Tested on SMDK5420.

This patch set depends on:
	1. [PATCH 0/5] MCPM backend for Exynos5420
	   http://www.spinics.net/lists/arm-kernel/msg327923.html

	2. [PATCH] arm: exynos: add generic function to calculate cpu number
	   http://www.spinics.net/lists/linux-samsung-soc/msg29446.html
	   http://www.spinics.net/lists/arm-kernel/msg324024.html
		
Changelog is in respective patches.
Chander Kashyap (5):
  driver: cpuidle-big-little: add of_device_id structure
  cpuidle: config: Add ARCH_EXYNOS entry to select cpuidle-big-little
    driver
  driver: cpuidle: cpuidle-big-little: init driver for Exynos5420
  exynos: cpuidle: do not allow cpuidle registration for Exynos5420
  mcpm: exynos: populate suspend and powered_up callbacks

 arch/arm/mach-exynos/cpuidle.c       |    3 +++
 arch/arm/mach-exynos/mcpm-exynos.c   |   34 ++++++++++++++++++++++++++++++++++
 drivers/cpuidle/Kconfig.arm          |    2 +-
 drivers/cpuidle/cpuidle-big_little.c |   12 +++++++++++-
 4 files changed, 49 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[Patch v4 1/5] driver: cpuidle-big-little: add of_device_id structure

[Chander Kashyap]

This driver will be used by many big.Little Soc's. As of now it does
string matching of hardcoded compatible string to init the driver. This
comparison list will keep on growing with addition of new SoC's.
Hence add of_device_id structure to collect the compatible strings of
SoC's using this driver.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
 drivers/cpuidle/cpuidle-big_little.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
index b45fc62..4cd02bd 100644
--- a/drivers/cpuidle/cpuidle-big_little.c
+++ b/drivers/cpuidle/cpuidle-big_little.c
@@ -163,14 +163,23 @@ static int __init bl_idle_driver_init(struct cpuidle_driver *drv, int cpu_id)
 	return 0;
 }
 
+static const struct of_device_id compatible_machine_match[] = {
+	{ .compatible = "arm,vexpress,v2p-ca15_a7" },
+	{},
+};
+
 static int __init bl_idle_init(void)
 {
 	int ret;
+	struct device_node *root = of_find_node_by_path("/");
+
+	if (!root)
+		return -ENODEV;
 
 	/*
 	 * Initialize the driver just for a compliant set of machines
 	 */
-	if (!of_machine_is_compatible("arm,vexpress,v2p-ca15_a7"))
+	if (!of_match_node(compatible_machine_match, root))
 		return -ENODEV;
 	/*
 	 * For now the differentiation between little and big cores
-- 
1.7.9.5

[Patch v4 2/5] cpuidle: config: Add ARCH_EXYNOS entry to select cpuidle-big-little driver

[Chander Kashyap]

Add support to select generic big-little cpuidle driver for Samsung Exynos
series SoC's. This is required for Exynos big-llittle SoC's eg, Exynos5420.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
Changes in v4:
	1. Typo fixed from SOC_EXYNOS5420 to ARCH_EXYNOS
	2. Commit message updated
Changes in v3: None
Changes in v2:
	1. Changed config macro from SOC_EXYNOS5420 to ARCH_EXYNOS
 drivers/cpuidle/Kconfig.arm |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 97ccc31..d9596e7 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -4,7 +4,7 @@
 
 config ARM_BIG_LITTLE_CPUIDLE
 	bool "Support for ARM big.LITTLE processors"
-	depends on ARCH_VEXPRESS_TC2_PM
+	depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS
 	select ARM_CPU_SUSPEND
 	select CPU_IDLE_MULTIPLE_DRIVERS
 	help
-- 
1.7.9.5

[Patch v4 3/5] driver: cpuidle: cpuidle-big-little: init driver for Exynos5420

[Chander Kashyap]

Add "samsung,exynos5420" compatible string to initialize generic
big-little cpuidle driver for Exynos5420.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
Changes in v4: None
Changes in v3:
	1. Add compatible string to of_device_id table insted comparing directoly
Changes in v2: none

 drivers/cpuidle/cpuidle-big_little.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
index 4cd02bd..344d79fa 100644
--- a/drivers/cpuidle/cpuidle-big_little.c
+++ b/drivers/cpuidle/cpuidle-big_little.c
@@ -165,6 +165,7 @@ static int __init bl_idle_driver_init(struct cpuidle_driver *drv, int cpu_id)
 
 static const struct of_device_id compatible_machine_match[] = {
 	{ .compatible = "arm,vexpress,v2p-ca15_a7" },
+	{ .compatible = "samsung,exynos5420" },
 	{},
 };
 
-- 
1.7.9.5

[Patch v4 4/5] exynos: cpuidle: do not allow cpuidle registration for Exynos5420

[Chander Kashyap]

Exynos5420 is big.Little Soc. It uses cpuidle-big-litle generic cpuidle driver.
Hence do not allow exynos cpuidle driver registration for Exynos5420.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/mach-exynos/cpuidle.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index c57cae0..242f75d 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -219,6 +219,9 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
 	int cpu_id, ret;
 	struct cpuidle_device *device;
 
+	if (soc_is_exynos5420())
+		return -ENODEV;
+
 	if (soc_is_exynos5250())
 		exynos5_core_down_clk();
 
-- 
1.7.9.5

[Patch v4 5/5] mcpm: exynos: populate suspend and powered_up callbacks

[Chander Kashyap]

In order to support cpuidle through mcpm, suspend and powered-up
callbacks are required in mcpm platform code.
Hence populate the same callbacks.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
Changes in v4: None
Changes in v3:
	1. Removed coherancy enablement after suspend failure.
	2. Use generic function to poweron cpu.
changes in v2:
	1. Fixed typo: enynos_pmu_cpunr to exynos_pmu_cpunr
 arch/arm/mach-exynos/mcpm-exynos.c |   34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
index d0f7461..6d4a907 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -256,10 +256,44 @@ static int exynos_power_down_finish(unsigned int cpu, unsigned int cluster)
 	return -ETIMEDOUT; /* timeout */
 }
 
+void exynos_powered_up(void)
+{
+	unsigned int mpidr, cpu, cluster;
+
+	mpidr = read_cpuid_mpidr();
+	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+	arch_spin_lock(&exynos_mcpm_lock);
+	if (cpu_use_count[cpu][cluster] == 0)
+		cpu_use_count[cpu][cluster] = 1;
+	arch_spin_unlock(&exynos_mcpm_lock);
+}
+
+static void exynos_suspend(u64 residency)
+{
+	unsigned int mpidr, cpunr;
+
+	mpidr = read_cpuid_mpidr();
+	cpunr = exynos_pmu_cpunr(mpidr);
+
+	__raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 0x1c);
+
+	exynos_power_down();
+
+	/*
+	 * Execution reaches here only if cpu did not power down.
+	 * Hence roll back the changes done in exynos_power_down function.
+	*/
+	exynos_cpu_powerup(cpunr);
+}
+
 static const struct mcpm_platform_ops exynos_power_ops = {
 	.power_up		= exynos_power_up,
 	.power_down		= exynos_power_down,
 	.power_down_finish	= exynos_power_down_finish,
+	.suspend		= exynos_suspend,
+	.powered_up		= exynos_powered_up,
 };
 
 static void __init exynos_mcpm_usage_count_init(void)
-- 
1.7.9.5

[Patch v4 5/5] mcpm: exynos: populate suspend and powered_up callbacks

[Lorenzo Pieralisi]

On Mon, May 05, 2014 at 10:27:20AM +0100, Chander Kashyap wrote:
> In order to support cpuidle through mcpm, suspend and powered-up
> callbacks are required in mcpm platform code.
> Hence populate the same callbacks.
> 
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
> ---
> Changes in v4: None
> Changes in v3:
> 	1. Removed coherancy enablement after suspend failure.

coherency

> 	2. Use generic function to poweron cpu.
> changes in v2:
> 	1. Fixed typo: enynos_pmu_cpunr to exynos_pmu_cpunr
>  arch/arm/mach-exynos/mcpm-exynos.c |   34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
> index d0f7461..6d4a907 100644
> --- a/arch/arm/mach-exynos/mcpm-exynos.c
> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
> @@ -256,10 +256,44 @@ static int exynos_power_down_finish(unsigned int cpu, unsigned int cluster)
>  	return -ETIMEDOUT; /* timeout */
>  }
>  
> +void exynos_powered_up(void)

static ?

> +{
> +	unsigned int mpidr, cpu, cluster;
> +
> +	mpidr = read_cpuid_mpidr();
> +	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> +	arch_spin_lock(&exynos_mcpm_lock);
> +	if (cpu_use_count[cpu][cluster] == 0)
> +		cpu_use_count[cpu][cluster] = 1;
> +	arch_spin_unlock(&exynos_mcpm_lock);
> +}
> +
> +static void exynos_suspend(u64 residency)
> +{
> +	unsigned int mpidr, cpunr;
> +
> +	mpidr = read_cpuid_mpidr();
> +	cpunr = exynos_pmu_cpunr(mpidr);

If I were to be picky, I would compute these values only if they
are needed, ie move the computation after exynos_power_down().

There is another quite horrible issue here. We know this code works
because the processors A15/A7 hit the caches with C bit in SCTLR cleared.

On processors where this is not true, this sequence would explode
if power down fails (in case core is gated but L2 is still powered on,
the stack is stuck in L2) since it is going to read stack data that is
in L2 but can't be read.

It is not related to this sequence only, but it is an issue in general
and wanted to mention that on the lists for public awareness.

The gist of what I am saying is, please add a comment to that extent,
here and it should be added in exynos_power_down() too.

> +	__raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 0x1c);

No magic numbers please (0x1c). You can add a macro/wrapper, as TC2 does.

> +	exynos_power_down();
> +
> +	/*
> +	 * Execution reaches here only if cpu did not power down.
> +	 * Hence roll back the changes done in exynos_power_down function.
> +	*/
> +	exynos_cpu_powerup(cpunr);

Please be aware that if this function returns MCPM will soft reboot, and
the CPUidle driver will have no way to detect a state entry failure.

I am just flagging this up, since fixing this behaviour is not easy, and
honestly, since power down failure should be the exception not the rule,
the idle stats should not be affected much.

I think this is the proper way of implementing the sequence but please
all keep in mind what I wrote above.

Lorenzo

> +}
> +
>  static const struct mcpm_platform_ops exynos_power_ops = {
>  	.power_up		= exynos_power_up,
>  	.power_down		= exynos_power_down,
>  	.power_down_finish	= exynos_power_down_finish,
> +	.suspend		= exynos_suspend,
> +	.powered_up		= exynos_powered_up,
>  };
>  
>  static void __init exynos_mcpm_usage_count_init(void)
> -- 
> 1.7.9.5
> 
> 

[Patch v4 5/5] mcpm: exynos: populate suspend and powered_up callbacks

[Chander Kashyap]

Hi Lorenzo

On 9 May 2014 21:02, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> On Mon, May 05, 2014 at 10:27:20AM +0100, Chander Kashyap wrote:
>> In order to support cpuidle through mcpm, suspend and powered-up
>> callbacks are required in mcpm platform code.
>> Hence populate the same callbacks.
>>
>> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
>> Signed-off-by: Chander Kashyap <k.chander@samsung.com>
>> ---
>> Changes in v4: None
>> Changes in v3:
>>       1. Removed coherancy enablement after suspend failure.
>
> coherency
>
>>       2. Use generic function to poweron cpu.
>> changes in v2:
>>       1. Fixed typo: enynos_pmu_cpunr to exynos_pmu_cpunr
>>  arch/arm/mach-exynos/mcpm-exynos.c |   34 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
>> index d0f7461..6d4a907 100644
>> --- a/arch/arm/mach-exynos/mcpm-exynos.c
>> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
>> @@ -256,10 +256,44 @@ static int exynos_power_down_finish(unsigned int cpu, unsigned int cluster)
>>       return -ETIMEDOUT; /* timeout */
>>  }
>>
>> +void exynos_powered_up(void)
>
> static ?

Ok, done

>
>> +{
>> +     unsigned int mpidr, cpu, cluster;
>> +
>> +     mpidr = read_cpuid_mpidr();
>> +     cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
>> +     cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
>> +
>> +     arch_spin_lock(&exynos_mcpm_lock);
>> +     if (cpu_use_count[cpu][cluster] == 0)
>> +             cpu_use_count[cpu][cluster] = 1;
>> +     arch_spin_unlock(&exynos_mcpm_lock);
>> +}
>> +
>> +static void exynos_suspend(u64 residency)
>> +{
>> +     unsigned int mpidr, cpunr;
>> +
>> +     mpidr = read_cpuid_mpidr();
>> +     cpunr = exynos_pmu_cpunr(mpidr);
>
> If I were to be picky, I would compute these values only if they
> are needed, ie move the computation after exynos_power_down().

Yes thats make sense. I will realign it.

>
> There is another quite horrible issue here. We know this code works
> because the processors A15/A7 hit the caches with C bit in SCTLR cleared.
>
> On processors where this is not true, this sequence would explode
> if power down fails (in case core is gated but L2 is still powered on,
> the stack is stuck in L2) since it is going to read stack data that is
> in L2 but can't be read.
>
> It is not related to this sequence only, but it is an issue in general
> and wanted to mention that on the lists for public awareness.
>

Can you please elaborate. I didn't understand.

> The gist of what I am saying is, please add a comment to that extent,
> here and it should be added in exynos_power_down() too.
>
>> +     __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 0x1c);
>
> No magic numbers please (0x1c). You can add a macro/wrapper, as TC2 does.

Yes i will remove it.

>
>> +     exynos_power_down();
>> +
>> +     /*
>> +      * Execution reaches here only if cpu did not power down.
>> +      * Hence roll back the changes done in exynos_power_down function.
>> +     */
>> +     exynos_cpu_powerup(cpunr);
>
> Please be aware that if this function returns MCPM will soft reboot, and
> the CPUidle driver will have no way to detect a state entry failure.
>
> I am just flagging this up, since fixing this behaviour is not easy, and
> honestly, since power down failure should be the exception not the rule,
> the idle stats should not be affected much.
>
> I think this is the proper way of implementing the sequence but please
> all keep in mind what I wrote above.
>
> Lorenzo
>
>> +}
>> +
>>  static const struct mcpm_platform_ops exynos_power_ops = {
>>       .power_up               = exynos_power_up,
>>       .power_down             = exynos_power_down,
>>       .power_down_finish      = exynos_power_down_finish,
>> +     .suspend                = exynos_suspend,
>> +     .powered_up             = exynos_powered_up,
>>  };
>>
>>  static void __init exynos_mcpm_usage_count_init(void)
>> --
>> 1.7.9.5
>>
>>
>



-- 
with warm regards,
Chander Kashyap

[Patch v4 5/5] mcpm: exynos: populate suspend and powered_up callbacks

[Lorenzo Pieralisi]

On Tue, May 13, 2014 at 12:43:31PM +0100, Chander Kashyap wrote:

[...]

> >> +static void exynos_suspend(u64 residency)
> >> +{
> >> +     unsigned int mpidr, cpunr;
> >> +
> >> +     mpidr = read_cpuid_mpidr();
> >> +     cpunr = exynos_pmu_cpunr(mpidr);
> >
> > If I were to be picky, I would compute these values only if they
> > are needed, ie move the computation after exynos_power_down().
> 
> Yes thats make sense. I will realign it.
> 
> >
> > There is another quite horrible issue here. We know this code works
> > because the processors A15/A7 hit the caches with C bit in SCTLR cleared.
> >
> > On processors where this is not true, this sequence would explode
> > if power down fails (in case core is gated but L2 is still powered on,
> > the stack is stuck in L2) since it is going to read stack data that is
> > in L2 but can't be read.
> >
> > It is not related to this sequence only, but it is an issue in general
> > and wanted to mention that on the lists for public awareness.
> >
> 
> Can you please elaborate. I didn't understand.

It is not related to this patch only. This function carries out writes to the
stack (which might end up in eg L2) and then disables the C bit in SCTLR
through MCPM.

A15 and A7 processors hit the cache with the C bit clear in the SCTLR
so the processor still "hits" the stack values if the power down fails.
On processors where caches are not hit with the C bit clear (eg A9) this code
would fail since the stack values that sit in the caches cannot be read with
the C bit clear in SCTLR until the SCTLR is restored, so it will have to
be implemented in assembly with no stack usage (or better, no cacheable data
usage).

So, all I am saying is, to avoid copy'n'paste havoc and to avoid running
this code on Exynos platforms where it must not be run as-is, please add
a comment along the line:

"This function requires the stack data to be visible through power down
and can only be executed on processors like A15 and A7 that hit the cache
with the C bit clear in the SCTLR register."

Please let me know if that's clear.

Lorenzo

> 
> > The gist of what I am saying is, please add a comment to that extent,
> > here and it should be added in exynos_power_down() too.
> >
> >> +     __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 0x1c);
> >
> > No magic numbers please (0x1c). You can add a macro/wrapper, as TC2 does.
> 
> Yes i will remove it.
> 
> >
> >> +     exynos_power_down();
> >> +
> >> +     /*
> >> +      * Execution reaches here only if cpu did not power down.
> >> +      * Hence roll back the changes done in exynos_power_down function.
> >> +     */
> >> +     exynos_cpu_powerup(cpunr);
> >
> > Please be aware that if this function returns MCPM will soft reboot, and
> > the CPUidle driver will have no way to detect a state entry failure.
> >
> > I am just flagging this up, since fixing this behaviour is not easy, and
> > honestly, since power down failure should be the exception not the rule,
> > the idle stats should not be affected much.
> >
> > I think this is the proper way of implementing the sequence but please
> > all keep in mind what I wrote above.
> >
> > Lorenzo
> >
> >> +}
> >> +
> >>  static const struct mcpm_platform_ops exynos_power_ops = {
> >>       .power_up               = exynos_power_up,
> >>       .power_down             = exynos_power_down,
> >>       .power_down_finish      = exynos_power_down_finish,
> >> +     .suspend                = exynos_suspend,
> >> +     .powered_up             = exynos_powered_up,
> >>  };
> >>
> >>  static void __init exynos_mcpm_usage_count_init(void)
> >> --
> >> 1.7.9.5
> >>
> >>
> >
> 
> 
> 
> -- 
> with warm regards,
> Chander Kashyap
> 

[Patch v4 5/5] mcpm: exynos: populate suspend and powered_up callbacks

[Chander Kashyap]

Hi Lorenzo,

On 13 May 2014 22:44, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> On Tue, May 13, 2014 at 12:43:31PM +0100, Chander Kashyap wrote:
>
> [...]
>
>> >> +static void exynos_suspend(u64 residency)
>> >> +{
>> >> +     unsigned int mpidr, cpunr;
>> >> +
>> >> +     mpidr = read_cpuid_mpidr();
>> >> +     cpunr = exynos_pmu_cpunr(mpidr);
>> >
>> > If I were to be picky, I would compute these values only if they
>> > are needed, ie move the computation after exynos_power_down().
>>
>> Yes thats make sense. I will realign it.
>>
>> >
>> > There is another quite horrible issue here. We know this code works
>> > because the processors A15/A7 hit the caches with C bit in SCTLR cleared.
>> >
>> > On processors where this is not true, this sequence would explode
>> > if power down fails (in case core is gated but L2 is still powered on,
>> > the stack is stuck in L2) since it is going to read stack data that is
>> > in L2 but can't be read.
>> >
>> > It is not related to this sequence only, but it is an issue in general
>> > and wanted to mention that on the lists for public awareness.
>> >
>>
>> Can you please elaborate. I didn't understand.
>
> It is not related to this patch only. This function carries out writes to the
> stack (which might end up in eg L2) and then disables the C bit in SCTLR
> through MCPM.
>
> A15 and A7 processors hit the cache with the C bit clear in the SCTLR
> so the processor still "hits" the stack values if the power down fails.
> On processors where caches are not hit with the C bit clear (eg A9) this code
> would fail since the stack values that sit in the caches cannot be read with
> the C bit clear in SCTLR until the SCTLR is restored, so it will have to
> be implemented in assembly with no stack usage (or better, no cacheable data
> usage).
>
> So, all I am saying is, to avoid copy'n'paste havoc and to avoid running
> this code on Exynos platforms where it must not be run as-is, please add
> a comment along the line:
>
> "This function requires the stack data to be visible through power down
> and can only be executed on processors like A15 and A7 that hit the cache
> with the C bit clear in the SCTLR register."
>
> Please let me know if that's clear.

It all clear now.
Thanks a lot.

>
> Lorenzo
>
>>
>> > The gist of what I am saying is, please add a comment to that extent,
>> > here and it should be added in exynos_power_down() too.
>> >
>> >> +     __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 0x1c);
>> >
>> > No magic numbers please (0x1c). You can add a macro/wrapper, as TC2 does.
>>
>> Yes i will remove it.
>>
>> >
>> >> +     exynos_power_down();
>> >> +
>> >> +     /*
>> >> +      * Execution reaches here only if cpu did not power down.
>> >> +      * Hence roll back the changes done in exynos_power_down function.
>> >> +     */
>> >> +     exynos_cpu_powerup(cpunr);
>> >
>> > Please be aware that if this function returns MCPM will soft reboot, and
>> > the CPUidle driver will have no way to detect a state entry failure.
>> >
>> > I am just flagging this up, since fixing this behaviour is not easy, and
>> > honestly, since power down failure should be the exception not the rule,
>> > the idle stats should not be affected much.
>> >
>> > I think this is the proper way of implementing the sequence but please
>> > all keep in mind what I wrote above.
>> >
>> > Lorenzo
>> >
>> >> +}
>> >> +
>> >>  static const struct mcpm_platform_ops exynos_power_ops = {
>> >>       .power_up               = exynos_power_up,
>> >>       .power_down             = exynos_power_down,
>> >>       .power_down_finish      = exynos_power_down_finish,
>> >> +     .suspend                = exynos_suspend,
>> >> +     .powered_up             = exynos_powered_up,
>> >>  };
>> >>
>> >>  static void __init exynos_mcpm_usage_count_init(void)
>> >> --
>> >> 1.7.9.5
>> >>
>> >>
>> >
>>
>>
>>
>> --
>> with warm regards,
>> Chander Kashyap
>>
>



-- 
with warm regards,
Chander Kashyap

[Patch v3 3/5] driver: cpuidle: cpuidle-big-little: init driver for Exynos5420

[Chander Kashyap]

Add "samsung,exynos5420" compatible string to initialize generic
big-little cpuidle driver for Exynos5420.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
Changes in v3:
	1. Add compatible string to of_device_id table insted comparing directoly
Changes in v2: none

 drivers/cpuidle/cpuidle-big_little.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
index 4cd02bd..344d79fa 100644
--- a/drivers/cpuidle/cpuidle-big_little.c
+++ b/drivers/cpuidle/cpuidle-big_little.c
@@ -165,6 +165,7 @@ static int __init bl_idle_driver_init(struct cpuidle_driver *drv, int cpu_id)
 
 static const struct of_device_id compatible_machine_match[] = {
 	{ .compatible = "arm,vexpress,v2p-ca15_a7" },
+	{ .compatible = "samsung,exynos5420" },
 	{},
 };
 
-- 
1.7.9.5

[Patch v3 4/5] exynos: cpuidle: do not allow cpuidle registration for Exynos5420

[Chander Kashyap]

Exynos5420 is big.Little Soc. It uses cpuidle-big-litle generic cpuidle driver.
Hence do not allow exynos cpuidle driver registration for Exynos5420.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/mach-exynos/cpuidle.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index c57cae0..242f75d 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -219,6 +219,9 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
 	int cpu_id, ret;
 	struct cpuidle_device *device;
 
+	if (soc_is_exynos5420())
+		return -ENODEV;
+
 	if (soc_is_exynos5250())
 		exynos5_core_down_clk();
 
-- 
1.7.9.5

[Patch v3 5/5] mcpm: exynos: populate suspend and powered_up callbacks

[Chander Kashyap]

In order to support cpuidle through mcpm, suspend and powered-up
callbacks are required in mcpm platform code.
Hence populate the same callbacks.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Chander Kashyap <k.chander@samsung.com>
---
Changes in v3:
	1. Removed coherance enablement after suspend failure.
	2. Use generic function to poweron cpu.
changes in v2:
	1. Fixed typo: enynos_pmu_cpunr to exynos_pmu_cpunr
 arch/arm/mach-exynos/mcpm-exynos.c |   34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
index d0f7461..6d4a907 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -256,10 +256,44 @@ static int exynos_power_down_finish(unsigned int cpu, unsigned int cluster)
 	return -ETIMEDOUT; /* timeout */
 }
 
+void exynos_powered_up(void)
+{
+	unsigned int mpidr, cpu, cluster;
+
+	mpidr = read_cpuid_mpidr();
+	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+	arch_spin_lock(&exynos_mcpm_lock);
+	if (cpu_use_count[cpu][cluster] == 0)
+		cpu_use_count[cpu][cluster] = 1;
+	arch_spin_unlock(&exynos_mcpm_lock);
+}
+
+static void exynos_suspend(u64 residency)
+{
+	unsigned int mpidr, cpunr;
+
+	mpidr = read_cpuid_mpidr();
+	cpunr = exynos_pmu_cpunr(mpidr);
+
+	__raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 0x1c);
+
+	exynos_power_down();
+
+	/*
+	 * Execution reaches here only if cpu did not power down.
+	 * Hence roll back the changes done in exynos_power_down function.
+	*/
+	exynos_cpu_powerup(cpunr);
+}
+
 static const struct mcpm_platform_ops exynos_power_ops = {
 	.power_up		= exynos_power_up,
 	.power_down		= exynos_power_down,
 	.power_down_finish	= exynos_power_down_finish,
+	.suspend		= exynos_suspend,
+	.powered_up		= exynos_powered_up,
 };
 
 static void __init exynos_mcpm_usage_count_init(void)
-- 
1.7.9.5