Re: [PATCH V2 17/17] ARM: exynos: config: Enable cpuidle

2014-04-08 Thread Bartlomiej Zolnierkiewicz 
On Tuesday, April 08, 2014 09:27:04 AM Sachin Kamat wrote:
> On 8 April 2014 00:03, Olof Johansson  wrote:
> > On Mon, Apr 7, 2014 at 10:44 AM, Bartlomiej Zolnierkiewicz
> >  wrote:
> >>
> >> Hi,
> >>
> >> On Monday, April 07, 2014 09:28:39 AM Olof Johansson wrote:
> >>> On Fri, Apr 4, 2014 at 9:09 AM, Bartlomiej Zolnierkiewicz
> >>>  wrote:
> >>>
> >>> [...]
> >
> > Does 5420 even work upstream? I have hardware access but nothing that
> > will run an upstream kernel as far as I know. Maybe SLSI has an SMDK
> > they can help out with?
> 
> By working upstream if you mean being bootable, then yes I am able to boot 
> both
> SMDK and Arndale Octa boards based on 5420 with the current upstream kernel.
> CPU idle however doesn't work though. I get a crash when I hot plug out all
> secondary CPUs on SMDK.

Thanks for testing.  This behavior is an expected one given the current code
(=> we need to disable AFTR on 5420 for now).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 17/17] ARM: exynos: config: Enable cpuidle

2014-04-08 Thread Bartlomiej Zolnierkiewicz 
On Monday, April 07, 2014 11:33:57 AM Olof Johansson wrote:
> On Mon, Apr 7, 2014 at 10:44 AM, Bartlomiej Zolnierkiewicz
>  wrote:
> >
> > Hi,
> >
> > On Monday, April 07, 2014 09:28:39 AM Olof Johansson wrote:
> >> On Fri, Apr 4, 2014 at 9:09 AM, Bartlomiej Zolnierkiewicz
> >>  wrote:
> >>
> >> [...]
> >> >   I know that this is mainly our problem but the issue is widespread on
> >> >   our targets and I believe that adding some workaround for it in cpuidle
> >> >   core would be beneficial for the whole cpuidle subsystem.
> >
> > On the second look at the cpuidle code there is a "cpuidle.off" kernel
> > parameter which should be sufficient to workaround this particular issue
> > for now.
> >
> >> Yes, we need to find a way forward without you guys holding the whole
> >> platform hostage with out-of-tree code (here and in general, since I
> >> think there are more areas in which this applies).
> >
> > Please explain this more because I really don't know what you're meaning
> > here (at least in case of SRPOL I feel that there are no such issues).
> >
> > In this particular case we have a problem with a modified uboot bootloader
> > versions being broken and incompatible with the advanced AFTR cpuidle mode.
> > This is not the only / main issue preventing AFTR mode and thus EXYNOS
> > cpuidle driver from being used by default so I really think that the your
> > comment was unfair.
> 
> Holding off features for users of the platform because your firmware
> is too old (and you can't control the feature per platform) is going
> to get harder and harder, so being able to enable these kind of things
> at runtime will be important. I.e. as features go in, it's something
> that needs to be considered (runtime checking vs ifdef).

In this particular case runtime detection of broken uboot versions is not
possible, otherwise the issue would have been addressed a long time ago.

Anyway this is not the main problem here and we can deal with it on our
side (we will just use "cpuidle.off" on our affected targets).

> Calling that hostage taking? Yeah, maybe a little on the harsh side.
> But the problem definitely exists.

I don't agree that there is some kind of general problem on our side
and you've not provided any specific issues that would fall under this
category.

> >> >  Namely there
> >> >   should be some way of telling cpuidle subsystem to either disable
> >> >   particular state(s) or limit the max available state.  I think that 
> >> > this
> >> >   can be also useful for testing and development of other cpuidle 
> >> > drivers.
> >> >
> >> > For now please change this patch to add CONFIG_CPU_IDLE=n instead (since
> >> > the config option is "default y" it will be auto-enabled if there is no
> >> > entry in the defconfig)..
> >>
> >> Can the code be refactored such that even if CPU_IDLE is on, it won't
> >> actually do anything useful on the platforms that have problems above?
> >> I.e. determined at runtime, not build time?
> >
> > We can disable AFTR mode by default on EXYNOS4x12 SoCs with secure mode
> > enabled and EXYNOS5420 SoCs (I would like somebody with the hardware to
> > verify that AFTR mode is not working first so we have 100% certainty
> > that we don't regress here).
> 
> Does 5420 even work upstream? I have hardware access but nothing that
> will run an upstream kernel as far as I know. Maybe SLSI has an SMDK
> they can help out with?

I don't know the current status of 5420.  The initial support was added by
Linaro (I added Chander and Sachin to cc:).

If the upstream support is working to check the AFTR mode one needs to
enable cpuidle driver (CONFIG_CPU_IDLE=y) and offline all CPUs except CPU0
(through sysfs using "echo 0 > /sys/devices/system/cpu/cpuX/online").

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 17/17] ARM: exynos: config: Enable cpuidle

2014-04-07 Thread Sachin Kamat 
On 8 April 2014 00:03, Olof Johansson  wrote:
> On Mon, Apr 7, 2014 at 10:44 AM, Bartlomiej Zolnierkiewicz
>  wrote:
>>
>> Hi,
>>
>> On Monday, April 07, 2014 09:28:39 AM Olof Johansson wrote:
>>> On Fri, Apr 4, 2014 at 9:09 AM, Bartlomiej Zolnierkiewicz
>>>  wrote:
>>>
>>> [...]
>
> Does 5420 even work upstream? I have hardware access but nothing that
> will run an upstream kernel as far as I know. Maybe SLSI has an SMDK
> they can help out with?

By working upstream if you mean being bootable, then yes I am able to boot both
SMDK and Arndale Octa boards based on 5420 with the current upstream kernel.
CPU idle however doesn't work though. I get a crash when I hot plug out all
secondary CPUs on SMDK.

-- 
With warm regards,
Sachin
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 17/17] ARM: exynos: config: Enable cpuidle

