[PATCH v3 1/7] cpuidle: Add common init interface and idle functionality
From: Turquette, Mike <hidden>
Date: 2012-01-24 23:46:55
On Mon, Jan 23, 2012 at 8:37 PM, Robert Lee [off-list ref] wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/cpuidle/common.c b/drivers/cpuidle/common.c new file mode 100644 index 0000000..dafa758 --- /dev/null +++ b/drivers/cpuidle/common.c@@ -0,0 +1,152 @@ +/* + * Copyright 2011 Freescale Semiconductor, Inc. + * Copyright 2011 Linaro Ltd. + * + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 or later at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +/* + * This code performs provides some commonly used cpuidle setup functionality
s/performs //
+static struct cpuidle_device __percpu * common_cpuidle_devices;
You can use DECLARE_PER_CPU here. Is there any particular reason to allocate these dynamically? You can replace the code above with, static DEFINE_PER_CPU(struct cpuidle_device, common_cpuidle_devices); I might change the variable name to "cpu_cpuidle_device" in that case since you are addressing a single CPU when using the per cpu accessor functions and "common_cpuidle_devices" sounds like an array or a list or something. No big deal to keep the current name though.
+static int simple_enter(struct cpuidle_device *dev,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cpuidle_driver *drv, int index)
+{
+ ? ? ? ktime_t time_start, time_end;
+
+ ? ? ? local_irq_disable();
+
+ ? ? ? time_start = ktime_get();
+
+ ? ? ? index = do_idle[index](dev, drv, index);
+
+ ? ? ? time_end = ktime_get();
+
+ ? ? ? local_irq_enable();
+
+ ? ? ? dev->last_residency =
+ ? ? ? ? ? ? ? (int)ktime_to_us(ktime_sub(time_end, time_start));
Sometimes an attempt to enter some C-state fails and the do_idle will
return immediately. What do you think about having do_idle return
-EERROR in this case and the conditionally setting last_residency to
zero in those cases? The point is that a C-state's total residency
time should not increase in the case where the hardware did not
successfully transition into that C-state. I've observed many times
where a specific low power state was actually achieved in the hardware
but /sys/devices/system/cpu/cpuN/cpuidle/stateM/time keeps
incrementing (albeit in very tiny increments). Something like,
if (IS_ERR(index))
dev->last_residency = 0;
else
...
Note: I haven't been through the CPUidle core in a while so maybe the
above suggestion violates some other requirements/assumptions...
+
+ ? ? ? return index;
+}
+
+void common_cpuidle_devices_uninit(void)
+{
+ ? ? ? int cpu_id;
+ ? ? ? struct cpuidle_device *dev;
+
+ ? ? ? for_each_possible_cpu(cpu_id) {
+ ? ? ? ? ? ? ? dev = per_cpu_ptr(common_cpuidle_devices, cpu_id);
+ ? ? ? ? ? ? ? cpuidle_unregister_device(dev);
+ ? ? ? }
+
+ ? ? ? free_percpu(common_cpuidle_devices);See note above about statically allocating the per-cpu variables.
+}
+
+/**
+ * common_cpuidle_init() - Provides some commonly used init functionality.
+ * @pdrv ? ? ? ? ? ? ? Pointer to your cpuidle_driver object.
+ * @simple ? ? ? ? ? ? Use the common simple_enter wrapper?
+ * @driver_data_init ? Pointer to your platform function to initialize your
+ * ? ? ? ? ? ? ? ? ? ? platform specific driver data. ?Use NULL if platform
+ * ? ? ? ? ? ? ? ? ? ? specific data is not needed.
+ *
+ * Common cpuidle init interface to provide common cpuidle functionality
+ * used by many platforms.
+ */
+int __init common_cpuidle_init(struct cpuidle_driver *pdrv, bool simple,
+ ? ? ? ? ? ? ? ? ? ? ? ?void (*driver_data_init)(struct cpuidle_device *dev))
+{
+ ? ? ? struct cpuidle_device *dev;
+ ? ? ? struct cpuidle_driver *drv;
+ ? ? ? int i, cpu_id, ret;
+
+ ? ? ? if (!pdrv || pdrv->state_count > CPUIDLE_STATE_MAX) {
+ ? ? ? ? ? ? ? pr_err("%s: Invalid Input\n", __func__);
+ ? ? ? ? ? ? ? return -EINVAL;
+ ? ? ? }
+
+ ? ? ? drv = kmalloc(sizeof(struct cpuidle_driver), GFP_KERNEL);
+ ? ? ? if (!drv) {
+ ? ? ? ? ? ? ? pr_err("%s: no memory for cpuidle driver\n", __func__);
+ ? ? ? ? ? ? ? return -ENOMEM;
+ ? ? ? }
+
+ ? ? ? *drv = *pdrv;
+
+ ? ? ? for (i = 0; simple && (i < drv->state_count); i++) {
+ ? ? ? ? ? ? ? do_idle[i] = drv->states[i].enter;
+ ? ? ? ? ? ? ? drv->states[i].enter = simple_enter;
+ ? ? ? }
+
+ ? ? ? ret = cpuidle_register_driver(drv);
+ ? ? ? if (ret) {
+ ? ? ? ? ? ? ? pr_err("%s: Failed to register cpuidle driver\n", __func__);
+ ? ? ? ? ? ? ? goto free_drv;
+ ? ? ? }
+
+ ? ? ? common_cpuidle_devices = alloc_percpu(struct cpuidle_device);
+ ? ? ? if (common_cpuidle_devices == NULL) {
+ ? ? ? ? ? ? ? ret = -ENOMEM;
+ ? ? ? ? ? ? ? goto unregister_drv;
+ ? ? ? }See note above about statically allocating these. Regards, Mike