On 08/23/2013 08:46 AM, Wang Dongsheng-B40534 wrote:

quoted

diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index 0e2cd5c..e805dcd 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig

Maybe drivers/cpuidle/Kconfig.powerpc is better? Like arm.

quoted

+obj-$(CONFIG_CPU_IDLE_IBM_POWER) += cpuidle-ibm-power.o

diff --git a/drivers/cpuidle/cpuidle-ibm-power.c

b/drivers/cpuidle/cpuidle-ibm-power.c
new file mode 100644
index 0000000..4ee5a94

--- /dev/null
+++ b/drivers/cpuidle/cpuidle-ibm-power.c

@@ -0,0 +1,304 @@
+/*
+ *  cpuidle-ibm-power - idle state cpuidle driver.
+ *  Adapted from drivers/idle/intel_idle.c and
+ *  drivers/acpi/processor_idle.c
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/moduleparam.h>
+#include <linux/cpuidle.h>
+#include <linux/cpu.h>
+#include <linux/notifier.h>
+
+#include <asm/paca.h>
+#include <asm/reg.h>
+#include <asm/machdep.h>
+#include <asm/firmware.h>
+#include <asm/runlatch.h>
+#include <asm/plpar_wrappers.h>
+
+struct cpuidle_driver power_idle_driver = {
+	.name             = "IBM-POWER-Idle",
+	.owner            = THIS_MODULE,
+};
+
+#define MAX_IDLE_STATE_COUNT	2
+
+static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;

Again, do not use the macro.

Wanting to re-use CPUIDLE_STATE_MAX macro, defined in linux/cpuidle.h
similar to what x86 uses. This is def scalable for various platforms ,
as demonstrated in drivers/idle/intel_idle.c

 >

quoted

+static struct cpuidle_state *cpuidle_state_table;
+
+static inline void idle_loop_prolog(unsigned long *in_purr)
+{
+	*in_purr = mfspr(SPRN_PURR);
+	/*
+	 * Indicate to the HV that we are idle. Now would be
+	 * a good time to find other work to dispatch.
+	 */
+	get_lppaca()->idle = 1;
+}
+
+static inline void idle_loop_epilog(unsigned long in_purr)
+{
+	get_lppaca()->wait_state_cycles += mfspr(SPRN_PURR) - in_purr;
+	get_lppaca()->idle = 0;
+}
+
+static int snooze_loop(struct cpuidle_device *dev,
+			struct cpuidle_driver *drv,
+			int index)
+{
+	unsigned long in_purr;
+
+	idle_loop_prolog(&in_purr);
+	local_irq_enable();

snooze_loop has already registered in cpuidle framework to handle snooze state.
where disable the irq? Why do "enable" here?

quoted

+/*
+ * States for dedicated partition case.
+ */
+static struct cpuidle_state dedicated_states[MAX_IDLE_STATE_COUNT] = {
+	{ /* Snooze */
+		.name = "snooze",
+		.desc = "snooze",
+		.flags = CPUIDLE_FLAG_TIME_VALID,
+		.exit_latency = 0,
+		.target_residency = 0,
+		.enter = &snooze_loop },
+	{ /* CEDE */
+		.name = "CEDE",
+		.desc = "CEDE",
+		.flags = CPUIDLE_FLAG_TIME_VALID,
+		.exit_latency = 10,
+		.target_residency = 100,
+		.enter = &dedicated_cede_loop },
+};
+
+/*
+ * States for shared partition case.
+ */
+static struct cpuidle_state shared_states[MAX_IDLE_STATE_COUNT] = {
+	{ /* Shared Cede */
+		.name = "Shared Cede",
+		.desc = "Shared Cede",
+		.flags = CPUIDLE_FLAG_TIME_VALID,
+		.exit_latency = 0,
+		.target_residency = 0,
+		.enter = &shared_cede_loop },
+};
+
+static void __exit power_processor_idle_exit(void)
+{
+
+	unregister_cpu_notifier(&setup_hotplug_notifier);

Remove a blank line.

quoted

+	cpuidle_unregister(&power_idle_driver);
+	return;
+}
+
+module_init(power_processor_idle_init);
+module_exit(power_processor_idle_exit);
+

Did you have tested the module? If not tested, please don't use the module.

quoted

+MODULE_AUTHOR("Deepthi Dharwar [off-list ref]");
+MODULE_DESCRIPTION("Cpuidle driver for IBM POWER platforms");
+MODULE_LICENSE("GPL");