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

2014-04-24 Thread Chander Kashyap
On 23 April 2014 21:32, Lorenzo Pieralisi  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 
>> Signed-off-by: Chander Kashyap 
>> ---
>> 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(
>> + "mrcp15, 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"
>> + "mcrp15, 0, %0, c1, c0, 1\n"
>> + : "=" (v), "=" (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(_mcpm_lock);
>> + if (cpu_use_count[cpu][cluster] == 0)
>> + cpu_use_count[cpu][cluster] = 1;
>> + arch_spin_unlock(_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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-04-24 Thread Chander Kashyap
On 23 April 2014 21:32, Lorenzo Pieralisi lorenzo.pieral...@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.kash...@linaro.org
 Signed-off-by: Chander Kashyap k.chan...@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(
 + mrcp15, 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
 + mcrp15, 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-04-23 Thread 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 
> Signed-off-by: Chander Kashyap 
> ---
> 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(
> + "mrcp15, 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"
> + "mcrp15, 0, %0, c1, c0, 1\n"
> + : "=" (v), "=" (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(_mcpm_lock);
> + if (cpu_use_count[cpu][cluster] == 0)
> + cpu_use_count[cpu][cluster] = 1;
> + arch_spin_unlock(_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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-04-23 Thread 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.kash...@linaro.org
 Signed-off-by: Chander Kashyap k.chan...@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(
 + mrcp15, 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
 + mcrp15, 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

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/