[PATCH 0/3] Add imx cpuidle

[PATCH 0/3] Add imx cpuidle

[Robert Lee]

Add common imx cpuidle initialization functionality and add a i.MX5 and i.MX6
platform cpuidle implementation.

Robert Lee (3):
  ARM: imx: Add common imx cpuidle init functionality.
  ARM: imx: Add imx5 cpuidle driver
  ARM: imx: Add imx6q cpuidle driver

 arch/arm/mach-imx/Makefile               |    4 +-
 arch/arm/mach-imx/cpuidle-imx5.c         |   55 ++++++++++++++++++
 arch/arm/mach-imx/cpuidle-imx6q.c        |   33 +++++++++++
 arch/arm/mach-imx/mach-imx6q.c           |    2 +
 arch/arm/mach-imx/mm-imx5.c              |   16 ++++--
 arch/arm/plat-mxc/Makefile               |    2 +
 arch/arm/plat-mxc/cpuidle.c              |   89 ++++++++++++++++++++++++++++++
 arch/arm/plat-mxc/include/mach/common.h  |    1 +
 arch/arm/plat-mxc/include/mach/cpuidle.h |   26 +++++++++
 9 files changed, 222 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/mach-imx/cpuidle-imx5.c
 create mode 100644 arch/arm/mach-imx/cpuidle-imx6q.c
 create mode 100644 arch/arm/plat-mxc/cpuidle.c
 create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h

-- 
1.7.10

[PATCH 0/3] Add imx cpuidle

[Robert Lee]

Add common imx cpuidle initialization functionality and add a i.MX5 and i.MX6
platform cpuidle implementation.

Robert Lee (3):
  ARM: imx: Add common imx cpuidle init functionality.
  ARM: imx: Add imx5 cpuidle driver
  ARM: imx: Add imx6q cpuidle driver

 arch/arm/mach-imx/Makefile               |    4 +-
 arch/arm/mach-imx/cpuidle-imx5.c         |   55 ++++++++++++++++++
 arch/arm/mach-imx/cpuidle-imx6q.c        |   33 +++++++++++
 arch/arm/mach-imx/mach-imx6q.c           |    2 +
 arch/arm/mach-imx/mm-imx5.c              |   16 ++++--
 arch/arm/plat-mxc/Makefile               |    2 +
 arch/arm/plat-mxc/cpuidle.c              |   89 ++++++++++++++++++++++++++++++
 arch/arm/plat-mxc/include/mach/common.h  |    1 +
 arch/arm/plat-mxc/include/mach/cpuidle.h |   26 +++++++++
 9 files changed, 222 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/mach-imx/cpuidle-imx5.c
 create mode 100644 arch/arm/mach-imx/cpuidle-imx6q.c
 create mode 100644 arch/arm/plat-mxc/cpuidle.c
 create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h

-- 
1.7.10

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Robert Lee]

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               |    2 +
 arch/arm/plat-mxc/cpuidle.c              |   89 ++++++++++++++++++++++++++++++
 arch/arm/plat-mxc/include/mach/cpuidle.h |   26 +++++++++
 3 files changed, 117 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..7c9e05f 100644
--- a/arch/arm/plat-mxc/Makefile
+++ b/arch/arm/plat-mxc/Makefile
@@ -16,6 +16,8 @@ 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..d1c9301
--- /dev/null
+++ b/arch/arm/plat-mxc/cpuidle.c
@@ -0,0 +1,89 @@
+/*
+ * 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;
+static struct cpuidle_driver *drv;
+
+void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
+{
+	drv = p;
+}
+
+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);
+}
+
+static int __init imx_cpuidle_init(void)
+{
+	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\n", __func__);
+		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;
+}
+late_initcall(imx_cpuidle_init);
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..6509f19
--- /dev/null
+++ b/arch/arm/plat-mxc/include/mach/cpuidle.h
@@ -0,0 +1,26 @@
+/*
+ * 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>
+
+extern int imx5_cpuidle_init(void);
+extern int imx6q_cpuidle_init(void);
+
+#ifdef CONFIG_CPU_IDLE
+extern void imx_cpuidle_devices_uninit(void);
+extern void imx_cpuidle_set_driver(struct cpuidle_driver *p);
+extern int imx5_cpuidle_init(void);
+extern int imx6q_cpuidle_init(void);
+#else
+static inline void imx_cpuidle_devices_uninit(void) {}
+static inline void imx_cpuidle_set_driver(struct cpuidle_driver *p) {}
+#endif
-- 
1.7.10

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Robert Lee]

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               |    2 +
 arch/arm/plat-mxc/cpuidle.c              |   89 ++++++++++++++++++++++++++++++
 arch/arm/plat-mxc/include/mach/cpuidle.h |   26 +++++++++
 3 files changed, 117 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..7c9e05f 100644
--- a/arch/arm/plat-mxc/Makefile
+++ b/arch/arm/plat-mxc/Makefile
@@ -16,6 +16,8 @@ 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..d1c9301
--- /dev/null
+++ b/arch/arm/plat-mxc/cpuidle.c
@@ -0,0 +1,89 @@
+/*
+ * 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;
+static struct cpuidle_driver *drv;
+
+void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
+{
+	drv = p;
+}
+
+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);
+}
+
+static int __init imx_cpuidle_init(void)
+{
+	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\n", __func__);
+		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;
+}
+late_initcall(imx_cpuidle_init);
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..6509f19
--- /dev/null
+++ b/arch/arm/plat-mxc/include/mach/cpuidle.h
@@ -0,0 +1,26 @@
+/*
+ * 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>
+
+extern int imx5_cpuidle_init(void);
+extern int imx6q_cpuidle_init(void);
+
+#ifdef CONFIG_CPU_IDLE
+extern void imx_cpuidle_devices_uninit(void);
+extern void imx_cpuidle_set_driver(struct cpuidle_driver *p);
+extern int imx5_cpuidle_init(void);
+extern int imx6q_cpuidle_init(void);
+#else
+static inline void imx_cpuidle_devices_uninit(void) {}
+static inline void imx_cpuidle_set_driver(struct cpuidle_driver *p) {}
+#endif
-- 
1.7.10

[PATCH 2/3] ARM: imx: Add imx5 cpuidle driver

[Robert Lee]

Add imx5 cpuidle driver.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-imx/Makefile              |    2 +-
 arch/arm/mach-imx/cpuidle-imx5.c        |   55 +++++++++++++++++++++++++++++++
 arch/arm/mach-imx/mm-imx5.c             |   16 ++++++---
 arch/arm/plat-mxc/include/mach/common.h |    1 +
 4 files changed, 69 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/mach-imx/cpuidle-imx5.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index ab939c5..84b8976 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_SOC_IMX27) += clock-imx27.o mm-imx27.o ehci-imx27.o
 obj-$(CONFIG_SOC_IMX31) += mm-imx3.o cpu-imx31.o clock-imx31.o iomux-imx31.o ehci-imx31.o pm-imx3.o
 obj-$(CONFIG_SOC_IMX35) += mm-imx3.o cpu-imx35.o clock-imx35.o ehci-imx35.o pm-imx3.o
 
-obj-$(CONFIG_SOC_IMX5) += cpu-imx5.o mm-imx5.o clock-mx51-mx53.o ehci-imx5.o pm-imx5.o cpu_op-mx51.o
+obj-$(CONFIG_SOC_IMX5) += cpu-imx5.o mm-imx5.o clock-mx51-mx53.o ehci-imx5.o pm-imx5.o cpu_op-mx51.o cpuidle-imx5.o
 
 # Support for CMOS sensor interface
 obj-$(CONFIG_MX1_VIDEO) += mx1-camera-fiq.o mx1-camera-fiq-ksym.o
diff --git a/arch/arm/mach-imx/cpuidle-imx5.c b/arch/arm/mach-imx/cpuidle-imx5.c
new file mode 100644
index 0000000..8f428fc
--- /dev/null
+++ b/arch/arm/mach-imx/cpuidle-imx5.c
@@ -0,0 +1,55 @@
+/*
+ * 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/init.h>
+#include <linux/export.h>
+#include <linux/cpuidle.h>
+#include <linux/err.h>
+#include <mach/common.h>
+#include <mach/cpuidle.h>
+#include <mach/hardware.h>
+
+static int imx5_cpuidle_enter(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int idx)
+{
+	int ret;
+
+	ret = imx5_idle();
+
+	if (ret < 0)
+		return ret;
+
+	return idx;
+}
+
+static struct cpuidle_driver imx5_cpuidle_driver = {
+	.name			= "imx5_cpuidle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
+	.states[0]	= {
+		.enter			= imx5_cpuidle_enter,
+		.exit_latency		= 20, /* max latency at 160MHz */
+		.target_residency	= 1,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "IMX5 SRPG",
+		.desc			= "CPU state retained,powered off",
+	},
+	.state_count		= 1,
+};
+
+int __init imx5_cpuidle_init(void)
+{
+	imx_cpuidle_set_driver(&imx5_cpuidle_driver);
+
+	return 0;
+}
diff --git a/arch/arm/mach-imx/mm-imx5.c b/arch/arm/mach-imx/mm-imx5.c
index e10f391..594915b 100644
--- a/arch/arm/mach-imx/mm-imx5.c
+++ b/arch/arm/mach-imx/mm-imx5.c
@@ -22,22 +22,29 @@
 #include <mach/common.h>
 #include <mach/devices-common.h>
 #include <mach/iomux-v3.h>
+#include <mach/cpuidle.h>
 
 static struct clk *gpc_dvfs_clk;
 
-static void imx5_idle(void)
+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");
 		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);
+
+	return ret;
 }
 
 /*
@@ -103,7 +110,7 @@ void __init imx51_init_early(void)
 	mxc_set_cpu_type(MXC_CPU_MX51);
 	mxc_iomux_v3_init(MX51_IO_ADDRESS(MX51_IOMUXC_BASE_ADDR));
 	mxc_arch_reset_init(MX51_IO_ADDRESS(MX51_WDOG1_BASE_ADDR));
-	arm_pm_idle = imx5_idle;
+	arm_pm_idle = (void *)imx5_idle;
 }
 
 void __init imx53_init_early(void)
@@ -203,7 +210,8 @@ void __init imx51_soc_init(void)
 	/* i.mx51 has the i.mx35 type sdma */
 	imx_add_imx_sdma("imx35-sdma", MX51_SDMA_BASE_ADDR, MX51_INT_SDMA, &imx51_sdma_pdata);
 
-	/* Setup AIPS registers */
+	imx5_cpuidle_init();
+
 	imx_set_aips(MX51_IO_ADDRESS(MX51_AIPS1_BASE_ADDR));
 	imx_set_aips(MX51_IO_ADDRESS(MX51_AIPS2_BASE_ADDR));
 
diff --git a/arch/arm/plat-mxc/include/mach/common.h b/arch/arm/plat-mxc/include/mach/common.h
index 0319c4a..128a572 100644
--- a/arch/arm/plat-mxc/include/mach/common.h
+++ b/arch/arm/plat-mxc/include/mach/common.h
@@ -95,6 +95,7 @@ enum mx3_cpu_pwr_mode {
 
 extern void mx3_cpu_lp_set(enum mx3_cpu_pwr_mode mode);
 extern void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode);
+extern int imx5_idle(void);
 extern void imx_print_silicon_rev(const char *cpu, int srev);
 
 void avic_handle_irq(struct pt_regs *);
-- 
1.7.10

[PATCH 2/3] ARM: imx: Add imx5 cpuidle driver

