Patchwork [RFC,v2,2/2] ARM: imx: Add mx5 cpuidle implmentation

login
register
mail settings
Submitter Robert Lee
Date Dec. 14, 2011, 7:02 a.m.
Message ID <1323846126-7516-3-git-send-email-rob.lee@linaro.org>
Download mbox | patch
Permalink /patch/131317/
State New
Headers show

Comments

Robert Lee - Dec. 14, 2011, 7:02 a.m.
Add mx5 cpuidle implmentation (based upon the new common cpuidle code).

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-mx5/Makefile          |    3 +-
 arch/arm/mach-mx5/clock-mx51-mx53.c |    3 ++
 arch/arm/mach-mx5/cpuidle.c         |   65 +++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-mx5/cpuidle.c
Mark Brown - Dec. 22, 2011, 5:50 p.m.
On Wed, Dec 14, 2011 at 01:02:06AM -0600, Robert Lee wrote:

> +	clk_enable(&gpc_dvfs_clk);

Should these enables be in the cpuidle code?  The device appears to have
been working fine without them thus far...  Alternatively, if they
should be on anyway does this need to be split out and sent as a bug
fix?
Robert Lee - Jan. 4, 2012, 11:35 p.m.
On 22 December 2011 11:50, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Dec 14, 2011 at 01:02:06AM -0600, Robert Lee wrote:
>
>> +     clk_enable(&gpc_dvfs_clk);
>
> Should these enables be in the cpuidle code?  The device appears to have
> been working fine without them thus far...  Alternatively, if they
> should be on anyway does this need to be split out and sent as a bug
> fix?

This clock is used by the existing pm_suspend code for i.MX51 and
other future code being worked on.  Since it uses extremely minimal
power and is required to be enabled during low power modes, it seemed
cleanest to just enable it during clock init.  But I forgot to remove
it from it's enabling from i.MX51 pm_suspend code so I can do that for
v3.
Shawn Guo - Jan. 5, 2012, 3:21 a.m.
On Wed, Jan 04, 2012 at 05:35:39PM -0600, Rob Lee wrote:
> On 22 December 2011 11:50, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > On Wed, Dec 14, 2011 at 01:02:06AM -0600, Robert Lee wrote:
> >
> >> +     clk_enable(&gpc_dvfs_clk);
> >
> > Should these enables be in the cpuidle code?  The device appears to have
> > been working fine without them thus far...  Alternatively, if they
> > should be on anyway does this need to be split out and sent as a bug
> > fix?
> 
> This clock is used by the existing pm_suspend code for i.MX51 and
> other future code being worked on.  Since it uses extremely minimal
> power and is required to be enabled during low power modes, it seemed
> cleanest to just enable it during clock init.

+1

Regards,
Shawn

> But I forgot to remove
> it from it's enabling from i.MX51 pm_suspend code so I can do that for
> v3.
>
Mark Brown - Jan. 5, 2012, 5:55 a.m.
On Wed, Jan 04, 2012 at 05:35:39PM -0600, Rob Lee wrote:
> On 22 December 2011 11:50, Mark Brown
> > On Wed, Dec 14, 2011 at 01:02:06AM -0600, Robert Lee wrote:

> >> +     clk_enable(&gpc_dvfs_clk);

> > Should these enables be in the cpuidle code?  The device appears to have
> > been working fine without them thus far...  Alternatively, if they
> > should be on anyway does this need to be split out and sent as a bug
> > fix?

> This clock is used by the existing pm_suspend code for i.MX51 and
> other future code being worked on.  Since it uses extremely minimal
> power and is required to be enabled during low power modes, it seemed
> cleanest to just enable it during clock init.  But I forgot to remove
> it from it's enabling from i.MX51 pm_suspend code so I can do that for
> v3.

Sounds like it's worth splitting out and getting it merged as quickly as
possible then?  It wasn't the code I was querying, it was the way it is
being merged.
Robert Lee - Jan. 6, 2012, 9:10 p.m.
On 4 January 2012 23:55, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Jan 04, 2012 at 05:35:39PM -0600, Rob Lee wrote:
>> On 22 December 2011 11:50, Mark Brown
>> > On Wed, Dec 14, 2011 at 01:02:06AM -0600, Robert Lee wrote:
>
>> >> +     clk_enable(&gpc_dvfs_clk);
>
>> > Should these enables be in the cpuidle code?  The device appears to have
>> > been working fine without them thus far...  Alternatively, if they
>> > should be on anyway does this need to be split out and sent as a bug
>> > fix?
>
>> This clock is used by the existing pm_suspend code for i.MX51 and
>> other future code being worked on.  Since it uses extremely minimal
>> power and is required to be enabled during low power modes, it seemed
>> cleanest to just enable it during clock init.  But I forgot to remove
>> it from it's enabling from i.MX51 pm_suspend code so I can do that for
>> v3.
>
> Sounds like it's worth splitting out and getting it merged as quickly as
> possible then?  It wasn't the code I was querying, it was the way it is
> being merged.

Sure, I can split this portion out as a separate patch.  I'd prefer to
keep it as part of this patch series though as the existing usage and
implementation of this clock works ok, it's just not the best
implementation once another driver needs to use this clock.  And it
may raise concern if implemented by itself with a pressing reason
being shown.  At least that's my thoughts.  I'm fairly new to the
community so please tolerate anything dumb I say and help understand
the ways of the force.

Patch

diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
index 0fc6080..2c348b4 100644
--- a/arch/arm/mach-mx5/Makefile
+++ b/arch/arm/mach-mx5/Makefile
@@ -3,8 +3,9 @@ 
 #
 
 # Object file lists.
-obj-y   := cpu.o mm.o clock-mx51-mx53.o ehci.o system.o
 
+obj-y   := cpu.o mm.o clock-mx51-mx53.o ehci.o system.o
+obj-$(CONFIG_CPU_IDLE) += cpuidle.o
 obj-$(CONFIG_PM) += pm-imx5.o
 obj-$(CONFIG_CPU_FREQ_IMX)    += cpu_op-mx51.o
 obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 4cb2769..12c8a2b 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -1533,6 +1533,7 @@  static struct clk_lookup mx53_lookups[] = {
 	_REGISTER_CLOCK("imx53-ahci.0", "ahci", sata_clk)
 	_REGISTER_CLOCK("imx53-ahci.0", "ahci_phy", ahci_phy_clk)
 	_REGISTER_CLOCK("imx53-ahci.0", "ahci_dma", ahci_dma_clk)
+	_REGISTER_CLOCK(NULL, "gpc_dvfs", gpc_dvfs_clk)
 };
 
 static void clk_tree_init(void)
@@ -1572,6 +1573,7 @@  int __init mx51_clocks_init(unsigned long ckil, unsigned long osc,
 
 	clk_enable(&cpu_clk);
 	clk_enable(&main_bus_clk);
+	clk_enable(&gpc_dvfs_clk);
 
 	clk_enable(&iim_clk);
 	imx_print_silicon_rev("i.MX51", mx51_revision());
@@ -1615,6 +1617,7 @@  int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
 	clk_set_parent(&uart_root_clk, &pll3_sw_clk);
 	clk_enable(&cpu_clk);
 	clk_enable(&main_bus_clk);
+	clk_enable(&gpc_dvfs_clk);
 
 	clk_enable(&iim_clk);
 	imx_print_silicon_rev("i.MX53", mx53_revision());
diff --git a/arch/arm/mach-mx5/cpuidle.c b/arch/arm/mach-mx5/cpuidle.c
new file mode 100644
index 0000000..7e8ad0a
--- /dev/null
+++ b/arch/arm/mach-mx5/cpuidle.c
@@ -0,0 +1,65 @@ 
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 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/init.h>
+#include <linux/export.h>
+#include <linux/cpuidle.h>
+#include <asm/proc-fns.h>
+#include <mach/common.h>
+
+int imx5_enter(struct cpuidle_device *dev,
+			struct cpuidle_driver *drv,
+			      int index)
+{
+	mx5_cpu_lp_set((unsigned int)dev->states_usage[index].driver_data);
+	cpu_do_idle();
+	return index;
+}
+
+static struct cpuidle_driver imx5_cpuidle_driver = {
+	.name	= "imx5_cpuidle",
+	.owner	= THIS_MODULE,
+	.states[0] = {
+		.enter			= imx5_enter,
+		.exit_latency		= 12,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "WFI",
+		.desc			= "idle cpu powered, unclocked.",
+	},
+	.states[1] = {
+		.enter			= imx5_enter,
+		.exit_latency		= 20,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "WFI SRPG",
+		.desc			= "idle cpu unpowered, unclocked.",
+	},
+	.state_count = 2,
+};
+
+/* use drive_data field to hold the mx5 idle parameter */
+static void __init imx5_dd_init(struct cpuidle_device * dev)
+{
+	dev->states_usage[0].driver_data = (void *)WAIT_UNCLOCKED;
+	dev->states_usage[1].driver_data = (void *)WAIT_UNCLOCKED_POWER_OFF;
+}
+
+static int __init imx5_init_cpuidle(void)
+{
+	int ret;
+
+	ret = common_cpuidle_init(&imx5_cpuidle_driver,
+					true,
+					imx5_dd_init);
+	return ret;
+}
+late_initcall(imx5_init_cpuidle);