[PATCH v3 1/7] cpuidle: Add common init interface and idle functionality

On 01/23/2012 10:37 PM, Robert Lee wrote:

The patch adds some cpuidle initialization functionality commonly
duplicated by many platforms.

Signed-off-by: Robert Lee <redacted>

A few cosmetic comments below, but otherwise looks good.

Reviewed-by: Rob Herring <redacted>

2012 now...

+ *
+ * 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
+ * used by many ARM SoC platforms.  Providing this functionality here
+ * reduces the duplication of this code for each ARM platform that uses it.
+ */
+
+#include <linux/kernel.h>
+#include <linux/io.h>
+#include <linux/cpuidle.h>
+#include <linux/hrtimer.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <asm/proc-fns.h>
+
+static struct cpuidle_device __percpu * common_cpuidle_devices;
+
+static int (*do_idle[CPUIDLE_STATE_MAX])(struct cpuidle_device *dev,
+			       struct cpuidle_driver *drv, int index);
+
+int cpuidle_def_idle(struct cpuidle_device *dev,
+			       struct cpuidle_driver *drv, int index)
+{
+	cpu_do_idle();
+	return index;
+}
+
+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));
+
+	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);
+}
+
+/**
+ * common_cpuidle_init() - Provides some commonly used init functionality.
+ * @pdrv		Pointer to your cpuidle_driver object.
+ * @simple		Use the common simple_enter wrapper?

remove the ?

+ * @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__);

Using pr_fmt rather than function name is preferred.

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

Perhaps a comment so it is clear you intend this to be struct copy.

+
+	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;
+	}
+
+	/* initialize state data for each cpuidle_device */
+	for_each_possible_cpu(cpu_id) {
+		dev = per_cpu_ptr(common_cpuidle_devices, cpu_id);
+		dev->cpu = cpu_id;
+		dev->state_count = drv->state_count;
+
+		if (driver_data_init)
+			driver_data_init(dev);
+
+		ret = cpuidle_register_device(dev);
+		if (ret) {
+			pr_err("%s: Failed to register cpu %u\n",
+				__func__, cpu_id);
+			goto uninit;
+		}
+	}
+
+	return 0;
+uninit:
+
+	common_cpuidle_devices_uninit();
+
+unregister_drv:
+
+	cpuidle_unregister_driver(drv);
+
+free_drv:
+
+	kfree(drv);
+
+	return ret;

Remove all the blank lines in the error handling.