[Robert Lee]

Add imx5 cpuidle driver.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-imx/Makefile              |    2 +-
 arch/arm/mach-imx/cpuidle-imx5.c        |   55 +++++++++++++++++++++++++++++++
 arch/arm/mach-imx/mm-imx5.c             |   16 ++++++---
 arch/arm/plat-mxc/include/mach/common.h |    1 +
 4 files changed, 69 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/mach-imx/cpuidle-imx5.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index ab939c5..84b8976 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_SOC_IMX27) += clock-imx27.o mm-imx27.o ehci-imx27.o
 obj-$(CONFIG_SOC_IMX31) += mm-imx3.o cpu-imx31.o clock-imx31.o iomux-imx31.o ehci-imx31.o pm-imx3.o
 obj-$(CONFIG_SOC_IMX35) += mm-imx3.o cpu-imx35.o clock-imx35.o ehci-imx35.o pm-imx3.o
 
-obj-$(CONFIG_SOC_IMX5) += cpu-imx5.o mm-imx5.o clock-mx51-mx53.o ehci-imx5.o pm-imx5.o cpu_op-mx51.o
+obj-$(CONFIG_SOC_IMX5) += cpu-imx5.o mm-imx5.o clock-mx51-mx53.o ehci-imx5.o pm-imx5.o cpu_op-mx51.o cpuidle-imx5.o
 
 # Support for CMOS sensor interface
 obj-$(CONFIG_MX1_VIDEO) += mx1-camera-fiq.o mx1-camera-fiq-ksym.o
diff --git a/arch/arm/mach-imx/cpuidle-imx5.c b/arch/arm/mach-imx/cpuidle-imx5.c
new file mode 100644
index 0000000..8f428fc
--- /dev/null
+++ b/arch/arm/mach-imx/cpuidle-imx5.c
@@ -0,0 +1,55 @@
+/*
+ * 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/init.h>
+#include <linux/export.h>
+#include <linux/cpuidle.h>
+#include <linux/err.h>
+#include <mach/common.h>
+#include <mach/cpuidle.h>
+#include <mach/hardware.h>
+
+static int imx5_cpuidle_enter(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int idx)
+{
+	int ret;
+
+	ret = imx5_idle();
+
+	if (ret < 0)
+		return ret;
+
+	return idx;
+}
+
+static struct cpuidle_driver imx5_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,
+};
+
+int __init imx5_cpuidle_init(void)
+{
+	imx_cpuidle_set_driver(&imx5_cpuidle_driver);
+
+	return 0;
+}
diff --git a/arch/arm/mach-imx/mm-imx5.c b/arch/arm/mach-imx/mm-imx5.c
index e10f391..594915b 100644
--- a/arch/arm/mach-imx/mm-imx5.c
+++ b/arch/arm/mach-imx/mm-imx5.c
@@ -22,22 +22,29 @@
 #include <mach/common.h>
 #include <mach/devices-common.h>
 #include <mach/iomux-v3.h>
+#include <mach/cpuidle.h>
 
 static struct clk *gpc_dvfs_clk;
 
-static void imx5_idle(void)
+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");
 		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);
+
+	return ret;
 }
 
 /*
@@ -103,7 +110,7 @@ void __init imx51_init_early(void)
 	mxc_set_cpu_type(MXC_CPU_MX51);
 	mxc_iomux_v3_init(MX51_IO_ADDRESS(MX51_IOMUXC_BASE_ADDR));
 	mxc_arch_reset_init(MX51_IO_ADDRESS(MX51_WDOG1_BASE_ADDR));
-	arm_pm_idle = imx5_idle;
+	arm_pm_idle = (void *)imx5_idle;
 }
 
 void __init imx53_init_early(void)
@@ -203,7 +210,8 @@ void __init imx51_soc_init(void)
 	/* i.mx51 has the i.mx35 type sdma */
 	imx_add_imx_sdma("imx35-sdma", MX51_SDMA_BASE_ADDR, MX51_INT_SDMA, &imx51_sdma_pdata);
 
-	/* Setup AIPS registers */
+	imx5_cpuidle_init();
+
 	imx_set_aips(MX51_IO_ADDRESS(MX51_AIPS1_BASE_ADDR));
 	imx_set_aips(MX51_IO_ADDRESS(MX51_AIPS2_BASE_ADDR));
 
diff --git a/arch/arm/plat-mxc/include/mach/common.h b/arch/arm/plat-mxc/include/mach/common.h
index 0319c4a..128a572 100644
--- a/arch/arm/plat-mxc/include/mach/common.h
+++ b/arch/arm/plat-mxc/include/mach/common.h
@@ -95,6 +95,7 @@ enum mx3_cpu_pwr_mode {
 
 extern void mx3_cpu_lp_set(enum mx3_cpu_pwr_mode mode);
 extern void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode);
+extern int imx5_idle(void);
 extern void imx_print_silicon_rev(const char *cpu, int srev);
 
 void avic_handle_irq(struct pt_regs *);
-- 
1.7.10

[PATCH 3/3] ARM: imx: Add imx6q cpuidle driver

[Robert Lee]

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 <rob.lee@linaro.org>
---
 arch/arm/mach-imx/Makefile        |    2 +-
 arch/arm/mach-imx/cpuidle-imx6q.c |   33 +++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/mach-imx6q.c    |    2 ++
 3 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-imx/cpuidle-imx6q.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 84b8976..530f0fd 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -70,7 +70,7 @@ obj-$(CONFIG_CPU_V7) += head-v7.o
 AFLAGS_head-v7.o :=-Wa,-march=armv7-a
 obj-$(CONFIG_SMP) += platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
-obj-$(CONFIG_SOC_IMX6Q) += clock-imx6q.o mach-imx6q.o
+obj-$(CONFIG_SOC_IMX6Q) += clock-imx6q.o mach-imx6q.o cpuidle-imx6q.o
 
 ifeq ($(CONFIG_PM),y)
 obj-$(CONFIG_SOC_IMX6Q) += pm-imx6q.o
diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c
new file mode 100644
index 0000000..b74557f
--- /dev/null
+++ b/arch/arm/mach-imx/cpuidle-imx6q.c
@@ -0,0 +1,33 @@
+/*
+ * 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/init.h>
+#include <linux/cpuidle.h>
+#include <linux/export.h>
+#include <asm/cpuidle.h>
+#include <mach/cpuidle.h>
+
+static struct cpuidle_driver imx6q_cpuidle_driver = {
+	.name			= "imx6q_cpuidle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
+	.states[0]		= ARM_CPUIDLE_WFI_STATE,
+	.state_count		= 1,
+};
+
+int __init imx6q_cpuidle_init(void)
+{
+	imx_cpuidle_set_driver(&imx6q_cpuidle_driver);
+
+	return 0;
+}
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index da6c1d9..6f31ca6 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -29,6 +29,7 @@
 #include <asm/system_misc.h>
 #include <mach/common.h>
 #include <mach/hardware.h>
+#include <mach/cpuidle.h>
 
 void imx6q_restart(char mode, const char *cmd)
 {
@@ -84,6 +85,7 @@ static void __init imx6q_init_machine(void)
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
 	imx6q_pm_init();
+	imx6q_cpuidle_init();
 }
 
 static void __init imx6q_map_io(void)
-- 
1.7.10

[PATCH 3/3] ARM: imx: Add imx6q cpuidle driver

[Robert Lee]

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 <rob.lee@linaro.org>
---
 arch/arm/mach-imx/Makefile        |    2 +-
 arch/arm/mach-imx/cpuidle-imx6q.c |   33 +++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/mach-imx6q.c    |    2 ++
 3 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-imx/cpuidle-imx6q.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 84b8976..530f0fd 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -70,7 +70,7 @@ obj-$(CONFIG_CPU_V7) += head-v7.o
 AFLAGS_head-v7.o :=-Wa,-march=armv7-a
 obj-$(CONFIG_SMP) += platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
-obj-$(CONFIG_SOC_IMX6Q) += clock-imx6q.o mach-imx6q.o
+obj-$(CONFIG_SOC_IMX6Q) += clock-imx6q.o mach-imx6q.o cpuidle-imx6q.o
 
 ifeq ($(CONFIG_PM),y)
 obj-$(CONFIG_SOC_IMX6Q) += pm-imx6q.o
diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c
new file mode 100644
index 0000000..b74557f
--- /dev/null
+++ b/arch/arm/mach-imx/cpuidle-imx6q.c
@@ -0,0 +1,33 @@
+/*
+ * 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/init.h>
+#include <linux/cpuidle.h>
+#include <linux/export.h>
+#include <asm/cpuidle.h>
+#include <mach/cpuidle.h>
+
+static struct cpuidle_driver imx6q_cpuidle_driver = {
+	.name			= "imx6q_cpuidle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
+	.states[0]		= ARM_CPUIDLE_WFI_STATE,
+	.state_count		= 1,
+};
+
+int __init imx6q_cpuidle_init(void)
+{
+	imx_cpuidle_set_driver(&imx6q_cpuidle_driver);
+
+	return 0;
+}
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index da6c1d9..6f31ca6 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -29,6 +29,7 @@
 #include <asm/system_misc.h>
 #include <mach/common.h>
 #include <mach/hardware.h>
+#include <mach/cpuidle.h>
 
 void imx6q_restart(char mode, const char *cmd)
 {
@@ -84,6 +85,7 @@ static void __init imx6q_init_machine(void)
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
 	imx6q_pm_init();
+	imx6q_cpuidle_init();
 }
 
 static void __init imx6q_map_io(void)
-- 
1.7.10

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Jesper Juhl]

On Mon, 16 Apr 2012, 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               |    2 +
>  arch/arm/plat-mxc/cpuidle.c              |   89 ++++++++++++++++++++++++++++++
>  arch/arm/plat-mxc/include/mach/cpuidle.h |   26 +++++++++
>  3 files changed, 117 insertions(+)
>  create mode 100644 arch/arm/plat-mxc/cpuidle.c
>  create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h
> 

A few pedantic/ignorable comments below.


> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
> index e81290c..7c9e05f 100644
> --- a/arch/arm/plat-mxc/Makefile
> +++ b/arch/arm/plat-mxc/Makefile
> @@ -16,6 +16,8 @@ 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..d1c9301
> --- /dev/null
> +++ b/arch/arm/plat-mxc/cpuidle.c
> @@ -0,0 +1,89 @@
> +/*
> + * 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;
> +static struct cpuidle_driver *drv;
> +
> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
> +{
> +	drv = p;
> +}
> +
> +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);
> +}
> +
> +static int __init imx_cpuidle_init(void)
> +{
> +	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);
> +

Pointless blank line. I'd suggest removing it.

> +	if (ret) {
> +		pr_err("%s: Failed to register cpuidle driver\n", __func__);
> +		return ret;
> +	}
> +
> +	imx_cpuidle_devices = alloc_percpu(struct cpuidle_device);
> +

Same here. Pointless blank line.

> +	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);
> +

Blank line here seems pointless.

> +	return ret;
> +}
> +late_initcall(imx_cpuidle_init);
> 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..6509f19
> --- /dev/null
> +++ b/arch/arm/plat-mxc/include/mach/cpuidle.h
> @@ -0,0 +1,26 @@
> +/*
> + * 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>
> +
> +extern int imx5_cpuidle_init(void);
> +extern int imx6q_cpuidle_init(void);
> +
> +#ifdef CONFIG_CPU_IDLE
> +extern void imx_cpuidle_devices_uninit(void);
> +extern void imx_cpuidle_set_driver(struct cpuidle_driver *p);
> +extern int imx5_cpuidle_init(void);
> +extern int imx6q_cpuidle_init(void);
> +#else
> +static inline void imx_cpuidle_devices_uninit(void) {}
> +static inline void imx_cpuidle_set_driver(struct cpuidle_driver *p) {}
> +#endif
> 

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Jesper Juhl]

On Mon, 16 Apr 2012, 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               |    2 +
>  arch/arm/plat-mxc/cpuidle.c              |   89 ++++++++++++++++++++++++++++++
>  arch/arm/plat-mxc/include/mach/cpuidle.h |   26 +++++++++
>  3 files changed, 117 insertions(+)
>  create mode 100644 arch/arm/plat-mxc/cpuidle.c
>  create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h
> 

A few pedantic/ignorable comments below.


> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
> index e81290c..7c9e05f 100644
> --- a/arch/arm/plat-mxc/Makefile
> +++ b/arch/arm/plat-mxc/Makefile
> @@ -16,6 +16,8 @@ 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..d1c9301
> --- /dev/null
> +++ b/arch/arm/plat-mxc/cpuidle.c
> @@ -0,0 +1,89 @@
> +/*
> + * 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;
> +static struct cpuidle_driver *drv;
> +
> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
> +{
> +	drv = p;
> +}
> +
> +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);
> +}
> +
> +static int __init imx_cpuidle_init(void)
> +{
> +	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);
> +

Pointless blank line. I'd suggest removing it.

> +	if (ret) {
> +		pr_err("%s: Failed to register cpuidle driver\n", __func__);
> +		return ret;
> +	}
> +
> +	imx_cpuidle_devices = alloc_percpu(struct cpuidle_device);
> +

Same here. Pointless blank line.

> +	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);
> +

Blank line here seems pointless.

> +	return ret;
> +}
> +late_initcall(imx_cpuidle_init);
> 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..6509f19
> --- /dev/null
> +++ b/arch/arm/plat-mxc/include/mach/cpuidle.h
> @@ -0,0 +1,26 @@
> +/*
> + * 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>
> +
> +extern int imx5_cpuidle_init(void);
> +extern int imx6q_cpuidle_init(void);
> +
> +#ifdef CONFIG_CPU_IDLE
> +extern void imx_cpuidle_devices_uninit(void);
> +extern void imx_cpuidle_set_driver(struct cpuidle_driver *p);
> +extern int imx5_cpuidle_init(void);
> +extern int imx6q_cpuidle_init(void);
> +#else
> +static inline void imx_cpuidle_devices_uninit(void) {}
> +static inline void imx_cpuidle_set_driver(struct cpuidle_driver *p) {}
> +#endif
> 

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Mon, Apr 16, 2012 at 06:50:12PM -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>
> ---
> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
> index e81290c..7c9e05f 100644
> --- a/arch/arm/plat-mxc/Makefile
> +++ b/arch/arm/plat-mxc/Makefile
> @@ -16,6 +16,8 @@ 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..d1c9301
> --- /dev/null
> +++ b/arch/arm/plat-mxc/cpuidle.c
> @@ -0,0 +1,89 @@
> +/*
> + * 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;
> +static struct cpuidle_driver *drv;
> +
> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
> +{
> +	drv = p;
> +}

You like it complicated, eh? Why do you introduce a function which sets
a variable...

> +
> +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);
> +}
> +
> +static int __init imx_cpuidle_init(void)

... instead of passing it directly to the function which uses it?

> +{
> +	struct cpuidle_device *dev;
> +	int cpu_id, ret;
> +
> +	if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
> +		pr_err("%s: Invalid Input\n", __func__);

You introduce a pointless error message on SoCs not setting a cpuidle
driver. When will people learn that initcalls do not only run on
machines they have on their desk?

> +		return -EINVAL;
> +	}
> +
> +	ret = cpuidle_register_driver(drv);
> +
> +	if (ret) {
> +		pr_err("%s: Failed to register cpuidle driver\n", __func__);

It's always nice to add the return value to the error message to get a
clue *what* went wrong. The function name though is rather
uninteresting.

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 |

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Mon, Apr 16, 2012 at 06:50:12PM -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>
> ---
> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
> index e81290c..7c9e05f 100644
> --- a/arch/arm/plat-mxc/Makefile
> +++ b/arch/arm/plat-mxc/Makefile
> @@ -16,6 +16,8 @@ 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..d1c9301
> --- /dev/null
> +++ b/arch/arm/plat-mxc/cpuidle.c
> @@ -0,0 +1,89 @@
> +/*
> + * 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;
> +static struct cpuidle_driver *drv;
> +
> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
> +{
> +	drv = p;
> +}

You like it complicated, eh? Why do you introduce a function which sets
a variable...

> +
> +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);
> +}
> +
> +static int __init imx_cpuidle_init(void)

... instead of passing it directly to the function which uses it?

> +{
> +	struct cpuidle_device *dev;
> +	int cpu_id, ret;
> +
> +	if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
> +		pr_err("%s: Invalid Input\n", __func__);

You introduce a pointless error message on SoCs not setting a cpuidle
driver. When will people learn that initcalls do not only run on
machines they have on their desk?

> +		return -EINVAL;
> +	}
> +
> +	ret = cpuidle_register_driver(drv);
> +
> +	if (ret) {
> +		pr_err("%s: Failed to register cpuidle driver\n", __func__);

It's always nice to add the return value to the error message to get a
clue *what* went wrong. The function name though is rather
uninteresting.

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 |

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

On Tue, Apr 17, 2012 at 2:43 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Mon, Apr 16, 2012 at 06:50:12PM -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>
>> ---
>> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
>> index e81290c..7c9e05f 100644
>> --- a/arch/arm/plat-mxc/Makefile
>> +++ b/arch/arm/plat-mxc/Makefile
>> @@ -16,6 +16,8 @@ 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..d1c9301
>> --- /dev/null
>> +++ b/arch/arm/plat-mxc/cpuidle.c
>> @@ -0,0 +1,89 @@
>> +/*
>> + * 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;
>> +static struct cpuidle_driver *drv;
>> +
>> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
>> +{
>> +     drv = p;
>> +}
>
> You like it complicated, eh? Why do you introduce a function which sets
> a variable...
>

This complication is used to deal with the timing of various levels of
init calls.  More explanation below.

>> +
>> +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);
>> +}
>> +
>> +static int __init imx_cpuidle_init(void)
>
> ... instead of passing it directly to the function which uses it?
>

If I called imx_cpuidle_init directly from imx5 or imx6q init
routines, it would be getting called before the coreinit_call of core
cpuidle causing a failure.  There were various other directions to
take and all seemed less desirable than this one.

One alternative would be to add a function to return the pointer to
the cpuidle driver object based on the machine type.  Functionality
exists to identify imx5 as a machine type but not imx6q, so I couldn't
use that machine based method without adding that extra code.

Another alternative would be to add a general platform lateinit_call
function to each platforms that support cpuidle.

>> +{
>> +     struct cpuidle_device *dev;
>> +     int cpu_id, ret;
>> +
>> +     if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
>> +             pr_err("%s: Invalid Input\n", __func__);
>
> You introduce a pointless error message on SoCs not setting a cpuidle
> driver. When will people learn that initcalls do not only run on
> machines they have on their desk?
>

Will fix in next version.

>> +             return -EINVAL;
>> +     }
>> +
>> +     ret = cpuidle_register_driver(drv);
>> +
>> +     if (ret) {
>> +             pr_err("%s: Failed to register cpuidle driver\n", __func__);
>
> It's always nice to add the return value to the error message to get a
> clue *what* went wrong. The function name though is rather
> uninteresting.

Will add the return value in the next version.

>
> 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 |

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

On Tue, Apr 17, 2012 at 2:43 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Mon, Apr 16, 2012 at 06:50:12PM -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>
>> ---
>> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
>> index e81290c..7c9e05f 100644
>> --- a/arch/arm/plat-mxc/Makefile
>> +++ b/arch/arm/plat-mxc/Makefile
>> @@ -16,6 +16,8 @@ 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..d1c9301
>> --- /dev/null
>> +++ b/arch/arm/plat-mxc/cpuidle.c
>> @@ -0,0 +1,89 @@
>> +/*
>> + * 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;
>> +static struct cpuidle_driver *drv;
>> +
>> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
>> +{
>> + ? ? drv = p;
>> +}
>
> You like it complicated, eh? Why do you introduce a function which sets
> a variable...
>

This complication is used to deal with the timing of various levels of
init calls.  More explanation below.

>> +
>> +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);
>> +}
>> +
>> +static int __init imx_cpuidle_init(void)
>
> ... instead of passing it directly to the function which uses it?
>

If I called imx_cpuidle_init directly from imx5 or imx6q init
routines, it would be getting called before the coreinit_call of core
cpuidle causing a failure.  There were various other directions to
take and all seemed less desirable than this one.

One alternative would be to add a function to return the pointer to
the cpuidle driver object based on the machine type.  Functionality
exists to identify imx5 as a machine type but not imx6q, so I couldn't
use that machine based method without adding that extra code.

Another alternative would be to add a general platform lateinit_call
function to each platforms that support cpuidle.

>> +{
>> + ? ? struct cpuidle_device *dev;
>> + ? ? int cpu_id, ret;
>> +
>> + ? ? if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
>> + ? ? ? ? ? ? pr_err("%s: Invalid Input\n", __func__);
>
> You introduce a pointless error message on SoCs not setting a cpuidle
> driver. When will people learn that initcalls do not only run on
> machines they have on their desk?
>

Will fix in next version.

>> + ? ? ? ? ? ? return -EINVAL;
>> + ? ? }
>> +
>> + ? ? ret = cpuidle_register_driver(drv);
>> +
>> + ? ? if (ret) {
>> + ? ? ? ? ? ? pr_err("%s: Failed to register cpuidle driver\n", __func__);
>
> It's always nice to add the return value to the error message to get a
> clue *what* went wrong. The function name though is rather
> uninteresting.

Will add the return value in the next version.

>
> 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 |

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Christian Robottom Reis]

On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
> >> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
> >> +{
> >> +     drv = p;
> >> +}
> >
> > You like it complicated, eh? Why do you introduce a function which sets
> > a variable...
> 
> This complication is used to deal with the timing of various levels of
> init calls.  More explanation below.

Regardless of how you end up solving this, it's probably a good idea
to document the rationale, perhaps cribbing from what you describe
below..

> If I called imx_cpuidle_init directly from imx5 or imx6q init
> routines, it would be getting called before the coreinit_call of core
> cpuidle causing a failure.  There were various other directions to
> take and all seemed less desirable than this one.
> 
> One alternative would be to add a function to return the pointer to
> the cpuidle driver object based on the machine type.  Functionality
> exists to identify imx5 as a machine type but not imx6q, so I couldn't
> use that machine based method without adding that extra code.
> 
> Another alternative would be to add a general platform lateinit_call
> function to each platforms that support cpuidle.

..in a comment; without it, the code indeed looks bizarre.
-- 
Christian Robottom Reis, Engineering VP
Brazil (GMT-3) | [+55] 16 9112 6430 | [+1] 612 216 4935
Linaro.org: Open Source Software for ARM SoCs

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Christian Robottom Reis]

On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
> >> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
> >> +{
> >> + ? ? drv = p;
> >> +}
> >
> > You like it complicated, eh? Why do you introduce a function which sets
> > a variable...
> 
> This complication is used to deal with the timing of various levels of
> init calls.  More explanation below.

Regardless of how you end up solving this, it's probably a good idea
to document the rationale, perhaps cribbing from what you describe
below..

> If I called imx_cpuidle_init directly from imx5 or imx6q init
> routines, it would be getting called before the coreinit_call of core
> cpuidle causing a failure.  There were various other directions to
> take and all seemed less desirable than this one.
> 
> One alternative would be to add a function to return the pointer to
> the cpuidle driver object based on the machine type.  Functionality
> exists to identify imx5 as a machine type but not imx6q, so I couldn't
> use that machine based method without adding that extra code.
> 
> Another alternative would be to add a general platform lateinit_call
> function to each platforms that support cpuidle.

..in a comment; without it, the code indeed looks bizarre.
-- 
Christian Robottom Reis, Engineering VP
Brazil (GMT-3) | [+55] 16 9112 6430 | [+1] 612 216 4935
Linaro.org: Open Source Software for ARM SoCs

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

On Tue, Apr 17, 2012 at 9:13 AM, Christian Robottom Reis
<kiko@linaro.org> wrote:
> On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
>> >> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
>> >> +{
>> >> +     drv = p;
>> >> +}
>> >
>> > You like it complicated, eh? Why do you introduce a function which sets
>> > a variable...
>>
>> This complication is used to deal with the timing of various levels of
>> init calls.  More explanation below.
>
> Regardless of how you end up solving this, it's probably a good idea
> to document the rationale, perhaps cribbing from what you describe
> below..

Agree.  Adding comments to the function itself seems to be the most
relevant location so I can add that information there if the function
remains.

>
>> If I called imx_cpuidle_init directly from imx5 or imx6q init
>> routines, it would be getting called before the coreinit_call of core
>> cpuidle causing a failure.  There were various other directions to
>> take and all seemed less desirable than this one.
>>
>> One alternative would be to add a function to return the pointer to
>> the cpuidle driver object based on the machine type.  Functionality
>> exists to identify imx5 as a machine type but not imx6q, so I couldn't
>> use that machine based method without adding that extra code.
>>
>> Another alternative would be to add a general platform lateinit_call
>> function to each platforms that support cpuidle.
>
> ..in a comment; without it, the code indeed looks bizarre.
> --
> Christian Robottom Reis, Engineering VP
> Brazil (GMT-3) | [+55] 16 9112 6430 | [+1] 612 216 4935
> Linaro.org: Open Source Software for ARM SoCs

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

On Tue, Apr 17, 2012 at 9:13 AM, Christian Robottom Reis
<kiko@linaro.org> wrote:
> On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
>> >> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
>> >> +{
>> >> + ? ? drv = p;
>> >> +}
>> >
>> > You like it complicated, eh? Why do you introduce a function which sets
>> > a variable...
>>
>> This complication is used to deal with the timing of various levels of
>> init calls. ?More explanation below.
>
> Regardless of how you end up solving this, it's probably a good idea
> to document the rationale, perhaps cribbing from what you describe
> below..

Agree.  Adding comments to the function itself seems to be the most
relevant location so I can add that information there if the function
remains.

>
>> If I called imx_cpuidle_init directly from imx5 or imx6q init
>> routines, it would be getting called before the coreinit_call of core
>> cpuidle causing a failure. ?There were various other directions to
>> take and all seemed less desirable than this one.
>>
>> One alternative would be to add a function to return the pointer to
>> the cpuidle driver object based on the machine type. ?Functionality
>> exists to identify imx5 as a machine type but not imx6q, so I couldn't
>> use that machine based method without adding that extra code.
>>
>> Another alternative would be to add a general platform lateinit_call
>> function to each platforms that support cpuidle.
>
> ..in a comment; without it, the code indeed looks bizarre.
> --
> Christian Robottom Reis, Engineering VP
> Brazil (GMT-3) | [+55] 16 9112 6430 | [+1] 612 216 4935
> Linaro.org: Open Source Software for ARM SoCs

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
> On Tue, Apr 17, 2012 at 2:43 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Mon, Apr 16, 2012 at 06:50:12PM -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>
> >> ---
> >> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
> >> index e81290c..7c9e05f 100644
> >> --- a/arch/arm/plat-mxc/Makefile
> >> +++ b/arch/arm/plat-mxc/Makefile
> >> @@ -16,6 +16,8 @@ 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..d1c9301
> >> --- /dev/null
> >> +++ b/arch/arm/plat-mxc/cpuidle.c
> >> @@ -0,0 +1,89 @@
> >> +/*
> >> + * 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;
> >> +static struct cpuidle_driver *drv;
> >> +
> >> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
> >> +{
> >> +     drv = p;
> >> +}
> >
> > You like it complicated, eh? Why do you introduce a function which sets
> > a variable...
> >
> 
> This complication is used to deal with the timing of various levels of
> init calls.  More explanation below.
> 
> >> +
> >> +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);
> >> +}
> >> +
> >> +static int __init imx_cpuidle_init(void)
> >
> > ... instead of passing it directly to the function which uses it?
> >
> 
> If I called imx_cpuidle_init directly from imx5 or imx6q init
> routines, it would be getting called before the coreinit_call of core
> cpuidle causing a failure.  There were various other directions to
> take and all seemed less desirable than this one.
> 
> One alternative would be to add a function to return the pointer to
> the cpuidle driver object based on the machine type.  Functionality
> exists to identify imx5 as a machine type but not imx6q, so I couldn't
> use that machine based method without adding that extra code.
> 
> Another alternative would be to add a general platform lateinit_call
> function to each platforms that support cpuidle.

Just put the initcall into mm-imx5.c and check the cpu type. Then you
also don't have to make imx5_idle global.

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 |

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
> On Tue, Apr 17, 2012 at 2:43 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Mon, Apr 16, 2012 at 06:50:12PM -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>
> >> ---
> >> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
> >> index e81290c..7c9e05f 100644
> >> --- a/arch/arm/plat-mxc/Makefile
> >> +++ b/arch/arm/plat-mxc/Makefile
> >> @@ -16,6 +16,8 @@ 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..d1c9301
> >> --- /dev/null
> >> +++ b/arch/arm/plat-mxc/cpuidle.c
> >> @@ -0,0 +1,89 @@
> >> +/*
> >> + * 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;
> >> +static struct cpuidle_driver *drv;
> >> +
> >> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
> >> +{
> >> + ? ? drv = p;
> >> +}
> >
> > You like it complicated, eh? Why do you introduce a function which sets
> > a variable...
> >
> 
> This complication is used to deal with the timing of various levels of
> init calls.  More explanation below.
> 
> >> +
> >> +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);
> >> +}
> >> +
> >> +static int __init imx_cpuidle_init(void)
> >
> > ... instead of passing it directly to the function which uses it?
> >
> 
> If I called imx_cpuidle_init directly from imx5 or imx6q init
> routines, it would be getting called before the coreinit_call of core
> cpuidle causing a failure.  There were various other directions to
> take and all seemed less desirable than this one.
> 
> One alternative would be to add a function to return the pointer to
> the cpuidle driver object based on the machine type.  Functionality
> exists to identify imx5 as a machine type but not imx6q, so I couldn't
> use that machine based method without adding that extra code.
> 
> Another alternative would be to add a general platform lateinit_call
> function to each platforms that support cpuidle.

Just put the initcall into mm-imx5.c and check the cpu type. Then you
also don't have to make imx5_idle global.

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 |

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

>> >> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
>> >> +{
>> >> +     drv = p;
>> >> +}
>> >
>> > You like it complicated, eh? Why do you introduce a function which sets
>> > a variable...
>> >
>>
>> This complication is used to deal with the timing of various levels of
>> init calls.  More explanation below.
>>
>> >> +
>> >> +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);
>> >> +}
>> >> +
>> >> +static int __init imx_cpuidle_init(void)
>> >
>> > ... instead of passing it directly to the function which uses it?
>> >
>>
>> If I called imx_cpuidle_init directly from imx5 or imx6q init
>> routines, it would be getting called before the coreinit_call of core
>> cpuidle causing a failure.  There were various other directions to
>> take and all seemed less desirable than this one.
>>
>> One alternative would be to add a function to return the pointer to
>> the cpuidle driver object based on the machine type.  Functionality
>> exists to identify imx5 as a machine type but not imx6q, so I couldn't
>> use that machine based method without adding that extra code.
>>
>> Another alternative would be to add a general platform lateinit_call
>> function to each platforms that support cpuidle.
>
> Just put the initcall into mm-imx5.c and check the cpu type. Then you
> also don't have to make imx5_idle global.

That solution is currently available for imx5 but for imx6q it implies
adding the cpu type support for imx6q.  Are you ok with that?

>
> 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 |

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

>> >> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
>> >> +{
>> >> + ? ? drv = p;
>> >> +}
>> >
>> > You like it complicated, eh? Why do you introduce a function which sets
>> > a variable...
>> >
>>
>> This complication is used to deal with the timing of various levels of
>> init calls. ?More explanation below.
>>
>> >> +
>> >> +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);
>> >> +}
>> >> +
>> >> +static int __init imx_cpuidle_init(void)
>> >
>> > ... instead of passing it directly to the function which uses it?
>> >
>>
>> If I called imx_cpuidle_init directly from imx5 or imx6q init
>> routines, it would be getting called before the coreinit_call of core
>> cpuidle causing a failure. ?There were various other directions to
>> take and all seemed less desirable than this one.
>>
>> One alternative would be to add a function to return the pointer to
>> the cpuidle driver object based on the machine type. ?Functionality
>> exists to identify imx5 as a machine type but not imx6q, so I couldn't
>> use that machine based method without adding that extra code.
>>
>> Another alternative would be to add a general platform lateinit_call
>> function to each platforms that support cpuidle.
>
> Just put the initcall into mm-imx5.c and check the cpu type. Then you
> also don't have to make imx5_idle global.

That solution is currently available for imx5 but for imx6q it implies
adding the cpu type support for imx6q.  Are you ok with that?

>
> 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 |

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

>>> If I called imx_cpuidle_init directly from imx5 or imx6q init
>>> routines, it would be getting called before the coreinit_call of core
>>> cpuidle causing a failure.  There were various other directions to
>>> take and all seemed less desirable than this one.
>>>
>>> One alternative would be to add a function to return the pointer to
>>> the cpuidle driver object based on the machine type.  Functionality
>>> exists to identify imx5 as a machine type but not imx6q, so I couldn't
>>> use that machine based method without adding that extra code.
>>>
>>> Another alternative would be to add a general platform lateinit_call
>>> function to each platforms that support cpuidle.
>>
>> Just put the initcall into mm-imx5.c and check the cpu type. Then you
>> also don't have to make imx5_idle global.
>
> That solution is currently available for imx5 but for imx6q it implies
> adding the cpu type support for imx6q.  Are you ok with that?

Sascha or Shawn, any further comments on my question?

Thanks,
Rob

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

>>> If I called imx_cpuidle_init directly from imx5 or imx6q init
>>> routines, it would be getting called before the coreinit_call of core
>>> cpuidle causing a failure. ?There were various other directions to
>>> take and all seemed less desirable than this one.
>>>
>>> One alternative would be to add a function to return the pointer to
>>> the cpuidle driver object based on the machine type. ?Functionality
>>> exists to identify imx5 as a machine type but not imx6q, so I couldn't
>>> use that machine based method without adding that extra code.
>>>
>>> Another alternative would be to add a general platform lateinit_call
>>> function to each platforms that support cpuidle.
>>
>> Just put the initcall into mm-imx5.c and check the cpu type. Then you
>> also don't have to make imx5_idle global.
>
> That solution is currently available for imx5 but for imx6q it implies
> adding the cpu type support for imx6q. ?Are you ok with that?

Sascha or Shawn, any further comments on my question?

Thanks,
Rob

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Wed, Apr 18, 2012 at 11:18:55PM -0500, Rob Lee wrote:
> >>> If I called imx_cpuidle_init directly from imx5 or imx6q init
> >>> routines, it would be getting called before the coreinit_call of core
> >>> cpuidle causing a failure.  There were various other directions to
> >>> take and all seemed less desirable than this one.
> >>>
> >>> One alternative would be to add a function to return the pointer to
> >>> the cpuidle driver object based on the machine type.  Functionality
> >>> exists to identify imx5 as a machine type but not imx6q, so I couldn't
> >>> use that machine based method without adding that extra code.
> >>>
> >>> Another alternative would be to add a general platform lateinit_call
> >>> function to each platforms that support cpuidle.
> >>
> >> Just put the initcall into mm-imx5.c and check the cpu type. Then you
> >> also don't have to make imx5_idle global.
> >
> > That solution is currently available for imx5 but for imx6q it implies
> > adding the cpu type support for imx6q.  Are you ok with that?
> 
> Sascha or Shawn, any further comments on my question?

I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
hook at device_initcall time can't be too wrong. Shawn?

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 |

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Wed, Apr 18, 2012 at 11:18:55PM -0500, Rob Lee wrote:
> >>> If I called imx_cpuidle_init directly from imx5 or imx6q init
> >>> routines, it would be getting called before the coreinit_call of core
> >>> cpuidle causing a failure. ?There were various other directions to
> >>> take and all seemed less desirable than this one.
> >>>
> >>> One alternative would be to add a function to return the pointer to
> >>> the cpuidle driver object based on the machine type. ?Functionality
> >>> exists to identify imx5 as a machine type but not imx6q, so I couldn't
> >>> use that machine based method without adding that extra code.
> >>>
> >>> Another alternative would be to add a general platform lateinit_call
> >>> function to each platforms that support cpuidle.
> >>
> >> Just put the initcall into mm-imx5.c and check the cpu type. Then you
> >> also don't have to make imx5_idle global.
> >
> > That solution is currently available for imx5 but for imx6q it implies
> > adding the cpu type support for imx6q. ?Are you ok with that?
> 
> Sascha or Shawn, any further comments on my question?

I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
hook at device_initcall time can't be too wrong. Shawn?

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 |

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Thu, Apr 19, 2012 at 08:43:08AM +0200, Sascha Hauer wrote:
> > Sascha or Shawn, any further comments on my question?
> 
Sorry for the late response, Rob.

> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
> hook at device_initcall time can't be too wrong. Shawn?
> 
Yep, it works for me.

-- 
Regards,
Shawn

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Thu, Apr 19, 2012 at 08:43:08AM +0200, Sascha Hauer wrote:
> > Sascha or Shawn, any further comments on my question?
> 
Sorry for the late response, Rob.

> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
> hook at device_initcall time can't be too wrong. Shawn?
> 
Yep, it works for me.

-- 
Regards,
Shawn

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

>> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
>> hook at device_initcall time can't be too wrong. Shawn?
>>
> Yep, it works for me.
>
Sascha, Shawn, thanks for the response.

Since device_initcall isn't platform specific, it seems I would still
need a cpu_is_imx6q() function or similiar functionality from a device
tree call.  Or do you have something else in mind that I'm not seeing?

Thanks,
Rob


> --
> Regards,
> Shawn

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

>> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
>> hook at device_initcall time can't be too wrong. Shawn?
>>
> Yep, it works for me.
>
Sascha, Shawn, thanks for the response.

Since device_initcall isn't platform specific, it seems I would still
need a cpu_is_imx6q() function or similiar functionality from a device
tree call.  Or do you have something else in mind that I'm not seeing?

Thanks,
Rob


> --
> Regards,
> Shawn

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
> >> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
> >> hook at device_initcall time can't be too wrong. Shawn?
> >>
> > Yep, it works for me.
> >
> Sascha, Shawn, thanks for the response.
> 
> Since device_initcall isn't platform specific, it seems I would still
> need a cpu_is_imx6q() function or similiar functionality from a device
> tree call.  Or do you have something else in mind that I'm not seeing?
> 
I guess Sascha is asking for something like the following.

static int __init imx_device_init(void)
{
	imx5_device_init();
	imx6_device_init();
}
device_initcall(imx_device_init)

static int __init imx6_device_init(void)
{
	/*
	 * do whatever needs to get done in device_initcall time
	 */
}

