* [PATCH v2 0/2] ARM: IMX5: cpuidle driver @ 2011-02-11 9:36 yong.shen at linaro.org 2011-02-11 9:36 ` [PATCH v2 1/2] " yong.shen at linaro.org 2011-02-11 9:36 ` [PATCH v2 2/2] ARM: IMX5 bbg: add cpuidle parameters yong.shen at linaro.org 0 siblings, 2 replies; 13+ messages in thread From: yong.shen at linaro.org @ 2011-02-11 9:36 UTC (permalink / raw) To: linux-arm-kernel change log: 1. move code to arch/arm/mach-mx5/, since it is cpu-type specific 2. provide a interface for various board to register board-specific params. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] ARM: IMX5: cpuidle driver 2011-02-11 9:36 [PATCH v2 0/2] ARM: IMX5: cpuidle driver yong.shen at linaro.org @ 2011-02-11 9:36 ` yong.shen at linaro.org 2011-02-15 18:27 ` Sascha Hauer 2011-02-16 8:11 ` [PATCH v2 " Sascha Hauer 2011-02-11 9:36 ` [PATCH v2 2/2] ARM: IMX5 bbg: add cpuidle parameters yong.shen at linaro.org 1 sibling, 2 replies; 13+ messages in thread From: yong.shen at linaro.org @ 2011-02-11 9:36 UTC (permalink / raw) To: linux-arm-kernel From: Yong Shen <yong.shen@freescale.com> implement cpuidle driver for iMX5 SOCs, leave cpuidle params to board related code. Signed-off-by: Yong Shen <yong.shen@freescale.com> --- arch/arm/mach-mx5/Makefile | 1 + arch/arm/mach-mx5/cpuidle.c | 113 +++++++++++++++++++++++++++++++++++++++++++ arch/arm/mach-mx5/cpuidle.h | 26 ++++++++++ 3 files changed, 140 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-mx5/cpuidle.c create mode 100644 arch/arm/mach-mx5/cpuidle.h diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile index 0d43be9..12239e0 100644 --- a/arch/arm/mach-mx5/Makefile +++ b/arch/arm/mach-mx5/Makefile @@ -7,6 +7,7 @@ obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o obj-$(CONFIG_SOC_IMX50) += mm-mx50.o obj-$(CONFIG_CPU_FREQ_IMX) += cpu_op-mx51.o +obj-$(CONFIG_CPU_IDLE) += cpuidle.o obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o obj-$(CONFIG_MACH_MX53_EVK) += board-mx53_evk.o diff --git a/arch/arm/mach-mx5/cpuidle.c b/arch/arm/mach-mx5/cpuidle.c new file mode 100644 index 0000000..9d77c47 --- /dev/null +++ b/arch/arm/mach-mx5/cpuidle.c @@ -0,0 +1,113 @@ +/* + * arch/arm/mach-mx5/cpuidle.c + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include <linux/io.h> +#include <linux/cpuidle.h> +#include <asm/proc-fns.h> +#include <mach/hardware.h> +#include "cpuidle.h" +#include "crm_regs.h" + +static struct imx_cpuidle_params *imx_cpuidle_params; +void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params) +{ + imx_cpuidle_params = cpuidle_params; +} + +extern int tzic_enable_wake(int is_idle); +static int imx_enter_idle(struct cpuidle_device *dev, + struct cpuidle_state *state) +{ + struct timeval before, after; + int idle_time; + u32 plat_lpc, arm_srpgcr, ccm_clpcr; + u32 empgc0, empgc1; + + local_irq_disable(); + do_gettimeofday(&before); + + plat_lpc = __raw_readl(MXC_CORTEXA8_PLAT_LPC) & + ~(MXC_CORTEXA8_PLAT_LPC_DSM); + ccm_clpcr = __raw_readl(MXC_CCM_CLPCR) & ~(MXC_CCM_CLPCR_LPM_MASK); + arm_srpgcr = __raw_readl(MXC_SRPG_ARM_SRPGCR) & ~(MXC_SRPGCR_PCR); + empgc0 = __raw_readl(MXC_SRPG_EMPGC0_SRPGCR) & ~(MXC_SRPGCR_PCR); + empgc1 = __raw_readl(MXC_SRPG_EMPGC1_SRPGCR) & ~(MXC_SRPGCR_PCR); + + if (state == &dev->states[WAIT_CLK_ON]) + ; + else if (state == &dev->states[WAIT_CLK_OFF]) + ccm_clpcr |= (0x1 << MXC_CCM_CLPCR_LPM_OFFSET); + else if (state == &dev->states[WAIT_CLK_OFF_POWER_OFF]) { + /* Wait unclocked, power off */ + plat_lpc |= MXC_CORTEXA8_PLAT_LPC_DSM + | MXC_CORTEXA8_PLAT_LPC_DBG_DSM; + arm_srpgcr |= MXC_SRPGCR_PCR; + ccm_clpcr |= (0x1 << MXC_CCM_CLPCR_LPM_OFFSET); + ccm_clpcr &= ~MXC_CCM_CLPCR_VSTBY; + ccm_clpcr &= ~MXC_CCM_CLPCR_SBYOS; + if (tzic_enable_wake(1) != 0) { + local_irq_enable(); + return 0; + } + } + + __raw_writel(plat_lpc, MXC_CORTEXA8_PLAT_LPC); + __raw_writel(ccm_clpcr, MXC_CCM_CLPCR); + __raw_writel(arm_srpgcr, MXC_SRPG_ARM_SRPGCR); + + cpu_do_idle(); + + do_gettimeofday(&after); + local_irq_enable(); + idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC + + (after.tv_usec - before.tv_usec); + return idle_time; +} + +static struct cpuidle_driver imx_cpuidle_driver = { + .name = "imx_idle", + .owner = THIS_MODULE, +}; + +static DEFINE_PER_CPU(struct cpuidle_device, imx_cpuidle_device); + +static int __init imx_cpuidle_init(void) +{ + struct cpuidle_device *device; + int i; + + if (imx_cpuidle_params == NULL) + return -ENODEV; + + cpuidle_register_driver(&imx_cpuidle_driver); + + device = &per_cpu(imx_cpuidle_device, smp_processor_id()); + device->state_count = IMX_MAX_CPUIDLE_STATE; + + for (i = 0; i < IMX_MAX_CPUIDLE_STATE && i < CPUIDLE_STATE_MAX; i++) { + device->states[i].enter = imx_enter_idle; + device->states[i].exit_latency = imx_cpuidle_params[i].latency; + device->states[i].flags = CPUIDLE_FLAG_TIME_VALID; + } + + strcpy(device->states[WAIT_CLK_ON].name, "WFI 0"); + strcpy(device->states[WAIT_CLK_ON].desc, "Wait with clock on"); + strcpy(device->states[WAIT_CLK_OFF].name, "WFI 1"); + strcpy(device->states[WAIT_CLK_OFF].desc, "Wait with clock off"); + strcpy(device->states[WAIT_CLK_OFF_POWER_OFF].name, "WFI 2"); + strcpy(device->states[WAIT_CLK_OFF_POWER_OFF].desc, + "Wait with clock off and power gating"); + + if (cpuidle_register_device(device)) { + printk(KERN_ERR "imx_cpuidle_init: Failed registering\n"); + return -ENODEV; + } + return 0; +} + +late_initcall(imx_cpuidle_init); diff --git a/arch/arm/mach-mx5/cpuidle.h b/arch/arm/mach-mx5/cpuidle.h new file mode 100644 index 0000000..e5ba495 --- /dev/null +++ b/arch/arm/mach-mx5/cpuidle.h @@ -0,0 +1,26 @@ +/* + * arch/arm/mach-mx5/cpuidle.h + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +enum { + WAIT_CLK_ON, /* c1 */ + WAIT_CLK_OFF, /* c2 */ + WAIT_CLK_OFF_POWER_OFF, /* c3 */ + IMX_MAX_CPUIDLE_STATE, +}; + +struct imx_cpuidle_params { + unsigned int latency; +}; + +#ifdef CONFIG_CPU_IDLE +extern void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params); +#else +inline void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params) +{} +#endif + -- 1.7.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] ARM: IMX5: cpuidle driver 2011-02-11 9:36 ` [PATCH v2 1/2] " yong.shen at linaro.org @ 2011-02-15 18:27 ` Sascha Hauer 2011-02-16 8:02 ` Yong Shen 2011-02-17 8:18 ` Yong Shen 2011-02-16 8:11 ` [PATCH v2 " Sascha Hauer 1 sibling, 2 replies; 13+ messages in thread From: Sascha Hauer @ 2011-02-15 18:27 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 11, 2011 at 10:36:12AM +0100, yong.shen at linaro.org wrote: > From: Yong Shen <yong.shen@freescale.com> > > implement cpuidle driver for iMX5 SOCs, leave cpuidle params to board > related code. > > Signed-off-by: Yong Shen <yong.shen@freescale.com> > --- > arch/arm/mach-mx5/Makefile | 1 + > arch/arm/mach-mx5/cpuidle.c | 113 +++++++++++++++++++++++++++++++++++++++++++ > arch/arm/mach-mx5/cpuidle.h | 26 ++++++++++ > 3 files changed, 140 insertions(+), 0 deletions(-) > create mode 100644 arch/arm/mach-mx5/cpuidle.c > create mode 100644 arch/arm/mach-mx5/cpuidle.h > > diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile > index 0d43be9..12239e0 100644 > --- a/arch/arm/mach-mx5/Makefile > +++ b/arch/arm/mach-mx5/Makefile > @@ -7,6 +7,7 @@ obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o > obj-$(CONFIG_SOC_IMX50) += mm-mx50.o > > obj-$(CONFIG_CPU_FREQ_IMX) += cpu_op-mx51.o > +obj-$(CONFIG_CPU_IDLE) += cpuidle.o > obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o > obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o > obj-$(CONFIG_MACH_MX53_EVK) += board-mx53_evk.o > diff --git a/arch/arm/mach-mx5/cpuidle.c b/arch/arm/mach-mx5/cpuidle.c > new file mode 100644 > index 0000000..9d77c47 > --- /dev/null > +++ b/arch/arm/mach-mx5/cpuidle.c > @@ -0,0 +1,113 @@ > +/* > + * arch/arm/mach-mx5/cpuidle.c > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#include <linux/io.h> > +#include <linux/cpuidle.h> > +#include <asm/proc-fns.h> > +#include <mach/hardware.h> > +#include "cpuidle.h" > +#include "crm_regs.h" > + > +static struct imx_cpuidle_params *imx_cpuidle_params; > +void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params) > +{ > + imx_cpuidle_params = cpuidle_params; > +} > + > +extern int tzic_enable_wake(int is_idle); Please put this into a header file. > +static int imx_enter_idle(struct cpuidle_device *dev, > + struct cpuidle_state *state) > +{ > + struct timeval before, after; > + int idle_time; > + u32 plat_lpc, arm_srpgcr, ccm_clpcr; > + u32 empgc0, empgc1; > + > + local_irq_disable(); > + do_gettimeofday(&before); > + > + plat_lpc = __raw_readl(MXC_CORTEXA8_PLAT_LPC) & > + ~(MXC_CORTEXA8_PLAT_LPC_DSM); > + ccm_clpcr = __raw_readl(MXC_CCM_CLPCR) & ~(MXC_CCM_CLPCR_LPM_MASK); > + arm_srpgcr = __raw_readl(MXC_SRPG_ARM_SRPGCR) & ~(MXC_SRPGCR_PCR); > + empgc0 = __raw_readl(MXC_SRPG_EMPGC0_SRPGCR) & ~(MXC_SRPGCR_PCR); > + empgc1 = __raw_readl(MXC_SRPG_EMPGC1_SRPGCR) & ~(MXC_SRPGCR_PCR); > + > + if (state == &dev->states[WAIT_CLK_ON]) > + ; An if without code? This looks strange. > + else if (state == &dev->states[WAIT_CLK_OFF]) > + ccm_clpcr |= (0x1 << MXC_CCM_CLPCR_LPM_OFFSET); > + else if (state == &dev->states[WAIT_CLK_OFF_POWER_OFF]) { > + /* Wait unclocked, power off */ > + plat_lpc |= MXC_CORTEXA8_PLAT_LPC_DSM > + | MXC_CORTEXA8_PLAT_LPC_DBG_DSM; > + arm_srpgcr |= MXC_SRPGCR_PCR; > + ccm_clpcr |= (0x1 << MXC_CCM_CLPCR_LPM_OFFSET); > + ccm_clpcr &= ~MXC_CCM_CLPCR_VSTBY; > + ccm_clpcr &= ~MXC_CCM_CLPCR_SBYOS; > + if (tzic_enable_wake(1) != 0) { > + local_irq_enable(); > + return 0; > + } > + } > + > + __raw_writel(plat_lpc, MXC_CORTEXA8_PLAT_LPC); > + __raw_writel(ccm_clpcr, MXC_CCM_CLPCR); > + __raw_writel(arm_srpgcr, MXC_SRPG_ARM_SRPGCR); > + > + cpu_do_idle(); > + > + do_gettimeofday(&after); > + local_irq_enable(); > + idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC + > + (after.tv_usec - before.tv_usec); > + return idle_time; > +} > + > +static struct cpuidle_driver imx_cpuidle_driver = { > + .name = "imx_idle", > + .owner = THIS_MODULE, > +}; > + > +static DEFINE_PER_CPU(struct cpuidle_device, imx_cpuidle_device); > + > +static int __init imx_cpuidle_init(void) > +{ > + struct cpuidle_device *device; > + int i; > + > + if (imx_cpuidle_params == NULL) > + return -ENODEV; > + > + cpuidle_register_driver(&imx_cpuidle_driver); > + > + device = &per_cpu(imx_cpuidle_device, smp_processor_id()); > + device->state_count = IMX_MAX_CPUIDLE_STATE; > + > + for (i = 0; i < IMX_MAX_CPUIDLE_STATE && i < CPUIDLE_STATE_MAX; i++) { > + device->states[i].enter = imx_enter_idle; > + device->states[i].exit_latency = imx_cpuidle_params[i].latency; > + device->states[i].flags = CPUIDLE_FLAG_TIME_VALID; > + } > + > + strcpy(device->states[WAIT_CLK_ON].name, "WFI 0"); > + strcpy(device->states[WAIT_CLK_ON].desc, "Wait with clock on"); > + strcpy(device->states[WAIT_CLK_OFF].name, "WFI 1"); > + strcpy(device->states[WAIT_CLK_OFF].desc, "Wait with clock off"); > + strcpy(device->states[WAIT_CLK_OFF_POWER_OFF].name, "WFI 2"); > + strcpy(device->states[WAIT_CLK_OFF_POWER_OFF].desc, > + "Wait with clock off and power gating"); > + > + if (cpuidle_register_device(device)) { > + printk(KERN_ERR "imx_cpuidle_init: Failed registering\n"); > + return -ENODEV; > + } > + return 0; > +} > + > +late_initcall(imx_cpuidle_init); We have a late_initcall here which needs to be protected from other cpus. On the other hand we depend on board code calling imx_cpuidle_board_params() before this initcall. I think the board code should call a imx_cpuidle_init(struct imx_cpuidle_params *cpuidle_params) instead which makes the flow of execution more clear. Also, the function should be named mx51_cpuidle_init(). > diff --git a/arch/arm/mach-mx5/cpuidle.h b/arch/arm/mach-mx5/cpuidle.h > new file mode 100644 > index 0000000..e5ba495 > --- /dev/null > +++ b/arch/arm/mach-mx5/cpuidle.h > @@ -0,0 +1,26 @@ > +/* > + * arch/arm/mach-mx5/cpuidle.h > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +enum { > + WAIT_CLK_ON, /* c1 */ > + WAIT_CLK_OFF, /* c2 */ > + WAIT_CLK_OFF_POWER_OFF, /* c3 */ > + IMX_MAX_CPUIDLE_STATE, > +}; > + > +struct imx_cpuidle_params { > + unsigned int latency; > +}; > + > +#ifdef CONFIG_CPU_IDLE > +extern void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params); > +#else > +inline void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params) > +{} > +#endif > + > -- > 1.7.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- 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 | ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] ARM: IMX5: cpuidle driver 2011-02-15 18:27 ` Sascha Hauer @ 2011-02-16 8:02 ` Yong Shen 2011-02-17 8:18 ` Yong Shen 1 sibling, 0 replies; 13+ messages in thread From: Yong Shen @ 2011-02-16 8:02 UTC (permalink / raw) To: linux-arm-kernel Hi Sascha, On Tue, Feb 15, 2011 at 7:27 PM, Sascha Hauer <s.hauer@pengutronix.de>wrote: > On Fri, Feb 11, 2011 at 10:36:12AM +0100, yong.shen at linaro.org wrote: > > From: Yong Shen <yong.shen@freescale.com> > > > > implement cpuidle driver for iMX5 SOCs, leave cpuidle params to board > > related code. > > > > Signed-off-by: Yong Shen <yong.shen@freescale.com> > > --- > > arch/arm/mach-mx5/Makefile | 1 + > > arch/arm/mach-mx5/cpuidle.c | 113 > +++++++++++++++++++++++++++++++++++++++++++ > > arch/arm/mach-mx5/cpuidle.h | 26 ++++++++++ > > 3 files changed, 140 insertions(+), 0 deletions(-) > > create mode 100644 arch/arm/mach-mx5/cpuidle.c > > create mode 100644 arch/arm/mach-mx5/cpuidle.h > > > > diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile > > index 0d43be9..12239e0 100644 > > --- a/arch/arm/mach-mx5/Makefile > > +++ b/arch/arm/mach-mx5/Makefile > > @@ -7,6 +7,7 @@ obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o > > obj-$(CONFIG_SOC_IMX50) += mm-mx50.o > > > > obj-$(CONFIG_CPU_FREQ_IMX) += cpu_op-mx51.o > > +obj-$(CONFIG_CPU_IDLE) += cpuidle.o > > obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o > > obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o > > obj-$(CONFIG_MACH_MX53_EVK) += board-mx53_evk.o > > diff --git a/arch/arm/mach-mx5/cpuidle.c b/arch/arm/mach-mx5/cpuidle.c > > new file mode 100644 > > index 0000000..9d77c47 > > --- /dev/null > > +++ b/arch/arm/mach-mx5/cpuidle.c > > @@ -0,0 +1,113 @@ > > +/* > > + * arch/arm/mach-mx5/cpuidle.c > > + * > > + * This file is licensed under the terms of the GNU General Public > > + * License version 2. This program is licensed "as is" without any > > + * warranty of any kind, whether express or implied. > > + */ > > + > > +#include <linux/io.h> > > +#include <linux/cpuidle.h> > > +#include <asm/proc-fns.h> > > +#include <mach/hardware.h> > > +#include "cpuidle.h" > > +#include "crm_regs.h" > > + > > +static struct imx_cpuidle_params *imx_cpuidle_params; > > +void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params) > > +{ > > + imx_cpuidle_params = cpuidle_params; > > +} > > + > > +extern int tzic_enable_wake(int is_idle); > > Please put this into a header file. > Ok, but I guess I should do it in another patch before this patch set. > > > +static int imx_enter_idle(struct cpuidle_device *dev, > > + struct cpuidle_state *state) > > +{ > > + struct timeval before, after; > > + int idle_time; > > + u32 plat_lpc, arm_srpgcr, ccm_clpcr; > > + u32 empgc0, empgc1; > > + > > + local_irq_disable(); > > + do_gettimeofday(&before); > > + > > + plat_lpc = __raw_readl(MXC_CORTEXA8_PLAT_LPC) & > > + ~(MXC_CORTEXA8_PLAT_LPC_DSM); > > + ccm_clpcr = __raw_readl(MXC_CCM_CLPCR) & ~(MXC_CCM_CLPCR_LPM_MASK); > > + arm_srpgcr = __raw_readl(MXC_SRPG_ARM_SRPGCR) & ~(MXC_SRPGCR_PCR); > > + empgc0 = __raw_readl(MXC_SRPG_EMPGC0_SRPGCR) & ~(MXC_SRPGCR_PCR); > > + empgc1 = __raw_readl(MXC_SRPG_EMPGC1_SRPGCR) & ~(MXC_SRPGCR_PCR); > > + > > + if (state == &dev->states[WAIT_CLK_ON]) > > + ; > > An if without code? This looks strange. > Yes, a little bit. I meant to treat this like a explanation for different cases, and I also can remove it and put a comments here. > > > + else if (state == &dev->states[WAIT_CLK_OFF]) > > + ccm_clpcr |= (0x1 << MXC_CCM_CLPCR_LPM_OFFSET); > > + else if (state == &dev->states[WAIT_CLK_OFF_POWER_OFF]) { > > + /* Wait unclocked, power off */ > > + plat_lpc |= MXC_CORTEXA8_PLAT_LPC_DSM > > + | MXC_CORTEXA8_PLAT_LPC_DBG_DSM; > > + arm_srpgcr |= MXC_SRPGCR_PCR; > > + ccm_clpcr |= (0x1 << MXC_CCM_CLPCR_LPM_OFFSET); > > + ccm_clpcr &= ~MXC_CCM_CLPCR_VSTBY; > > + ccm_clpcr &= ~MXC_CCM_CLPCR_SBYOS; > > + if (tzic_enable_wake(1) != 0) { > > + local_irq_enable(); > > + return 0; > > + } > > + } > > + > > + __raw_writel(plat_lpc, MXC_CORTEXA8_PLAT_LPC); > > + __raw_writel(ccm_clpcr, MXC_CCM_CLPCR); > > + __raw_writel(arm_srpgcr, MXC_SRPG_ARM_SRPGCR); > > + > > + cpu_do_idle(); > > + > > + do_gettimeofday(&after); > > + local_irq_enable(); > > + idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC + > > + (after.tv_usec - before.tv_usec); > > + return idle_time; > > +} > > + > > +static struct cpuidle_driver imx_cpuidle_driver = { > > + .name = "imx_idle", > > + .owner = THIS_MODULE, > > +}; > > + > > +static DEFINE_PER_CPU(struct cpuidle_device, imx_cpuidle_device); > > + > > +static int __init imx_cpuidle_init(void) > > +{ > > + struct cpuidle_device *device; > > + int i; > > + > > + if (imx_cpuidle_params == NULL) > > + return -ENODEV; > > + > > + cpuidle_register_driver(&imx_cpuidle_driver); > > + > > + device = &per_cpu(imx_cpuidle_device, smp_processor_id()); > > + device->state_count = IMX_MAX_CPUIDLE_STATE; > > + > > + for (i = 0; i < IMX_MAX_CPUIDLE_STATE && i < CPUIDLE_STATE_MAX; > i++) { > > + device->states[i].enter = imx_enter_idle; > > + device->states[i].exit_latency = > imx_cpuidle_params[i].latency; > > + device->states[i].flags = CPUIDLE_FLAG_TIME_VALID; > > + } > > + > > + strcpy(device->states[WAIT_CLK_ON].name, "WFI 0"); > > + strcpy(device->states[WAIT_CLK_ON].desc, "Wait with clock on"); > > + strcpy(device->states[WAIT_CLK_OFF].name, "WFI 1"); > > + strcpy(device->states[WAIT_CLK_OFF].desc, "Wait with clock off"); > > + strcpy(device->states[WAIT_CLK_OFF_POWER_OFF].name, "WFI 2"); > > + strcpy(device->states[WAIT_CLK_OFF_POWER_OFF].desc, > > + "Wait with clock off and power gating"); > > + > > + if (cpuidle_register_device(device)) { > > + printk(KERN_ERR "imx_cpuidle_init: Failed registering\n"); > > + return -ENODEV; > > + } > > + return 0; > > +} > > + > > +late_initcall(imx_cpuidle_init); > > We have a late_initcall here which needs to be protected from other > cpus. On the other hand we depend on board code calling > imx_cpuidle_board_params() before this initcall. I think the board code > should call a imx_cpuidle_init(struct imx_cpuidle_params > *cpuidle_params) instead which makes the flow of execution more clear. > Also, the function should be named mx51_cpuidle_init(). > Then, I assume the best way might be that remove imx_cpuidle_board_params(), and let board code call mx51_cpuidle_init(), at the same time the params can be passed in by the same funciton. > > > diff --git a/arch/arm/mach-mx5/cpuidle.h b/arch/arm/mach-mx5/cpuidle.h > > new file mode 100644 > > index 0000000..e5ba495 > > --- /dev/null > > +++ b/arch/arm/mach-mx5/cpuidle.h > > @@ -0,0 +1,26 @@ > > +/* > > + * arch/arm/mach-mx5/cpuidle.h > > + * > > + * This file is licensed under the terms of the GNU General Public > > + * License version 2. This program is licensed "as is" without any > > + * warranty of any kind, whether express or implied. > > + */ > > + > > +enum { > > + WAIT_CLK_ON, /* c1 */ > > + WAIT_CLK_OFF, /* c2 */ > > + WAIT_CLK_OFF_POWER_OFF, /* c3 */ > > + IMX_MAX_CPUIDLE_STATE, > > +}; > > + > > +struct imx_cpuidle_params { > > + unsigned int latency; > > +}; > > + > > +#ifdef CONFIG_CPU_IDLE > > +extern void imx_cpuidle_board_params(struct imx_cpuidle_params > *cpuidle_params); > > +#else > > +inline void imx_cpuidle_board_params(struct imx_cpuidle_params > *cpuidle_params) > > +{} > > +#endif > > + > > -- > > 1.7.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel at lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > -- > 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 | > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110216/fd146cd2/attachment-0001.html> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] ARM: IMX5: cpuidle driver 2011-02-15 18:27 ` Sascha Hauer 2011-02-16 8:02 ` Yong Shen @ 2011-02-17 8:18 ` Yong Shen 2011-02-17 10:54 ` Sascha Hauer 1 sibling, 1 reply; 13+ messages in thread From: Yong Shen @ 2011-02-17 8:18 UTC (permalink / raw) To: linux-arm-kernel > > > > + return 0; > > +} > > + > > +late_initcall(imx_cpuidle_init); > > We have a late_initcall here which needs to be protected from other > cpus. On the other hand we depend on board code calling > imx_cpuidle_board_params() before this initcall. I think the board code > should call a imx_cpuidle_init(struct imx_cpuidle_params > *cpuidle_params) instead which makes the flow of execution more clear. > > imx_cpuidle_init can not be called directly in board code, since it is too early to register cpuidle driver and device which depend on some other system resource. Yong -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110217/407ee15f/attachment-0001.html> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] ARM: IMX5: cpuidle driver 2011-02-17 8:18 ` Yong Shen @ 2011-02-17 10:54 ` Sascha Hauer 2011-02-20 14:58 ` Yong Shen 0 siblings, 1 reply; 13+ messages in thread From: Sascha Hauer @ 2011-02-17 10:54 UTC (permalink / raw) To: linux-arm-kernel On Thu, Feb 17, 2011 at 09:18:11AM +0100, Yong Shen wrote: > > > > > > > + return 0; > > > +} > > > + > > > +late_initcall(imx_cpuidle_init); > > > > We have a late_initcall here which needs to be protected from other > > cpus. On the other hand we depend on board code calling > > imx_cpuidle_board_params() before this initcall. I think the board code > > should call a imx_cpuidle_init(struct imx_cpuidle_params > > *cpuidle_params) instead which makes the flow of execution more clear. > > > > imx_cpuidle_init can not be called directly in board code, since it is too > early to register cpuidle driver and device which depend on some other > system resource. I see. Maybe we should make this a platform driver then like for example davinci does. 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 | ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] ARM: IMX5: cpuidle driver 2011-02-17 10:54 ` Sascha Hauer @ 2011-02-20 14:58 ` Yong Shen 2011-02-23 15:41 ` [PATCH v3 " Yong Shen 0 siblings, 1 reply; 13+ messages in thread From: Yong Shen @ 2011-02-20 14:58 UTC (permalink / raw) To: linux-arm-kernel Hi Sascha, I had sent out v3 patch before the your last comments. I noticed how davinci is doing, but some SOCs like omap, they also do it in another way like my code. However, if you prefer the way davinci is doing, I will redo it. Please confirm. thanks Yong On Thu, Feb 17, 2011 at 11:54 AM, Sascha Hauer <s.hauer@pengutronix.de>wrote: > On Thu, Feb 17, 2011 at 09:18:11AM +0100, Yong Shen wrote: > > > > > > > > > > + return 0; > > > > +} > > > > + > > > > +late_initcall(imx_cpuidle_init); > > > > > > We have a late_initcall here which needs to be protected from other > > > cpus. On the other hand we depend on board code calling > > > imx_cpuidle_board_params() before this initcall. I think the board code > > > should call a imx_cpuidle_init(struct imx_cpuidle_params > > > *cpuidle_params) instead which makes the flow of execution more clear. > > > > > > imx_cpuidle_init can not be called directly in board code, since it is > too > > early to register cpuidle driver and device which depend on some other > > system resource. > > I see. Maybe we should make this a platform driver then like for example > davinci does. > > 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 | > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110220/59d8ac2c/attachment-0001.html> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 1/2] ARM: IMX5: cpuidle driver 2011-02-20 14:58 ` Yong Shen @ 2011-02-23 15:41 ` Yong Shen 0 siblings, 0 replies; 13+ messages in thread From: Yong Shen @ 2011-02-23 15:41 UTC (permalink / raw) To: linux-arm-kernel Hi Sascha, I noticed there were no comments for v3, but you had comments for v2 which was posted after v3. See below: >>I see. Maybe we should make this a platform driver then like for example >>davinci does. >I had sent out v3 patch before the your last comments. >I noticed how davinci is doing, but some SOCs like omap, they also do it in another way like my code. >However, if you prefer the way davinci is doing, I will redo it. Please confirm Shall we still follow davinci's code? Thanks Yong -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110223/510859b3/attachment-0001.html> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] ARM: IMX5: cpuidle driver 2011-02-11 9:36 ` [PATCH v2 1/2] " yong.shen at linaro.org 2011-02-15 18:27 ` Sascha Hauer @ 2011-02-16 8:11 ` Sascha Hauer 2011-02-16 8:37 ` Yong Shen 1 sibling, 1 reply; 13+ messages in thread From: Sascha Hauer @ 2011-02-16 8:11 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 11, 2011 at 10:36:12AM +0100, yong.shen at linaro.org wrote: > From: Yong Shen <yong.shen@freescale.com> > > implement cpuidle driver for iMX5 SOCs, leave cpuidle params to board > related code. > > Signed-off-by: Yong Shen <yong.shen@freescale.com> > --- > arch/arm/mach-mx5/Makefile | 1 + > arch/arm/mach-mx5/cpuidle.c | 113 +++++++++++++++++++++++++++++++++++++++++++ > arch/arm/mach-mx5/cpuidle.h | 26 ++++++++++ > 3 files changed, 140 insertions(+), 0 deletions(-) > create mode 100644 arch/arm/mach-mx5/cpuidle.c > create mode 100644 arch/arm/mach-mx5/cpuidle.h > > diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile > index 0d43be9..12239e0 100644 > --- a/arch/arm/mach-mx5/Makefile > +++ b/arch/arm/mach-mx5/Makefile > @@ -7,6 +7,7 @@ obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o > obj-$(CONFIG_SOC_IMX50) += mm-mx50.o > > obj-$(CONFIG_CPU_FREQ_IMX) += cpu_op-mx51.o > +obj-$(CONFIG_CPU_IDLE) += cpuidle.o > obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o > obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o > obj-$(CONFIG_MACH_MX53_EVK) += board-mx53_evk.o > diff --git a/arch/arm/mach-mx5/cpuidle.c b/arch/arm/mach-mx5/cpuidle.c > new file mode 100644 > index 0000000..9d77c47 > --- /dev/null > +++ b/arch/arm/mach-mx5/cpuidle.c > @@ -0,0 +1,113 @@ > +/* > + * arch/arm/mach-mx5/cpuidle.c > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#include <linux/io.h> > +#include <linux/cpuidle.h> > +#include <asm/proc-fns.h> > +#include <mach/hardware.h> > +#include "cpuidle.h" > +#include "crm_regs.h" > + > +static struct imx_cpuidle_params *imx_cpuidle_params; > +void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params) > +{ > + imx_cpuidle_params = cpuidle_params; > +} > + > +extern int tzic_enable_wake(int is_idle); > +static int imx_enter_idle(struct cpuidle_device *dev, > + struct cpuidle_state *state) > +{ > + struct timeval before, after; > + int idle_time; > + u32 plat_lpc, arm_srpgcr, ccm_clpcr; > + u32 empgc0, empgc1; > + > + local_irq_disable(); > + do_gettimeofday(&before); > + > + plat_lpc = __raw_readl(MXC_CORTEXA8_PLAT_LPC) & > + ~(MXC_CORTEXA8_PLAT_LPC_DSM); One thing that strikes me here is the fact that this code can probably run on i.MX53 aswell, right? It's only that these registers have different addresses on i.MX53. The MXC_ prefix is therefore not a good idea. Switching this to MX51_ and having an additional MX53_ register leads to code duplication. This shows that it's a bad idea to code fixed addresses in the code. We should go for base + offset instead so that this code will have a better start on i.MX53. This of course needs changes in the current crm_regs.h and probably in the i.MX51/53 clock code. > + ccm_clpcr = __raw_readl(MXC_CCM_CLPCR) & ~(MXC_CCM_CLPCR_LPM_MASK); > + arm_srpgcr = __raw_readl(MXC_SRPG_ARM_SRPGCR) & ~(MXC_SRPGCR_PCR); > + empgc0 = __raw_readl(MXC_SRPG_EMPGC0_SRPGCR) & ~(MXC_SRPGCR_PCR); > + empgc1 = __raw_readl(MXC_SRPG_EMPGC1_SRPGCR) & ~(MXC_SRPGCR_PCR); > + > + if (state == &dev->states[WAIT_CLK_ON]) > + ; > + else if (state == &dev->states[WAIT_CLK_OFF]) > + ccm_clpcr |= (0x1 << MXC_CCM_CLPCR_LPM_OFFSET); > + else if (state == &dev->states[WAIT_CLK_OFF_POWER_OFF]) { > + /* Wait unclocked, power off */ > + plat_lpc |= MXC_CORTEXA8_PLAT_LPC_DSM > + | MXC_CORTEXA8_PLAT_LPC_DBG_DSM; > + arm_srpgcr |= MXC_SRPGCR_PCR; > + ccm_clpcr |= (0x1 << MXC_CCM_CLPCR_LPM_OFFSET); > + ccm_clpcr &= ~MXC_CCM_CLPCR_VSTBY; > + ccm_clpcr &= ~MXC_CCM_CLPCR_SBYOS; > + if (tzic_enable_wake(1) != 0) { > + local_irq_enable(); > + return 0; > + } > + } > + > + __raw_writel(plat_lpc, MXC_CORTEXA8_PLAT_LPC); > + __raw_writel(ccm_clpcr, MXC_CCM_CLPCR); > + __raw_writel(arm_srpgcr, MXC_SRPG_ARM_SRPGCR); > + > + cpu_do_idle(); > + > + do_gettimeofday(&after); > + local_irq_enable(); > + idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC + > + (after.tv_usec - before.tv_usec); > + return idle_time; > +} > + > +static struct cpuidle_driver imx_cpuidle_driver = { > + .name = "imx_idle", > + .owner = THIS_MODULE, > +}; > + > +static DEFINE_PER_CPU(struct cpuidle_device, imx_cpuidle_device); > + > +static int __init imx_cpuidle_init(void) > +{ > + struct cpuidle_device *device; > + int i; > + > + if (imx_cpuidle_params == NULL) > + return -ENODEV; > + > + cpuidle_register_driver(&imx_cpuidle_driver); > + > + device = &per_cpu(imx_cpuidle_device, smp_processor_id()); > + device->state_count = IMX_MAX_CPUIDLE_STATE; > + > + for (i = 0; i < IMX_MAX_CPUIDLE_STATE && i < CPUIDLE_STATE_MAX; i++) { > + device->states[i].enter = imx_enter_idle; > + device->states[i].exit_latency = imx_cpuidle_params[i].latency; > + device->states[i].flags = CPUIDLE_FLAG_TIME_VALID; > + } > + > + strcpy(device->states[WAIT_CLK_ON].name, "WFI 0"); > + strcpy(device->states[WAIT_CLK_ON].desc, "Wait with clock on"); > + strcpy(device->states[WAIT_CLK_OFF].name, "WFI 1"); > + strcpy(device->states[WAIT_CLK_OFF].desc, "Wait with clock off"); > + strcpy(device->states[WAIT_CLK_OFF_POWER_OFF].name, "WFI 2"); > + strcpy(device->states[WAIT_CLK_OFF_POWER_OFF].desc, > + "Wait with clock off and power gating"); > + > + if (cpuidle_register_device(device)) { > + printk(KERN_ERR "imx_cpuidle_init: Failed registering\n"); > + return -ENODEV; > + } > + return 0; > +} > + > +late_initcall(imx_cpuidle_init); > diff --git a/arch/arm/mach-mx5/cpuidle.h b/arch/arm/mach-mx5/cpuidle.h > new file mode 100644 > index 0000000..e5ba495 > --- /dev/null > +++ b/arch/arm/mach-mx5/cpuidle.h > @@ -0,0 +1,26 @@ > +/* > + * arch/arm/mach-mx5/cpuidle.h > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +enum { > + WAIT_CLK_ON, /* c1 */ > + WAIT_CLK_OFF, /* c2 */ > + WAIT_CLK_OFF_POWER_OFF, /* c3 */ > + IMX_MAX_CPUIDLE_STATE, > +}; > + > +struct imx_cpuidle_params { > + unsigned int latency; > +}; > + > +#ifdef CONFIG_CPU_IDLE > +extern void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params); > +#else > +inline void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params) > +{} > +#endif > + > -- > 1.7.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- 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 | ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] ARM: IMX5: cpuidle driver 2011-02-16 8:11 ` [PATCH v2 " Sascha Hauer @ 2011-02-16 8:37 ` Yong Shen 2011-02-16 9:13 ` Sascha Hauer 0 siblings, 1 reply; 13+ messages in thread From: Yong Shen @ 2011-02-16 8:37 UTC (permalink / raw) To: linux-arm-kernel Hi Sascha, > > + local_irq_disable(); > > + do_gettimeofday(&before); > > + > > + plat_lpc = __raw_readl(MXC_CORTEXA8_PLAT_LPC) & > > + ~(MXC_CORTEXA8_PLAT_LPC_DSM); > > One thing that strikes me here is the fact that this code can probably > run on i.MX53 aswell, right? It's only that these registers have > different addresses on i.MX53. The MXC_ prefix is therefore not a good > idea. Switching this to MX51_ and having an additional MX53_ register > leads to code duplication. This shows that it's a bad idea to code > fixed addresses in the code. We should go for base + offset instead > so that this code will have a better start on i.MX53. This of course > needs changes in the current crm_regs.h and probably in the i.MX51/53 > clock code. > Yes, for mx53, it is similar. But for the case you are talking about, is it easier that we keep MXC_ prefix in this file and define MXC_ to MX51 or MX53 in crm_regs.h according to which board is running? In addition, registers for this code are not in one section, which means many BASEx + offset there, if I understand right. Do you have a sample for 'base + offset' case? since mx53 just came in, I am not sure about such case. yong > > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110216/a8359ff2/attachment-0001.html> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] ARM: IMX5: cpuidle driver 2011-02-16 8:37 ` Yong Shen @ 2011-02-16 9:13 ` Sascha Hauer 2011-02-16 9:25 ` Yong Shen 0 siblings, 1 reply; 13+ messages in thread From: Sascha Hauer @ 2011-02-16 9:13 UTC (permalink / raw) To: linux-arm-kernel On Wed, Feb 16, 2011 at 09:37:47AM +0100, Yong Shen wrote: > Hi Sascha, > > > > > + local_irq_disable(); > > > + do_gettimeofday(&before); > > > + > > > + plat_lpc = __raw_readl(MXC_CORTEXA8_PLAT_LPC) & > > > + ~(MXC_CORTEXA8_PLAT_LPC_DSM); > > > > One thing that strikes me here is the fact that this code can probably > > run on i.MX53 aswell, right? It's only that these registers have > > different addresses on i.MX53. The MXC_ prefix is therefore not a good > > idea. Switching this to MX51_ and having an additional MX53_ register > > leads to code duplication. This shows that it's a bad idea to code > > fixed addresses in the code. We should go for base + offset instead > > so that this code will have a better start on i.MX53. This of course > > needs changes in the current crm_regs.h and probably in the i.MX51/53 > > clock code. > > > Yes, for mx53, it is similar. > But for the case you are talking about, is it easier that we keep MXC_ > prefix in this file and define MXC_ to MX51 or MX53 in crm_regs.h according > to which board is running? I don't understand. How can we 'define' (which is compile time) to something depending on the board (which is runtime)? > In addition, registers for this code are not in one section, which means > many BASEx + offset there, if I understand right. Do you have a sample for > 'base + offset' case? since mx53 just came in, I am not sure about such > case. Forget it. I just realized that more or less by accident the virtual addresses for the i.MX51 and i.MX53 are the same. 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 | ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] ARM: IMX5: cpuidle driver 2011-02-16 9:13 ` Sascha Hauer @ 2011-02-16 9:25 ` Yong Shen 0 siblings, 0 replies; 13+ messages in thread From: Yong Shen @ 2011-02-16 9:25 UTC (permalink / raw) To: linux-arm-kernel On Wed, Feb 16, 2011 at 10:13 AM, Sascha Hauer <s.hauer@pengutronix.de>wrote: > On Wed, Feb 16, 2011 at 09:37:47AM +0100, Yong Shen wrote: > > Hi Sascha, > > > > > > > > + local_irq_disable(); > > > > + do_gettimeofday(&before); > > > > + > > > > + plat_lpc = __raw_readl(MXC_CORTEXA8_PLAT_LPC) & > > > > + ~(MXC_CORTEXA8_PLAT_LPC_DSM); > > > > > > One thing that strikes me here is the fact that this code can probably > > > run on i.MX53 aswell, right? It's only that these registers have > > > different addresses on i.MX53. The MXC_ prefix is therefore not a good > > > idea. Switching this to MX51_ and having an additional MX53_ register > > > leads to code duplication. This shows that it's a bad idea to code > > > fixed addresses in the code. We should go for base + offset instead > > > so that this code will have a better start on i.MX53. This of course > > > needs changes in the current crm_regs.h and probably in the i.MX51/53 > > > clock code. > > > > > Yes, for mx53, it is similar. > > But for the case you are talking about, is it easier that we keep MXC_ > > prefix in this file and define MXC_ to MX51 or MX53 in crm_regs.h > according > > to which board is running? > > I don't understand. How can we 'define' (which is compile time) to > something depending on the board (which is runtime)? I ignored the goal is one image running on multiple SOCs. > > > In addition, registers for this code are not in one section, which means > > many BASEx + offset there, if I understand right. Do you have a sample > for > > 'base + offset' case? since mx53 just came in, I am not sure about such > > case. > > Forget it. I just realized that more or less by accident the virtual > addresses for the i.MX51 and i.MX53 are the same. > So, the conclusion is: still using MXC_ prefix in this period. right? Correct me. Yong > > 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 | > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110216/1d858dad/attachment-0001.html> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/2] ARM: IMX5 bbg: add cpuidle parameters 2011-02-11 9:36 [PATCH v2 0/2] ARM: IMX5: cpuidle driver yong.shen at linaro.org 2011-02-11 9:36 ` [PATCH v2 1/2] " yong.shen at linaro.org @ 2011-02-11 9:36 ` yong.shen at linaro.org 1 sibling, 0 replies; 13+ messages in thread From: yong.shen at linaro.org @ 2011-02-11 9:36 UTC (permalink / raw) To: linux-arm-kernel From: Yong Shen <yong.shen@freescale.com> This parameters are workable, but need further tuning. Signed-off-by: Yong Shen <yong.shen@freescale.com> --- arch/arm/mach-mx5/board-mx51_babbage.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-mx5/board-mx51_babbage.c b/arch/arm/mach-mx5/board-mx51_babbage.c index d9d402e..168b09c 100644 --- a/arch/arm/mach-mx5/board-mx51_babbage.c +++ b/arch/arm/mach-mx5/board-mx51_babbage.c @@ -37,6 +37,7 @@ #include "devices-imx51.h" #include "devices.h" #include "cpu_op-mx51.h" +#include "cpuidle.h" #define BABBAGE_USB_HUB_RESET IMX_GPIO_NR(1, 7) #define BABBAGE_USBH1_STP IMX_GPIO_NR(1, 27) @@ -333,6 +334,11 @@ static const struct spi_imx_master mx51_babbage_spi_pdata __initconst = { .num_chipselect = ARRAY_SIZE(mx51_babbage_spi_cs), }; +static struct imx_cpuidle_params babage_cpuidle_params[] = { + {100,}, + {150,}, + {1000,}, +}; /* * Board specific initialization. */ @@ -383,6 +389,8 @@ static void __init mxc_board_init(void) ARRAY_SIZE(mx51_babbage_spi_board_info)); imx51_add_ecspi(0, &mx51_babbage_spi_pdata); imx51_add_imx2_wdt(0, NULL); + + imx_cpuidle_board_params(babage_cpuidle_params); } static void __init mx51_babbage_timer_init(void) -- 1.7.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-02-23 15:41 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-11 9:36 [PATCH v2 0/2] ARM: IMX5: cpuidle driver yong.shen at linaro.org 2011-02-11 9:36 ` [PATCH v2 1/2] " yong.shen at linaro.org 2011-02-15 18:27 ` Sascha Hauer 2011-02-16 8:02 ` Yong Shen 2011-02-17 8:18 ` Yong Shen 2011-02-17 10:54 ` Sascha Hauer 2011-02-20 14:58 ` Yong Shen 2011-02-23 15:41 ` [PATCH v3 " Yong Shen 2011-02-16 8:11 ` [PATCH v2 " Sascha Hauer 2011-02-16 8:37 ` Yong Shen 2011-02-16 9:13 ` Sascha Hauer 2011-02-16 9:25 ` Yong Shen 2011-02-11 9:36 ` [PATCH v2 2/2] ARM: IMX5 bbg: add cpuidle parameters yong.shen at linaro.org
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).