amit daniel kachhap wrote: > > 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? I’d preferred to use 2nd :) > 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. > Sounds good to me. Thanks, Kukjin > Thanks, > Amit Daniel > > On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano > <daniel.lezcano@xxxxxxxxxx> 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@xxxxxxxxxxx> > >>>>> --- > >>>>> > >>>>> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |