| Message ID | 1323846126-7516-3-git-send-email-rob.lee@linaro.org |
|---|---|
| State | New |
| Headers | show |
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?
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.
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. >
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.
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.
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);
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