Patchwork [v2,1/3] ARM: imx: Add common imx cpuidle init functionality.

login
register
mail settings
Submitter Robert Lee
Date May 2, 2012, 2:12 a.m.
Message ID <1335924760-796-2-git-send-email-rob.lee@linaro.org>
Download mbox | patch
Permalink /patch/156318/
State New
Headers show

Comments

Robert Lee - May 2, 2012, 2:12 a.m.
Add common cpuidle init functionality that can be used by various
imx platforms.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/plat-mxc/Makefile               |    1 +
 arch/arm/plat-mxc/cpuidle.c              |   80 ++++++++++++++++++++++++++++++
 arch/arm/plat-mxc/include/mach/cpuidle.h |   22 ++++++++
 3 files changed, 103 insertions(+)
 create mode 100644 arch/arm/plat-mxc/cpuidle.c
 create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h
Shawn Guo - May 2, 2012, 3:13 a.m.
On Tue, May 01, 2012 at 09:12:38PM -0500, Robert Lee wrote:
> Add common cpuidle init functionality that can be used by various
> imx platforms.
> 
> Signed-off-by: Robert Lee <rob.lee@linaro.org>
> ---
>  arch/arm/plat-mxc/Makefile               |    1 +
>  arch/arm/plat-mxc/cpuidle.c              |   80 ++++++++++++++++++++++++++++++
>  arch/arm/plat-mxc/include/mach/cpuidle.h |   22 ++++++++
>  3 files changed, 103 insertions(+)
>  create mode 100644 arch/arm/plat-mxc/cpuidle.c
>  create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h
> 
> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
> index e81290c..63b064b 100644
> --- a/arch/arm/plat-mxc/Makefile
> +++ b/arch/arm/plat-mxc/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_MXC_ULPI) += ulpi.o
>  obj-$(CONFIG_MXC_USE_EPIT) += epit.o
>  obj-$(CONFIG_MXC_DEBUG_BOARD) += 3ds_debugboard.o
>  obj-$(CONFIG_CPU_FREQ_IMX)    += cpufreq.o
> +obj-$(CONFIG_CPU_IDLE) += cpuidle.o
>  ifdef CONFIG_SND_IMX_SOC
>  obj-y += ssi-fiq.o
>  obj-y += ssi-fiq-ksym.o
> diff --git a/arch/arm/plat-mxc/cpuidle.c b/arch/arm/plat-mxc/cpuidle.c
> new file mode 100644
> index 0000000..b7a5e1c
> --- /dev/null
> +++ b/arch/arm/plat-mxc/cpuidle.c
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + * Copyright 2012 Linaro Ltd.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/io.h>
> +#include <linux/cpuidle.h>
> +#include <linux/hrtimer.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +
> +static struct cpuidle_device __percpu * imx_cpuidle_devices;
> +
> +void imx_cpuidle_devices_uninit(void)
> +{
> +	int cpu_id;
> +	struct cpuidle_device *dev;
> +
> +	for_each_possible_cpu(cpu_id) {
> +		dev = per_cpu_ptr(imx_cpuidle_devices, cpu_id);
> +		cpuidle_unregister_device(dev);
> +	}
> +
> +	free_percpu(imx_cpuidle_devices);
> +}

Does this function need to be exported?  I haven't seen it being
used anywhere outside this file.  Also, can it be __init?

> +
> +int __init imx_cpuidle_init(struct cpuidle_driver *drv)
> +{
> +	struct cpuidle_device *dev;
> +	int cpu_id, ret;
> +
> +	if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
> +		pr_err("%s: Invalid Input\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	ret = cpuidle_register_driver(drv);
> +	if (ret) {
> +		pr_err("%s: Failed to register cpuidle driver with error: %d\n",
> +			 __func__, ret);
> +		return ret;
> +	}
> +
> +	imx_cpuidle_devices = alloc_percpu(struct cpuidle_device);
> +	if (imx_cpuidle_devices == NULL) {
> +		ret = -ENOMEM;
> +		goto unregister_drv;
> +	}
> +
> +	/* initialize state data for each cpuidle_device */
> +	for_each_possible_cpu(cpu_id) {
> +		dev = per_cpu_ptr(imx_cpuidle_devices, cpu_id);
> +		dev->cpu = cpu_id;
> +		dev->state_count = drv->state_count;
> +
> +		ret = cpuidle_register_device(dev);
> +		if (ret) {
> +			pr_err("%s: Failed to register cpu %u\n",
> +				__func__, cpu_id);

Nit: print ret (error code) too?

> +			goto uninit;
> +		}
> +	}
> +
> +	return 0;
> +
> +uninit:
> +	imx_cpuidle_devices_uninit();
> +
> +unregister_drv:
> +	cpuidle_unregister_driver(drv);
> +	return ret;
> +}
> diff --git a/arch/arm/plat-mxc/include/mach/cpuidle.h b/arch/arm/plat-mxc/include/mach/cpuidle.h
> new file mode 100644
> index 0000000..8612510
> --- /dev/null
> +++ b/arch/arm/plat-mxc/include/mach/cpuidle.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + * Copyright 2012 Linaro Ltd.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/cpuidle.h>
> +
> +#ifdef CONFIG_CPU_IDLE
> +extern void imx_cpuidle_devices_uninit(void);
> +extern int imx_cpuidle_init(struct cpuidle_driver *drv);
> +#else
> +static inline void imx_cpuidle_devices_uninit(void) {}
> +static inline int imx_cpuidle_init(struct cpuidle_driver *drv)
> +{ return -ENODEV; }

Nit: if it can not be in the same line with function name, we usually
have it be:

{
	return -ENODEV;
}

> +#endif
> -- 
> 1.7.10
>
Sascha Hauer - May 2, 2012, 7:27 a.m.
On Tue, May 01, 2012 at 09:12:38PM -0500, Robert Lee wrote:
> Add common cpuidle init functionality that can be used by various
> imx platforms.
> 
> Signed-off-by: Robert Lee <rob.lee@linaro.org>
> ---
> +
> +int __init imx_cpuidle_init(struct cpuidle_driver *drv)
> +{
> +	struct cpuidle_device *dev;
> +	int cpu_id, ret;
> +
> +	if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {

Please don't check for !drv here. When someone calls this function with
a NULL pointer he should get a nive stack trace allowing him to figure
out what went wrong.

> +		pr_err("%s: Invalid Input\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	ret = cpuidle_register_driver(drv);
> +	if (ret) {
> +		pr_err("%s: Failed to register cpuidle driver with error: %d\n",
> +			 __func__, ret);
> +		return ret;
> +	}
> +
> +	imx_cpuidle_devices = alloc_percpu(struct cpuidle_device);
> +	if (imx_cpuidle_devices == NULL) {
> +		ret = -ENOMEM;
> +		goto unregister_drv;
> +	}
> +
> +	/* initialize state data for each cpuidle_device */
> +	for_each_possible_cpu(cpu_id) {
> +		dev = per_cpu_ptr(imx_cpuidle_devices, cpu_id);
> +		dev->cpu = cpu_id;
> +		dev->state_count = drv->state_count;
> +
> +		ret = cpuidle_register_device(dev);
> +		if (ret) {
> +			pr_err("%s: Failed to register cpu %u\n",
> +				__func__, cpu_id);
> +			goto uninit;

You should only unregister the cpuidle devices you successfully
registered. Unregistering not yet registered cpuidle devices probably
has unwanted side effects.

Sascha

> +		}
> +	}
> +
> +	return 0;
> +
> +uninit:
> +	imx_cpuidle_devices_uninit();
> +
> +unregister_drv:
> +	cpuidle_unregister_driver(drv);
> +	return ret;
> +}
> diff --git a/arch/arm/plat-mxc/include/mach/cpuidle.h b/arch/arm/plat-mxc/include/mach/cpuidle.h
> new file mode 100644
> index 0000000..8612510
> --- /dev/null
> +++ b/arch/arm/plat-mxc/include/mach/cpuidle.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + * Copyright 2012 Linaro Ltd.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/cpuidle.h>
> +
> +#ifdef CONFIG_CPU_IDLE
> +extern void imx_cpuidle_devices_uninit(void);
> +extern int imx_cpuidle_init(struct cpuidle_driver *drv);
> +#else
> +static inline void imx_cpuidle_devices_uninit(void) {}
> +static inline int imx_cpuidle_init(struct cpuidle_driver *drv)
> +{ return -ENODEV; }
> +#endif
> -- 
> 1.7.10
> 
>
Robert Lee - May 2, 2012, 1:59 p.m.
Shawn,

On Tue, May 1, 2012 at 10:13 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Tue, May 01, 2012 at 09:12:38PM -0500, Robert Lee wrote:
>> Add common cpuidle init functionality that can be used by various
>> imx platforms.
>>
>> Signed-off-by: Robert Lee <rob.lee@linaro.org>
>> ---
>>  arch/arm/plat-mxc/Makefile               |    1 +
>>  arch/arm/plat-mxc/cpuidle.c              |   80 ++++++++++++++++++++++++++++++
>>  arch/arm/plat-mxc/include/mach/cpuidle.h |   22 ++++++++
>>  3 files changed, 103 insertions(+)
>>  create mode 100644 arch/arm/plat-mxc/cpuidle.c
>>  create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h
>>
>> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
>> index e81290c..63b064b 100644
>> --- a/arch/arm/plat-mxc/Makefile
>> +++ b/arch/arm/plat-mxc/Makefile
>> @@ -16,6 +16,7 @@ obj-$(CONFIG_MXC_ULPI) += ulpi.o
>>  obj-$(CONFIG_MXC_USE_EPIT) += epit.o
>>  obj-$(CONFIG_MXC_DEBUG_BOARD) += 3ds_debugboard.o
>>  obj-$(CONFIG_CPU_FREQ_IMX)    += cpufreq.o
>> +obj-$(CONFIG_CPU_IDLE) += cpuidle.o
>>  ifdef CONFIG_SND_IMX_SOC
>>  obj-y += ssi-fiq.o
>>  obj-y += ssi-fiq-ksym.o
>> diff --git a/arch/arm/plat-mxc/cpuidle.c b/arch/arm/plat-mxc/cpuidle.c
>> new file mode 100644
>> index 0000000..b7a5e1c
>> --- /dev/null
>> +++ b/arch/arm/plat-mxc/cpuidle.c
>> @@ -0,0 +1,80 @@
>> +/*
>> + * Copyright 2012 Freescale Semiconductor, Inc.
>> + * Copyright 2012 Linaro Ltd.
>> + *
>> + * The code contained herein is licensed under the GNU General Public
>> + * License. You may obtain a copy of the GNU General Public License
>> + * Version 2 or later at the following locations:
>> + *
>> + * http://www.opensource.org/licenses/gpl-license.html
>> + * http://www.gnu.org/copyleft/gpl.html
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/io.h>
>> +#include <linux/cpuidle.h>
>> +#include <linux/hrtimer.h>
>> +#include <linux/err.h>
>> +#include <linux/slab.h>
>> +
>> +static struct cpuidle_device __percpu * imx_cpuidle_devices;
>> +
>> +void imx_cpuidle_devices_uninit(void)
>> +{
>> +     int cpu_id;
>> +     struct cpuidle_device *dev;
>> +
>> +     for_each_possible_cpu(cpu_id) {
>> +             dev = per_cpu_ptr(imx_cpuidle_devices, cpu_id);
>> +             cpuidle_unregister_device(dev);
>> +     }
>> +
>> +     free_percpu(imx_cpuidle_devices);
>> +}
>
> Does this function need to be exported?  I haven't seen it being
> used anywhere outside this file.  Also, can it be __init?
>

Yes to both for now and can be changed back if necessary in the future.

>> +
>> +int __init imx_cpuidle_init(struct cpuidle_driver *drv)
>> +{
>> +     struct cpuidle_device *dev;
>> +     int cpu_id, ret;
>> +
>> +     if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
>> +             pr_err("%s: Invalid Input\n", __func__);
>> +             return -EINVAL;
>> +     }
>> +
>> +     ret = cpuidle_register_driver(drv);
>> +     if (ret) {
>> +             pr_err("%s: Failed to register cpuidle driver with error: %d\n",
>> +                      __func__, ret);
>> +             return ret;
>> +     }
>> +
>> +     imx_cpuidle_devices = alloc_percpu(struct cpuidle_device);
>> +     if (imx_cpuidle_devices == NULL) {
>> +             ret = -ENOMEM;
>> +             goto unregister_drv;
>> +     }
>> +
>> +     /* initialize state data for each cpuidle_device */
>> +     for_each_possible_cpu(cpu_id) {
>> +             dev = per_cpu_ptr(imx_cpuidle_devices, cpu_id);
>> +             dev->cpu = cpu_id;
>> +             dev->state_count = drv->state_count;
>> +
>> +             ret = cpuidle_register_device(dev);
>> +             if (ret) {
>> +                     pr_err("%s: Failed to register cpu %u\n",
>> +                             __func__, cpu_id);
>
> Nit: print ret (error code) too?
>

I added the printing of the error code based on Sascha's suggestion in
v1 of this submission.

>> +                     goto uninit;
>> +             }
>> +     }
>> +
>> +     return 0;
>> +
>> +uninit:
>> +     imx_cpuidle_devices_uninit();
>> +
>> +unregister_drv:
>> +     cpuidle_unregister_driver(drv);
>> +     return ret;
>> +}
>> diff --git a/arch/arm/plat-mxc/include/mach/cpuidle.h b/arch/arm/plat-mxc/include/mach/cpuidle.h
>> new file mode 100644
>> index 0000000..8612510
>> --- /dev/null
>> +++ b/arch/arm/plat-mxc/include/mach/cpuidle.h
>> @@ -0,0 +1,22 @@
>> +/*
>> + * Copyright 2012 Freescale Semiconductor, Inc.
>> + * Copyright 2012 Linaro Ltd.
>> + *
>> + * The code contained herein is licensed under the GNU General Public
>> + * License. You may obtain a copy of the GNU General Public License
>> + * Version 2 or later at the following locations:
>> + *
>> + * http://www.opensource.org/licenses/gpl-license.html
>> + * http://www.gnu.org/copyleft/gpl.html
>> + */
>> +
>> +#include <linux/cpuidle.h>
>> +
>> +#ifdef CONFIG_CPU_IDLE
>> +extern void imx_cpuidle_devices_uninit(void);
>> +extern int imx_cpuidle_init(struct cpuidle_driver *drv);
>> +#else
>> +static inline void imx_cpuidle_devices_uninit(void) {}
>> +static inline int imx_cpuidle_init(struct cpuidle_driver *drv)
>> +{ return -ENODEV; }
>
> Nit: if it can not be in the same line with function name, we usually
> have it be:
>
> {
>        return -ENODEV;
> }

Understood.  I was just going by what I have seen used in other places
(like include/linux/cpuidle.h).

Thanks,
Rob

>
>> +#endif
>> --
>> 1.7.10
>>
>
> --
> Regards,
> Shawn
Shawn Guo - May 2, 2012, 2:07 p.m.
On 2 May 2012 21:59, Rob Lee <rob.lee@linaro.org> wrote:
>>> +             ret = cpuidle_register_device(dev);
>>> +             if (ret) {
>>> +                     pr_err("%s: Failed to register cpu %u\n",
>>> +                             __func__, cpu_id);
>>
>> Nit: print ret (error code) too?
>>
>
> I added the printing of the error code based on Sascha's suggestion in
> v1 of this submission.
>
But you did not print "ret" in above pr_err.

Regards,
Shawn
Robert Lee - May 2, 2012, 2:09 p.m.
On Wed, May 2, 2012 at 9:07 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On 2 May 2012 21:59, Rob Lee <rob.lee@linaro.org> wrote:
>>>> +             ret = cpuidle_register_device(dev);
>>>> +             if (ret) {
>>>> +                     pr_err("%s: Failed to register cpu %u\n",
>>>> +                             __func__, cpu_id);
>>>
>>> Nit: print ret (error code) too?
>>>
>>
>> I added the printing of the error code based on Sascha's suggestion in
>> v1 of this submission.
>>
> But you did not print "ret" in above pr_err.
>

Argh, sorry, still trying to wake up I guess.  Thanks.

> Regards,
> Shawn
Robert Lee - May 2, 2012, 7:16 p.m.
Sascha,

On Wed, May 2, 2012 at 2:27 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Tue, May 01, 2012 at 09:12:38PM -0500, Robert Lee wrote:
>> Add common cpuidle init functionality that can be used by various
>> imx platforms.
>>
>> Signed-off-by: Robert Lee <rob.lee@linaro.org>
>> ---
>> +
>> +int __init imx_cpuidle_init(struct cpuidle_driver *drv)
>> +{
>> +     struct cpuidle_device *dev;
>> +     int cpu_id, ret;
>> +
>> +     if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
>
> Please don't check for !drv here. When someone calls this function with
> a NULL pointer he should get a nive stack trace allowing him to figure
> out what went wrong.
>

Ok, I will change this in v3.  Given your statement, my understanding
is that I should avoid adding checks to make sure a valid driver
object was given as the stack trace information is all the handling
that is needed.  If there is any further logic needed in that rule,
could you elaborate so that I don't make this mistake in the future,
or so that I don't add a check on a driver object in a case that I
should?

>> +             pr_err("%s: Invalid Input\n", __func__);
>> +             return -EINVAL;
>> +     }
>> +
>> +     ret = cpuidle_register_driver(drv);
>> +     if (ret) {
>> +             pr_err("%s: Failed to register cpuidle driver with error: %d\n",
>> +                      __func__, ret);
>> +             return ret;
>> +     }
>> +
>> +     imx_cpuidle_devices = alloc_percpu(struct cpuidle_device);
>> +     if (imx_cpuidle_devices == NULL) {
>> +             ret = -ENOMEM;
>> +             goto unregister_drv;
>> +     }
>> +
>> +     /* initialize state data for each cpuidle_device */
>> +     for_each_possible_cpu(cpu_id) {
>> +             dev = per_cpu_ptr(imx_cpuidle_devices, cpu_id);
>> +             dev->cpu = cpu_id;
>> +             dev->state_count = drv->state_count;
>> +
>> +             ret = cpuidle_register_device(dev);
>> +             if (ret) {
>> +                     pr_err("%s: Failed to register cpu %u\n",
>> +                             __func__, cpu_id);
>> +                     goto uninit;
>
> You should only unregister the cpuidle devices you successfully
> registered. Unregistering not yet registered cpuidle devices probably
> has unwanted side effects.
>

I did not add in this handling because the cpuidle_unregister_device()
call already has a "registered" check so extra handling seemed
unnecessary.  But if you still think it is needed just let me know.

cpuidle_unregister_device()
{
...
	if (dev->registered == 0)
		return;
...

Thanks,
Rob

> Sascha
>
>> +             }
>> +     }
>> +
>> +     return 0;
>> +
>> +uninit:
>> +     imx_cpuidle_devices_uninit();
>> +
>> +unregister_drv:
>> +     cpuidle_unregister_driver(drv);
>> +     return ret;
>> +}
>> diff --git a/arch/arm/plat-mxc/include/mach/cpuidle.h b/arch/arm/plat-mxc/include/mach/cpuidle.h
>> new file mode 100644
>> index 0000000..8612510
>> --- /dev/null
>> +++ b/arch/arm/plat-mxc/include/mach/cpuidle.h
>> @@ -0,0 +1,22 @@
>> +/*
>> + * Copyright 2012 Freescale Semiconductor, Inc.
>> + * Copyright 2012 Linaro Ltd.
>> + *
>> + * The code contained herein is licensed under the GNU General Public
>> + * License. You may obtain a copy of the GNU General Public License
>> + * Version 2 or later at the following locations:
>> + *
>> + * http://www.opensource.org/licenses/gpl-license.html
>> + * http://www.gnu.org/copyleft/gpl.html
>> + */
>> +
>> +#include <linux/cpuidle.h>
>> +
>> +#ifdef CONFIG_CPU_IDLE
>> +extern void imx_cpuidle_devices_uninit(void);
>> +extern int imx_cpuidle_init(struct cpuidle_driver *drv);
>> +#else
>> +static inline void imx_cpuidle_devices_uninit(void) {}
>> +static inline int imx_cpuidle_init(struct cpuidle_driver *drv)
>> +{ return -ENODEV; }
>> +#endif
>> --
>> 1.7.10
>>
>>
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Sascha Hauer - May 2, 2012, 8:21 p.m.
On Wed, May 02, 2012 at 02:16:36PM -0500, Rob Lee wrote:
> Sascha,
> 
> >> +int __init imx_cpuidle_init(struct cpuidle_driver *drv)
> >> +{
> >> +     struct cpuidle_device *dev;
> >> +     int cpu_id, ret;
> >> +
> >> +     if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
> >
> > Please don't check for !drv here. When someone calls this function with
> > a NULL pointer he should get a nive stack trace allowing him to figure
> > out what went wrong.
> >
> 
> Ok, I will change this in v3.  Given your statement, my understanding
> is that I should avoid adding checks to make sure a valid driver
> object was given as the stack trace information is all the handling
> that is needed.  If there is any further logic needed in that rule,
> could you elaborate so that I don't make this mistake in the future,
> or so that I don't add a check on a driver object in a case that I
> should?

Here we have the case that only a Kernel developer will add a call to
this function. For a kernel developer a stack trace is more useful
than a pr_err. Of course this is different when not testing for a NULL
pointer causes subtle bugs in unrelated code.

> >
> > You should only unregister the cpuidle devices you successfully
> > registered. Unregistering not yet registered cpuidle devices probably
> > has unwanted side effects.
> >
> 
> I did not add in this handling because the cpuidle_unregister_device()
> call already has a "registered" check so extra handling seemed
> unnecessary.  But if you still think it is needed just let me know.
> 

It's ok then. I didn't check cpuidle_unregister_device.

Sascha

Patch

diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
index e81290c..63b064b 100644
--- a/arch/arm/plat-mxc/Makefile
+++ b/arch/arm/plat-mxc/Makefile
@@ -16,6 +16,7 @@  obj-$(CONFIG_MXC_ULPI) += ulpi.o
 obj-$(CONFIG_MXC_USE_EPIT) += epit.o
 obj-$(CONFIG_MXC_DEBUG_BOARD) += 3ds_debugboard.o
 obj-$(CONFIG_CPU_FREQ_IMX)    += cpufreq.o
+obj-$(CONFIG_CPU_IDLE) += cpuidle.o
 ifdef CONFIG_SND_IMX_SOC
 obj-y += ssi-fiq.o
 obj-y += ssi-fiq-ksym.o
diff --git a/arch/arm/plat-mxc/cpuidle.c b/arch/arm/plat-mxc/cpuidle.c
new file mode 100644
index 0000000..b7a5e1c
--- /dev/null
+++ b/arch/arm/plat-mxc/cpuidle.c
@@ -0,0 +1,80 @@ 
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright 2012 Linaro Ltd.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/kernel.h>
+#include <linux/io.h>
+#include <linux/cpuidle.h>
+#include <linux/hrtimer.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+
+static struct cpuidle_device __percpu * imx_cpuidle_devices;
+
+void imx_cpuidle_devices_uninit(void)
+{
+	int cpu_id;
+	struct cpuidle_device *dev;
+
+	for_each_possible_cpu(cpu_id) {
+		dev = per_cpu_ptr(imx_cpuidle_devices, cpu_id);
+		cpuidle_unregister_device(dev);
+	}
+
+	free_percpu(imx_cpuidle_devices);
+}
+
+int __init imx_cpuidle_init(struct cpuidle_driver *drv)
+{
+	struct cpuidle_device *dev;
+	int cpu_id, ret;
+
+	if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
+		pr_err("%s: Invalid Input\n", __func__);
+		return -EINVAL;
+	}
+
+	ret = cpuidle_register_driver(drv);
+	if (ret) {
+		pr_err("%s: Failed to register cpuidle driver with error: %d\n",
+			 __func__, ret);
+		return ret;
+	}
+
+	imx_cpuidle_devices = alloc_percpu(struct cpuidle_device);
+	if (imx_cpuidle_devices == NULL) {
+		ret = -ENOMEM;
+		goto unregister_drv;
+	}
+
+	/* initialize state data for each cpuidle_device */
+	for_each_possible_cpu(cpu_id) {
+		dev = per_cpu_ptr(imx_cpuidle_devices, cpu_id);
+		dev->cpu = cpu_id;
+		dev->state_count = drv->state_count;
+
+		ret = cpuidle_register_device(dev);
+		if (ret) {
+			pr_err("%s: Failed to register cpu %u\n",
+				__func__, cpu_id);
+			goto uninit;
+		}
+	}
+
+	return 0;
+
+uninit:
+	imx_cpuidle_devices_uninit();
+
+unregister_drv:
+	cpuidle_unregister_driver(drv);
+	return ret;
+}
diff --git a/arch/arm/plat-mxc/include/mach/cpuidle.h b/arch/arm/plat-mxc/include/mach/cpuidle.h
new file mode 100644
index 0000000..8612510
--- /dev/null
+++ b/arch/arm/plat-mxc/include/mach/cpuidle.h
@@ -0,0 +1,22 @@ 
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright 2012 Linaro Ltd.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/cpuidle.h>
+
+#ifdef CONFIG_CPU_IDLE
+extern void imx_cpuidle_devices_uninit(void);
+extern int imx_cpuidle_init(struct cpuidle_driver *drv);
+#else
+static inline void imx_cpuidle_devices_uninit(void) {}
+static inline int imx_cpuidle_init(struct cpuidle_driver *drv)
+{ return -ENODEV; }
+#endif