-- 
Regards,
Shawn

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
> >> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
> >> hook at device_initcall time can't be too wrong. Shawn?
> >>
> > Yep, it works for me.
> >
> Sascha, Shawn, thanks for the response.
> 
> Since device_initcall isn't platform specific, it seems I would still
> need a cpu_is_imx6q() function or similiar functionality from a device
> tree call.  Or do you have something else in mind that I'm not seeing?
> 
I guess Sascha is asking for something like the following.

static int __init imx_device_init(void)
{
	imx5_device_init();
	imx6_device_init();
}
device_initcall(imx_device_init)

static int __init imx6_device_init(void)
{
	/*
	 * do whatever needs to get done in device_initcall time
	 */
}

-- 
Regards,
Shawn

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
> On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
> > >> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
> > >> hook at device_initcall time can't be too wrong. Shawn?
> > >>
> > > Yep, it works for me.
> > >
> > Sascha, Shawn, thanks for the response.
> > 
> > Since device_initcall isn't platform specific, it seems I would still
> > need a cpu_is_imx6q() function or similiar functionality from a device
> > tree call.  Or do you have something else in mind that I'm not seeing?
> > 
> I guess Sascha is asking for something like the following.
> 
> static int __init imx_device_init(void)
> {
> 	imx5_device_init();
> 	imx6_device_init();
> }
> device_initcall(imx_device_init)
> 
> static int __init imx6_device_init(void)
> {
> 	/*
> 	 * do whatever needs to get done in device_initcall time
> 	 */
> }

