[PATCH] regulator: remove unnecessary of_node_get() to parent

[PATCH] regulator: remove unnecessary of_node_get() to parent

[Guodong Xu]

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(-)

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");
-- 
1.9.1

[PATCH] regulator: remove unnecessary of_node_get() to parent

[Guodong Xu]

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(-)

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");
-- 
1.9.1

Re: [PATCH] regulator: remove unnecessary of_node_get() to parent

[Axel Lin]

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);

[PATCH] regulator: remove unnecessary of_node_get() to parent

[Axel Lin]

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);

Re: [PATCH] regulator: remove unnecessary of_node_get() to parent

[Axel Lin]

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)

[PATCH] regulator: remove unnecessary of_node_get() to parent

[Axel Lin]

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)

Re: [PATCH] regulator: remove unnecessary of_node_get() to parent

[Guodong Xu]

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

[PATCH] regulator: remove unnecessary of_node_get() to parent

[Guodong Xu]

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

Re: [PATCH] regulator: remove unnecessary of_node_get() to parent

[Axel Lin]

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

[PATCH] regulator: remove unnecessary of_node_get() to parent

[Axel Lin]

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