2014-04-21 11:49:54

by Chander Kashyap

[permalink] [raw]
Subject: [PATCH 0/4] add cpuidle support for Exynos5420

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


2014-04-21 11:50:10

by Chander Kashyap

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

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 <[email protected]>
Signed-off-by: Chander Kashyap <[email protected]>
---
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

2014-04-21 11:50:15

by Chander Kashyap

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

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 <[email protected]>
Signed-off-by: Chander Kashyap <[email protected]>
---
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

2014-04-21 11:50:03

by Chander Kashyap

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

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

Signed-off-by: Chander Kashyap <[email protected]>
Signed-off-by: Chander Kashyap <[email protected]>
---
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

2014-04-21 11:50:00

by Chander Kashyap

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

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 <[email protected]>
Signed-off-by: Chander Kashyap <[email protected]>
---
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

2014-04-22 10:38:07

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 3/4] exynos: cpuidle: do not allow cpuidle registration for Exynos5420

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 <[email protected]>
> Signed-off-by: Chander Kashyap <[email protected]>
> ---

Acked-by: Daniel Lezcano <[email protected]>

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

2014-04-22 10:39:09

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 2/4] driver: cpuidle: cpuidle-big-little: init driver for Exynos5420

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 <[email protected]>
> Signed-off-by: Chander Kashyap <[email protected]>
> ---

To be migrated to platform_driver but until that:

Acked-by: Daniel Lezcano <[email protected]>

> 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

2014-04-22 10:41:45

by Daniel Lezcano

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

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 <[email protected]>
> Signed-off-by: Chander Kashyap <[email protected]>
> ---
> 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

2014-04-22 10:50:46

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 4/4] mcpm: exynos: populate suspend and powered_up callbacks

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 <[email protected]>
> Signed-off-by: Chander Kashyap <[email protected]>
> ---
> 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

2014-04-22 11:12:16

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 2/4] driver: cpuidle: cpuidle-big-little: init driver for Exynos5420

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 <[email protected]>
> Signed-off-by: Chander Kashyap <[email protected]>
> ---
> 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

2014-04-23 06:20:06

by Chander Kashyap

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

Hi Daniel,

On 22 April 2014 16:12, Daniel Lezcano <[email protected]> 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 <[email protected]>
>> Signed-off-by: Chander Kashyap <[email protected]>
>> ---
>> 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

2014-04-23 08:22:48

by Chander Kashyap

[permalink] [raw]
Subject: Re: [PATCH 4/4] mcpm: exynos: populate suspend and powered_up callbacks

On 22 April 2014 16:21, Daniel Lezcano <[email protected]> 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 <[email protected]>
>> Signed-off-by: Chander Kashyap <[email protected]>
>> ---
>> 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

2014-04-23 09:26:23

by Chander Kashyap

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

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

Signed-off-by: Chander Kashyap <[email protected]>
Signed-off-by: Chander Kashyap <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
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

2014-04-23 09:26:35

by Chander Kashyap

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

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 <[email protected]>
Signed-off-by: Chander Kashyap <[email protected]>
---
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

2014-04-23 09:26:32

by Chander Kashyap

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

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 <[email protected]>
Signed-off-by: Chander Kashyap <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
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

2014-04-23 09:26:18

by Chander Kashyap

[permalink] [raw]
Subject: [Patch v2 0/4] add cpuidle support for Exynos5420

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

2014-04-23 09:29:31

by Chander Kashyap

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

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 <[email protected]>
Signed-off-by: Chander Kashyap <[email protected]>
---
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

2014-04-23 10:02:06

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [Patch v2 0/4] add cpuidle support for Exynos5420

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.

2014-04-23 15:42:39

by Kukjin Kim

[permalink] [raw]
Subject: Re: [Patch v2 0/4] add cpuidle support for Exynos5420

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

2014-04-23 16:03:08

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [Patch v2 4/4] mcpm: exynos: populate suspend and powered_up callbacks

[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 <[email protected]>
> Signed-off-by: Chander Kashyap <[email protected]>
> ---
> 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

2014-04-23 16:33:00

by Lorenzo Pieralisi

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

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 <[email protected]>
> Signed-off-by: Chander Kashyap <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>
> ---
> 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

2014-04-24 07:44:47

by Chander Kashyap

[permalink] [raw]
Subject: Re: [Patch v2 4/4] mcpm: exynos: populate suspend and powered_up callbacks

On 23 April 2014 21:32, Lorenzo Pieralisi <[email protected]> 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 <[email protected]>
>> Signed-off-by: Chander Kashyap <[email protected]>
>> ---
>> 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

2014-04-24 07:47:11

by Chander Kashyap

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

On 23 April 2014 22:02, Lorenzo Pieralisi <[email protected]> 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 <[email protected]>
>> Signed-off-by: Chander Kashyap <[email protected]>
>> Acked-by: Daniel Lezcano <[email protected]>
>> ---
>> 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