After checking the linux history I found no state where it was explicite
allowed to register a platform device twice. In the early days there was
an bug and in some cases linux did populated the same device again. This
was fixed in 2014 by commit c6e126de43e7 ("of: Keep track of populated
platform devices") and since then this wasn't possible anymore.
The for_each_device() solution had two issues:
1) We are looping over all devices each time
of_platform_device_create() gets called.
2) The logic around 'if (!dev->resource)' then 'continue' seems to be
broken. This suggest that we can populate a device without
resource(s) first and adding resource(s) later which isn't the case.
Instead the current logic will populate the same device again but
this time (maybe) with resources. So the caller can add the same
device as many times as possible without resources.
3) The device_node gets modified if the new device_node to add has the
same resources region.
Add a device reference to the device_node to track an registered device
seems to:
1) Simplify the code.
2) Align the logic with the current linux state with the exception that
we are returning the already created device.
3) Avoid looping over each device all the time.
4) Fix the memory leak which can occure if of_platform_device_create()
gets called for the same device without any resources.
I added the missing check for amba devices too while on it.
Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
---
v2:
- new patch
drivers/of/platform.c | 51 +++++++++++++++++--------------------------
include/of.h | 1 +
2 files changed, 21 insertions(+), 31 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 21c7cce1a5..01de6f98af 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -101,11 +101,18 @@ struct device_d *of_platform_device_create(struct device_node *np,
struct device_d *dev;
struct resource *res = NULL, temp_res;
resource_size_t resinval;
- int i, j, ret, num_reg = 0, match;
+ int i, ret, num_reg = 0;
if (!of_device_is_available(np))
return NULL;
+ /*
+ * Linux uses the OF_POPULATED flag to skip already populated/created
+ * devices.
+ */
+ if (np->dev)
+ return np->dev;
+
/* count the io resources */
if (of_can_translate_address(np))
while (of_address_to_resource(np, num_reg, &temp_res) == 0)
@@ -121,35 +128,6 @@ struct device_d *of_platform_device_create(struct device_node *np,
return NULL;
}
}
-
- /*
- * A device may already be registered as platform_device.
- * Instead of registering the same device again, just
- * add this node to the existing device.
- */
- for_each_device(dev) {
- if (!dev->resource)
- continue;
-
- for (i = 0, match = 0; i < num_reg; i++)
- for (j = 0; j < dev->num_resources; j++)
- if (dev->resource[j].start ==
- res[i].start &&
- dev->resource[j].end ==
- res[i].end) {
- match++;
- break;
- }
-
- /* check if all address resources match */
- if (match == num_reg) {
- debug("connecting %s to %s\n",
- np->name, dev_name(dev));
- dev->device_node = np;
- free(res);
- return dev;
- }
- }
}
/* setup generic device info */
@@ -170,8 +148,10 @@ struct device_d *of_platform_device_create(struct device_node *np,
(num_reg) ? &dev->resource[0].start : &resinval);
ret = platform_device_register(dev);
- if (!ret)
+ if (!ret) {
+ np->dev = dev;
return dev;
+ }
free(dev);
if (num_reg)
@@ -252,6 +232,13 @@ static struct device_d *of_amba_device_create(struct device_node *np)
if (!of_device_is_available(np))
return NULL;
+ /*
+ * Linux uses the OF_POPULATED flag to skip already populated/created
+ * devices.
+ */
+ if (np->dev)
+ return np->dev;
+
dev = xzalloc(sizeof(*dev));
/* setup generic device info */
@@ -275,6 +262,8 @@ static struct device_d *of_amba_device_create(struct device_node *np)
if (ret)
goto amba_err_free;
+ np->dev = &dev->dev;
+
return &dev->dev;
amba_err_free:
diff --git a/include/of.h b/include/of.h
index fc36f7a21a..f2dad9a6c2 100644
--- a/include/of.h
+++ b/include/of.h
@@ -35,6 +35,7 @@ struct device_node {
struct list_head parent_list;
struct list_head list;
phandle phandle;
+ struct device_d *dev;
};
struct of_device_id {
--
2.20.1
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
