Thread (60 messages) 60 messages, 4 authors, 2014-07-03
STALE4380d

[PATCH 07/16] ARM: mvebu: Make the CPU idle initialization more generic

From: Thomas Petazzoni <hidden>
Date: 2014-06-30 14:07:27
Also in: linux-pm

Gregory,

On Fri, 27 Jun 2014 15:22:48 +0200, Gregory CLEMENT wrote:
+static bool (*mvebu_v7_cpu_idle_init)(void);
+
+static __init bool armada_xp_cpuidle_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric");
+	if (!np)
+		return false;
+	of_node_put(np);
+
+	mvebu_v7_cpuidle_device.dev.platform_data = armada_xp_370_cpu_suspend;
+	return true;
+}
+
+static struct of_device_id of_cpuidle_table[] __initdata = {
+	{ .compatible = "marvell,armadaxp",
+	  .data = (void *)armada_xp_cpuidle_init,
+	},
+	{ /* end of list */ },
+};
+
 static int __init mvebu_v7_cpu_pm_init(void)
 {
 	struct device_node *np;
+	const struct of_device_id *match;
+
+	np = of_find_matching_node_and_match(NULL, of_cpuidle_table,
+					&match);
I'm not sure I like using of_find_matching_node_and_match() on the
entire tree to find the root node compatible string. Wouldn't it be
simpler and shorter to just do:

	if (of_machine_is_compatible("marvell,armadaxp"))
		ret = armadaxp_cpuidle_init();
	else if (of_machine_is_compatible("marvell,armada370"))
		ret = armada370_cpuidle_init();
	else if (of_machine_is_compaitble("marvell,armada380"))
		ret = armada38x_cpuidle_init();

	if (ret)
		return ret;

Also, using a boolean to return a success/error status is not the
kernel way of doing things, you should use an int instead and use
proper error codes or return 0 on success.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help