Re: [RFC PATCH v2 0/4] Core device subsystem

[Marc Zyngier <marc.zyngier@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

On 08/07/11 14:08, Marc Zyngier wrote:
> So you're basically folding of_core_device_populate() and
> core_driver_init_class() into one call, and generating the
> of_device_ids on the fly. If you're going down that road,
> it would be even simpler to directly use of_device_ids
> instead of core_device_ids and skip the generation altogether.
> 
> That would also remove the static declaration of devices to be
> probed in the architecture support code...
> 
> Let me think of it and prototype that.

See the attached patch against branch dt_gic_twd from
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git

It boots fine on my PB11MP.

What do you think?

	M.
-- 
Jazz is not dead. It just smells funny...

[-- Attachment #2: code_dev.patch.txt --]
[-- Type: text/plain, Size: 6152 bytes --]

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 74825c2..e7d9922 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -712,9 +712,9 @@ static int __init gic_core_init(struct core_device *dev)
 	return 0;
 }
 
-static struct core_device_id gic_ids[] __initdata = {
-	{ .name = "arm,gic-spi" }, /* DT */
-	{ .name = "arm_gic" },     /* Static device */
+static struct of_device_id gic_ids[] __initdata = {
+	{ .compatible = "arm,gic-spi" }, /* DT */
+	{ .name = "arm_gic" },		 /* Static device */
 	{ },
 };
 
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index 0b0681c..1cb4bd7 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -114,15 +114,10 @@ void set_irq_flags(unsigned int irq, unsigned int iflags)
 	irq_modify_status(irq, clr, set & ~clr);
 }
 
-static struct of_device_id of_irq_controller_ids[] __initdata = {
-	{ .compatible   = "arm,gic-spi", },
-	{},
-};
-
 void __init init_IRQ(void)
 {
 	machine_desc->init_irq();
-	of_core_device_populate(CORE_DEV_CLASS_IRQ, of_irq_controller_ids);
+	of_core_device_populate(CORE_DEV_CLASS_IRQ);
 	core_driver_init_class(CORE_DEV_CLASS_IRQ, core_device_irq_sort);
 }
 
diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index b879698..80f7ad2 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -150,18 +150,13 @@ device_initcall(timer_init_syscore_ops);
 
 void (* __initdata arm_late_time_init)(void);
 
-static struct of_device_id of_timer_ids[] __initdata = {
-	{ .compatible = "arm,smp-twd", },
-	{},
-};
-
 static void __init __arm_late_time_init(void)
 {
 	if (arm_late_time_init)
 		arm_late_time_init();
 
 #ifdef CONFIG_LOCAL_TIMER_DEVICES
-	of_core_device_populate(CORE_DEV_CLASS_TIMER, of_timer_ids);
+	of_core_device_populate(CORE_DEV_CLASS_TIMER);
 	core_driver_init_class(CORE_DEV_CLASS_TIMER, NULL);
 #endif
 }
diff --git a/drivers/base/core_device.c b/drivers/base/core_device.c
index a8df59d..0f1c585 100644
--- a/drivers/base/core_device.c
+++ b/drivers/base/core_device.c
@@ -20,20 +20,22 @@ static struct list_head anchors[CORE_DEV_CLASS_MAX] __initdata = {
 };
 
 static int __init core_device_match(struct core_device *dev,
-				    struct core_device_id *ids)
+				    struct of_device_id *ids)
 {
-	int i;
 #ifdef CONFIG_OF
-	if (dev->of_node)
-		for (i = 0; ids[i].name != NULL; i++)
-			if (of_device_is_compatible(dev->of_node,
-						    ids[i].name))
-				return 1;
+	if (dev->of_node) {
+		if (of_match_node(ids, dev->of_node))
+			return 1;
+
+		return 0;
+	}
 #endif
-	for (i = 0; ids[i].name != NULL; i++)
-		if (!strcmp(dev->name, ids[i].name))
+	while (ids->name[0] || ids->type[0] || ids->compatible[0]) {
+		if (ids->name[0] && !strcmp(dev->name, ids->name))
 			return 1;
 
+		ids++;
+	}
 	return 0;
 }
 