The problem is more how we can detect that we are actually running a
i.MX6 SoC. We could directly ask the devicetree in an initcall or we
could introduce a cpu_is_mx6() just like we have a macro for all other
i.MX SoCs.

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 |

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
> On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
> > >> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
> > >> hook at device_initcall time can't be too wrong. Shawn?
> > >>
> > > Yep, it works for me.
> > >
> > Sascha, Shawn, thanks for the response.
> > 
> > Since device_initcall isn't platform specific, it seems I would still
> > need a cpu_is_imx6q() function or similiar functionality from a device
> > tree call.  Or do you have something else in mind that I'm not seeing?
> > 
> I guess Sascha is asking for something like the following.
> 
> static int __init imx_device_init(void)
> {
> 	imx5_device_init();
> 	imx6_device_init();
> }
> device_initcall(imx_device_init)
> 
> static int __init imx6_device_init(void)
> {
> 	/*
> 	 * do whatever needs to get done in device_initcall time
> 	 */
> }

The problem is more how we can detect that we are actually running a
i.MX6 SoC. We could directly ask the devicetree in an initcall or we
could introduce a cpu_is_mx6() just like we have a macro for all other
i.MX SoCs.

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 |

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
> On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
> > On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
> > > >> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
> > > >> hook at device_initcall time can't be too wrong. Shawn?
> > > >>
> > > > Yep, it works for me.
> > > >
> > > Sascha, Shawn, thanks for the response.
> > > 
> > > Since device_initcall isn't platform specific, it seems I would still
> > > need a cpu_is_imx6q() function or similiar functionality from a device
> > > tree call.  Or do you have something else in mind that I'm not seeing?
> > > 
> > I guess Sascha is asking for something like the following.
> > 
> > static int __init imx_device_init(void)
> > {
> > 	imx5_device_init();
> > 	imx6_device_init();
> > }
> > device_initcall(imx_device_init)
> > 
> > static int __init imx6_device_init(void)
> > {
> > 	/*
> > 	 * do whatever needs to get done in device_initcall time
> > 	 */
> > }
> 
> The problem is more how we can detect that we are actually running a
> i.MX6 SoC. We could directly ask the devicetree in an initcall or we
> could introduce a cpu_is_mx6() just like we have a macro for all other
> i.MX SoCs.
> 
Oops, my reply was a brain-dead one.  Hmm, then it seems that we have
to introduce cpu_is_mx6() which I tried hard to avoid.  I do not have
a preference between defining a macro and asking device tree.

-- 
Regards,
Shawn

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
> On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
> > On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
> > > >> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
> > > >> hook at device_initcall time can't be too wrong. Shawn?
> > > >>
> > > > Yep, it works for me.
> > > >
> > > Sascha, Shawn, thanks for the response.
> > > 
> > > Since device_initcall isn't platform specific, it seems I would still
> > > need a cpu_is_imx6q() function or similiar functionality from a device
> > > tree call.  Or do you have something else in mind that I'm not seeing?
> > > 
> > I guess Sascha is asking for something like the following.
> > 
> > static int __init imx_device_init(void)
> > {
> > 	imx5_device_init();
> > 	imx6_device_init();
> > }
> > device_initcall(imx_device_init)
> > 
> > static int __init imx6_device_init(void)
> > {
> > 	/*
> > 	 * do whatever needs to get done in device_initcall time
> > 	 */
> > }
> 
> The problem is more how we can detect that we are actually running a
> i.MX6 SoC. We could directly ask the devicetree in an initcall or we
> could introduce a cpu_is_mx6() just like we have a macro for all other
> i.MX SoCs.
> 
Oops, my reply was a brain-dead one.  Hmm, then it seems that we have
to introduce cpu_is_mx6() which I tried hard to avoid.  I do not have
a preference between defining a macro and asking device tree.

-- 
Regards,
Shawn

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Mon, Apr 23, 2012 at 02:53:01PM +0800, Shawn Guo wrote:
> On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
> > On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
> > > On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
> > > > >> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
> > > > >> hook at device_initcall time can't be too wrong. Shawn?
> > > > >>
> > > > > Yep, it works for me.
> > > > >
> > > > Sascha, Shawn, thanks for the response.
> > > > 
> > > > Since device_initcall isn't platform specific, it seems I would still
> > > > need a cpu_is_imx6q() function or similiar functionality from a device
> > > > tree call.  Or do you have something else in mind that I'm not seeing?
> > > > 
> > > I guess Sascha is asking for something like the following.
> > > 
> > > static int __init imx_device_init(void)
> > > {
> > > 	imx5_device_init();
> > > 	imx6_device_init();
> > > }
> > > device_initcall(imx_device_init)
> > > 
> > > static int __init imx6_device_init(void)
> > > {
> > > 	/*
> > > 	 * do whatever needs to get done in device_initcall time
> > > 	 */
> > > }
> > 
> > The problem is more how we can detect that we are actually running a
> > i.MX6 SoC. We could directly ask the devicetree in an initcall or we
> > could introduce a cpu_is_mx6() just like we have a macro for all other
> > i.MX SoCs.
> > 
> Oops, my reply was a brain-dead one.  Hmm, then it seems that we have
> to introduce cpu_is_mx6() which I tried hard to avoid.  I do not have
> a preference between defining a macro and asking device tree.

