linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: daniel.lezcano@linaro.org (Daniel Lezcano)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 05/18] cpuidle: make a single register function for all
Date: Wed, 10 Apr 2013 16:22:10 +0200	[thread overview]
Message-ID: <1365603743-5618-6-git-send-email-daniel.lezcano@linaro.org> (raw)
In-Reply-To: <1365603743-5618-1-git-send-email-daniel.lezcano@linaro.org>

The usual scheme to initialize a cpuidle driver on a SMP is:

	cpuidle_register_driver(drv);
	for_each_possible_cpu(cpu) {
		device = &per_cpu(cpuidle_dev, cpu);
		cpuidle_register_device(device);
	}

This code is duplicated in each cpuidle driver.

On UP systems, it is done this way:

	cpuidle_register_driver(drv);
	device = &per_cpu(cpuidle_dev, cpu);
	cpuidle_register_device(device);

On UP, the macro 'for_each_cpu' does one iteration:

#define for_each_cpu(cpu, mask)                 \
        for ((cpu) = 0; (cpu) < 1; (cpu)++, (void)mask)

Hence, the initialization loop is the same for UP than SMP.

Beside, we saw different bugs / mis-initialization / return code unchecked in
the different drivers, the code is duplicated including bugs. After fixing all
these ones, it appears the initialization pattern is the same for everyone.

Let's add a wrapper function doing this initialization with a cpumask parameter
for the coupled idle states and use it for all the drivers.

That will save a lot of LOC, consolidate the code, and the modifications in the
future could be done in a single place. Another benefit is the consolidation of
the cpuidle_device variable which is now in the cpuidle framework and no longer
spread accross the different arch specific drivers.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/cpuidle/cpuidle.c |   65 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/cpuidle.h   |    9 +++++--
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 0da795b..3a5454b 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -24,6 +24,7 @@
 #include "cpuidle.h"
 
 DEFINE_PER_CPU(struct cpuidle_device *, cpuidle_devices);
+DEFINE_PER_CPU(struct cpuidle_device, cpuidle_dev);
 
 DEFINE_MUTEX(cpuidle_lock);
 LIST_HEAD(cpuidle_detected_devices);
@@ -453,6 +454,70 @@ void cpuidle_unregister_device(struct cpuidle_device *dev)
 
 EXPORT_SYMBOL_GPL(cpuidle_unregister_device);
 
