[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[amit daniel kachhap]

Hi Daniel/Tomasz,

>From the discussion I can conclude that SOC check is needed in the
cpuidle driver for deeper C states. Only the question is where to
insert this.
Also to perform the SOC there can be 2 ways like
1) each SOC check 4120, 4412, 5250 etc (long list)
2) negate the nonsupporting SOC's like 5440 (small list like current patch)
Any opinion?
 As Bartlomiej suggested that this patch conflicts with Daniel's
earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
So I can re-base my patch on top of this one if needed.

Thanks,
Amit Daniel

On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
<daniel.lezcano at linaro.org> wrote:
> On 07/28/2013 11:22 AM, Tomasz Figa wrote:
>> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
>>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
>>>> Amit Daniel Kachhap wrote:
>>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
>>>>> exynos5440
>>>>> soc as this soc does not support this state. All the cpu's only
>>>>> allows the basic
>>>>> C0 state.
>>>>>
>>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel at samsung.com>
>>>>> ---
>>>>>
>>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>>>>> exynos/cpuidle.c
>>>>> index 17a18ff..9a776a1 100644
>>>>> --- a/arch/arm/mach-exynos/cpuidle.c
>>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
>>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>>>>>
>>>>>            device->cpu = cpu_id;
>>>>>
>>>>>            /* Support IDLE only */
>>>>>
>>>>> -          if (cpu_id != 0)
>>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
>>>>>
>>>>>                    device->state_count = 1;
>>>>>
>>>>>            ret = cpuidle_register_device(device);
>>>>>
>>>>> --
>>>>> 1.7.1
>>>>
>>>> Applied, thanks.
>>>
>>> You shouldn't have. This patch means exynos5540 has no cpuidle driver at
>>> all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
>>> an exynos5540.
>>
>> To shed more light on this, let me add that you need to register a cpuidle
>> driver only if you have more states than a simple WFI or you need some
>> crazy steps to enter WFI. Default setup falls back to generic ARM WFI.
>> (Daniel, do we get the nice idle stats as provided by cpuidle core then?)
>
> Nope, but with one state, idle vs busy stats do the trick.
>
> BTW, I am writing a tool to do some stats based on the idle events [1].
> It is still at a very early development stage but we can get some
> interesting informations.
>
>
>> Anyway, Exynos cpuidle is using an initcall to initialize and we support
>> multiple Exynos SoCs in single zImage, so deselecting CONFIG_CPU_IDLE is
>> not an option.
>
> Good point.
>
>> Considering multiplatform requirements, the driver has to
>> be modified to initialize only on supported platforms, either by:
>>
>>  a) dropping the initcall and calling the init function directly from
>> arch/arm/mach-exynos
>>
>>  or
>>
>>  b) checking if machine we are running on is supported, which would mean a
>> long list of all Exynos SoCs that needs to be checked.
>>
>> An evolution of option a) is registering a platform device somewhere in
>> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
>
> Yes, I am favorable to this solution [2].
>
>> The
>> problem is that you must register a static platform device from arch code,
>> because cpuidle is not a real hardware block that can be put into device
>> tree.
>
> Thanks
>   -- Daniel
>
> [1] https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summary
> [2] http://patches.linaro.org/18368/
>
> --
>  <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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html