2014-04-07 Thread Olof Johansson 
On Mon, Apr 7, 2014 at 10:44 AM, Bartlomiej Zolnierkiewicz
 wrote:
>
> Hi,
>
> On Monday, April 07, 2014 09:28:39 AM Olof Johansson wrote:
>> On Fri, Apr 4, 2014 at 9:09 AM, Bartlomiej Zolnierkiewicz
>>  wrote:
>>
>> [...]
>> >   I know that this is mainly our problem but the issue is widespread on
>> >   our targets and I believe that adding some workaround for it in cpuidle
>> >   core would be beneficial for the whole cpuidle subsystem.
>
> On the second look at the cpuidle code there is a "cpuidle.off" kernel
> parameter which should be sufficient to workaround this particular issue
> for now.
>
>> Yes, we need to find a way forward without you guys holding the whole
>> platform hostage with out-of-tree code (here and in general, since I
>> think there are more areas in which this applies).
>
> Please explain this more because I really don't know what you're meaning
> here (at least in case of SRPOL I feel that there are no such issues).
>
> In this particular case we have a problem with a modified uboot bootloader
> versions being broken and incompatible with the advanced AFTR cpuidle mode.
> This is not the only / main issue preventing AFTR mode and thus EXYNOS
> cpuidle driver from being used by default so I really think that the your
> comment was unfair.

