Lost of calls to of_platform_populate() are not unbalanced by a call to of_platform_depopulate(). This create issues while drivers are bind/unbind.
In way to solve those issues is to add devm_of_platform_populate() which will call of_platform_depopulate() when the device is unbound from the bus. This also could make drivers more robust in case that probe failed after calling of_platform_populate().
Benjamin Gaignard (2): of: add devm_ functions for populate and depopulate drm: sti: make driver use devm_of_platform_populate()
drivers/gpu/drm/sti/sti_drv.c | 3 +- drivers/of/platform.c | 77 +++++++++++++++++++++++++++++++++++++++++++ include/linux/of_platform.h | 20 +++++++++++ 3 files changed, 98 insertions(+), 2 deletions(-)
Lost of calls to of_platform_populate() are not unbalanced by a call to of_platform_depopulate(). This create issues while drivers are bind/unbind.
In way to solve those issues is to add devm_of_platform_populate() which will call of_platform_depopulate() when the device is unbound from the bus.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/of/platform.c | 77 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_platform.h | 20 ++++++++++++ 2 files changed, 97 insertions(+)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b8064bc..3dbebf7 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -571,6 +571,83 @@ void of_platform_depopulate(struct device *parent) } EXPORT_SYMBOL_GPL(of_platform_depopulate);
+static void devm_of_platform_populate_release(struct device *dev, void *res) +{ + of_platform_depopulate(*(struct device **)res); +} + +/** + * devm_of_platform_populate() - Populate platform_devices from device tree data + * @dev: device that requested to populate from device tree data + * @root: parent of the first level to probe or NULL for the root of the tree + * @matches: match table, NULL to use the default + * @lookup: auxdata table for matching id and platform_data with device nodes + * @parent: parent to hook devices from, NULL for toplevel + * + * Similar to of_platform_populate(), but will automatically call + * of_platform_depopulate() when the device is unbound from the bus. + * + * Returns 0 on success, < 0 on failure. + */ +int devm_of_platform_populate(struct device *dev, + struct device_node *root, + const struct of_device_id *matches, + const struct of_dev_auxdata *lookup, + struct device *parent) +{ + struct device **ptr; + int ret; + + ptr = devres_alloc(devm_of_platform_populate_release, + sizeof(*ptr), GFP_KERNEL); + if (!ptr) + return -ENOMEM; + + ret = of_platform_populate(root, matches, lookup, parent); + if (ret) { + devres_free(ptr); + } else { + *ptr = parent; + devres_add(dev, ptr); + } + + return ret; +} +EXPORT_SYMBOL_GPL(devm_of_platform_populate); + +static int devm_of_platform_match(struct device *dev, void *res, void *data) +{ + struct device **ptr = res; + + if (!ptr) { + WARN_ON(!ptr); + return 0; + } + + return *ptr == data; +} + +/** + * devm_of_platform_depopulate() - Remove devices populated from device tree + * @dev: device that requested to populate from device tree data + * @parent: device which children will be removed + * + * Complementary to devm_of_platform_populate(), this function removes children + * of the given device (and, recurrently, their children) that have been + * created from their respective device tree nodes (and only those, + * leaving others - eg. manually created - unharmed). + */ +void devm_of_platform_depopulate(struct device *dev, struct device *parent) +{ + int ret; + + ret = devres_release(dev, devm_of_platform_populate_release, + devm_of_platform_match, parent); + + WARN_ON(ret); +} +EXPORT_SYMBOL_GPL(devm_of_platform_depopulate); + #ifdef CONFIG_OF_DYNAMIC static int of_platform_notify(struct notifier_block *nb, unsigned long action, void *arg) diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h index 956a100..282fae3 100644 --- a/include/linux/of_platform.h +++ b/include/linux/of_platform.h @@ -76,6 +76,14 @@ extern int of_platform_default_populate(struct device_node *root, const struct of_dev_auxdata *lookup, struct device *parent); extern void of_platform_depopulate(struct device *parent); + +extern int devm_of_platform_populate(struct device *dev, + struct device_node *root, + const struct of_device_id *matches, + const struct of_dev_auxdata *lookup, + struct device *parent); +extern void devm_of_platform_depopulate(struct device *dev, + struct device *parent); #else static inline int of_platform_populate(struct device_node *root, const struct of_device_id *matches, @@ -91,6 +99,18 @@ static inline int of_platform_default_populate(struct device_node *root, return -ENODEV; } static inline void of_platform_depopulate(struct device *parent) { } + +static inline int devm_of_platform_populate(struct device *dev, + struct device_node *root, + const struct of_device_id *matches, + const struct of_dev_auxdata *lookup, + struct device *parent) +{ + return -ENODEV; +} + +static inline void devm_of_platform_depopulate(struct device *dev, + struct device *parent) { } #endif
#if defined(CONFIG_OF_DYNAMIC) && defined(CONFIG_OF_ADDRESS)
On Fri, Feb 17, 2017 at 8:31 AM, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
Lost of calls to of_platform_populate() are not unbalanced by a call
s/Lost/Lots/
to of_platform_depopulate(). This create issues while drivers are bind/unbind.
In way to solve those issues is to add devm_of_platform_populate() which will call of_platform_depopulate() when the device is unbound from the bus.
One complication is of_platform_populate is designed to be called multiple times. We call it with the default match table and then platforms can call it again with a custom match table for example.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/of/platform.c | 77 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_platform.h | 20 ++++++++++++ 2 files changed, 97 insertions(+)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b8064bc..3dbebf7 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -571,6 +571,83 @@ void of_platform_depopulate(struct device *parent) } EXPORT_SYMBOL_GPL(of_platform_depopulate);
+static void devm_of_platform_populate_release(struct device *dev, void *res) +{
of_platform_depopulate(*(struct device **)res);+}
+/**
- devm_of_platform_populate() - Populate platform_devices from device tree data
- @dev: device that requested to populate from device tree data
- @root: parent of the first level to probe or NULL for the root of the tree
- @matches: match table, NULL to use the default
NULL is no match table, not the default which means only populate immediate children.
- @lookup: auxdata table for matching id and platform_data with device nodes
- @parent: parent to hook devices from, NULL for toplevel
I think this needs to be a bit different args as the use is limited. root and parent must not be NULL. dev should be the same as parent. lookup was for legacy, so drop that.
- Similar to of_platform_populate(), but will automatically call
- of_platform_depopulate() when the device is unbound from the bus.
- Returns 0 on success, < 0 on failure.
- */
2017-02-24 15:17 GMT+01:00 Rob Herring robh+dt@kernel.org:
On Fri, Feb 17, 2017 at 8:31 AM, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
Lost of calls to of_platform_populate() are not unbalanced by a call
s/Lost/Lots/
to of_platform_depopulate(). This create issues while drivers are bind/unbind.
In way to solve those issues is to add devm_of_platform_populate() which will call of_platform_depopulate() when the device is unbound from the bus.
One complication is of_platform_populate is designed to be called multiple times. We call it with the default match table and then platforms can call it again with a custom match table for example.
I do not plan to use it every where but only in some drivers probe() to avoid adding goto to call of_platform_depopulate() for each error cases which may occur after calling populate.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/of/platform.c | 77 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_platform.h | 20 ++++++++++++ 2 files changed, 97 insertions(+)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b8064bc..3dbebf7 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -571,6 +571,83 @@ void of_platform_depopulate(struct device *parent) } EXPORT_SYMBOL_GPL(of_platform_depopulate);
+static void devm_of_platform_populate_release(struct device *dev, void *res) +{
of_platform_depopulate(*(struct device **)res);+}
+/**
- devm_of_platform_populate() - Populate platform_devices from device tree data
- @dev: device that requested to populate from device tree data
- @root: parent of the first level to probe or NULL for the root of the tree
- @matches: match table, NULL to use the default
NULL is no match table, not the default which means only populate immediate children.
I have copy of_platform_populate() description... I replace it by: @matches: match table (could be NULL)
- @lookup: auxdata table for matching id and platform_data with device nodes
- @parent: parent to hook devices from, NULL for toplevel
I think this needs to be a bit different args as the use is limited. root and parent must not be NULL. dev should be the same as parent. lookup was for legacy, so drop that.
So function prototype will become: int devm_of_platform_populate(struct device *dev, struct device_node *root, const struct of_device_id *matches)
- Similar to of_platform_populate(), but will automatically call
- of_platform_depopulate() when the device is unbound from the bus.
- Returns 0 on success, < 0 on failure.
- */
On Fri, Feb 24, 2017 at 9:13 AM, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
2017-02-24 15:17 GMT+01:00 Rob Herring robh+dt@kernel.org:
On Fri, Feb 17, 2017 at 8:31 AM, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
Lost of calls to of_platform_populate() are not unbalanced by a call
s/Lost/Lots/
to of_platform_depopulate(). This create issues while drivers are bind/unbind.
In way to solve those issues is to add devm_of_platform_populate() which will call of_platform_depopulate() when the device is unbound from the bus.
One complication is of_platform_populate is designed to be called multiple times. We call it with the default match table and then platforms can call it again with a custom match table for example.
I do not plan to use it every where but only in some drivers probe() to avoid adding goto to call of_platform_depopulate() for each error cases which may occur after calling populate.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/of/platform.c | 77 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_platform.h | 20 ++++++++++++ 2 files changed, 97 insertions(+)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b8064bc..3dbebf7 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -571,6 +571,83 @@ void of_platform_depopulate(struct device *parent) } EXPORT_SYMBOL_GPL(of_platform_depopulate);
+static void devm_of_platform_populate_release(struct device *dev, void *res) +{
of_platform_depopulate(*(struct device **)res);+}
+/**
- devm_of_platform_populate() - Populate platform_devices from device tree data
- @dev: device that requested to populate from device tree data
- @root: parent of the first level to probe or NULL for the root of the tree
- @matches: match table, NULL to use the default
NULL is no match table, not the default which means only populate immediate children.
I have copy of_platform_populate() description... I replace it by: @matches: match table (could be NULL)
- @lookup: auxdata table for matching id and platform_data with device nodes
- @parent: parent to hook devices from, NULL for toplevel
I think this needs to be a bit different args as the use is limited. root and parent must not be NULL. dev should be the same as parent. lookup was for legacy, so drop that.
So function prototype will become: int devm_of_platform_populate(struct device *dev, struct device_node *root, const struct of_device_id *matches)
I just noticed something else too. dev->of_node should equal root I think, so we can get rid of root param.
Rob
2017-02-24 16:20 GMT+01:00 Rob Herring robh+dt@kernel.org:
On Fri, Feb 24, 2017 at 9:13 AM, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
2017-02-24 15:17 GMT+01:00 Rob Herring robh+dt@kernel.org:
On Fri, Feb 17, 2017 at 8:31 AM, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
Lost of calls to of_platform_populate() are not unbalanced by a call
s/Lost/Lots/
to of_platform_depopulate(). This create issues while drivers are bind/unbind.
In way to solve those issues is to add devm_of_platform_populate() which will call of_platform_depopulate() when the device is unbound from the bus.
One complication is of_platform_populate is designed to be called multiple times. We call it with the default match table and then platforms can call it again with a custom match table for example.
I do not plan to use it every where but only in some drivers probe() to avoid adding goto to call of_platform_depopulate() for each error cases which may occur after calling populate.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/of/platform.c | 77 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_platform.h | 20 ++++++++++++ 2 files changed, 97 insertions(+)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b8064bc..3dbebf7 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -571,6 +571,83 @@ void of_platform_depopulate(struct device *parent) } EXPORT_SYMBOL_GPL(of_platform_depopulate);
+static void devm_of_platform_populate_release(struct device *dev, void *res) +{
of_platform_depopulate(*(struct device **)res);+}
+/**
- devm_of_platform_populate() - Populate platform_devices from device tree data
- @dev: device that requested to populate from device tree data
- @root: parent of the first level to probe or NULL for the root of the tree
- @matches: match table, NULL to use the default
NULL is no match table, not the default which means only populate immediate children.
I have copy of_platform_populate() description... I replace it by: @matches: match table (could be NULL)
- @lookup: auxdata table for matching id and platform_data with device nodes
- @parent: parent to hook devices from, NULL for toplevel
I think this needs to be a bit different args as the use is limited. root and parent must not be NULL. dev should be the same as parent. lookup was for legacy, so drop that.
So function prototype will become: int devm_of_platform_populate(struct device *dev, struct device_node *root, const struct of_device_id *matches)
I just noticed something else too. dev->of_node should equal root I think, so we can get rid of root param.
match parameter is very often set to NULL since this function will have a limited scope why not also remove it and only keep dev ?
Rob
On Fri, Feb 24, 2017 at 9:26 AM, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
2017-02-24 16:20 GMT+01:00 Rob Herring robh+dt@kernel.org:
On Fri, Feb 24, 2017 at 9:13 AM, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
2017-02-24 15:17 GMT+01:00 Rob Herring robh+dt@kernel.org:
On Fri, Feb 17, 2017 at 8:31 AM, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
Lost of calls to of_platform_populate() are not unbalanced by a call
s/Lost/Lots/
to of_platform_depopulate(). This create issues while drivers are bind/unbind.
In way to solve those issues is to add devm_of_platform_populate() which will call of_platform_depopulate() when the device is unbound from the bus.
One complication is of_platform_populate is designed to be called multiple times. We call it with the default match table and then platforms can call it again with a custom match table for example.
I do not plan to use it every where but only in some drivers probe() to avoid adding goto to call of_platform_depopulate() for each error cases which may occur after calling populate.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/of/platform.c | 77 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_platform.h | 20 ++++++++++++ 2 files changed, 97 insertions(+)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b8064bc..3dbebf7 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -571,6 +571,83 @@ void of_platform_depopulate(struct device *parent) } EXPORT_SYMBOL_GPL(of_platform_depopulate);
+static void devm_of_platform_populate_release(struct device *dev, void *res) +{
of_platform_depopulate(*(struct device **)res);+}
+/**
- devm_of_platform_populate() - Populate platform_devices from device tree data
- @dev: device that requested to populate from device tree data
- @root: parent of the first level to probe or NULL for the root of the tree
- @matches: match table, NULL to use the default
NULL is no match table, not the default which means only populate immediate children.
I have copy of_platform_populate() description... I replace it by: @matches: match table (could be NULL)
- @lookup: auxdata table for matching id and platform_data with device nodes
- @parent: parent to hook devices from, NULL for toplevel
I think this needs to be a bit different args as the use is limited. root and parent must not be NULL. dev should be the same as parent. lookup was for legacy, so drop that.
So function prototype will become: int devm_of_platform_populate(struct device *dev, struct device_node *root, const struct of_device_id *matches)
I just noticed something else too. dev->of_node should equal root I think, so we can get rid of root param.
match parameter is very often set to NULL since this function will have a limited scope why not also remove it and only keep dev ?
That's fine with me.
Rob
This make sure that of_platform_depopulate() is called if an error occur in probe after populating the date from the device tree.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/gpu/drm/sti/sti_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c index ff71e25..01ef9a4 100644 --- a/drivers/gpu/drm/sti/sti_drv.c +++ b/drivers/gpu/drm/sti/sti_drv.c @@ -438,7 +438,7 @@ static int sti_platform_probe(struct platform_device *pdev)
dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
- of_platform_populate(node, NULL, NULL, dev); + devm_of_platform_populate(dev, node, NULL, NULL, dev);
child_np = of_get_next_available_child(node, NULL);
@@ -454,7 +454,6 @@ static int sti_platform_probe(struct platform_device *pdev) static int sti_platform_remove(struct platform_device *pdev) { component_master_del(&pdev->dev, &sti_ops); - of_platform_depopulate(&pdev->dev);
return 0; }
linaro-kernel@lists.linaro.org
-
Benjamin Gaignard -
Rob Herring