On 03/18/2015 02:14 AM, Stephen Boyd wrote:
On 03/17/15 04:29, Lorenzo Pieralisi wrote:
On Mon, Mar 16, 2015 at 10:08:19PM +0000, Stephen Boyd wrote:
On 03/03/15 04:29, Daniel Lezcano wrote:
The code is optimized to use the __init section intensively in order to reduce
the memory footprint after the driver is initialized and unify the function
names with ARM64.
In order to prevent multiple declarations and the specific cpuidle ops to be
spread across the different headers, a mechanism, similar to the cgroup subsys,
has been introduced.
A new platform willing to add its cpuidle ops must add an entry in the file
cpuidle_ops.h in the current form:
#if IS_ENABLED(CONFIG_ARM_FOO_CPUIDLE)
CPUIDLE_OPS(foo)
#endif
... and use the variable name in the specific low level code:
struct cpuidle_ops foo_cpuidle_ops;
The CPUIDLE_OPS macro will be processed in different way in the cpuidle.c file,
thus allowing to keep untouched the arm cpuidle core code in the future when
a new platform is added.
[...]
diff --git a/arch/arm/include/asm/cpuidle_ops.h b/arch/arm/include/asm/cpuidle_ops.h
new file mode 100644
index 0000000..be0a612
--- /dev/null
+++ b/arch/arm/include/asm/cpuidle_ops.h
@@ -0,0 +1,3 @@
+/*
+ * List of cpuidle operations
+ */
diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
index 45969f8..25e9789c 100644
--- a/arch/arm/kernel/cpuidle.c
+++ b/arch/arm/kernel/cpuidle.c
@@ -10,8 +10,29 @@
*/
#include <linux/cpuidle.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
#include <asm/cpuidle.h>
+#define CPUIDLE_OPS(__x) extern struct cpuidle_ops __x ## _cpuidle_ops;
+#include <asm/cpuidle_ops.h>
+#undef CPUIDLE_OPS
+
+#define CPUIDLE_OPS(__x) __x ## _cpuidle_ops_id,
+enum cpuidle_ops_id {
+#include <asm/cpuidle_ops.h>
+ CPUIDLE_OPS_COUNT,
+};
+#undef CPUIDLE_OPS
+
+#define CPUIDLE_OPS(__x) [__x ## _cpuidle_ops_id ] = &__x ## _cpuidle_ops,
+static struct cpuidle_ops *supported_cpuidle_ops[] __initconst = {
+#include <asm/cpuidle_ops.h>
+};
+#undef CPUIDLE_OPS
+
+static struct cpuidle_ops cpuidle_ops[NR_CPUS];
Is there any reason why we aren't putting these structures into a linker
section like we do for the smp operations structures?
I think it can be done with an OF_TABLE, it is a bit of shame cpuidle_ops
should work on UP too otherwise they could have been merged in
smp_ops to create cpu_ops, like arm64 does.
We should merge the two and remove the SMP dependency on arm32.
I will be happy to do that but right now it would be nice to keep
focused on bringing the cpuidle ops first, even if we have a bit of code
duplicated, in order to unblock the cpuidle drivers awaiting for this
code to be merged.
The nice thing about using the linker is it makes it clearer at the
location where we define the structure that it's actually used by
something. Right now the structures are defined non-static in a file and
then we have to know that a CPUIDLE_OPS() define has been made in
another architecture specific asm header file so that this macro magic
works. The commit text says something about multiple declarations and
ops spread across header files, which shouldn't apply if we're using the
linker to find these ops and merge them into an array we can iterate over.
It makes sense, see above for UP vs SMP. I wonder if we can't find
something to overcome the UP limitation nicely, the init code in
arch/arm/kernel/devtree.c is identical for smp_ops and cpuidle_ops,
apart from the CONFIG_SMP ifdeffery.
It should be possible to replace the arm32 smp_operations structure with
something like the arm64 cpu_operations structure. Yes we would have to
drop the SMP dependency, but that will be ok. It would require some work
to make arm32 and arm64 the same, but for these purposes that isn't
really required as long as we can put the cpu idle hook there.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
