[PATCH v5] clk: sprd: Hold reference returned by of_get_parent()

[PATCH v5] clk: sprd: Hold reference returned by of_get_parent()

[Liang He]

We should hold the reference returned by of_get_parent() and use it
to call of_node_put() for refcount balance.

Fixes: f95e8c7923d1 ("clk: sprd: support to get regmap from parent node")
Signed-off-by: Liang He <windhl@126.com>
---
 changelog:

 v5: fix bug reported by Chunyan and confirmed by Orson
 v4: fix another bug in the same place, missing in v3
 v3: (1) keep original 'if-else if-else' coding style adviesd by Orson
     (2) fix typo in commit-log: of_node_out --> of_node_put
 v2: minimize the effective range of of_get_parent() advised by Orson
 v1: hold reference returned by of_get_parent()


 drivers/clk/sprd/common.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
index d620bbbcdfc8..24db1df09075 100644
--- a/drivers/clk/sprd/common.c
+++ b/drivers/clk/sprd/common.c
@@ -41,7 +41,7 @@ int sprd_clk_regmap_init(struct platform_device *pdev,
 {
 	void __iomem *base;
 	struct device *dev = &pdev->dev;
-	struct device_node *node = dev->of_node;
+	struct device_node *node = dev->of_node, *np;
 	struct regmap *regmap;
 
 	if (of_find_property(node, "sprd,syscon", NULL)) {
@@ -50,9 +50,10 @@ int sprd_clk_regmap_init(struct platform_device *pdev,
 			pr_err("%s: failed to get syscon regmap\n", __func__);
 			return PTR_ERR(regmap);
 		}
-	} else if (of_device_is_compatible(of_get_parent(dev->of_node),
-			   "syscon")) {
-		regmap = device_node_to_regmap(of_get_parent(dev->of_node));
+	} else if (of_device_is_compatible(np =	of_get_parent(node), "syscon")
+		|| (of_node_put(np), 0)) {
+		regmap = device_node_to_regmap(np);
+		of_node_put(np);
 		if (IS_ERR(regmap)) {
 			dev_err(dev, "failed to get regmap from its parent.\n");
 			return PTR_ERR(regmap);
-- 
2.25.1

Re: [PATCH v5] clk: sprd: Hold reference returned by of_get_parent()

[Orson Zhai]

Liang,

On Mon, Jul 4, 2022 at 8:47 AM Liang He <windhl@126.com> wrote:
>
> We should hold the reference returned by of_get_parent() and use it
> to call of_node_put() for refcount balance.
>
> Fixes: f95e8c7923d1 ("clk: sprd: support to get regmap from parent node")
> Signed-off-by: Liang He <windhl@126.com>

Reviewed-by: Orson Zhai <orsonzhai@gmail.com>

Great job.
Thanks.

-Orson

> ---
>  changelog:
>
>  v5: fix bug reported by Chunyan and confirmed by Orson
>  v4: fix another bug in the same place, missing in v3
>  v3: (1) keep original 'if-else if-else' coding style adviesd by Orson
>      (2) fix typo in commit-log: of_node_out --> of_node_put
>  v2: minimize the effective range of of_get_parent() advised by Orson
>  v1: hold reference returned by of_get_parent()
>
>
>  drivers/clk/sprd/common.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
> index d620bbbcdfc8..24db1df09075 100644
> --- a/drivers/clk/sprd/common.c
> +++ b/drivers/clk/sprd/common.c
> @@ -41,7 +41,7 @@ int sprd_clk_regmap_init(struct platform_device *pdev,
>  {
>         void __iomem *base;
>         struct device *dev = &pdev->dev;
> -       struct device_node *node = dev->of_node;
> +       struct device_node *node = dev->of_node, *np;
>         struct regmap *regmap;
>
>         if (of_find_property(node, "sprd,syscon", NULL)) {
> @@ -50,9 +50,10 @@ int sprd_clk_regmap_init(struct platform_device *pdev,
>                         pr_err("%s: failed to get syscon regmap\n", __func__);
>                         return PTR_ERR(regmap);
>                 }
> -       } else if (of_device_is_compatible(of_get_parent(dev->of_node),
> -                          "syscon")) {
> -               regmap = device_node_to_regmap(of_get_parent(dev->of_node));
> +       } else if (of_device_is_compatible(np = of_get_parent(node), "syscon")
> +               || (of_node_put(np), 0)) {
> +               regmap = device_node_to_regmap(np);
> +               of_node_put(np);
>                 if (IS_ERR(regmap)) {
>                         dev_err(dev, "failed to get regmap from its parent.\n");
>                         return PTR_ERR(regmap);
> --
> 2.25.1
>

Re: [PATCH v5] clk: sprd: Hold reference returned by of_get_parent()

[Stephen Boyd]

Quoting Liang He (2022-07-03 17:47:29)
> We should hold the reference returned by of_get_parent() and use it
> to call of_node_put() for refcount balance.
> 
> Fixes: f95e8c7923d1 ("clk: sprd: support to get regmap from parent node")
> Signed-off-by: Liang He <windhl@126.com>
> ---
>  changelog:
> 
>  v5: fix bug reported by Chunyan and confirmed by Orson
>  v4: fix another bug in the same place, missing in v3
>  v3: (1) keep original 'if-else if-else' coding style adviesd by Orson
>      (2) fix typo in commit-log: of_node_out --> of_node_put
>  v2: minimize the effective range of of_get_parent() advised by Orson
>  v1: hold reference returned by of_get_parent()
> 
> 
>  drivers/clk/sprd/common.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
> index d620bbbcdfc8..24db1df09075 100644
> --- a/drivers/clk/sprd/common.c
> +++ b/drivers/clk/sprd/common.c
> @@ -41,7 +41,7 @@ int sprd_clk_regmap_init(struct platform_device *pdev,
>  {
>         void __iomem *base;
>         struct device *dev = &pdev->dev;
> -       struct device_node *node = dev->of_node;
> +       struct device_node *node = dev->of_node, *np;
>         struct regmap *regmap;
>  
>         if (of_find_property(node, "sprd,syscon", NULL)) {
> @@ -50,9 +50,10 @@ int sprd_clk_regmap_init(struct platform_device *pdev,
>                         pr_err("%s: failed to get syscon regmap\n", __func__);
>                         return PTR_ERR(regmap);
>                 }
> -       } else if (of_device_is_compatible(of_get_parent(dev->of_node),
> -                          "syscon")) {
> -               regmap = device_node_to_regmap(of_get_parent(dev->of_node));
> +       } else if (of_device_is_compatible(np = of_get_parent(node), "syscon")
> +               || (of_node_put(np), 0)) {

This is gross.

Re: [PATCH v5] clk: sprd: Hold reference returned by of_get_parent()

[Stephen Boyd]

Quoting Liang He (2022-07-03 17:47:29)
> We should hold the reference returned by of_get_parent() and use it
> to call of_node_put() for refcount balance.
> 
> Fixes: f95e8c7923d1 ("clk: sprd: support to get regmap from parent node")
> Signed-off-by: Liang He <windhl@126.com>
> ---

Applied to clk-next