Holding off features for users of the platform because your firmware
is too old (and you can't control the feature per platform) is going
to get harder and harder, so being able to enable these kind of things
at runtime will be important. I.e. as features go in, it's something
that needs to be considered (runtime checking vs ifdef).

Calling that hostage taking? Yeah, maybe a little on the harsh side.
But the problem definitely exists.

>> >  Namely there
>> >   should be some way of telling cpuidle subsystem to either disable
>> >   particular state(s) or limit the max available state.  I think that this
>> >   can be also useful for testing and development of other cpuidle drivers.
>> >
>> > For now please change this patch to add CONFIG_CPU_IDLE=n instead (since
>> > the config option is "default y" it will be auto-enabled if there is no
>> > entry in the defconfig)..
>>
>> Can the code be refactored such that even if CPU_IDLE is on, it won't
>> actually do anything useful on the platforms that have problems above?
>> I.e. determined at runtime, not build time?
>
> We can disable AFTR mode by default on EXYNOS4x12 SoCs with secure mode
> enabled and EXYNOS5420 SoCs (I would like somebody with the hardware to
> verify that AFTR mode is not working first so we have 100% certainty
> that we don't regress here).

Does 5420 even work upstream? I have hardware access but nothing that
will run an upstream kernel as far as I know. Maybe SLSI has an SMDK
they can help out with?


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 17/17] ARM: exynos: config: Enable cpuidle

2014-04-07 Thread Bartlomiej Zolnierkiewicz 

Hi,

On Monday, April 07, 2014 09:28:39 AM Olof Johansson wrote:
> On Fri, Apr 4, 2014 at 9:09 AM, Bartlomiej Zolnierkiewicz
>  wrote:
> 
> [...]
> >   I know that this is mainly our problem but the issue is widespread on
> >   our targets and I believe that adding some workaround for it in cpuidle
> >   core would be beneficial for the whole cpuidle subsystem.

On the second look at the cpuidle code there is a "cpuidle.off" kernel
parameter which should be sufficient to workaround this particular issue
for now.

> Yes, we need to find a way forward without you guys holding the whole
> platform hostage with out-of-tree code (here and in general, since I
> think there are more areas in which this applies).

Please explain this more because I really don't know what you're meaning
here (at least in case of SRPOL I feel that there are no such issues).

In this particular case we have a problem with a modified uboot bootloader
versions being broken and incompatible with the advanced AFTR cpuidle mode.
This is not the only / main issue preventing AFTR mode and thus EXYNOS
cpuidle driver from being used by default so I really think that the your
comment was unfair.

> >  Namely there
> >   should be some way of telling cpuidle subsystem to either disable
> >   particular state(s) or limit the max available state.  I think that this
> >   can be also useful for testing and development of other cpuidle drivers.
> >
> > For now please change this patch to add CONFIG_CPU_IDLE=n instead (since
> > the config option is "default y" it will be auto-enabled if there is no
> > entry in the defconfig)..
> 
> Can the code be refactored such that even if CPU_IDLE is on, it won't
> actually do anything useful on the platforms that have problems above?
> I.e. determined at runtime, not build time?

We can disable AFTR mode by default on EXYNOS4x12 SoCs with secure mode
enabled and EXYNOS5420 SoCs (I would like somebody with the hardware to
verify that AFTR mode is not working first so we have 100% certainty
that we don't regress here).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 17/17] ARM: exynos: config: Enable cpuidle

2014-04-07 Thread Olof Johansson 
On Fri, Apr 4, 2014 at 9:09 AM, Bartlomiej Zolnierkiewicz
 wrote:

[...]
>   I know that this is mainly our problem but the issue is widespread on
>   our targets and I believe that adding some workaround for it in cpuidle
>   core would be beneficial for the whole cpuidle subsystem.

Yes, we need to find a way forward without you guys holding the whole
platform hostage with out-of-tree code (here and in general, since I
think there are more areas in which this applies).

>  Namely there
>   should be some way of telling cpuidle subsystem to either disable
>   particular state(s) or limit the max available state.  I think that this
>   can be also useful for testing and development of other cpuidle drivers.
>
> For now please change this patch to add CONFIG_CPU_IDLE=n instead (since
> the config option is "default y" it will be auto-enabled if there is no
> entry in the defconfig)..

Can the code be refactored such that even if CPU_IDLE is on, it won't
actually do anything useful on the platforms that have problems above?
I.e. determined at runtime, not build time?


(I have not yet looked at the rest of the series).


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 17/17] ARM: exynos: config: Enable cpuidle

2014-04-07 Thread Olof Johansson 
On Mon, Apr 7, 2014 at 9:19 AM, Bartlomiej Zolnierkiewicz
 wrote:
> On Monday, April 07, 2014 11:43:47 AM Daniel Lezcano wrote:
>> On 04/04/2014 06:09 PM, Bartlomiej Zolnierkiewicz wrote:
>> >
>> > On Friday, April 04, 2014 03:43:09 PM Daniel Lezcano wrote:
>> >> The cpuidle driver is broken since v3.11 and now we are at v3.14.
>> >>
>> >> Default the cpuidle driver to favorize a better detection next time.
>> >>
>> >> Signed-off-by: Daniel Lezcano 
>> >> Reviewed-by: Viresh Kumar 
>> >> ---
>> >>   arch/arm/configs/exynos_defconfig |1 +
>> >>   1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/arch/arm/configs/exynos_defconfig 
>> >> b/arch/arm/configs/exynos_defconfig
>> >> index 4ce7b70..6ed4b34 100644
>> >> --- a/arch/arm/configs/exynos_defconfig
>> >> +++ b/arch/arm/configs/exynos_defconfig
>> >> @@ -132,3 +132,4 @@ CONFIG_DEBUG_INFO=y
>> >>   CONFIG_DEBUG_USER=y
>> >>   CONFIG_CRYPTO_SHA256=y
>> >>   CONFIG_CRC_CCITT=y
>> >> +CONFIG_CPU_IDLE=y
>> >
>> > Sorry but I have to NAK this change.  There are three issues with AFTR
>> > mode that should be resolved first before this patch can be merged:
>> >
>> > * The upstream Exynos cpuidle code lacks support for secure firmware and
>> >it is used on i.e. Trats2 boards (Exynos4412).  Attempts to use AFTR on
>> >such hardware results in oops + lockup.
>> >I have patches adding secure firmware support to Exynos cpuidle driver
>> >but they depend on Tomasz Figa's PM changes which are not yet upstream.
>>
>> Ok.
>>
>> > * Somebody needs to verify that the current Exynos cpuidle driver works
>> >in the AFTR mode on Exynos5420 for which support has been added recently
>> >(I really doubt it looking at some internal trees).
>>
>> Rajeshwari (cc'ed) is working on creating a big.Little driver for this
>> board, so it will be a separate cpuidle driver.
>>
>> > * Some of our u-boot bootloader versions are incompatible with AFTR.  We
>> >have observed the problem happening on Trats board (Exynos4210) on which
>> >it can be fixed by using the upstream u-boot version and Universal C210
>> >board (Exynos4210 with broken SMP support) on which there is no upstream
>> >u-boot available (IIRC) and because of the broken SMP support enabling
>> >cpuidle results in attempt to enter AFTR during boot + immediate lockup.
>> >
>> >I know that this is mainly our problem but the issue is widespread on
>> >our targets and I believe that adding some workaround for it in cpuidle
>> >core would be beneficial for the whole cpuidle subsystem.  Namely there
>> >should be some way of telling cpuidle subsystem to either disable
>> >particular state(s) or limit the max available state.  I think that this
>> >can be also useful for testing and development of other cpuidle drivers.
>> >
>> > For now please change this patch to add CONFIG_CPU_IDLE=n instead (since
>> > the config option is "default y" it will be auto-enabled if there is no
>> > entry in the defconfig)..
>>
>> Ok.
>>
>> Kukjin, is it possible you merge patches 1->16 ?
>
> It is OK given that patch #17 is fixed and merged at the same time
> (preferably it should go before patches #1-16 to preserve bisectability).
> Alternatively 'default y' should be removed from patch #16 before
> the merge.  We really don't want to have EXYNOS cpuidle driver enabled
> by default yet.

Default y is almost always the wrong thing to use, so that should be
taken out no matter what.


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 17/17] ARM: exynos: config: Enable cpuidle

2014-04-07 Thread Bartlomiej Zolnierkiewicz 
On Monday, April 07, 2014 11:43:47 AM Daniel Lezcano wrote:
> On 04/04/2014 06:09 PM, Bartlomiej Zolnierkiewicz wrote:
> >
> > On Friday, April 04, 2014 03:43:09 PM Daniel Lezcano wrote:
> >> The cpuidle driver is broken since v3.11 and now we are at v3.14.
> >>
> >> Default the cpuidle driver to favorize a better detection next time.
> >>
> >> Signed-off-by: Daniel Lezcano 
> >> Reviewed-by: Viresh Kumar 
> >> ---
> >>   arch/arm/configs/exynos_defconfig |1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/arch/arm/configs/exynos_defconfig 
> >> b/arch/arm/configs/exynos_defconfig
> >> index 4ce7b70..6ed4b34 100644
> >> --- a/arch/arm/configs/exynos_defconfig
> >> +++ b/arch/arm/configs/exynos_defconfig
> >> @@ -132,3 +132,4 @@ CONFIG_DEBUG_INFO=y
> >>   CONFIG_DEBUG_USER=y
> >>   CONFIG_CRYPTO_SHA256=y
> >>   CONFIG_CRC_CCITT=y
> >> +CONFIG_CPU_IDLE=y
> >
> > Sorry but I have to NAK this change.  There are three issues with AFTR
> > mode that should be resolved first before this patch can be merged:
> >
> > * The upstream Exynos cpuidle code lacks support for secure firmware and
> >it is used on i.e. Trats2 boards (Exynos4412).  Attempts to use AFTR on
> >such hardware results in oops + lockup.
> >I have patches adding secure firmware support to Exynos cpuidle driver
> >but they depend on Tomasz Figa's PM changes which are not yet upstream.
> 
> Ok.
> 
> > * Somebody needs to verify that the current Exynos cpuidle driver works
> >in the AFTR mode on Exynos5420 for which support has been added recently
> >(I really doubt it looking at some internal trees).
> 
> Rajeshwari (cc'ed) is working on creating a big.Little driver for this 
> board, so it will be a separate cpuidle driver.
> 
> > * Some of our u-boot bootloader versions are incompatible with AFTR.  We
> >have observed the problem happening on Trats board (Exynos4210) on which
> >it can be fixed by using the upstream u-boot version and Universal C210
> >board (Exynos4210 with broken SMP support) on which there is no upstream
> >u-boot available (IIRC) and because of the broken SMP support enabling
> >cpuidle results in attempt to enter AFTR during boot + immediate lockup.
> >
> >I know that this is mainly our problem but the issue is widespread on
> >our targets and I believe that adding some workaround for it in cpuidle
> >core would be beneficial for the whole cpuidle subsystem.  Namely there
> >should be some way of telling cpuidle subsystem to either disable
> >particular state(s) or limit the max available state.  I think that this
> >can be also useful for testing and development of other cpuidle drivers.
> >
> > For now please change this patch to add CONFIG_CPU_IDLE=n instead (since
> > the config option is "default y" it will be auto-enabled if there is no
> > entry in the defconfig)..
> 
> Ok.
> 
> Kukjin, is it possible you merge patches 1->16 ?

It is OK given that patch #17 is fixed and merged at the same time
(preferably it should go before patches #1-16 to preserve bisectability).
Alternatively 'default y' should be removed from patch #16 before
the merge.  We really don't want to have EXYNOS cpuidle driver enabled
by default yet.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 17/17] ARM: exynos: config: Enable cpuidle

2014-04-07 Thread Daniel Lezcano 

On 04/04/2014 06:09 PM, Bartlomiej Zolnierkiewicz wrote:


On Friday, April 04, 2014 03:43:09 PM Daniel Lezcano wrote:

The cpuidle driver is broken since v3.11 and now we are at v3.14.

Default the cpuidle driver to favorize a better detection next time.

Signed-off-by: Daniel Lezcano 
Reviewed-by: Viresh Kumar 
---
  arch/arm/configs/exynos_defconfig |1 +
  1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/exynos_defconfig 
b/arch/arm/configs/exynos_defconfig
index 4ce7b70..6ed4b34 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -132,3 +132,4 @@ CONFIG_DEBUG_INFO=y
  CONFIG_DEBUG_USER=y
  CONFIG_CRYPTO_SHA256=y
  CONFIG_CRC_CCITT=y
+CONFIG_CPU_IDLE=y


Sorry but I have to NAK this change.  There are three issues with AFTR
mode that should be resolved first before this patch can be merged:

* The upstream Exynos cpuidle code lacks support for secure firmware and
   it is used on i.e. Trats2 boards (Exynos4412).  Attempts to use AFTR on
   such hardware results in oops + lockup.
   I have patches adding secure firmware support to Exynos cpuidle driver
   but they depend on Tomasz Figa's PM changes which are not yet upstream.


Ok.


* Somebody needs to verify that the current Exynos cpuidle driver works
   in the AFTR mode on Exynos5420 for which support has been added recently
   (I really doubt it looking at some internal trees).


Rajeshwari (cc'ed) is working on creating a big.Little driver for this 
board, so it will be a separate cpuidle driver.



* Some of our u-boot bootloader versions are incompatible with AFTR.  We
   have observed the problem happening on Trats board (Exynos4210) on which
   it can be fixed by using the upstream u-boot version and Universal C210
   board (Exynos4210 with broken SMP support) on which there is no upstream
   u-boot available (IIRC) and because of the broken SMP support enabling
   cpuidle results in attempt to enter AFTR during boot + immediate lockup.

   I know that this is mainly our problem but the issue is widespread on
   our targets and I believe that adding some workaround for it in cpuidle
   core would be beneficial for the whole cpuidle subsystem.  Namely there
   should be some way of telling cpuidle subsystem to either disable
   particular state(s) or limit the max available state.  I think that this
   can be also useful for testing and development of other cpuidle drivers.

For now please change this patch to add CONFIG_CPU_IDLE=n instead (since
the config option is "default y" it will be auto-enabled if there is no
entry in the defconfig)..


Ok.

Kukjin, is it possible you merge patches 1->16 ?

Thanks

  -- Daniel


--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics




--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 17/17] ARM: exynos: config: Enable cpuidle

2014-04-04 Thread Bartlomiej Zolnierkiewicz 

On Friday, April 04, 2014 03:43:09 PM Daniel Lezcano wrote:
> The cpuidle driver is broken since v3.11 and now we are at v3.14.
> 
> Default the cpuidle driver to favorize a better detection next time.
> 
> Signed-off-by: Daniel Lezcano 
> Reviewed-by: Viresh Kumar 
> ---
>  arch/arm/configs/exynos_defconfig |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/configs/exynos_defconfig 
> b/arch/arm/configs/exynos_defconfig
> index 4ce7b70..6ed4b34 100644
> --- a/arch/arm/configs/exynos_defconfig
> +++ b/arch/arm/configs/exynos_defconfig
> @@ -132,3 +132,4 @@ CONFIG_DEBUG_INFO=y
>  CONFIG_DEBUG_USER=y
>  CONFIG_CRYPTO_SHA256=y
>  CONFIG_CRC_CCITT=y
> +CONFIG_CPU_IDLE=y

Sorry but I have to NAK this change.  There are three issues with AFTR
mode that should be resolved first before this patch can be merged:

* The upstream Exynos cpuidle code lacks support for secure firmware and
  it is used on i.e. Trats2 boards (Exynos4412).  Attempts to use AFTR on
  such hardware results in oops + lockup.

  I have patches adding secure firmware support to Exynos cpuidle driver
  but they depend on Tomasz Figa's PM changes which are not yet upstream.

* Somebody needs to verify that the current Exynos cpuidle driver works
  in the AFTR mode on Exynos5420 for which support has been added recently
  (I really doubt it looking at some internal trees).

* Some of our u-boot bootloader versions are incompatible with AFTR.  We
  have observed the problem happening on Trats board (Exynos4210) on which
  it can be fixed by using the upstream u-boot version and Universal C210
  board (Exynos4210 with broken SMP support) on which there is no upstream
  u-boot available (IIRC) and because of the broken SMP support enabling
  cpuidle results in attempt to enter AFTR during boot + immediate lockup.

  I know that this is mainly our problem but the issue is widespread on
  our targets and I believe that adding some workaround for it in cpuidle
  core would be beneficial for the whole cpuidle subsystem.  Namely there
  should be some way of telling cpuidle subsystem to either disable
  particular state(s) or limit the max available state.  I think that this
  can be also useful for testing and development of other cpuidle drivers.

For now please change this patch to add CONFIG_CPU_IDLE=n instead (since
the config option is "default y" it will be auto-enabled if there is no
entry in the defconfig)..

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html