| Message ID | 1487341886-13829-2-git-send-email-benjamin.gaignard@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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)
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(+)