Since we already have a place in early setup code in which we know that
we are running on an i.MX6 I suggest for the sake of the symmetry of the
universe that we introduce a cpu_is_mx6.

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 |

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Mon, Apr 23, 2012 at 02:53:01PM +0800, Shawn Guo wrote:
> On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
> > On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
> > > On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
> > > > >> I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific
> > > > >> hook at device_initcall time can't be too wrong. Shawn?
> > > > >>
> > > > > Yep, it works for me.
> > > > >
> > > > Sascha, Shawn, thanks for the response.
> > > > 
> > > > Since device_initcall isn't platform specific, it seems I would still
> > > > need a cpu_is_imx6q() function or similiar functionality from a device
> > > > tree call.  Or do you have something else in mind that I'm not seeing?
> > > > 
> > > I guess Sascha is asking for something like the following.
> > > 
> > > static int __init imx_device_init(void)
> > > {
> > > 	imx5_device_init();
> > > 	imx6_device_init();
> > > }
> > > device_initcall(imx_device_init)
> > > 
> > > static int __init imx6_device_init(void)
> > > {
> > > 	/*
> > > 	 * do whatever needs to get done in device_initcall time
> > > 	 */
> > > }
> > 
> > The problem is more how we can detect that we are actually running a
> > i.MX6 SoC. We could directly ask the devicetree in an initcall or we
> > could introduce a cpu_is_mx6() just like we have a macro for all other
> > i.MX SoCs.
> > 
> Oops, my reply was a brain-dead one.  Hmm, then it seems that we have
> to introduce cpu_is_mx6() which I tried hard to avoid.  I do not have
> a preference between defining a macro and asking device tree.

Since we already have a place in early setup code in which we know that
we are running on an i.MX6 I suggest for the sake of the symmetry of the
universe that we introduce a cpu_is_mx6.

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 |

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Mon, Apr 23, 2012 at 08:56:23AM +0200, Sascha Hauer wrote:
> On Mon, Apr 23, 2012 at 02:53:01PM +0800, Shawn Guo wrote:
> > On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
> > > On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
> > > > On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
...
> > > > > Since device_initcall isn't platform specific, it seems I would still
> > > > > need a cpu_is_imx6q() function or similiar functionality from a device
> > > > > tree call.  Or do you have something else in mind that I'm not seeing?
> > > > > 
> > > > I guess Sascha is asking for something like the following.
> > > > 
> > > > static int __init imx_device_init(void)
> > > > {
> > > > 	imx5_device_init();
> > > > 	imx6_device_init();
> > > > }
> > > > device_initcall(imx_device_init)
> > > > 
> > > > static int __init imx6_device_init(void)
> > > > {
> > > > 	/*
> > > > 	 * do whatever needs to get done in device_initcall time
> > > > 	 */
> > > > }
> > > 
> > > The problem is more how we can detect that we are actually running a
> > > i.MX6 SoC. We could directly ask the devicetree in an initcall or we
> > > could introduce a cpu_is_mx6() just like we have a macro for all other
> > > i.MX SoCs.
> > > 
> > Oops, my reply was a brain-dead one.  Hmm, then it seems that we have
> > to introduce cpu_is_mx6() which I tried hard to avoid.  I do not have
> > a preference between defining a macro and asking device tree.
> 
> Since we already have a place in early setup code in which we know that
> we are running on an i.MX6 I suggest for the sake of the symmetry of the
> universe that we introduce a cpu_is_mx6.
> 
Let me try last time.  What about having a late_initcall hook in
machine_desc?

Regards,
Shawn

8<---

diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
index d7692ca..0b1c94b 100644
--- a/arch/arm/include/asm/mach/arch.h
+++ b/arch/arm/include/asm/mach/arch.h
@@ -43,6 +43,7 @@ struct machine_desc {
        void                    (*init_irq)(void);
        struct sys_timer        *timer;         /* system tick timer    */
        void                    (*init_machine)(void);
+       void                    (*init_late)(void);
 #ifdef CONFIG_MULTI_IRQ_HANDLER
        void                    (*handle_irq)(struct pt_regs *);
 #endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index ebfac78..549f036 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -800,6 +800,14 @@ static int __init customize_machine(void)
 }
 arch_initcall(customize_machine);

+static int __init init_machine_late(void)
+{
+       if (machine_desc->init_late)
+               machine_desc->init_late();
+       return 0;
+}
+late_initcall(init_machine_late);
+
 #ifdef CONFIG_KEXEC
 static inline unsigned long long get_total_mem(void)
 {
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index da6c1d9..0e3640f 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)")
        .handle_irq     = imx6q_handle_irq,
        .timer          = &imx6q_timer,
        .init_machine   = imx6q_init_machine,
+       .init_late      = imx6q_init_late,
        .dt_compat      = imx6q_dt_compat,
        .restart        = imx6q_restart,
 MACHINE_END

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Mon, Apr 23, 2012 at 08:56:23AM +0200, Sascha Hauer wrote:
> On Mon, Apr 23, 2012 at 02:53:01PM +0800, Shawn Guo wrote:
> > On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
> > > On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
> > > > On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
...
> > > > > Since device_initcall isn't platform specific, it seems I would still
> > > > > need a cpu_is_imx6q() function or similiar functionality from a device
> > > > > tree call.  Or do you have something else in mind that I'm not seeing?
> > > > > 
> > > > I guess Sascha is asking for something like the following.
> > > > 
> > > > static int __init imx_device_init(void)
> > > > {
> > > > 	imx5_device_init();
> > > > 	imx6_device_init();
> > > > }
> > > > device_initcall(imx_device_init)
> > > > 
> > > > static int __init imx6_device_init(void)
> > > > {
> > > > 	/*
> > > > 	 * do whatever needs to get done in device_initcall time
> > > > 	 */
> > > > }
> > > 
> > > The problem is more how we can detect that we are actually running a
> > > i.MX6 SoC. We could directly ask the devicetree in an initcall or we
> > > could introduce a cpu_is_mx6() just like we have a macro for all other
> > > i.MX SoCs.
> > > 
> > Oops, my reply was a brain-dead one.  Hmm, then it seems that we have
> > to introduce cpu_is_mx6() which I tried hard to avoid.  I do not have
> > a preference between defining a macro and asking device tree.
> 
> Since we already have a place in early setup code in which we know that
> we are running on an i.MX6 I suggest for the sake of the symmetry of the
> universe that we introduce a cpu_is_mx6.
> 
Let me try last time.  What about having a late_initcall hook in
machine_desc?

Regards,
Shawn

8<---

diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
index d7692ca..0b1c94b 100644
--- a/arch/arm/include/asm/mach/arch.h
+++ b/arch/arm/include/asm/mach/arch.h
@@ -43,6 +43,7 @@ struct machine_desc {
        void                    (*init_irq)(void);
        struct sys_timer        *timer;         /* system tick timer    */
        void                    (*init_machine)(void);
+       void                    (*init_late)(void);
 #ifdef CONFIG_MULTI_IRQ_HANDLER
        void                    (*handle_irq)(struct pt_regs *);
 #endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index ebfac78..549f036 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -800,6 +800,14 @@ static int __init customize_machine(void)
 }
 arch_initcall(customize_machine);

+static int __init init_machine_late(void)
+{
+       if (machine_desc->init_late)
+               machine_desc->init_late();
+       return 0;
+}
+late_initcall(init_machine_late);
+
 #ifdef CONFIG_KEXEC
 static inline unsigned long long get_total_mem(void)
 {
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index da6c1d9..0e3640f 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)")
        .handle_irq     = imx6q_handle_irq,
        .timer          = &imx6q_timer,
        .init_machine   = imx6q_init_machine,
+       .init_late      = imx6q_init_late,
        .dt_compat      = imx6q_dt_compat,
        .restart        = imx6q_restart,
 MACHINE_END

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Mon, Apr 23, 2012 at 03:10:15PM +0800, Shawn Guo wrote:
> On Mon, Apr 23, 2012 at 08:56:23AM +0200, Sascha Hauer wrote:
> > On Mon, Apr 23, 2012 at 02:53:01PM +0800, Shawn Guo wrote:
> > > On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
> > > > On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
> > > > > On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
> ...
> > > > i.MX6 SoC. We could directly ask the devicetree in an initcall or we
> > > > could introduce a cpu_is_mx6() just like we have a macro for all other
> > > > i.MX SoCs.
> > > > 
> > > Oops, my reply was a brain-dead one.  Hmm, then it seems that we have
> > > to introduce cpu_is_mx6() which I tried hard to avoid.  I do not have
> > > a preference between defining a macro and asking device tree.
> > 
> > Since we already have a place in early setup code in which we know that
> > we are running on an i.MX6 I suggest for the sake of the symmetry of the
> > universe that we introduce a cpu_is_mx6.
> > 
> Let me try last time.  What about having a late_initcall hook in
> machine_desc?

Also fine with me.

> 
> Regards,
> Shawn
> 
> 8<---
> 
> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
> index d7692ca..0b1c94b 100644
> --- a/arch/arm/include/asm/mach/arch.h
> +++ b/arch/arm/include/asm/mach/arch.h
> @@ -43,6 +43,7 @@ struct machine_desc {
>         void                    (*init_irq)(void);
>         struct sys_timer        *timer;         /* system tick timer    */
>         void                    (*init_machine)(void);
> +       void                    (*init_late)(void);
>  #ifdef CONFIG_MULTI_IRQ_HANDLER
>         void                    (*handle_irq)(struct pt_regs *);
>  #endif
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index ebfac78..549f036 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -800,6 +800,14 @@ static int __init customize_machine(void)
>  }
>  arch_initcall(customize_machine);
> 
> +static int __init init_machine_late(void)
> +{
> +       if (machine_desc->init_late)
> +               machine_desc->init_late();
> +       return 0;
> +}
> +late_initcall(init_machine_late);
> +
>  #ifdef CONFIG_KEXEC
>  static inline unsigned long long get_total_mem(void)
>  {
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index da6c1d9..0e3640f 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)")
>         .handle_irq     = imx6q_handle_irq,
>         .timer          = &imx6q_timer,
>         .init_machine   = imx6q_init_machine,
> +       .init_late      = imx6q_init_late,
>         .dt_compat      = imx6q_dt_compat,
>         .restart        = imx6q_restart,
>  MACHINE_END
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@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 |

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Sascha Hauer]

On Mon, Apr 23, 2012 at 03:10:15PM +0800, Shawn Guo wrote:
> On Mon, Apr 23, 2012 at 08:56:23AM +0200, Sascha Hauer wrote:
> > On Mon, Apr 23, 2012 at 02:53:01PM +0800, Shawn Guo wrote:
> > > On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
> > > > On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
> > > > > On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
> ...
> > > > i.MX6 SoC. We could directly ask the devicetree in an initcall or we
> > > > could introduce a cpu_is_mx6() just like we have a macro for all other
> > > > i.MX SoCs.
> > > > 
> > > Oops, my reply was a brain-dead one.  Hmm, then it seems that we have
> > > to introduce cpu_is_mx6() which I tried hard to avoid.  I do not have
> > > a preference between defining a macro and asking device tree.
> > 
> > Since we already have a place in early setup code in which we know that
> > we are running on an i.MX6 I suggest for the sake of the symmetry of the
> > universe that we introduce a cpu_is_mx6.
> > 
> Let me try last time.  What about having a late_initcall hook in
> machine_desc?

Also fine with me.