@@ -82,29 +84,37 @@ void __init core_driver_init_class(enum core_device_class class,
 }
 
 #ifdef CONFIG_OF
-void __init of_core_device_populate(enum core_device_class class,
-			     struct of_device_id *matches)
+void __init of_core_device_populate(enum core_device_class class)
 {
+	const struct core_driver_setup_block *b;
 	struct device_node *np;
 	struct core_device *dev;
 	int num_res;
 
-	for_each_matching_node(np, matches) {
-		pr_debug("core: registering OF device %s\n", np->full_name);
-		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-		if (!dev)
-			return;
-		dev->name = np->name;
-		dev->of_node = np;
+	for (b = __core_driver_block_start; b < __core_driver_block_end; b++) {
+		if (b->class != class)
+			continue;
+
+		for_each_matching_node(np, b->drv->ids) {
+			pr_debug("core: registering OF device %s\n",
+				 np->full_name);
+			dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+			if (!dev) {
+				of_node_put(np);
+				return;
+			}
+			dev->resource = of_device_alloc_resources(np, &num_res);
+			if (num_res < 0) {
+				of_node_put(np);
+				kfree(dev);
+				return;
+			}
 
-		dev->resource = of_device_alloc_resources(np, &num_res);
-		if (num_res < 0) {
-			kfree(dev);
-			return;
+			dev->num_resources = num_res;
+			dev->of_node = of_node_get(np);
+			dev->name = np->name;
+			core_device_register(class, dev);
 		}
-		dev->num_resources = num_res;
-
-		core_device_register(class, dev);
 	}
 }
 
diff --git a/drivers/clocksource/arm_smp_twd.c b/drivers/clocksource/arm_smp_twd.c
index 8d45000..717b4da 100644
--- a/drivers/clocksource/arm_smp_twd.c
+++ b/drivers/clocksource/arm_smp_twd.c
@@ -389,9 +389,9 @@ static int __init twd_core_init(struct core_device *dev)
 	return 0;
 }
 
-static struct core_device_id twd_core_ids[] __initdata = {
+static struct of_device_id twd_core_ids[] __initdata = {
 	{ .name	= "arm_smp_twd", },
-	{ .name = "arm,smp-twd", },
+	{ .compatible = "arm,smp-twd", },
 	{},
 };
 
diff --git a/include/linux/core_device.h b/include/linux/core_device.h
index e632868..f21caba 100644
--- a/include/linux/core_device.h
+++ b/include/linux/core_device.h
@@ -16,10 +16,6 @@
 #include <linux/ioport.h>
 #include <linux/of.h>
 
-struct core_device_id {
-	const char		*name;
-};
-
 enum core_device_class {
 	CORE_DEV_CLASS_IRQ,
 	CORE_DEV_CLASS_TIMER,
@@ -38,7 +34,7 @@ struct core_device {
 
 struct core_driver {
 	int			(*init)(struct core_device *);
-	struct core_device_id	*ids;
+	struct of_device_id	*ids;
 };
 
 void core_device_register(enum core_device_class class,
@@ -46,12 +42,10 @@ void core_device_register(enum core_device_class class,
 void core_driver_init_class(enum core_device_class class,
 			    void (*sort)(struct list_head *));
 #ifdef CONFIG_OF
-void of_core_device_populate(enum core_device_class class,
-			     struct of_device_id *matches);
+void of_core_device_populate(enum core_device_class class);
 void core_device_irq_sort(struct list_head *head);
 #else
-static inline void of_core_device_populate(enum core_device_class class,
-					   struct of_device_id *matches)
+static inline void of_core_device_populate(enum core_device_class class)
 {
 }
 #define core_device_irq_sort	NULL