[1/2] dt: check for devices already created fron DT scan
diff mbox

Message ID 1305829704-11774-2-git-send-email-robherring2@gmail.com
State New, archived
Headers show

Commit Message

Rob Herring May 19, 2011, 6:28 p.m. UTC
From: Rob Herring <rob.herring@calxeda.com>

of_platform_bus_create may create duplicate devices if already
added when scanning other buses like amba bus.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/of/platform.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

Comments

Grant Likely May 19, 2011, 7:54 p.m. UTC | #1
On Thu, May 19, 2011 at 01:28:23PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> of_platform_bus_create may create duplicate devices if already
> added when scanning other buses like amba bus.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
>  drivers/of/platform.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 9b785be..7bf74fb 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -234,6 +234,13 @@ static int of_platform_bus_create(struct device_node *bus,
>  		return 0;
>  	}
>  
> +	/* Has the device already been registered? */
> +	if (of_find_device_by_node(bus)) {
> +		pr_debug("%s() - skipping %s, already registered\n",
> +			 __func__, bus->full_name);
> +		return 0;
> +	}
> +

Intriguing, I hadn't though about doing it this way, and it would
hugely simplify the of_platform_prepare() logic that I posted a few
weeks back.  I need to think about this...

However, there is a potential race condition with static device
registrations that also use of_platform_prepare().  If there every
exists a multi-threaded device registration scenario, then it could be
possible for this check to pass, the other thread register a
conflicting device, and then this finishes creating it.  However,
since device registration is pretty much always a single threaded
affair, I'm probably just being paranoid.

Also, of_find_device_by_node() casts a rather wide net.  There are a
few small use-cases where multiple devices will have the same node
pointer.  It would be better to restrict it to devices with the same
parent, or devices on the same bus type.

>  	dev = of_platform_device_create(bus, NULL, parent);
>  	if (!dev || !of_match_node(matches, bus))
>  		return 0;
> @@ -326,4 +333,5 @@ int of_platform_populate(struct device_node *root,
>  	of_node_put(root);
>  	return rc;
>  }
> +EXPORT_SYMBOL(of_platform_populate);

This is an unrelated change.  Does it belong in this patch?

>  #endif /* !CONFIG_SPARC */
> -- 
> 1.7.4.1
>

Patch
diff mbox

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 9b785be..7bf74fb 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -234,6 +234,13 @@  static int of_platform_bus_create(struct device_node *bus,
 		return 0;
 	}
 
+	/* Has the device already been registered? */
+	if (of_find_device_by_node(bus)) {
+		pr_debug("%s() - skipping %s, already registered\n",
+			 __func__, bus->full_name);
+		return 0;
+	}
+
 	dev = of_platform_device_create(bus, NULL, parent);
 	if (!dev || !of_match_node(matches, bus))
 		return 0;
@@ -326,4 +333,5 @@  int of_platform_populate(struct device_node *root,
 	of_node_put(root);
 	return rc;
 }
+EXPORT_SYMBOL(of_platform_populate);
 #endif /* !CONFIG_SPARC */