> 
> Regards,
> Shawn
> 
> 8<---
> 
> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
> index d7692ca..0b1c94b 100644
> --- a/arch/arm/include/asm/mach/arch.h
> +++ b/arch/arm/include/asm/mach/arch.h
> @@ -43,6 +43,7 @@ struct machine_desc {
>         void                    (*init_irq)(void);
>         struct sys_timer        *timer;         /* system tick timer    */
>         void                    (*init_machine)(void);
> +       void                    (*init_late)(void);
>  #ifdef CONFIG_MULTI_IRQ_HANDLER
>         void                    (*handle_irq)(struct pt_regs *);
>  #endif
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index ebfac78..549f036 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -800,6 +800,14 @@ static int __init customize_machine(void)
>  }
>  arch_initcall(customize_machine);
> 
> +static int __init init_machine_late(void)
> +{
> +       if (machine_desc->init_late)
> +               machine_desc->init_late();
> +       return 0;
> +}
> +late_initcall(init_machine_late);
> +
>  #ifdef CONFIG_KEXEC
>  static inline unsigned long long get_total_mem(void)
>  {
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index da6c1d9..0e3640f 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)")
>         .handle_irq     = imx6q_handle_irq,
>         .timer          = &imx6q_timer,
>         .init_machine   = imx6q_init_machine,
> +       .init_late      = imx6q_init_late,
>         .dt_compat      = imx6q_dt_compat,
>         .restart        = imx6q_restart,
>  MACHINE_END
> 
> _______________________________________________
> 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 |

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

>> Let me try last time.  What about having a late_initcall hook in
>> machine_desc?
>
> Also fine with me.
>

Shall I add Shawn's patch to my imx cpuidle patchset or should the
arch/arm/kernel/setup.c and arch.h changes be submitted separately?
If separately, Shawn, did you want to submit this patch or should I?

Thanks,
Rob

>>
>> Regards,
>> Shawn
>>
>> 8<---
>>
>> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
>> index d7692ca..0b1c94b 100644
>> --- a/arch/arm/include/asm/mach/arch.h
>> +++ b/arch/arm/include/asm/mach/arch.h
>> @@ -43,6 +43,7 @@ struct machine_desc {
>>         void                    (*init_irq)(void);
>>         struct sys_timer        *timer;         /* system tick timer    */
>>         void                    (*init_machine)(void);
>> +       void                    (*init_late)(void);
>>  #ifdef CONFIG_MULTI_IRQ_HANDLER
>>         void                    (*handle_irq)(struct pt_regs *);
>>  #endif
>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
>> index ebfac78..549f036 100644
>> --- a/arch/arm/kernel/setup.c
>> +++ b/arch/arm/kernel/setup.c
>> @@ -800,6 +800,14 @@ static int __init customize_machine(void)
>>  }
>>  arch_initcall(customize_machine);
>>
>> +static int __init init_machine_late(void)
>> +{
>> +       if (machine_desc->init_late)
>> +               machine_desc->init_late();
>> +       return 0;
>> +}
>> +late_initcall(init_machine_late);
>> +
>>  #ifdef CONFIG_KEXEC
>>  static inline unsigned long long get_total_mem(void)
>>  {
>> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
>> index da6c1d9..0e3640f 100644
>> --- a/arch/arm/mach-imx/mach-imx6q.c
>> +++ b/arch/arm/mach-imx/mach-imx6q.c
>> @@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)")
>>         .handle_irq     = imx6q_handle_irq,
>>         .timer          = &imx6q_timer,
>>         .init_machine   = imx6q_init_machine,
>> +       .init_late      = imx6q_init_late,
>>         .dt_compat      = imx6q_dt_compat,
>>         .restart        = imx6q_restart,
>>  MACHINE_END
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@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 |

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

>> Let me try last time. ?What about having a late_initcall hook in
>> machine_desc?
>
> Also fine with me.
>

Shall I add Shawn's patch to my imx cpuidle patchset or should the
arch/arm/kernel/setup.c and arch.h changes be submitted separately?
If separately, Shawn, did you want to submit this patch or should I?

Thanks,
Rob

>>
>> Regards,
>> Shawn
>>
>> 8<---
>>
>> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
>> index d7692ca..0b1c94b 100644
>> --- a/arch/arm/include/asm/mach/arch.h
>> +++ b/arch/arm/include/asm/mach/arch.h
>> @@ -43,6 +43,7 @@ struct machine_desc {
>> ? ? ? ? void ? ? ? ? ? ? ? ? ? ?(*init_irq)(void);
>> ? ? ? ? struct sys_timer ? ? ? ?*timer; ? ? ? ? /* system tick timer ? ?*/
>> ? ? ? ? void ? ? ? ? ? ? ? ? ? ?(*init_machine)(void);
>> + ? ? ? void ? ? ? ? ? ? ? ? ? ?(*init_late)(void);
>> ?#ifdef CONFIG_MULTI_IRQ_HANDLER
>> ? ? ? ? void ? ? ? ? ? ? ? ? ? ?(*handle_irq)(struct pt_regs *);
>> ?#endif
>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
>> index ebfac78..549f036 100644
>> --- a/arch/arm/kernel/setup.c
>> +++ b/arch/arm/kernel/setup.c
>> @@ -800,6 +800,14 @@ static int __init customize_machine(void)
>> ?}
>> ?arch_initcall(customize_machine);
>>
>> +static int __init init_machine_late(void)
>> +{
>> + ? ? ? if (machine_desc->init_late)
>> + ? ? ? ? ? ? ? machine_desc->init_late();
>> + ? ? ? return 0;
>> +}
>> +late_initcall(init_machine_late);
>> +
>> ?#ifdef CONFIG_KEXEC
>> ?static inline unsigned long long get_total_mem(void)
>> ?{
>> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
>> index da6c1d9..0e3640f 100644
>> --- a/arch/arm/mach-imx/mach-imx6q.c
>> +++ b/arch/arm/mach-imx/mach-imx6q.c
>> @@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)")
>> ? ? ? ? .handle_irq ? ? = imx6q_handle_irq,
>> ? ? ? ? .timer ? ? ? ? ?= &imx6q_timer,
>> ? ? ? ? .init_machine ? = imx6q_init_machine,
>> + ? ? ? .init_late ? ? ?= imx6q_init_late,
>> ? ? ? ? .dt_compat ? ? ?= imx6q_dt_compat,
>> ? ? ? ? .restart ? ? ? ?= imx6q_restart,
>> ?MACHINE_END
>>
>> _______________________________________________
>> 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 |

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
> >> Let me try last time.  What about having a late_initcall hook in
> >> machine_desc?
> >
> > Also fine with me.
> >
> 
> Shall I add Shawn's patch to my imx cpuidle patchset or should the
> arch/arm/kernel/setup.c and arch.h changes be submitted separately?
> If separately, Shawn, did you want to submit this patch or should I?
> 
Strange.  Russell is not in the Cc list.  I remember I added Russell
into Cc when I propose the idea.  Added him again.

Rob,

I suggest you have changes on arch/arm/kernel/setup.c and arch.h be
a separate patch, but you can still have it in the series to show why
we need the changes.  Cc Russell when posting the series, and see if
Russell is fine with the patch.  If he is, we can ask his preference
how the patch should go in, submitting it to his patch tracker or we
can have it go though arm-soc along with the series to save the
dependency.

Regards,
Shawn

> >> 8<---
> >>
> >> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
> >> index d7692ca..0b1c94b 100644
> >> --- a/arch/arm/include/asm/mach/arch.h
> >> +++ b/arch/arm/include/asm/mach/arch.h
> >> @@ -43,6 +43,7 @@ struct machine_desc {
> >>         void                    (*init_irq)(void);
> >>         struct sys_timer        *timer;         /* system tick timer    */
> >>         void                    (*init_machine)(void);
> >> +       void                    (*init_late)(void);
> >>  #ifdef CONFIG_MULTI_IRQ_HANDLER
> >>         void                    (*handle_irq)(struct pt_regs *);
> >>  #endif
> >> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> >> index ebfac78..549f036 100644
> >> --- a/arch/arm/kernel/setup.c
> >> +++ b/arch/arm/kernel/setup.c
> >> @@ -800,6 +800,14 @@ static int __init customize_machine(void)
> >>  }
> >>  arch_initcall(customize_machine);
> >>
> >> +static int __init init_machine_late(void)
> >> +{
> >> +       if (machine_desc->init_late)
> >> +               machine_desc->init_late();
> >> +       return 0;
> >> +}
> >> +late_initcall(init_machine_late);
> >> +
> >>  #ifdef CONFIG_KEXEC
> >>  static inline unsigned long long get_total_mem(void)
> >>  {
> >> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> >> index da6c1d9..0e3640f 100644
> >> --- a/arch/arm/mach-imx/mach-imx6q.c
> >> +++ b/arch/arm/mach-imx/mach-imx6q.c
> >> @@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)")
> >>         .handle_irq     = imx6q_handle_irq,
> >>         .timer          = &imx6q_timer,
> >>         .init_machine   = imx6q_init_machine,
> >> +       .init_late      = imx6q_init_late,
> >>         .dt_compat      = imx6q_dt_compat,
> >>         .restart        = imx6q_restart,
> >>  MACHINE_END

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
> >> Let me try last time. ?What about having a late_initcall hook in
> >> machine_desc?
> >
> > Also fine with me.
> >
> 
> Shall I add Shawn's patch to my imx cpuidle patchset or should the
> arch/arm/kernel/setup.c and arch.h changes be submitted separately?
> If separately, Shawn, did you want to submit this patch or should I?
> 
Strange.  Russell is not in the Cc list.  I remember I added Russell
into Cc when I propose the idea.  Added him again.

Rob,

I suggest you have changes on arch/arm/kernel/setup.c and arch.h be
a separate patch, but you can still have it in the series to show why
we need the changes.  Cc Russell when posting the series, and see if
Russell is fine with the patch.  If he is, we can ask his preference
how the patch should go in, submitting it to his patch tracker or we
can have it go though arm-soc along with the series to save the
dependency.

Regards,
Shawn

> >> 8<---
> >>
> >> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
> >> index d7692ca..0b1c94b 100644
> >> --- a/arch/arm/include/asm/mach/arch.h
> >> +++ b/arch/arm/include/asm/mach/arch.h
> >> @@ -43,6 +43,7 @@ struct machine_desc {
> >> ? ? ? ? void ? ? ? ? ? ? ? ? ? ?(*init_irq)(void);
> >> ? ? ? ? struct sys_timer ? ? ? ?*timer; ? ? ? ? /* system tick timer ? ?*/
> >> ? ? ? ? void ? ? ? ? ? ? ? ? ? ?(*init_machine)(void);
> >> + ? ? ? void ? ? ? ? ? ? ? ? ? ?(*init_late)(void);
> >> ?#ifdef CONFIG_MULTI_IRQ_HANDLER
> >> ? ? ? ? void ? ? ? ? ? ? ? ? ? ?(*handle_irq)(struct pt_regs *);
> >> ?#endif
> >> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> >> index ebfac78..549f036 100644
> >> --- a/arch/arm/kernel/setup.c
> >> +++ b/arch/arm/kernel/setup.c
> >> @@ -800,6 +800,14 @@ static int __init customize_machine(void)
> >> ?}
> >> ?arch_initcall(customize_machine);
> >>
> >> +static int __init init_machine_late(void)
> >> +{
> >> + ? ? ? if (machine_desc->init_late)
> >> + ? ? ? ? ? ? ? machine_desc->init_late();
> >> + ? ? ? return 0;
> >> +}
> >> +late_initcall(init_machine_late);
> >> +
> >> ?#ifdef CONFIG_KEXEC
> >> ?static inline unsigned long long get_total_mem(void)
> >> ?{
> >> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> >> index da6c1d9..0e3640f 100644
> >> --- a/arch/arm/mach-imx/mach-imx6q.c
> >> +++ b/arch/arm/mach-imx/mach-imx6q.c
> >> @@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)")
> >> ? ? ? ? .handle_irq ? ? = imx6q_handle_irq,
> >> ? ? ? ? .timer ? ? ? ? ?= &imx6q_timer,
> >> ? ? ? ? .init_machine ? = imx6q_init_machine,
> >> + ? ? ? .init_late ? ? ?= imx6q_init_late,
> >> ? ? ? ? .dt_compat ? ? ?= imx6q_dt_compat,
> >> ? ? ? ? .restart ? ? ? ?= imx6q_restart,
> >> ?MACHINE_END

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Russell King - ARM Linux]

