| Message ID | 1410321039-26888-1-git-send-email-guodong.xu@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-09-10 11:50 GMT+08:00 Guodong Xu <guodong.xu@linaro.org>: > These of_node_get() were added to balance refcount decrements inside of > of_find_node_by_name(). > See: commit c92f5dd2c42f ("regulator: Add missing of_node_put()") > > However of_find_node_by_name() was then replaced by of_get_child_by_name(), > which doesn't call of_node_put() against its input parameter. > > So, need to remove these unnecessary of_node_get() calls. The of_node_get() and of_node_put() is a pair. You need to either keep both or remove both. BTW, I think either the comment of of_get_child_by_name() needs fix or the implementation needs fix. The implementation does not increment refcount. /** * of_get_child_by_name - Find the child node by name for a given parent * @node: parent node * @name: child name to look for. * * This function looks for child node for given matching name * * Returns a node pointer if found, with refcount incremented, use * of_node_put() on it when done. * Returns NULL if node is not found. */ struct device_node *of_get_child_by_name(const struct device_node *node, const char *name) { struct device_node *child; for_each_child_of_node(node, child) if (child->name && (of_node_cmp(child->name, name) == 0)) break; return child; } EXPORT_SYMBOL(of_get_child_by_name);
2014-09-10 12:20 GMT+08:00 Axel Lin <axel.lin@ingics.com>: > 2014-09-10 11:50 GMT+08:00 Guodong Xu <guodong.xu@linaro.org>: >> These of_node_get() were added to balance refcount decrements inside of >> of_find_node_by_name(). >> See: commit c92f5dd2c42f ("regulator: Add missing of_node_put()") >> >> However of_find_node_by_name() was then replaced by of_get_child_by_name(), >> which doesn't call of_node_put() against its input parameter. >> >> So, need to remove these unnecessary of_node_get() calls. > > The of_node_get() and of_node_put() is a pair. > You need to either keep both or remove both. > > > BTW, > I think either the comment of of_get_child_by_name() needs fix or the > implementation > needs fix. The implementation does not increment refcount. Ah, I see the of_node_get() and of_node_put() in __of_get_next_child. So of_get_child_by_name() is correct.(both comment and implementation)
On 09/10/2014 12:23 PM, Axel Lin wrote: > 2014-09-10 12:20 GMT+08:00 Axel Lin <axel.lin@ingics.com>: >> 2014-09-10 11:50 GMT+08:00 Guodong Xu <guodong.xu@linaro.org>: >>> These of_node_get() were added to balance refcount decrements inside of >>> of_find_node_by_name(). >>> See: commit c92f5dd2c42f ("regulator: Add missing of_node_put()") >>> >>> However of_find_node_by_name() was then replaced by of_get_child_by_name(), >>> which doesn't call of_node_put() against its input parameter. >>> >>> So, need to remove these unnecessary of_node_get() calls. >> >> The of_node_get() and of_node_put() is a pair. >> You need to either keep both or remove both. >> >> >> BTW, >> I think either the comment of of_get_child_by_name() needs fix or the >> implementation >> needs fix. The implementation does not increment refcount. > > Ah, I see the of_node_get() and of_node_put() in __of_get_next_child. > So of_get_child_by_name() is correct.(both comment and implementation) > That's right. You only need to call of_node_put() once on the node of_get_child_by_name() returns. That's why I submit this patch to remove of_node_get() _before_ calling to of_get_child_by_name(). -Guodong
2014-09-10 17:23 GMT+08:00 Guodong Xu <guodong.xu@linaro.org>: > > > On 09/10/2014 12:23 PM, Axel Lin wrote: >> 2014-09-10 12:20 GMT+08:00 Axel Lin <axel.lin@ingics.com>: >>> 2014-09-10 11:50 GMT+08:00 Guodong Xu <guodong.xu@linaro.org>: >>>> These of_node_get() were added to balance refcount decrements inside of >>>> of_find_node_by_name(). >>>> See: commit c92f5dd2c42f ("regulator: Add missing of_node_put()") >>>> >>>> However of_find_node_by_name() was then replaced by of_get_child_by_name(), >>>> which doesn't call of_node_put() against its input parameter. >>>> >>>> So, need to remove these unnecessary of_node_get() calls. >>> >>> The of_node_get() and of_node_put() is a pair. >>> You need to either keep both or remove both. >>> >>> >>> BTW, >>> I think either the comment of of_get_child_by_name() needs fix or the >>> implementation >>> needs fix. The implementation does not increment refcount. >> >> Ah, I see the of_node_get() and of_node_put() in __of_get_next_child. >> So of_get_child_by_name() is correct.(both comment and implementation) >> > > That's right. You only need to call of_node_put() once on the node > of_get_child_by_name() returns. That's why I submit this patch to remove > of_node_get() _before_ calling to of_get_child_by_name(). Reviewed-by: Axel Lin <axel.lin@ingics.com> Thanks, Axel
diff --git a/drivers/regulator/88pm8607.c b/drivers/regulator/88pm8607.c index 337634a..6d77dcd 100644 --- a/drivers/regulator/88pm8607.c +++ b/drivers/regulator/88pm8607.c @@ -319,7 +319,7 @@ static int pm8607_regulator_dt_init(struct platform_device *pdev, struct regulator_config *config) { struct device_node *nproot, *np; - nproot = of_node_get(pdev->dev.parent->of_node); + nproot = pdev->dev.parent->of_node; if (!nproot) return -ENODEV; nproot = of_get_child_by_name(nproot, "regulators"); diff --git a/drivers/regulator/da9052-regulator.c b/drivers/regulator/da9052-regulator.c index fdb6ea8..0003362 100644 --- a/drivers/regulator/da9052-regulator.c +++ b/drivers/regulator/da9052-regulator.c @@ -422,9 +422,9 @@ static int da9052_regulator_probe(struct platform_device *pdev) config.init_data = pdata->regulators[pdev->id]; } else { #ifdef CONFIG_OF - struct device_node *nproot, *np; + struct device_node *nproot = da9052->dev->of_node; + struct device_node *np; - nproot = of_node_get(da9052->dev->of_node); if (!nproot) return -ENODEV; diff --git a/drivers/regulator/max8907-regulator.c b/drivers/regulator/max8907-regulator.c index 9623e9e..3426be8 100644 --- a/drivers/regulator/max8907-regulator.c +++ b/drivers/regulator/max8907-regulator.c @@ -226,7 +226,7 @@ static int max8907_regulator_parse_dt(struct platform_device *pdev) struct device_node *np, *regulators; int ret; - np = of_node_get(pdev->dev.parent->of_node); + np = pdev->dev.parent->of_node; if (!np) return 0; diff --git a/drivers/regulator/max8925-regulator.c b/drivers/regulator/max8925-regulator.c index dad2bcd..7770777 100644 --- a/drivers/regulator/max8925-regulator.c +++ b/drivers/regulator/max8925-regulator.c @@ -250,7 +250,7 @@ static int max8925_regulator_dt_init(struct platform_device *pdev, struct device_node *nproot, *np; int rcount; - nproot = of_node_get(pdev->dev.parent->of_node); + nproot = pdev->dev.parent->of_node; if (!nproot) return -ENODEV; np = of_get_child_by_name(nproot, "regulators"); diff --git a/drivers/regulator/max8997.c b/drivers/regulator/max8997.c index 90b4c53..9c31e21 100644 --- a/drivers/regulator/max8997.c +++ b/drivers/regulator/max8997.c @@ -917,7 +917,7 @@ static int max8997_pmic_dt_parse_pdata(struct platform_device *pdev, struct max8997_regulator_data *rdata; unsigned int i, dvs_voltage_nr = 1, ret; - pmic_np = of_node_get(iodev->dev->of_node); + pmic_np = iodev->dev->of_node; if (!pmic_np) { dev_err(&pdev->dev, "could not find pmic sub-node\n"); return -ENODEV; diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c index a7ce34d..1878e5b 100644 --- a/drivers/regulator/palmas-regulator.c +++ b/drivers/regulator/palmas-regulator.c @@ -1427,7 +1427,6 @@ static void palmas_dt_to_pdata(struct device *dev, u32 prop; int idx, ret; - node = of_node_get(node); regulators = of_get_child_by_name(node, "regulators"); if (!regulators) { dev_info(dev, "regulator node not found\n"); diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c index 2064507..18fc991 100644 --- a/drivers/regulator/tps65910-regulator.c +++ b/drivers/regulator/tps65910-regulator.c @@ -1014,7 +1014,7 @@ static struct tps65910_board *tps65910_parse_dt_reg_data( if (!pmic_plat_data) return NULL; - np = of_node_get(pdev->dev.parent->of_node); + np = pdev->dev.parent->of_node; regulators = of_get_child_by_name(np, "regulators"); if (!regulators) { dev_err(&pdev->dev, "regulator node not found\n");
These of_node_get() were added to balance refcount decrements inside of of_find_node_by_name(). See: commit c92f5dd2c42f ("regulator: Add missing of_node_put()") However of_find_node_by_name() was then replaced by of_get_child_by_name(), which doesn't call of_node_put() against its input parameter. So, need to remove these unnecessary of_node_get() calls. Signed-off-by: Guodong Xu <guodong.xu@linaro.org> --- drivers/regulator/88pm8607.c | 2 +- drivers/regulator/da9052-regulator.c | 4 ++-- drivers/regulator/max8907-regulator.c | 2 +- drivers/regulator/max8925-regulator.c | 2 +- drivers/regulator/max8997.c | 2 +- drivers/regulator/palmas-regulator.c | 1 - drivers/regulator/tps65910-regulator.c | 2 +- 7 files changed, 7 insertions(+), 8 deletions(-)