+/*
+ * cpuidle_unregister: unregister a driver and the devices. This function
+ * can be used only if the driver has been previously registered through
+ * the cpuidle_register function.
+ *
+ * @drv: a valid pointer to a struct cpuidle_driver
+ */
+void cpuidle_unregister(struct cpuidle_driver *drv)
+{
+	int cpu;
+	struct cpuidle_device *device;
+
+	for_each_possible_cpu(cpu) {
+		device = &per_cpu(cpuidle_dev, cpu);
+		cpuidle_unregister_device(device);
+	}
+
+	cpuidle_unregister_driver(drv);
+}
+EXPORT_SYMBOL_GPL(cpuidle_unregister);
+
+/**
+ * cpuidle_register: registers the driver and the cpu devices with the
+ * coupled_cpus passed as parameter. This function is used for all common
+ * initialization pattern there are in the arch specific drivers. The
+ * devices is globally defined in this file.
+ *
+ * @drv         : a valid pointer to a struct cpuidle_driver
+ * @coupled_cpus: a cpumask for the coupled states
+ *
+ * Returns 0 on success, < 0 otherwise
+ */
+int cpuidle_register(struct cpuidle_driver *drv,
+		     const struct cpumask *const coupled_cpus)
+{
+	int ret, cpu;
+	struct cpuidle_device *device;
+
+	ret = cpuidle_register_driver(drv);
+	if (ret) {
+		printk(KERN_ERR "failed to register cpuidle driver\n");
+		return ret;
+	}
+
+	for_each_possible_cpu(cpu) {
+		device = &per_cpu(cpuidle_dev, cpu);
+		device->cpu = cpu;
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
+		device->coupled_cpus = *coupled_cpus;
+#endif
+		ret = cpuidle_register_device(device);
+		if (!ret)
+			continue;
+
+		printk(KERN_ERR "Failed to register cpuidle "
+		       "device for cpu%d\n", cpu);
+		cpuidle_unregister(drv);
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cpuidle_register);
+
 #ifdef CONFIG_SMP
 
 static void smp_callback(void *v)
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 79e3811..3c86faa 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -123,7 +123,9 @@ extern void cpuidle_driver_unref(void);
 extern void cpuidle_unregister_driver(struct cpuidle_driver *drv);
 extern int cpuidle_register_device(struct cpuidle_device *dev);
 extern void cpuidle_unregister_device(struct cpuidle_device *dev);
-
+extern int cpuidle_register(struct cpuidle_driver *drv,
+			    const struct cpumask *const coupled_cpus);
+extern void cpuidle_unregister(struct cpuidle_driver *drv);
 extern void cpuidle_pause_and_lock(void);
 extern void cpuidle_resume_and_unlock(void);
 extern void cpuidle_pause(void);
@@ -148,7 +150,10 @@ static inline void cpuidle_unregister_driver(struct cpuidle_driver *drv) { }
 static inline int cpuidle_register_device(struct cpuidle_device *dev)
 {return -ENODEV; }
 static inline void cpuidle_unregister_device(struct cpuidle_device *dev) { }
-
+static inline int cpuidle_register(struct cpuidle_driver *drv,
+				   const struct cpumask *const coupled_cpus)
+{return -ENODEV; }
+static inline void cpuidle_unregister(struct cpuidle_driver *drv) { }
 static inline void cpuidle_pause_and_lock(void) { }
 static inline void cpuidle_resume_and_unlock(void) { }
 static inline void cpuidle_pause(void) { }
-- 
1.7.9.5

  parent reply	other threads:[~2013-04-10 14:22 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-10 14:22 [PATCH 00/18] cpuidle: code consolidation Daniel Lezcano
2013-04-10 14:22 ` [PATCH 01/18] ARM: OMAP3: remove cpuidle_wrap_enter Daniel Lezcano
2013-04-10 14:22 ` [PATCH 02/18] cpuidle: remove en_core_tk_irqen flag Daniel Lezcano
2013-04-10 14:22 ` [PATCH 03/18] ARM: ux500: cpuidle: replace for_each_online_cpu by for_each_possible_cpu Daniel Lezcano
2013-04-10 14:22 ` [PATCH 04/18] ARM: imx: cpuidle: create separate drivers for imx5/imx6 Daniel Lezcano
2013-04-12  6:05   ` Shawn Guo
2013-04-12  6:58     ` Daniel Lezcano
2013-04-12  7:11       ` Shawn Guo
2013-04-12  7:12         ` Daniel Lezcano
2013-04-10 14:22 ` Daniel Lezcano [this message]
2013-04-10 14:59   ` [PATCH 05/18] cpuidle: make a single register function for all Rob Herring
2013-04-10 15:05     ` Daniel Lezcano
2013-04-22  6:57     ` Daniel Lezcano
2013-04-10 16:55   ` Andrew Lunn
2013-04-10 18:04     ` Daniel Lezcano
2013-04-10 18:18       ` Andrew Lunn
2013-04-10 22:07         ` Daniel Lezcano
2013-04-10 17:04   ` Andrew Lunn
2013-04-10 18:02     ` Daniel Lezcano
2013-04-10 18:23       ` Andrew Lunn
2013-04-10 19:59         ` Daniel Lezcano
2013-04-10 18:22   ` Colin Cross
2013-04-10 19:59     ` Daniel Lezcano
2013-04-10 14:22 ` [PATCH 06/18] ARM: ux500: cpuidle: use init/exit common routine Daniel Lezcano
2013-04-10 14:22 ` [PATCH 07/18] ARM: at91: " Daniel Lezcano
2013-04-10 14:39   ` Nicolas Ferre
2013-04-10 14:22 ` [PATCH 08/18] ARM: OMAP3: " Daniel Lezcano
2013-04-10 14:22 ` [PATCH 09/18] ARM: s3c64xx: " Daniel Lezcano
2013-04-10 14:22 ` [PATCH 10/18] ARM: tegra1: " Daniel Lezcano
2013-04-10 17:29   ` Stephen Warren
2013-04-10 18:05     ` Daniel Lezcano
2013-04-10 14:22 ` [PATCH 11/18] ARM: shmobile: " Daniel Lezcano
2013-04-11  2:46   ` Simon Horman
2013-04-10 14:22 ` [PATCH 12/18] ARM: OMAP4: " Daniel Lezcano
2013-04-10 14:22 ` [PATCH 13/18] ARM: tegra2: " Daniel Lezcano
2013-04-11  1:01   ` Joseph Lo
2013-04-11  5:58     ` Daniel Lezcano
2013-04-10 14:22 ` [PATCH 14/18] ARM: tegra3: " Daniel Lezcano
2013-04-10 14:22 ` [PATCH 15/18] ARM: calxeda: " Daniel Lezcano
2013-04-10 14:31   ` Rob Herring
2013-04-10 14:22 ` [PATCH 16/18] ARM: kirkwood: " Daniel Lezcano
2013-04-10 14:22 ` [PATCH 17/18] ARM: davinci: " Daniel Lezcano
2013-04-10 14:22 ` [PATCH 18/18] ARM: imx: " Daniel Lezcano
2013-04-10 19:12 ` [PATCH 00/18] cpuidle: code consolidation Rafael J. Wysocki
2013-04-10 20:01   ` Daniel Lezcano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1365603743-5618-6-git-send-email-daniel.lezcano@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).