On Tue, Apr 24, 2012 at 09:38:43AM +0800, Shawn Guo wrote:
> On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
> > >> Let me try last time.  What about having a late_initcall hook in
> > >> machine_desc?
> > >
> > > Also fine with me.
> > >
> > 
> > Shall I add Shawn's patch to my imx cpuidle patchset or should the
> > arch/arm/kernel/setup.c and arch.h changes be submitted separately?
> > If separately, Shawn, did you want to submit this patch or should I?
> > 
> Strange.  Russell is not in the Cc list.  I remember I added Russell
> into Cc when I propose the idea.  Added him again.

I didn't see any message in this thread cc'd to me, but that's not to
say I hadn't already read this patch.  I don't have any comment against
it, but I do wonder how often this hook would be used.

We do seem to have quite a number of late_initcall()s in arch/arm/mach-*,
so it seems to be a good idea - provided someone's willing to convert all
those users of late_initcall()s.

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Russell King - ARM Linux]

On Tue, Apr 24, 2012 at 09:38:43AM +0800, Shawn Guo wrote:
> On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
> > >> Let me try last time. ?What about having a late_initcall hook in
> > >> machine_desc?
> > >
> > > Also fine with me.
> > >
> > 
> > Shall I add Shawn's patch to my imx cpuidle patchset or should the
> > arch/arm/kernel/setup.c and arch.h changes be submitted separately?
> > If separately, Shawn, did you want to submit this patch or should I?
> > 
> Strange.  Russell is not in the Cc list.  I remember I added Russell
> into Cc when I propose the idea.  Added him again.

I didn't see any message in this thread cc'd to me, but that's not to
say I hadn't already read this patch.  I don't have any comment against
it, but I do wonder how often this hook would be used.

We do seem to have quite a number of late_initcall()s in arch/arm/mach-*,
so it seems to be a good idea - provided someone's willing to convert all
those users of late_initcall()s.

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Tue, Apr 24, 2012 at 08:54:26AM +0100, Russell King - ARM Linux wrote:
> On Tue, Apr 24, 2012 at 09:38:43AM +0800, Shawn Guo wrote:
> > On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
> > > >> Let me try last time.  What about having a late_initcall hook in
> > > >> machine_desc?
> > > >
> > > > Also fine with me.
> > > >
> > > 
> > > Shall I add Shawn's patch to my imx cpuidle patchset or should the
> > > arch/arm/kernel/setup.c and arch.h changes be submitted separately?
> > > If separately, Shawn, did you want to submit this patch or should I?
> > > 
> > Strange.  Russell is not in the Cc list.  I remember I added Russell
> > into Cc when I propose the idea.  Added him again.
> 
> I didn't see any message in this thread cc'd to me, but that's not to
> say I hadn't already read this patch.  I don't have any comment against
> it, but I do wonder how often this hook would be used.
> 
I guess mach-* that use common cpuidle will likely need this hook.

> We do seem to have quite a number of late_initcall()s in arch/arm/mach-*,
> so it seems to be a good idea - provided someone's willing to convert all
> those users of late_initcall()s.

Agreed.  The late_initcall()s in arch/arm/mach-* will not scale for
long time, since we are moving toward single build.

-- 
Regards,
Shawn

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On Tue, Apr 24, 2012 at 08:54:26AM +0100, Russell King - ARM Linux wrote:
> On Tue, Apr 24, 2012 at 09:38:43AM +0800, Shawn Guo wrote:
> > On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
> > > >> Let me try last time. ?What about having a late_initcall hook in
> > > >> machine_desc?
> > > >
> > > > Also fine with me.
> > > >
> > > 
> > > Shall I add Shawn's patch to my imx cpuidle patchset or should the
> > > arch/arm/kernel/setup.c and arch.h changes be submitted separately?
> > > If separately, Shawn, did you want to submit this patch or should I?
> > > 
> > Strange.  Russell is not in the Cc list.  I remember I added Russell
> > into Cc when I propose the idea.  Added him again.
> 
> I didn't see any message in this thread cc'd to me, but that's not to
> say I hadn't already read this patch.  I don't have any comment against
> it, but I do wonder how often this hook would be used.
> 
I guess mach-* that use common cpuidle will likely need this hook.

> We do seem to have quite a number of late_initcall()s in arch/arm/mach-*,
> so it seems to be a good idea - provided someone's willing to convert all
> those users of late_initcall()s.

Agreed.  The late_initcall()s in arch/arm/mach-* will not scale for
long time, since we are moving toward single build.

-- 
Regards,
Shawn

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

On Tue, Apr 24, 2012 at 3:36 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Tue, Apr 24, 2012 at 08:54:26AM +0100, Russell King - ARM Linux wrote:
>> On Tue, Apr 24, 2012 at 09:38:43AM +0800, Shawn Guo wrote:
>> > On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
>> > > >> Let me try last time.  What about having a late_initcall hook in
>> > > >> machine_desc?
>> > > >
>> > > > Also fine with me.
>> > > >
>> > >
>> > > Shall I add Shawn's patch to my imx cpuidle patchset or should the
>> > > arch/arm/kernel/setup.c and arch.h changes be submitted separately?
>> > > If separately, Shawn, did you want to submit this patch or should I?
>> > >
>> > Strange.  Russell is not in the Cc list.  I remember I added Russell
>> > into Cc when I propose the idea.  Added him again.
>>
>> I didn't see any message in this thread cc'd to me, but that's not to
>> say I hadn't already read this patch.  I don't have any comment against
>> it, but I do wonder how often this hook would be used.
>>
> I guess mach-* that use common cpuidle will likely need this hook.
>
>> We do seem to have quite a number of late_initcall()s in arch/arm/mach-*,
>> so it seems to be a good idea - provided someone's willing to convert all
>> those users of late_initcall()s.
>
> Agreed.  The late_initcall()s in arch/arm/mach-* will not scale for
> long time, since we are moving toward single build.
>

Thanks for the attention on this.  From what I've understood, I will
send another submission that includes the imx cpuidle patchset and
Shawn's device tree late initcall patchset and I'll provide
explanation of the two separate patchsets in the cover sheet.

> --
> Regards,
> Shawn

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Rob Lee]

On Tue, Apr 24, 2012 at 3:36 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Tue, Apr 24, 2012 at 08:54:26AM +0100, Russell King - ARM Linux wrote:
>> On Tue, Apr 24, 2012 at 09:38:43AM +0800, Shawn Guo wrote:
>> > On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
>> > > >> Let me try last time. ?What about having a late_initcall hook in
>> > > >> machine_desc?
>> > > >
>> > > > Also fine with me.
>> > > >
>> > >
>> > > Shall I add Shawn's patch to my imx cpuidle patchset or should the
>> > > arch/arm/kernel/setup.c and arch.h changes be submitted separately?
>> > > If separately, Shawn, did you want to submit this patch or should I?
>> > >
>> > Strange. ?Russell is not in the Cc list. ?I remember I added Russell
>> > into Cc when I propose the idea. ?Added him again.
>>
>> I didn't see any message in this thread cc'd to me, but that's not to
>> say I hadn't already read this patch. ?I don't have any comment against
>> it, but I do wonder how often this hook would be used.
>>
> I guess mach-* that use common cpuidle will likely need this hook.
>
>> We do seem to have quite a number of late_initcall()s in arch/arm/mach-*,
>> so it seems to be a good idea - provided someone's willing to convert all
>> those users of late_initcall()s.
>
> Agreed. ?The late_initcall()s in arch/arm/mach-* will not scale for
> long time, since we are moving toward single build.
>

Thanks for the attention on this.  From what I've understood, I will
send another submission that includes the imx cpuidle patchset and
Shawn's device tree late initcall patchset and I'll provide
explanation of the two separate patchsets in the cover sheet.

> --
> Regards,
> Shawn

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Russell King - ARM Linux]

On Tue, Apr 24, 2012 at 10:40:35AM -0500, Rob Lee wrote:
> Thanks for the attention on this.  From what I've understood, I will
> send another submission that includes the imx cpuidle patchset and
> Shawn's device tree late initcall patchset and I'll provide
> explanation of the two separate patchsets in the cover sheet.

What I also want to see _along with_ the addition of the init_late
callback is a patch set from the _same_ people fixing up the rest
of arch/arm/mach-* to use it - you know, like what I do when I
introduce these kinds of things.

Otherwise I'm going to NAK the change.  We can't have new stuff
introduced in this way and the older platforms ignored.  Forward
progress across the board please.

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Russell King - ARM Linux]

On Tue, Apr 24, 2012 at 10:40:35AM -0500, Rob Lee wrote:
> Thanks for the attention on this.  From what I've understood, I will
> send another submission that includes the imx cpuidle patchset and
> Shawn's device tree late initcall patchset and I'll provide
> explanation of the two separate patchsets in the cover sheet.

What I also want to see _along with_ the addition of the init_late
callback is a patch set from the _same_ people fixing up the rest
of arch/arm/mach-* to use it - you know, like what I do when I
introduce these kinds of things.

Otherwise I'm going to NAK the change.  We can't have new stuff
introduced in this way and the older platforms ignored.  Forward
progress across the board please.

Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On 25 April 2012 03:51, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Tue, Apr 24, 2012 at 10:40:35AM -0500, Rob Lee wrote:
>> Thanks for the attention on this.  From what I've understood, I will
>> send another submission that includes the imx cpuidle patchset and
>> Shawn's device tree late initcall patchset and I'll provide
>> explanation of the two separate patchsets in the cover sheet.
>
> What I also want to see _along with_ the addition of the init_late
> callback is a patch set from the _same_ people fixing up the rest
> of arch/arm/mach-* to use it - you know, like what I do when I
> introduce these kinds of things.
>
> Otherwise I'm going to NAK the change.  We can't have new stuff
> introduced in this way and the older platforms ignored.  Forward
> progress across the board please.

I'm working on a series to clean them up.

Regards,
Shawn

[PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.

[Shawn Guo]

On 25 April 2012 03:51, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Tue, Apr 24, 2012 at 10:40:35AM -0500, Rob Lee wrote:
>> Thanks for the attention on this. ?From what I've understood, I will
>> send another submission that includes the imx cpuidle patchset and
>> Shawn's device tree late initcall patchset and I'll provide
>> explanation of the two separate patchsets in the cover sheet.
>
> What I also want to see _along with_ the addition of the init_late
> callback is a patch set from the _same_ people fixing up the rest
> of arch/arm/mach-* to use it - you know, like what I do when I
> introduce these kinds of things.
>
> Otherwise I'm going to NAK the change. ?We can't have new stuff
> introduced in this way and the older platforms ignored. ?Forward
> progress across the board please.

I'm working on a series to clean them up.

Regards,
Shawn