RE: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

-----Original Message-----
From: Rob Herring [mailto:robherring2@gmail.com]
Sent: Monday, June 11, 2012 11:33 PM
To: Jia Hongtao-B38951
Cc: devicetree-discuss@lists.ozlabs.org; linux-kernel@vger.kernel.org;
Greg KH
Subject: Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[adding lkml and Greg KH]

On 06/08/2012 04:43 AM, Jia Hongtao wrote:

quoted

We changed the pcie controller driver to platform driver so that the

PCI

quoted

of platform devices need to be created earlier in the arch_initcall

stage

quoted

according to the original timing of calling fsl_add_bridge(). So we do

PCI

quoted

probing separately from other devices. But probing more than once could
cause duplication warning. We add check if the devices have already

probed

quoted

before probing any devices to avoid duplication warning.

Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---

Where's v1 and v2 history?

Sorry, there is no V1 and V2.
These are a series of patches and only this one related to device-tree operation.
V3 are for other patches.
I will aware of that in the future.
Thanks for mentioning.

quoted

 drivers/of/platform.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index a37330e..3aab01f 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c

@@ -139,6 +139,18 @@ struct platform_device *of_device_alloc(struct

device_node *np,

quoted

 	if (!dev)
 		return NULL;

+	dev->dev.of_node = of_node_get(np);
+	if (bus_id)
+		dev_set_name(&dev->dev, "%s", bus_id);
+	else
+		of_device_make_bus_id(&dev->dev);
+
+	if (kset_find_obj(dev->dev.kobj.kset, kobject_name(&dev->dev.kobj)))

{

Is this a fast operation? I worry that we'd be doing this for every
device creation. Looking at kobj's seems too low level as well.

To us this is also the main concern.

-Jia Hongtao.
 

Perhaps moving the warning up the call stack and making the relevant
functions __must_check would work.

Rob

quoted

+		kfree(dev);
+		of_node_put(np);
+		return NULL;
+	}
+
 	/* count the io and irq resources */
 	while (of_address_to_resource(np, num_reg, &temp_res) == 0)
 		num_reg++;

@@ -161,17 +173,11 @@ struct platform_device *of_device_alloc(struct

device_node *np,

quoted

 		WARN_ON(of_irq_to_resource_table(np, res, num_irq) !=

num_irq);

quoted

 	}

-	dev->dev.of_node = of_node_get(np);
 #if defined(CONFIG_MICROBLAZE)
 	dev->dev.dma_mask = &dev->archdata.dma_mask;
 #endif
 	dev->dev.parent = parent;

-	if (bus_id)
-		dev_set_name(&dev->dev, "%s", bus_id);
-	else
-		of_device_make_bus_id(&dev->dev);
-
 	return dev;
 }
 EXPORT_SYMBOL(of_device_alloc);