Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul

Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Lucas Stach]

Am Donnerstag, den 25.08.2016, 00:13 +0800 schrieb Anson Huang:
> This patch enables cpuidle driver for i.MX6UL, it
> reuses i.MX6SX's cpuidle driver, 3 levels of cpuidle
> supported:
> 
> 1. ARM WFI;
> 2. SOC in WAIT mode;
> 3. SOC in WAIT mode + ARM power off.
> 
> As i.MX6UL has cortex-A7 CORE with an internal L2
> cache, so need to add L2 cache type check to decide
> if to flush L2 when entering idle with ARM power off,
> this is different from i.MX6SX which has PL310 external
> L2 cache.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
>  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach-imx/cpuidle-imx6sx.c
> index 41cdce6..b4fcc8ae 100644
> --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> @@ -9,14 +9,21 @@
>  #include <linux/cpuidle.h>
>  #include <linux/cpu_pm.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <asm/cacheflush.h>
>  #include <asm/cpuidle.h>
>  #include <asm/suspend.h>
>  
>  #include "common.h"
>  #include "cpuidle.h"
>  
> +static bool pl310_available;
> +
>  static int imx6sx_idle_finish(unsigned long val)
>  {
> +	/* check if need to flush internal L2 cache */
> +	if (!pl310_available)
> +		flush_cache_all();

I think this is only necessary when entering the deepest idle state with
the ARM core powered off, right? It's a serious overhead for the shallow
idle states.

Also flush_cache_all() only flushes architected/inner caches, so it
should do the right thing for this use-case even if the outer PL310
cache is present. There is no need to make this conditional.

Regards,
Lucas
 
>  	cpu_do_idle();
>  
>  	return 0;
> @@ -101,5 +108,8 @@ int __init imx6sx_cpuidle_init(void)
>  	imx_gpc_set_arm_power_up_timing(2, 1);
>  	imx_gpc_set_arm_power_down_timing(1, 1);
>  
> +	if (of_find_compatible_node(NULL, NULL, "arm,pl310-cache"))
> +		pl310_available = true;
> +
>  	return cpuidle_register(&imx6sx_cpuidle_driver, NULL);
>  }
> diff --git a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
> index c2cd61c..9c5e8f3 100644
> --- a/arch/arm/mach-imx/mach-imx6ul.c
> +++ b/arch/arm/mach-imx/mach-imx6ul.c
> @@ -16,6 +16,7 @@
>  #include <asm/mach/map.h>
>  
>  #include "common.h"
> +#include "cpuidle.h"
>  
>  static void __init imx6ul_enet_clk_init(void)
>  {
> @@ -80,6 +81,8 @@ static void __init imx6ul_init_irq(void)
>  
>  static void __init imx6ul_init_late(void)
>  {
> +	imx6sx_cpuidle_init();
> +
>  	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
>  		imx6_pm_opp_init();
>  		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Lucas Stach]

Am Donnerstag, den 25.08.2016, 00:13 +0800 schrieb Anson Huang:
> This patch enables cpuidle driver for i.MX6UL, it
> reuses i.MX6SX's cpuidle driver, 3 levels of cpuidle
> supported:
> 
> 1. ARM WFI;
> 2. SOC in WAIT mode;
> 3. SOC in WAIT mode + ARM power off.
> 
> As i.MX6UL has cortex-A7 CORE with an internal L2
> cache, so need to add L2 cache type check to decide
> if to flush L2 when entering idle with ARM power off,
> this is different from i.MX6SX which has PL310 external
> L2 cache.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
>  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach-imx/cpuidle-imx6sx.c
> index 41cdce6..b4fcc8ae 100644
> --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> @@ -9,14 +9,21 @@
>  #include <linux/cpuidle.h>
>  #include <linux/cpu_pm.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <asm/cacheflush.h>
>  #include <asm/cpuidle.h>
>  #include <asm/suspend.h>
>  
>  #include "common.h"
>  #include "cpuidle.h"
>  
> +static bool pl310_available;
> +
>  static int imx6sx_idle_finish(unsigned long val)
>  {
> +	/* check if need to flush internal L2 cache */
> +	if (!pl310_available)
> +		flush_cache_all();

I think this is only necessary when entering the deepest idle state with
the ARM core powered off, right? It's a serious overhead for the shallow
idle states.

Also flush_cache_all() only flushes architected/inner caches, so it
should do the right thing for this use-case even if the outer PL310
cache is present. There is no need to make this conditional.

Regards,
Lucas
 
>  	cpu_do_idle();
>  
>  	return 0;
> @@ -101,5 +108,8 @@ int __init imx6sx_cpuidle_init(void)
>  	imx_gpc_set_arm_power_up_timing(2, 1);
>  	imx_gpc_set_arm_power_down_timing(1, 1);
>  
> +	if (of_find_compatible_node(NULL, NULL, "arm,pl310-cache"))
> +		pl310_available = true;
> +
>  	return cpuidle_register(&imx6sx_cpuidle_driver, NULL);
>  }
> diff --git a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
> index c2cd61c..9c5e8f3 100644
> --- a/arch/arm/mach-imx/mach-imx6ul.c
> +++ b/arch/arm/mach-imx/mach-imx6ul.c
> @@ -16,6 +16,7 @@
>  #include <asm/mach/map.h>
>  
>  #include "common.h"
> +#include "cpuidle.h"
>  
>  static void __init imx6ul_enet_clk_init(void)
>  {
> @@ -80,6 +81,8 @@ static void __init imx6ul_init_irq(void)
>  
>  static void __init imx6ul_init_late(void)
>  {
> +	imx6sx_cpuidle_init();
> +
>  	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
>  		imx6_pm_opp_init();
>  		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);

RE: [PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Lucas Stach [mailto:l.stach@pengutronix.de]
> Sent: 2016-08-24 5:09 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Fabio
> Estevam <fabio.estevam@nxp.com>; shawnguo@kernel.org;
> linux@armlinux.org.uk; kernel@pengutronix.de
> Subject: Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul
> 
> Am Donnerstag, den 25.08.2016, 00:13 +0800 schrieb Anson Huang:
> > This patch enables cpuidle driver for i.MX6UL, it reuses i.MX6SX's
> > cpuidle driver, 3 levels of cpuidle
> > supported:
> >
> > 1. ARM WFI;
> > 2. SOC in WAIT mode;
> > 3. SOC in WAIT mode + ARM power off.
> >
> > As i.MX6UL has cortex-A7 CORE with an internal L2 cache, so need to
> > add L2 cache type check to decide if to flush L2 when entering idle
> > with ARM power off, this is different from i.MX6SX which has PL310
> > external
> > L2 cache.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
> >  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
> >  2 files changed, 13 insertions(+)
> >
> > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > index 41cdce6..b4fcc8ae 100644
> > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > @@ -9,14 +9,21 @@
> >  #include <linux/cpuidle.h>
> >  #include <linux/cpu_pm.h>
> >  #include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <asm/cacheflush.h>
> >  #include <asm/cpuidle.h>
> >  #include <asm/suspend.h>
> >
> >  #include "common.h"
> >  #include "cpuidle.h"
> >
> > +static bool pl310_available;
> > +
> >  static int imx6sx_idle_finish(unsigned long val)  {
> > +	/* check if need to flush internal L2 cache */
> > +	if (!pl310_available)
> > +		flush_cache_all();
> 
> I think this is only necessary when entering the deepest idle state with the
> ARM core powered off, right? It's a serious overhead for the shallow idle states.
> 
> Also flush_cache_all() only flushes architected/inner caches, so it should do the
> right thing for this use-case even if the outer PL310 cache is present. There is
> no need to make this conditional.
> 

Sorry, my mutt is NOT working now, so I just reply you using outlook, will work on my mutt later.

Yes, the imx6sx_idle_finish is only called when entering deepest idle with ARM powered off.

The common cpu_suspend framework already takes care of L1 cache, for i.MX6SX which is with a 
PL310 external L2 cache, as L2 cache memory is NOT powered down, so we no need to flush it.

For i.MX6UL which is cortex A7 and has integrated L2 cache, the L2 memory power will be also
turned off when ARM core is powered off, so we have to flush it before power off.

Regards,
Anson


> Regards,
> Lucas
> 
> >  	cpu_do_idle();
> >
> >  	return 0;
> > @@ -101,5 +108,8 @@ int __init imx6sx_cpuidle_init(void)
> >  	imx_gpc_set_arm_power_up_timing(2, 1);
> >  	imx_gpc_set_arm_power_down_timing(1, 1);
> >
> > +	if (of_find_compatible_node(NULL, NULL, "arm,pl310-cache"))
> > +		pl310_available = true;
> > +
> >  	return cpuidle_register(&imx6sx_cpuidle_driver, NULL);  } diff --git
> > a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
> > index c2cd61c..9c5e8f3 100644
> > --- a/arch/arm/mach-imx/mach-imx6ul.c
> > +++ b/arch/arm/mach-imx/mach-imx6ul.c
> > @@ -16,6 +16,7 @@
> >  #include <asm/mach/map.h>
> >
> >  #include "common.h"
> > +#include "cpuidle.h"
> >
> >  static void __init imx6ul_enet_clk_init(void)  { @@ -80,6 +81,8 @@
> > static void __init imx6ul_init_irq(void)
> >
> >  static void __init imx6ul_init_late(void)  {
> > +	imx6sx_cpuidle_init();
> > +
> >  	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
> >  		imx6_pm_opp_init();
> >  		platform_device_register_simple("imx6q-cpufreq", -1, NULL,
> 0);
> 

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Lucas Stach [mailto:l.stach at pengutronix.de]
> Sent: 2016-08-24 5:09 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; Fabio
> Estevam <fabio.estevam@nxp.com>; shawnguo at kernel.org;
> linux at armlinux.org.uk; kernel at pengutronix.de
> Subject: Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul
> 
> Am Donnerstag, den 25.08.2016, 00:13 +0800 schrieb Anson Huang:
> > This patch enables cpuidle driver for i.MX6UL, it reuses i.MX6SX's
> > cpuidle driver, 3 levels of cpuidle
> > supported:
> >
> > 1. ARM WFI;
> > 2. SOC in WAIT mode;
> > 3. SOC in WAIT mode + ARM power off.
> >
> > As i.MX6UL has cortex-A7 CORE with an internal L2 cache, so need to
> > add L2 cache type check to decide if to flush L2 when entering idle
> > with ARM power off, this is different from i.MX6SX which has PL310
> > external
> > L2 cache.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
> >  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
> >  2 files changed, 13 insertions(+)
> >
> > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > index 41cdce6..b4fcc8ae 100644
> > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > @@ -9,14 +9,21 @@
> >  #include <linux/cpuidle.h>
> >  #include <linux/cpu_pm.h>
> >  #include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <asm/cacheflush.h>
> >  #include <asm/cpuidle.h>
> >  #include <asm/suspend.h>
> >
> >  #include "common.h"
> >  #include "cpuidle.h"
> >
> > +static bool pl310_available;
> > +
> >  static int imx6sx_idle_finish(unsigned long val)  {
> > +	/* check if need to flush internal L2 cache */
> > +	if (!pl310_available)
> > +		flush_cache_all();
> 
> I think this is only necessary when entering the deepest idle state with the
> ARM core powered off, right? It's a serious overhead for the shallow idle states.
> 
> Also flush_cache_all() only flushes architected/inner caches, so it should do the
> right thing for this use-case even if the outer PL310 cache is present. There is
> no need to make this conditional.
> 

Sorry, my mutt is NOT working now, so I just reply you using outlook, will work on my mutt later.

Yes, the imx6sx_idle_finish is only called when entering deepest idle with ARM powered off.

The common cpu_suspend framework already takes care of L1 cache, for i.MX6SX which is with a 
PL310 external L2 cache, as L2 cache memory is NOT powered down, so we no need to flush it.

For i.MX6UL which is cortex A7 and has integrated L2 cache, the L2 memory power will be also
turned off when ARM core is powered off, so we have to flush it before power off.

Regards,
Anson


> Regards,
> Lucas
> 
> >  	cpu_do_idle();
> >
> >  	return 0;
> > @@ -101,5 +108,8 @@ int __init imx6sx_cpuidle_init(void)
> >  	imx_gpc_set_arm_power_up_timing(2, 1);
> >  	imx_gpc_set_arm_power_down_timing(1, 1);
> >
> > +	if (of_find_compatible_node(NULL, NULL, "arm,pl310-cache"))
> > +		pl310_available = true;
> > +
> >  	return cpuidle_register(&imx6sx_cpuidle_driver, NULL);  } diff --git
> > a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
> > index c2cd61c..9c5e8f3 100644
> > --- a/arch/arm/mach-imx/mach-imx6ul.c
> > +++ b/arch/arm/mach-imx/mach-imx6ul.c
> > @@ -16,6 +16,7 @@
> >  #include <asm/mach/map.h>
> >
> >  #include "common.h"
> > +#include "cpuidle.h"
> >
> >  static void __init imx6ul_enet_clk_init(void)  { @@ -80,6 +81,8 @@
> > static void __init imx6ul_init_irq(void)
> >
> >  static void __init imx6ul_init_late(void)  {
> > +	imx6sx_cpuidle_init();
> > +
> >  	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
> >  		imx6_pm_opp_init();
> >  		platform_device_register_simple("imx6q-cpufreq", -1, NULL,
> 0);
> 

Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Lucas Stach]

Am Mittwoch, den 24.08.2016, 09:53 +0000 schrieb Yongcai Huang:
> 
> 
> Best Regards!
> Anson Huang
> 
> 
> 
> > -----Original Message-----
> > From: Lucas Stach [mailto:l.stach@pengutronix.de]
> > Sent: 2016-08-24 5:09 PM
> > To: Yongcai Huang <anson.huang@nxp.com>
> > Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Fabio
> > Estevam <fabio.estevam@nxp.com>; shawnguo@kernel.org;
> > linux@armlinux.org.uk; kernel@pengutronix.de
> > Subject: Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul
> > 
> > Am Donnerstag, den 25.08.2016, 00:13 +0800 schrieb Anson Huang:
> > > This patch enables cpuidle driver for i.MX6UL, it reuses i.MX6SX's
> > > cpuidle driver, 3 levels of cpuidle
> > > supported:
> > >
> > > 1. ARM WFI;
> > > 2. SOC in WAIT mode;
> > > 3. SOC in WAIT mode + ARM power off.
> > >
> > > As i.MX6UL has cortex-A7 CORE with an internal L2 cache, so need to
> > > add L2 cache type check to decide if to flush L2 when entering idle
> > > with ARM power off, this is different from i.MX6SX which has PL310
> > > external
> > > L2 cache.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > >  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
> > >  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
> > >  2 files changed, 13 insertions(+)
> > >
> > > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > index 41cdce6..b4fcc8ae 100644
> > > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > @@ -9,14 +9,21 @@
> > >  #include <linux/cpuidle.h>
> > >  #include <linux/cpu_pm.h>
> > >  #include <linux/module.h>
> > > +#include <linux/of.h>
> > > +#include <asm/cacheflush.h>
> > >  #include <asm/cpuidle.h>
> > >  #include <asm/suspend.h>
> > >
> > >  #include "common.h"
> > >  #include "cpuidle.h"
> > >
> > > +static bool pl310_available;
> > > +
> > >  static int imx6sx_idle_finish(unsigned long val)  {
> > > +	/* check if need to flush internal L2 cache */
> > > +	if (!pl310_available)
> > > +		flush_cache_all();
> > 
> > I think this is only necessary when entering the deepest idle state with the
> > ARM core powered off, right? It's a serious overhead for the shallow idle states.
> > 
> > Also flush_cache_all() only flushes architected/inner caches, so it should do the
> > right thing for this use-case even if the outer PL310 cache is present. There is
> > no need to make this conditional.
> > 
> 
> Sorry, my mutt is NOT working now, so I just reply you using outlook, will work on my mutt later.
> 
> Yes, the imx6sx_idle_finish is only called when entering deepest idle with ARM powered off.
> 
Okay, makes sense.

> The common cpu_suspend framework already takes care of L1 cache, for i.MX6SX which is with a 
> PL310 external L2 cache, as L2 cache memory is NOT powered down, so we no need to flush it.
> 
As far as I can see flush_cache_all() only flushes architected caches.
The PL310 is NOT an architected, but an outer cache, so it will not be
touched by flush_cache_all() even if it is present. So the condition to
only do this when the PL310 isn't present isn't needed.

Personally I would just remove the condition, but if you are concerned
about the double L1 flush overhead (I wouldn't worry about this, it
should be negligible) you should really make this conditional on an
architected L2 being present. Making it conditional on the outer cache
being absent is confusing.

> For i.MX6UL which is cortex A7 and has integrated L2 cache, the L2 memory power will be also
> turned off when ARM core is powered off, so we have to flush it before power off.
> 
Yes, this is completely right.

Regards,
Lucas

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Lucas Stach]

Am Mittwoch, den 24.08.2016, 09:53 +0000 schrieb Yongcai Huang:
> 
> 
> Best Regards!
> Anson Huang
> 
> 
> 
> > -----Original Message-----
> > From: Lucas Stach [mailto:l.stach at pengutronix.de]
> > Sent: 2016-08-24 5:09 PM
> > To: Yongcai Huang <anson.huang@nxp.com>
> > Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; Fabio
> > Estevam <fabio.estevam@nxp.com>; shawnguo at kernel.org;
> > linux at armlinux.org.uk; kernel at pengutronix.de
> > Subject: Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul
> > 
> > Am Donnerstag, den 25.08.2016, 00:13 +0800 schrieb Anson Huang:
> > > This patch enables cpuidle driver for i.MX6UL, it reuses i.MX6SX's
> > > cpuidle driver, 3 levels of cpuidle
> > > supported:
> > >
> > > 1. ARM WFI;
> > > 2. SOC in WAIT mode;
> > > 3. SOC in WAIT mode + ARM power off.
> > >
> > > As i.MX6UL has cortex-A7 CORE with an internal L2 cache, so need to
> > > add L2 cache type check to decide if to flush L2 when entering idle
> > > with ARM power off, this is different from i.MX6SX which has PL310
> > > external
> > > L2 cache.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > >  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
> > >  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
> > >  2 files changed, 13 insertions(+)
> > >
> > > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > index 41cdce6..b4fcc8ae 100644
> > > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > @@ -9,14 +9,21 @@
> > >  #include <linux/cpuidle.h>
> > >  #include <linux/cpu_pm.h>
> > >  #include <linux/module.h>
> > > +#include <linux/of.h>
> > > +#include <asm/cacheflush.h>
> > >  #include <asm/cpuidle.h>
> > >  #include <asm/suspend.h>
> > >
> > >  #include "common.h"
> > >  #include "cpuidle.h"
> > >
> > > +static bool pl310_available;
> > > +
> > >  static int imx6sx_idle_finish(unsigned long val)  {
> > > +	/* check if need to flush internal L2 cache */
> > > +	if (!pl310_available)
> > > +		flush_cache_all();
> > 
> > I think this is only necessary when entering the deepest idle state with the
> > ARM core powered off, right? It's a serious overhead for the shallow idle states.
> > 
> > Also flush_cache_all() only flushes architected/inner caches, so it should do the
> > right thing for this use-case even if the outer PL310 cache is present. There is
> > no need to make this conditional.
> > 
> 
> Sorry, my mutt is NOT working now, so I just reply you using outlook, will work on my mutt later.
> 
> Yes, the imx6sx_idle_finish is only called when entering deepest idle with ARM powered off.
> 
Okay, makes sense.

> The common cpu_suspend framework already takes care of L1 cache, for i.MX6SX which is with a 
> PL310 external L2 cache, as L2 cache memory is NOT powered down, so we no need to flush it.
> 
As far as I can see flush_cache_all() only flushes architected caches.
The PL310 is NOT an architected, but an outer cache, so it will not be
touched by flush_cache_all() even if it is present. So the condition to
only do this when the PL310 isn't present isn't needed.

Personally I would just remove the condition, but if you are concerned
about the double L1 flush overhead (I wouldn't worry about this, it
should be negligible) you should really make this conditional on an
architected L2 being present. Making it conditional on the outer cache
being absent is confusing.

> For i.MX6UL which is cortex A7 and has integrated L2 cache, the L2 memory power will be also
> turned off when ARM core is powered off, so we have to flush it before power off.
> 
Yes, this is completely right.

Regards,
Lucas

Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Russell King - ARM Linux]

On Wed, Aug 24, 2016 at 12:04:50PM +0200, Lucas Stach wrote:
> Am Mittwoch, den 24.08.2016, 09:53 +0000 schrieb Yongcai Huang:
> > 
> > 
> > Best Regards!
> > Anson Huang
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Lucas Stach [mailto:l.stach@pengutronix.de]
> > > Sent: 2016-08-24 5:09 PM
> > > To: Yongcai Huang <anson.huang@nxp.com>
> > > Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Fabio
> > > Estevam <fabio.estevam@nxp.com>; shawnguo@kernel.org;
> > > linux@armlinux.org.uk; kernel@pengutronix.de
> > > Subject: Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul
> > > 
> > > Am Donnerstag, den 25.08.2016, 00:13 +0800 schrieb Anson Huang:
> > > > This patch enables cpuidle driver for i.MX6UL, it reuses i.MX6SX's
> > > > cpuidle driver, 3 levels of cpuidle
> > > > supported:
> > > >
> > > > 1. ARM WFI;
> > > > 2. SOC in WAIT mode;
> > > > 3. SOC in WAIT mode + ARM power off.
> > > >
> > > > As i.MX6UL has cortex-A7 CORE with an internal L2 cache, so need to
> > > > add L2 cache type check to decide if to flush L2 when entering idle
> > > > with ARM power off, this is different from i.MX6SX which has PL310
> > > > external
> > > > L2 cache.
> > > >
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > > ---
> > > >  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
> > > >  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
> > > >  2 files changed, 13 insertions(+)
> > > >
> > > > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > > b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > > index 41cdce6..b4fcc8ae 100644
> > > > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > > @@ -9,14 +9,21 @@
> > > >  #include <linux/cpuidle.h>
> > > >  #include <linux/cpu_pm.h>
> > > >  #include <linux/module.h>
> > > > +#include <linux/of.h>
> > > > +#include <asm/cacheflush.h>
> > > >  #include <asm/cpuidle.h>
> > > >  #include <asm/suspend.h>
> > > >
> > > >  #include "common.h"
> > > >  #include "cpuidle.h"
> > > >
> > > > +static bool pl310_available;
> > > > +
> > > >  static int imx6sx_idle_finish(unsigned long val)  {
> > > > +	/* check if need to flush internal L2 cache */
> > > > +	if (!pl310_available)
> > > > +		flush_cache_all();
> > > 
> > > I think this is only necessary when entering the deepest idle state with the
> > > ARM core powered off, right? It's a serious overhead for the shallow idle states.
> > > 
> > > Also flush_cache_all() only flushes architected/inner caches, so it should do the
> > > right thing for this use-case even if the outer PL310 cache is present. There is
> > > no need to make this conditional.
> > > 
> > 
> > Sorry, my mutt is NOT working now, so I just reply you using outlook, will work on my mutt later.
> > 
> > Yes, the imx6sx_idle_finish is only called when entering deepest idle with ARM powered off.
> > 
> Okay, makes sense.
> 
> > The common cpu_suspend framework already takes care of L1 cache, for i.MX6SX which is with a 
> > PL310 external L2 cache, as L2 cache memory is NOT powered down, so we no need to flush it.
> > 
> As far as I can see flush_cache_all() only flushes architected caches.
> The PL310 is NOT an architected, but an outer cache, so it will not be
> touched by flush_cache_all() even if it is present. So the condition to
> only do this when the PL310 isn't present isn't needed.

Look again.  The patch is only calling flush_cache_all() if the L2C-310
is _not_ present.  So, the question about flush_cache_all() touching
the L2C-310 is irrelevant.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Russell King - ARM Linux]

On Wed, Aug 24, 2016 at 12:04:50PM +0200, Lucas Stach wrote:
> Am Mittwoch, den 24.08.2016, 09:53 +0000 schrieb Yongcai Huang:
> > 
> > 
> > Best Regards!
> > Anson Huang
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Lucas Stach [mailto:l.stach at pengutronix.de]
> > > Sent: 2016-08-24 5:09 PM
> > > To: Yongcai Huang <anson.huang@nxp.com>
> > > Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; Fabio
> > > Estevam <fabio.estevam@nxp.com>; shawnguo at kernel.org;
> > > linux at armlinux.org.uk; kernel at pengutronix.de
> > > Subject: Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul
> > > 
> > > Am Donnerstag, den 25.08.2016, 00:13 +0800 schrieb Anson Huang:
> > > > This patch enables cpuidle driver for i.MX6UL, it reuses i.MX6SX's
> > > > cpuidle driver, 3 levels of cpuidle
> > > > supported:
> > > >
> > > > 1. ARM WFI;
> > > > 2. SOC in WAIT mode;
> > > > 3. SOC in WAIT mode + ARM power off.
> > > >
> > > > As i.MX6UL has cortex-A7 CORE with an internal L2 cache, so need to
> > > > add L2 cache type check to decide if to flush L2 when entering idle
> > > > with ARM power off, this is different from i.MX6SX which has PL310
> > > > external
> > > > L2 cache.
> > > >
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > > ---
> > > >  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
> > > >  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
> > > >  2 files changed, 13 insertions(+)
> > > >
> > > > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > > b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > > index 41cdce6..b4fcc8ae 100644
> > > > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > > > @@ -9,14 +9,21 @@
> > > >  #include <linux/cpuidle.h>
> > > >  #include <linux/cpu_pm.h>
> > > >  #include <linux/module.h>
> > > > +#include <linux/of.h>
> > > > +#include <asm/cacheflush.h>
> > > >  #include <asm/cpuidle.h>
> > > >  #include <asm/suspend.h>
> > > >
> > > >  #include "common.h"
> > > >  #include "cpuidle.h"
> > > >
> > > > +static bool pl310_available;
> > > > +
> > > >  static int imx6sx_idle_finish(unsigned long val)  {
> > > > +	/* check if need to flush internal L2 cache */
> > > > +	if (!pl310_available)
> > > > +		flush_cache_all();
> > > 
> > > I think this is only necessary when entering the deepest idle state with the
> > > ARM core powered off, right? It's a serious overhead for the shallow idle states.
> > > 
> > > Also flush_cache_all() only flushes architected/inner caches, so it should do the
> > > right thing for this use-case even if the outer PL310 cache is present. There is
> > > no need to make this conditional.
> > > 
> > 
> > Sorry, my mutt is NOT working now, so I just reply you using outlook, will work on my mutt later.
> > 
> > Yes, the imx6sx_idle_finish is only called when entering deepest idle with ARM powered off.
> > 
> Okay, makes sense.
> 
> > The common cpu_suspend framework already takes care of L1 cache, for i.MX6SX which is with a 
> > PL310 external L2 cache, as L2 cache memory is NOT powered down, so we no need to flush it.
> > 
> As far as I can see flush_cache_all() only flushes architected caches.
> The PL310 is NOT an architected, but an outer cache, so it will not be
> touched by flush_cache_all() even if it is present. So the condition to
> only do this when the PL310 isn't present isn't needed.

Look again.  The patch is only calling flush_cache_all() if the L2C-310
is _not_ present.  So, the question about flush_cache_all() touching
the L2C-310 is irrelevant.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Anson Huang]

This patch enables cpuidle driver for i.MX6UL, it
reuses i.MX6SX's cpuidle driver, 3 levels of cpuidle
supported:

1. ARM WFI;
2. SOC in WAIT mode;
3. SOC in WAIT mode + ARM power off.

As i.MX6UL has cortex-A7 CORE with an internal L2
cache, so need to add L2 cache type check to decide
if to flush L2 when entering idle with ARM power off,
this is different from i.MX6SX which has PL310 external
L2 cache.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
 arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach-imx/cpuidle-imx6sx.c
index 41cdce6..b4fcc8ae 100644
--- a/arch/arm/mach-imx/cpuidle-imx6sx.c
+++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
@@ -9,14 +9,21 @@
 #include <linux/cpuidle.h>
 #include <linux/cpu_pm.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <asm/cacheflush.h>
 #include <asm/cpuidle.h>
 #include <asm/suspend.h>
 
 #include "common.h"
 #include "cpuidle.h"
 
+static bool pl310_available;
+
 static int imx6sx_idle_finish(unsigned long val)
 {
+	/* check if need to flush internal L2 cache */
+	if (!pl310_available)
+		flush_cache_all();
 	cpu_do_idle();
 
 	return 0;
@@ -101,5 +108,8 @@ int __init imx6sx_cpuidle_init(void)
 	imx_gpc_set_arm_power_up_timing(2, 1);
 	imx_gpc_set_arm_power_down_timing(1, 1);
 
+	if (of_find_compatible_node(NULL, NULL, "arm,pl310-cache"))
+		pl310_available = true;
+
 	return cpuidle_register(&imx6sx_cpuidle_driver, NULL);
 }
diff --git a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
index c2cd61c..9c5e8f3 100644
--- a/arch/arm/mach-imx/mach-imx6ul.c
+++ b/arch/arm/mach-imx/mach-imx6ul.c
@@ -16,6 +16,7 @@
 #include <asm/mach/map.h>
 
 #include "common.h"
+#include "cpuidle.h"
 
 static void __init imx6ul_enet_clk_init(void)
 {
@@ -80,6 +81,8 @@ static void __init imx6ul_init_irq(void)
 
 static void __init imx6ul_init_late(void)
 {
+	imx6sx_cpuidle_init();
+
 	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
 		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
-- 
1.9.1

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Anson Huang]

This patch enables cpuidle driver for i.MX6UL, it
reuses i.MX6SX's cpuidle driver, 3 levels of cpuidle
supported:

1. ARM WFI;
2. SOC in WAIT mode;
3. SOC in WAIT mode + ARM power off.

As i.MX6UL has cortex-A7 CORE with an internal L2
cache, so need to add L2 cache type check to decide
if to flush L2 when entering idle with ARM power off,
this is different from i.MX6SX which has PL310 external
L2 cache.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
 arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach-imx/cpuidle-imx6sx.c
index 41cdce6..b4fcc8ae 100644
--- a/arch/arm/mach-imx/cpuidle-imx6sx.c
+++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
@@ -9,14 +9,21 @@
 #include <linux/cpuidle.h>
 #include <linux/cpu_pm.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <asm/cacheflush.h>
 #include <asm/cpuidle.h>
 #include <asm/suspend.h>
 
 #include "common.h"
 #include "cpuidle.h"
 
+static bool pl310_available;
+
 static int imx6sx_idle_finish(unsigned long val)
 {
+	/* check if need to flush internal L2 cache */
+	if (!pl310_available)
+		flush_cache_all();
 	cpu_do_idle();
 
 	return 0;
@@ -101,5 +108,8 @@ int __init imx6sx_cpuidle_init(void)
 	imx_gpc_set_arm_power_up_timing(2, 1);
 	imx_gpc_set_arm_power_down_timing(1, 1);
 
+	if (of_find_compatible_node(NULL, NULL, "arm,pl310-cache"))
+		pl310_available = true;
+
 	return cpuidle_register(&imx6sx_cpuidle_driver, NULL);
 }
diff --git a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
index c2cd61c..9c5e8f3 100644
--- a/arch/arm/mach-imx/mach-imx6ul.c
+++ b/arch/arm/mach-imx/mach-imx6ul.c
@@ -16,6 +16,7 @@
 #include <asm/mach/map.h>
 
 #include "common.h"
+#include "cpuidle.h"
 
 static void __init imx6ul_enet_clk_init(void)
 {
@@ -80,6 +81,8 @@ static void __init imx6ul_init_irq(void)
 
 static void __init imx6ul_init_late(void)
 {
+	imx6sx_cpuidle_init();
+
 	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
 		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
-- 
1.9.1

Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Peter Chen]

>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
>  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
>  2 files changed, 13 insertions(+)
>
> diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach-imx/cpuidle-imx6sx.c
> index 41cdce6..b4fcc8ae 100644
> --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> @@ -9,14 +9,21 @@
>  #include <linux/cpuidle.h>
>  #include <linux/cpu_pm.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <asm/cacheflush.h>
>  #include <asm/cpuidle.h>
>  #include <asm/suspend.h>
>
>  #include "common.h"
>  #include "cpuidle.h"
>
> +static bool pl310_available;
> +


>  static int imx6sx_idle_finish(unsigned long val)
>  {
> +       /* check if need to flush internal L2 cache */
> +       if (!pl310_available)

Just wondering why you need an extra variable here?
Calling of_find_compatible_node takes long time at that time?

Peter

> +               flush_cache_all();
>         cpu_do_idle();
>
>         return 0;
> @@ -101,5 +108,8 @@ int __init imx6sx_cpuidle_init(void)
>         imx_gpc_set_arm_power_up_timing(2, 1);
>         imx_gpc_set_arm_power_down_timing(1, 1);
>
> +       if (of_find_compatible_node(NULL, NULL, "arm,pl310-cache"))
> +               pl310_available = true;
> +
>         return cpuidle_register(&imx6sx_cpuidle_driver, NULL);
>  }
> diff --git a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
> index c2cd61c..9c5e8f3 100644
> --- a/arch/arm/mach-imx/mach-imx6ul.c
> +++ b/arch/arm/mach-imx/mach-imx6ul.c
> @@ -16,6 +16,7 @@
>  #include <asm/mach/map.h>
>
>  #include "common.h"
> +#include "cpuidle.h"
>
>  static void __init imx6ul_enet_clk_init(void)
>  {
> @@ -80,6 +81,8 @@ static void __init imx6ul_init_irq(void)
>
>  static void __init imx6ul_init_late(void)
>  {
> +       imx6sx_cpuidle_init();
> +
>         if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
>                 imx6_pm_opp_init();
>                 platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Peter Chen]

>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
>  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
>  2 files changed, 13 insertions(+)
>
> diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach-imx/cpuidle-imx6sx.c
> index 41cdce6..b4fcc8ae 100644
> --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> @@ -9,14 +9,21 @@
>  #include <linux/cpuidle.h>
>  #include <linux/cpu_pm.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <asm/cacheflush.h>
>  #include <asm/cpuidle.h>
>  #include <asm/suspend.h>
>
>  #include "common.h"
>  #include "cpuidle.h"
>
> +static bool pl310_available;
> +


>  static int imx6sx_idle_finish(unsigned long val)
>  {
> +       /* check if need to flush internal L2 cache */
> +       if (!pl310_available)

Just wondering why you need an extra variable here?
Calling of_find_compatible_node takes long time at that time?

Peter

> +               flush_cache_all();
>         cpu_do_idle();
>
>         return 0;
> @@ -101,5 +108,8 @@ int __init imx6sx_cpuidle_init(void)
>         imx_gpc_set_arm_power_up_timing(2, 1);
>         imx_gpc_set_arm_power_down_timing(1, 1);
>
> +       if (of_find_compatible_node(NULL, NULL, "arm,pl310-cache"))
> +               pl310_available = true;
> +
>         return cpuidle_register(&imx6sx_cpuidle_driver, NULL);
>  }
> diff --git a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
> index c2cd61c..9c5e8f3 100644
> --- a/arch/arm/mach-imx/mach-imx6ul.c
> +++ b/arch/arm/mach-imx/mach-imx6ul.c
> @@ -16,6 +16,7 @@
>  #include <asm/mach/map.h>
>
>  #include "common.h"
> +#include "cpuidle.h"
>
>  static void __init imx6ul_enet_clk_init(void)
>  {
> @@ -80,6 +81,8 @@ static void __init imx6ul_init_irq(void)
>
>  static void __init imx6ul_init_late(void)
>  {
> +       imx6sx_cpuidle_init();
> +
>         if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
>                 imx6_pm_opp_init();
>                 platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen@gmail.com]
> Sent: 2016-08-25 3:09 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; lkml <linux-kernel@vger.kernel.org>;
> Fabio Estevam <fabio.estevam@nxp.com>; shawnguo@kernel.org;
> linux@armlinux.org.uk; kernel@pengutronix.de
> Subject: Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul
> 
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
> >  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
> >  2 files changed, 13 insertions(+)
> >
> > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > index 41cdce6..b4fcc8ae 100644
> > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > @@ -9,14 +9,21 @@
> >  #include <linux/cpuidle.h>
> >  #include <linux/cpu_pm.h>
> >  #include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <asm/cacheflush.h>
> >  #include <asm/cpuidle.h>
> >  #include <asm/suspend.h>
> >
> >  #include "common.h"
> >  #include "cpuidle.h"
> >
> > +static bool pl310_available;
> > +
> 
> 
> >  static int imx6sx_idle_finish(unsigned long val)  {
> > +       /* check if need to flush internal L2 cache */
> > +       if (!pl310_available)
> 
> Just wondering why you need an extra variable here?
> Calling of_find_compatible_node takes long time at that time?
> 
> Peter
> 

Because cpu idle enter/exit is called very frequently, so calling
of_find_compatible_node in every cpu idle thread is adding too
many latency, I think adding a variable is better.

Anson

> > +               flush_cache_all();
> >         cpu_do_idle();
> >
> >         return 0;
> > @@ -101,5 +108,8 @@ int __init imx6sx_cpuidle_init(void)
> >         imx_gpc_set_arm_power_up_timing(2, 1);
> >         imx_gpc_set_arm_power_down_timing(1, 1);
> >
> > +       if (of_find_compatible_node(NULL, NULL, "arm,pl310-cache"))
> > +               pl310_available = true;
> > +
> >         return cpuidle_register(&imx6sx_cpuidle_driver, NULL);  } diff
> > --git a/arch/arm/mach-imx/mach-imx6ul.c
> > b/arch/arm/mach-imx/mach-imx6ul.c index c2cd61c..9c5e8f3 100644
> > --- a/arch/arm/mach-imx/mach-imx6ul.c
> > +++ b/arch/arm/mach-imx/mach-imx6ul.c
> > @@ -16,6 +16,7 @@
> >  #include <asm/mach/map.h>
> >
> >  #include "common.h"
> > +#include "cpuidle.h"
> >
> >  static void __init imx6ul_enet_clk_init(void)  { @@ -80,6 +81,8 @@
> > static void __init imx6ul_init_irq(void)
> >
> >  static void __init imx6ul_init_late(void)  {
> > +       imx6sx_cpuidle_init();
> > +
> >         if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
> >                 imx6_pm_opp_init();
> >                 platform_device_register_simple("imx6q-cpufreq", -1,
> > NULL, 0);
> > --
> > 1.9.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen at gmail.com]
> Sent: 2016-08-25 3:09 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; lkml <linux-kernel@vger.kernel.org>;
> Fabio Estevam <fabio.estevam@nxp.com>; shawnguo at kernel.org;
> linux at armlinux.org.uk; kernel at pengutronix.de
> Subject: Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul
> 
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  arch/arm/mach-imx/cpuidle-imx6sx.c | 10 ++++++++++
> >  arch/arm/mach-imx/mach-imx6ul.c    |  3 +++
> >  2 files changed, 13 insertions(+)
> >
> > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > index 41cdce6..b4fcc8ae 100644
> > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> > @@ -9,14 +9,21 @@
> >  #include <linux/cpuidle.h>
> >  #include <linux/cpu_pm.h>
> >  #include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <asm/cacheflush.h>
> >  #include <asm/cpuidle.h>
> >  #include <asm/suspend.h>
> >
> >  #include "common.h"
> >  #include "cpuidle.h"
> >
> > +static bool pl310_available;
> > +
> 
> 
> >  static int imx6sx_idle_finish(unsigned long val)  {
> > +       /* check if need to flush internal L2 cache */
> > +       if (!pl310_available)
> 
> Just wondering why you need an extra variable here?
> Calling of_find_compatible_node takes long time at that time?
> 
> Peter
> 

Because cpu idle enter/exit is called very frequently, so calling
of_find_compatible_node in every cpu idle thread is adding too
many latency, I think adding a variable is better.

Anson

> > +               flush_cache_all();
> >         cpu_do_idle();
> >
> >         return 0;
> > @@ -101,5 +108,8 @@ int __init imx6sx_cpuidle_init(void)
> >         imx_gpc_set_arm_power_up_timing(2, 1);
> >         imx_gpc_set_arm_power_down_timing(1, 1);
> >
> > +       if (of_find_compatible_node(NULL, NULL, "arm,pl310-cache"))
> > +               pl310_available = true;
> > +
> >         return cpuidle_register(&imx6sx_cpuidle_driver, NULL);  } diff
> > --git a/arch/arm/mach-imx/mach-imx6ul.c
> > b/arch/arm/mach-imx/mach-imx6ul.c index c2cd61c..9c5e8f3 100644
> > --- a/arch/arm/mach-imx/mach-imx6ul.c
> > +++ b/arch/arm/mach-imx/mach-imx6ul.c
> > @@ -16,6 +16,7 @@
> >  #include <asm/mach/map.h>
> >
> >  #include "common.h"
> > +#include "cpuidle.h"
> >
> >  static void __init imx6ul_enet_clk_init(void)  { @@ -80,6 +81,8 @@
> > static void __init imx6ul_init_irq(void)
> >
> >  static void __init imx6ul_init_late(void)  {
> > +       imx6sx_cpuidle_init();
> > +
> >         if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
> >                 imx6_pm_opp_init();
> >                 platform_device_register_simple("imx6q-cpufreq", -1,
> > NULL, 0);
> > --
> > 1.9.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Shawn Guo]

On Wed, Aug 24, 2016 at 12:04:50PM +0200, Lucas Stach wrote:
> Personally I would just remove the condition, but if you are concerned
> about the double L1 flush overhead (I wouldn't worry about this, it
> should be negligible) you should really make this conditional on an
> architected L2 being present. Making it conditional on the outer cache
> being absent is confusing.

Anson,

Is there any concern or problem if we follow Lucas' suggestion to
unconditionally calls flush_cache_all() here?

Shawn

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Shawn Guo]

On Wed, Aug 24, 2016 at 12:04:50PM +0200, Lucas Stach wrote:
> Personally I would just remove the condition, but if you are concerned
> about the double L1 flush overhead (I wouldn't worry about this, it
> should be negligible) you should really make this conditional on an
> architected L2 being present. Making it conditional on the outer cache
> being absent is confusing.

Anson,

Is there any concern or problem if we follow Lucas' suggestion to
unconditionally calls flush_cache_all() here?

Shawn

RE: [PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo@kernel.org]
> Sent: 2016-08-29 3:24 PM
> To: Lucas Stach <l.stach@pengutronix.de>; Yongcai Huang
> <anson.huang@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Fabio
> Estevam <fabio.estevam@nxp.com>; linux@armlinux.org.uk;
> kernel@pengutronix.de
> Subject: Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul
> 
> On Wed, Aug 24, 2016 at 12:04:50PM +0200, Lucas Stach wrote:
> > Personally I would just remove the condition, but if you are concerned
> > about the double L1 flush overhead (I wouldn't worry about this, it
> > should be negligible) you should really make this conditional on an
> > architected L2 being present. Making it conditional on the outer cache
> > being absent is confusing.
> 
> Anson,
> 
> Is there any concern or problem if we follow Lucas' suggestion to
> unconditionally calls flush_cache_all() here?
> 
> Shawn

Because this code is in idle thread, my original aim is to make the latency as
small as possible, but since the double L1 flush here should finish very quick at this
stage and compare to hardware ARM core power down/up latency, it should be
negligible as Lucas mentioned, yes, I agree to remove condition check here and
just call L1 flush again to avoid any confusion. Will send out a V2 patch later, thanks.

Anson.

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo at kernel.org]
> Sent: 2016-08-29 3:24 PM
> To: Lucas Stach <l.stach@pengutronix.de>; Yongcai Huang
> <anson.huang@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; Fabio
> Estevam <fabio.estevam@nxp.com>; linux at armlinux.org.uk;
> kernel at pengutronix.de
> Subject: Re: [PATCH] ARM: imx: add cpuidle support for i.mx6ul
> 
> On Wed, Aug 24, 2016 at 12:04:50PM +0200, Lucas Stach wrote:
> > Personally I would just remove the condition, but if you are concerned
> > about the double L1 flush overhead (I wouldn't worry about this, it
> > should be negligible) you should really make this conditional on an
> > architected L2 being present. Making it conditional on the outer cache
> > being absent is confusing.
> 
> Anson,
> 
> Is there any concern or problem if we follow Lucas' suggestion to
> unconditionally calls flush_cache_all() here?
> 
> Shawn

Because this code is in idle thread, my original aim is to make the latency as
small as possible, but since the double L1 flush here should finish very quick at this
stage and compare to hardware ARM core power down/up latency, it should be
negligible as Lucas mentioned, yes, I agree to remove condition check here and
just call L1 flush again to avoid any confusion. Will send out a V2 patch later, thanks.

Anson.

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Anson Huang]

This patch introduces an independent cpuidle driver for
i.MX6UL, totally 3 levels of cpuidle are supported as below:

1. ARM WFI;
2. SOC in WAIT mode;
3. SOC in WAIT mode + ARM power off.

Signed-off-by: Anson Huang <b20788@freescale.com>
---
 arch/arm/mach-imx/Makefile         |   1 +
 arch/arm/mach-imx/cpuidle-imx6ul.c | 106 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/cpuidle.h        |   5 ++
 arch/arm/mach-imx/mach-imx6ul.c    |   8 +++
 4 files changed, 120 insertions(+)
 create mode 100644 arch/arm/mach-imx/cpuidle-imx6ul.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index fb689d8..1d7df9c 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_SOC_IMX5) += cpuidle-imx5.o
 obj-$(CONFIG_SOC_IMX6Q) += cpuidle-imx6q.o
 obj-$(CONFIG_SOC_IMX6SL) += cpuidle-imx6sl.o
 obj-$(CONFIG_SOC_IMX6SX) += cpuidle-imx6sx.o
+obj-$(CONFIG_SOC_IMX6UL) += cpuidle-imx6ul.o
 endif
 
 ifdef CONFIG_SND_IMX_SOC
diff --git a/arch/arm/mach-imx/cpuidle-imx6ul.c b/arch/arm/mach-imx/cpuidle-imx6ul.c
new file mode 100644
index 0000000..93793a1
--- /dev/null
+++ b/arch/arm/mach-imx/cpuidle-imx6ul.c
@@ -0,0 +1,106 @@
+/*
+ * Copyright (C) 2015 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+#include <linux/module.h>
+#include <asm/cacheflush.h>
+#include <asm/cpuidle.h>
+#include <asm/suspend.h>
+
+#include "common.h"
+#include "cpuidle.h"
+
+static int imx6ul_idle_finish(unsigned long val)
+{
+	flush_cache_all();
+	cpu_do_idle();
+
+	return 0;
+}
+
+static int imx6ul_enter_wait(struct cpuidle_device *dev,
+			    struct cpuidle_driver *drv, int index)
+{
+	imx6_set_lpm(WAIT_UNCLOCKED);
+
+	switch (index) {
+	case 1:
+		cpu_do_idle();
+		break;
+	case 2:
+		imx6_enable_rbc(true);
+		imx_gpc_set_arm_power_in_lpm(true);
+		imx_set_cpu_jump(0, v7_cpu_resume);
+		/* Need to notify there is a cpu pm operation. */
+		cpu_pm_enter();
+		cpu_cluster_pm_enter();
+
+		cpu_suspend(0, imx6ul_idle_finish);
+
+		cpu_cluster_pm_exit();
+		cpu_pm_exit();
+		imx_gpc_set_arm_power_in_lpm(false);
+		imx6_enable_rbc(false);
+		break;
+	default:
+		break;
+	}
+
+	imx6_set_lpm(WAIT_CLOCKED);
+
+	return index;
+}
+
+static struct cpuidle_driver imx6ul_cpuidle_driver = {
+	.name = "imx6ul_cpuidle",
+	.owner = THIS_MODULE,
+	.states = {
+		/* WFI */
+		ARM_CPUIDLE_WFI_STATE,
+		/* WAIT */
+		{
+			.exit_latency = 50,
+			.target_residency = 75,
+			.flags = CPUIDLE_FLAG_TIMER_STOP,
+			.enter = imx6ul_enter_wait,
+			.name = "WAIT",
+			.desc = "Clock off",
+		},
+		/* WAIT + ARM power off  */
+		{
+			/*
+			 * ARM gating 31us * 5 + RBC clear 65us
+			 * and some margin for SW execution, here set it
+			 * to 300us.
+			 */
+			.exit_latency = 300,
+			.target_residency = 500,
+			.enter = imx6ul_enter_wait,
+			.name = "LOW-POWER-IDLE",
+			.desc = "ARM power off",
+		},
+	},
+	.state_count = 3,
+	.safe_state_index = 0,
+};
+
+int __init imx6ul_cpuidle_init(void)
+{
+	imx6_enable_rbc(false);
+	/*
+	 * set ARM power up/down timing to the fastest,
+	 * sw2iso and sw can be set to one 32K cycle = 31us
+	 * except for power up sw2iso which need to be
+	 * larger than LDO ramp up time.
+	 */
+	imx_gpc_set_arm_power_up_timing(2, 1);
+	imx_gpc_set_arm_power_down_timing(1, 1);
+
+	return cpuidle_register(&imx6ul_cpuidle_driver, NULL);
+}
diff --git a/arch/arm/mach-imx/cpuidle.h b/arch/arm/mach-imx/cpuidle.h
index f914012..4dcf71d 100644
--- a/arch/arm/mach-imx/cpuidle.h
+++ b/arch/arm/mach-imx/cpuidle.h
@@ -15,6 +15,7 @@ extern int imx5_cpuidle_init(void);
 extern int imx6q_cpuidle_init(void);
 extern int imx6sl_cpuidle_init(void);
 extern int imx6sx_cpuidle_init(void);
+extern int imx6ul_cpuidle_init(void);
 #else
 static inline int imx5_cpuidle_init(void)
 {
@@ -32,4 +33,8 @@ static inline int imx6sx_cpuidle_init(void)
 {
 	return 0;
 }
+static inline int imx6ul_cpuidle_init(void)
+{
+	return 0;
+}
 #endif
diff --git a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
index f206506..0f4b95c 100644
--- a/arch/arm/mach-imx/mach-imx6ul.c
+++ b/arch/arm/mach-imx/mach-imx6ul.c
@@ -11,6 +11,7 @@
 #include <asm/mach/map.h>
 
 #include "common.h"
+#include "cpuidle.h"
 
 static void __init imx6ul_init_machine(void)
 {
@@ -29,6 +30,12 @@ static void __init imx6ul_init_irq(void)
 	imx_init_revision_from_anatop();
 	imx_src_init();
 	irqchip_init();
+	imx6_pm_ccm_init("fsl,imx6ul-ccm");
+}
+
+static void __init imx6ul_init_late(void)
+{
+	imx6ul_cpuidle_init();
 }
 
 static const char *imx6ul_dt_compat[] __initconst = {
@@ -40,4 +47,5 @@ DT_MACHINE_START(IMX6UL, "Freescale i.MX6 Ultralite (Device Tree)")
 	.init_irq	= imx6ul_init_irq,
 	.init_machine	= imx6ul_init_machine,
 	.dt_compat	= imx6ul_dt_compat,
+	.init_late	= imx6ul_init_late,
 MACHINE_END
-- 
1.9.1

[PATCH] ARM: imx: add cpuidle support for i.mx6ul

[Lucas Stach]

Am Mittwoch, den 05.08.2015, 00:32 +0800 schrieb Anson Huang:
> This patch introduces an independent cpuidle driver for
> i.MX6UL, totally 3 levels of cpuidle are supported as below:
> 
> 1. ARM WFI;
> 2. SOC in WAIT mode;
> 3. SOC in WAIT mode + ARM power off.
> 
> Signed-off-by: Anson Huang <b20788@freescale.com>
> ---
>  arch/arm/mach-imx/Makefile         |   1 +
>  arch/arm/mach-imx/cpuidle-imx6ul.c | 106 +++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-imx/cpuidle.h        |   5 ++
>  arch/arm/mach-imx/mach-imx6ul.c    |   8 +++
>  4 files changed, 120 insertions(+)
>  create mode 100644 arch/arm/mach-imx/cpuidle-imx6ul.c
> 
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index fb689d8..1d7df9c 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_SOC_IMX5) += cpuidle-imx5.o
>  obj-$(CONFIG_SOC_IMX6Q) += cpuidle-imx6q.o
>  obj-$(CONFIG_SOC_IMX6SL) += cpuidle-imx6sl.o
>  obj-$(CONFIG_SOC_IMX6SX) += cpuidle-imx6sx.o
> +obj-$(CONFIG_SOC_IMX6UL) += cpuidle-imx6ul.o
>  endif
>  
>  ifdef CONFIG_SND_IMX_SOC
> diff --git a/arch/arm/mach-imx/cpuidle-imx6ul.c b/arch/arm/mach-imx/cpuidle-imx6ul.c
> new file mode 100644
> index 0000000..93793a1
> --- /dev/null
> +++ b/arch/arm/mach-imx/cpuidle-imx6ul.c
> @@ -0,0 +1,106 @@
> +/*
> + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/cpuidle.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/module.h>
> +#include <asm/cacheflush.h>
> +#include <asm/cpuidle.h>
> +#include <asm/suspend.h>
> +
> +#include "common.h"
> +#include "cpuidle.h"
> +
> +static int imx6ul_idle_finish(unsigned long val)
> +{
> +	flush_cache_all();
> +	cpu_do_idle();
> +
> +	return 0;
> +}
> +
> +static int imx6ul_enter_wait(struct cpuidle_device *dev,
> +			    struct cpuidle_driver *drv, int index)
> +{
> +	imx6_set_lpm(WAIT_UNCLOCKED);
> +
> +	switch (index) {
> +	case 1:
> +		cpu_do_idle();
> +		break;
> +	case 2:
> +		imx6_enable_rbc(true);
> +		imx_gpc_set_arm_power_in_lpm(true);
> +		imx_set_cpu_jump(0, v7_cpu_resume);
> +		/* Need to notify there is a cpu pm operation. */
> +		cpu_pm_enter();
> +		cpu_cluster_pm_enter();
> +
> +		cpu_suspend(0, imx6ul_idle_finish);
> +
> +		cpu_cluster_pm_exit();
> +		cpu_pm_exit();
> +		imx_gpc_set_arm_power_in_lpm(false);
> +		imx6_enable_rbc(false);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	imx6_set_lpm(WAIT_CLOCKED);
> +
> +	return index;
> +}
> +
> +static struct cpuidle_driver imx6ul_cpuidle_driver = {
> +	.name = "imx6ul_cpuidle",
> +	.owner = THIS_MODULE,
> +	.states = {
> +		/* WFI */
> +		ARM_CPUIDLE_WFI_STATE,
> +		/* WAIT */
> +		{
> +			.exit_latency = 50,
> +			.target_residency = 75,
> +			.flags = CPUIDLE_FLAG_TIMER_STOP,
> +			.enter = imx6ul_enter_wait,
> +			.name = "WAIT",
> +			.desc = "Clock off",
> +		},
> +		/* WAIT + ARM power off  */
> +		{
> +			/*
> +			 * ARM gating 31us * 5 + RBC clear 65us
> +			 * and some margin for SW execution, here set it
> +			 * to 300us.
> +			 */
> +			.exit_latency = 300,
> +			.target_residency = 500,

Why is ".flags = CPUIDLE_FLAG_TIMER_STOP," omitted here?

> +			.enter = imx6ul_enter_wait,
> +			.name = "LOW-POWER-IDLE",
> +			.desc = "ARM power off",
> +		},
> +	},
> +	.state_count = 3,
> +	.safe_state_index = 0,
> +};
> +
> +int __init imx6ul_cpuidle_init(void)
> +{
> +	imx6_enable_rbc(false);
> +	/*
> +	 * set ARM power up/down timing to the fastest,
> +	 * sw2iso and sw can be set to one 32K cycle = 31us
> +	 * except for power up sw2iso which need to be
> +	 * larger than LDO ramp up time.
> +	 */
> +	imx_gpc_set_arm_power_up_timing(2, 1);
> +	imx_gpc_set_arm_power_down_timing(1, 1);
> +
> +	return cpuidle_register(&imx6ul_cpuidle_driver, NULL);
> +}
> diff --git a/arch/arm/mach-imx/cpuidle.h b/arch/arm/mach-imx/cpuidle.h
> index f914012..4dcf71d 100644
> --- a/arch/arm/mach-imx/cpuidle.h
> +++ b/arch/arm/mach-imx/cpuidle.h
> @@ -15,6 +15,7 @@ extern int imx5_cpuidle_init(void);
>  extern int imx6q_cpuidle_init(void);
>  extern int imx6sl_cpuidle_init(void);
>  extern int imx6sx_cpuidle_init(void);
> +extern int imx6ul_cpuidle_init(void);
>  #else
>  static inline int imx5_cpuidle_init(void)
>  {
> @@ -32,4 +33,8 @@ static inline int imx6sx_cpuidle_init(void)
>  {
>  	return 0;
>  }
> +static inline int imx6ul_cpuidle_init(void)
> +{
> +	return 0;
> +}
>  #endif
> diff --git a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
> index f206506..0f4b95c 100644
> --- a/arch/arm/mach-imx/mach-imx6ul.c
> +++ b/arch/arm/mach-imx/mach-imx6ul.c
> @@ -11,6 +11,7 @@
>  #include <asm/mach/map.h>
>  
>  #include "common.h"
> +#include "cpuidle.h"
>  
>  static void __init imx6ul_init_machine(void)
>  {
> @@ -29,6 +30,12 @@ static void __init imx6ul_init_irq(void)
>  	imx_init_revision_from_anatop();
>  	imx_src_init();
>  	irqchip_init();
> +	imx6_pm_ccm_init("fsl,imx6ul-ccm");
> +}
> +
> +static void __init imx6ul_init_late(void)
> +{
> +	imx6ul_cpuidle_init();
>  }
>  
>  static const char *imx6ul_dt_compat[] __initconst = {
> @@ -40,4 +47,5 @@ DT_MACHINE_START(IMX6UL, "Freescale i.MX6 Ultralite (Device Tree)")
>  	.init_irq	= imx6ul_init_irq,
>  	.init_machine	= imx6ul_init_machine,
>  	.dt_compat	= imx6ul_dt_compat,
> +	.init_late	= imx6ul_init_late,
>  MACHINE_END

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |