From: Robert Lee <hidden> Date: 2012-05-07 21:16:59
Add common cpuidle init functionality that can be used by various
imx platforms.
Signed-off-by: Robert Lee <redacted>
---
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
@@ -0,0 +1,80 @@+/*+*Copyright2012FreescaleSemiconductor,Inc.+*Copyright2012LinaroLtd.+*+*ThecodecontainedhereinislicensedundertheGNUGeneralPublic+*License.YoumayobtainacopyoftheGNUGeneralPublicLicense+*Version2orlateratthefollowinglocations:+*+*http://www.opensource.org/licenses/gpl-license.html+*http://www.gnu.org/copyleft/gpl.html+*/++#include<linux/cpuidle.h>+#include<linux/err.h>+#include<linux/hrtimer.h>+#include<linux/io.h>+#include<linux/kernel.h>+#include<linux/slab.h>++staticstructcpuidle_device__percpu*imx_cpuidle_devices;++staticvoid__initimx_cpuidle_devices_uninit(void)+{+intcpu_id;+structcpuidle_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__initimx_cpuidle_init(structcpuidle_driver*drv)+{+structcpuidle_device*dev;+intcpu_id,ret;++if(drv->state_count>CPUIDLE_STATE_MAX){+pr_err("%s: state_count exceeds maximum\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);+returnret;+}++imx_cpuidle_devices=alloc_percpu(structcpuidle_device);+if(imx_cpuidle_devices==NULL){+ret=-ENOMEM;+gotounregister_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, error: %d\n",+__func__,cpu_id,ret);+gotouninit;+}+}++return0;++uninit:+imx_cpuidle_devices_uninit();++unregister_drv:+cpuidle_unregister_driver(drv);+returnret;+}
@@ -20,26 +20,61 @@#include<mach/hardware.h>#include<mach/common.h>+#include<mach/cpuidle.h>#include<mach/devices-common.h>#include<mach/iomux-v3.h>staticstructclk*gpc_dvfs_clk;-staticvoidimx5_idle(void)+staticintimx5_idle(void){+intret=0;+/* gpc clock is needed for SRPG */if(gpc_dvfs_clk==NULL){gpc_dvfs_clk=clk_get(NULL,"gpc_dvfs");if(IS_ERR(gpc_dvfs_clk))-return;+return-ENODEV;}clk_enable(gpc_dvfs_clk);mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);if(!tzic_enable_wake())cpu_do_idle();+else+ret=-EBUSY;clk_disable(gpc_dvfs_clk);++returnret;+}++staticintimx5_cpuidle_enter(structcpuidle_device*dev,+structcpuidle_driver*drv,intidx)+{+intret;++ret=imx5_idle();++if(ret<0)+returnret;++returnidx;}+staticstructcpuidle_driverimx5_cpuidle_driver={+.name="imx5_cpuidle",+.owner=THIS_MODULE,+.en_core_tk_irqen=1,+.states[0]={+.enter=imx5_cpuidle_enter,+.exit_latency=20,/* max latency@160MHz */+.target_residency=1,+.flags=CPUIDLE_FLAG_TIME_VALID,+.name="IMX5 SRPG",+.desc="CPU state retained,powered off",+},+.state_count=1,+};+/**DefinetheMX50memorymap.*/
From: Robert Lee <hidden> Date: 2012-05-07 21:17:15
Add basic imx6q cpuidle driver. For now, only basic WFI state is
supported. Deeper idle states will be added in the future.
Signed-off-by: Robert Lee <redacted>
---
arch/arm/mach-imx/mach-imx6q.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
@@ -20,26 +20,61 @@#include<mach/hardware.h>#include<mach/common.h>+#include<mach/cpuidle.h>#include<mach/devices-common.h>#include<mach/iomux-v3.h>staticstructclk*gpc_dvfs_clk;-staticvoidimx5_idle(void)+staticintimx5_idle(void){+intret=0;+/* gpc clock is needed for SRPG */if(gpc_dvfs_clk==NULL){gpc_dvfs_clk=clk_get(NULL,"gpc_dvfs");
This clk_get should go away here and be moved somewhere to
initialization. Also, if getting this clock fails we can still
do regular cpu_do_idle. Additionally, if clk_get fails, we'll
have a ERR_PTR value in gpc_dvfs_clk in which case the
gpc_dvfs_clk == NULL won't trigger next time you are here and
then you'll enable a nonexisting clock below.
I wonder why you don't add the default ARM_CPUIDLE_WFI_STATE_PWR state.
The above is something different, right? It has a greater exit latency
than ARM_CPUIDLE_WFI_STATE_PWR, so why don't we add it here aswell?
Still this looks suspicious. Reading this will lead to the question why
this prototype is casted. Please just add a imx5_pm_idle with the
correct prototype.
?#include <mach/hardware.h>
?#include <mach/common.h>
+#include <mach/cpuidle.h>
?#include <mach/devices-common.h>
?#include <mach/iomux-v3.h>
?static struct clk *gpc_dvfs_clk;
-static void imx5_idle(void)
+static int imx5_idle(void)
?{
+ ? ? int ret = 0;
+
? ? ? /* gpc clock is needed for SRPG */
? ? ? if (gpc_dvfs_clk == NULL) {
? ? ? ? ? ? ? gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs");
This clk_get should go away here and be moved somewhere to
initialization. Also, if getting this clock fails we can still
do regular cpu_do_idle. Additionally, if clk_get fails, we'll
have a ERR_PTR value in gpc_dvfs_clk in which case the
gpc_dvfs_clk == NULL won't trigger next time you are here and
then you'll enable a nonexisting clock below.
Agree. I'd prefer to enable this clock during intialization and just
leave it running. It is supposed to be a very low power clock and I
couldn't measuring any power difference with and without it being
enabled
I wonder why you don't add the default ARM_CPUIDLE_WFI_STATE_PWR state.
The above is something different, right? It has a greater exit latency
than ARM_CPUIDLE_WFI_STATE_PWR, so why don't we add it here aswell?
Yes and no. Yes this is a different state but no, it doesn't have a
significantly greater exit latency, or at least a large enough exit
latency to warrant an extra state in my opinion. According to the
i.MX5 documentation, the extra exit time beyond basic WFI required for
the "WAIT_UNCLOCKED_POWER_OFF" state is 500ns (this is due to a
difference in i.MX5 hardware implementation compared to all other ARM
platforms). In reality, it did require a few more microseconds to
perform in my testing just based on the extra register writes in
mx5_cpu_lp_set(). I'd like to clean up mx5_cpu_lp_set() and add a
global variable to track the previous state and to just exit out if
the new state is the same as the old. I could do this cleanup as part
of this patchset if you prefer that.
Still this looks suspicious. Reading this will lead to the question why
this prototype is casted. Please just add a imx5_pm_idle with the
correct prototype.
On Wed, May 09, 2012 at 09:27:02AM -0500, Rob Lee wrote:
Sascha,
quoted
This clk_get should go away here and be moved somewhere to
initialization. Also, if getting this clock fails we can still
do regular cpu_do_idle. Additionally, if clk_get fails, we'll
have a ERR_PTR value in gpc_dvfs_clk in which case the
gpc_dvfs_clk == NULL won't trigger next time you are here and
then you'll enable a nonexisting clock below.
Agree. I'd prefer to enable this clock during intialization and just
leave it running. It is supposed to be a very low power clock and I
couldn't measuring any power difference with and without it being
enabled
Ok, even better.
quoted
I wonder why you don't add the default ARM_CPUIDLE_WFI_STATE_PWR state.
The above is something different, right? It has a greater exit latency
than ARM_CPUIDLE_WFI_STATE_PWR, so why don't we add it here aswell?
Yes and no. Yes this is a different state but no, it doesn't have a
significantly greater exit latency, or at least a large enough exit
latency to warrant an extra state in my opinion. According to the
i.MX5 documentation, the extra exit time beyond basic WFI required for
the "WAIT_UNCLOCKED_POWER_OFF" state is 500ns (this is due to a
difference in i.MX5 hardware implementation compared to all other ARM
platforms). In reality, it did require a few more microseconds to
perform in my testing just based on the extra register writes in
mx5_cpu_lp_set(). I'd like to clean up mx5_cpu_lp_set() and add a
global variable to track the previous state and to just exit out if
the new state is the same as the old.
Do you think it's worth it? You buy skipping the read with an additional
test.
I could do this cleanup as part of this patchset if you prefer that.
Yes please. Cleanups before adding new features is always a good reason
to apply a patch series ;)
Sascha
--
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 |
On Thu, May 10, 2012 at 7:41 AM, Sascha Hauer [off-list ref] wrote:
On Wed, May 09, 2012 at 09:27:02AM -0500, Rob Lee wrote:
quoted
Sascha,
quoted
This clk_get should go away here and be moved somewhere to
initialization. Also, if getting this clock fails we can still
do regular cpu_do_idle. Additionally, if clk_get fails, we'll
have a ERR_PTR value in gpc_dvfs_clk in which case the
gpc_dvfs_clk == NULL won't trigger next time you are here and
then you'll enable a nonexisting clock below.
Agree. ?I'd prefer to enable this clock during intialization and just
leave it running. ?It is supposed to be a very low power clock and I
couldn't measuring any power difference with and without it being
enabled
Ok, even better.
quoted
quoted
I wonder why you don't add the default ARM_CPUIDLE_WFI_STATE_PWR state.
The above is something different, right? It has a greater exit latency
than ARM_CPUIDLE_WFI_STATE_PWR, so why don't we add it here aswell?
Yes and no. ?Yes this is a different state but no, it doesn't have a
significantly greater exit latency, or at least a large enough exit
latency to warrant an extra state in my opinion. ?According to the
i.MX5 documentation, the extra exit time beyond basic WFI required for
the ?"WAIT_UNCLOCKED_POWER_OFF" state is 500ns (this is due to a
difference in i.MX5 hardware implementation compared to all other ARM
platforms). ?In reality, it did require a few more microseconds to
perform in my testing just based on the extra register writes in
mx5_cpu_lp_set(). ?I'd like to clean up mx5_cpu_lp_set() and add a
global variable to track the previous state and to just exit out if
the new state is the same as the old.
Do you think it's worth it? You buy skipping the read with an additional
test.
I'll run some tests to check.
Thanks,
Rob
quoted
I could do this cleanup as part of this patchset if you prefer that.
Yes please. Cleanups before adding new features is always a good reason
to apply a patch series ;)
Sascha
--
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 |