* [PATCH] clk/sprd: Hold reference returned by of_get_parent()
@ 2022-06-24 10:38 Liang He
2022-06-25 8:36 ` Orson Zhai
0 siblings, 1 reply; 6+ messages in thread
From: Liang He @ 2022-06-24 10:38 UTC (permalink / raw)
To: mturquette, sboyd, orsonzhai, baolin.wang7, zhang.lyra,
linux-clk, windhl
We should hold the reference returned by of_get_parent() and use it
to call of_node_out() for refcount balance.
Fixes: f95e8c7923d1 ("clk: sprd: support to get regmap from parent node")
Signed-off-by: Liang He <windhl@126.com>
---
these bugs are found in 5.19rc2
these bugs are confirmed not fixed by lore.kernel.org
these bugs are compiled tested in 5.19rc2 with at91_dt_defconfig
drivers/clk/sprd/common.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
index d620bbbcdfc8..08c1d7a9ec8b 100644
--- a/drivers/clk/sprd/common.c
+++ b/drivers/clk/sprd/common.c
@@ -41,35 +41,41 @@ 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;
+ np = of_get_parent(dev->of_node);
if (of_find_property(node, "sprd,syscon", NULL)) {
regmap = syscon_regmap_lookup_by_phandle(node, "sprd,syscon");
if (IS_ERR(regmap)) {
pr_err("%s: failed to get syscon regmap\n", __func__);
+ of_node_put(np);
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, "syscon")) {
+ regmap = device_node_to_regmap(np);
if (IS_ERR(regmap)) {
+ of_node_put(np);
dev_err(dev, "failed to get regmap from its parent.\n");
return PTR_ERR(regmap);
}
} else {
base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(base))
+ if (IS_ERR(base)) {
+ of_node_put(np);
return PTR_ERR(base);
+ }
regmap = devm_regmap_init_mmio(&pdev->dev, base,
&sprdclk_regmap_config);
if (IS_ERR(regmap)) {
+ of_node_put(np);
pr_err("failed to init regmap\n");
return PTR_ERR(regmap);
}
}
+ of_node_put(np);
sprd_clk_set_regmap(desc, regmap);
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] clk/sprd: Hold reference returned by of_get_parent()
2022-06-24 10:38 [PATCH] clk/sprd: Hold reference returned by of_get_parent() Liang He
@ 2022-06-25 8:36 ` Orson Zhai
2022-06-25 15:32 ` Liang He
0 siblings, 1 reply; 6+ messages in thread
From: Orson Zhai @ 2022-06-25 8:36 UTC (permalink / raw)
To: Liang He; +Cc: mturquette, Stephen Boyd, Baolin Wang, Lyra Zhang, linux-clk
Hi Liang,
On Fri, Jun 24, 2022 at 6:39 PM Liang He <windhl@126.com> wrote:
>
> We should hold the reference returned by of_get_parent() and use it
> to call of_node_out() for refcount balance.
>
> Fixes: f95e8c7923d1 ("clk: sprd: support to get regmap from parent node")
> Signed-off-by: Liang He <windhl@126.com>
> ---
> these bugs are found in 5.19rc2
> these bugs are confirmed not fixed by lore.kernel.org
> these bugs are compiled tested in 5.19rc2 with at91_dt_defconfig
>
> drivers/clk/sprd/common.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
> index d620bbbcdfc8..08c1d7a9ec8b 100644
> --- a/drivers/clk/sprd/common.c
> +++ b/drivers/clk/sprd/common.c
> @@ -41,35 +41,41 @@ 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;
>
> + np = of_get_parent(dev->of_node);
> if (of_find_property(node, "sprd,syscon", NULL)) {
> regmap = syscon_regmap_lookup_by_phandle(node, "sprd,syscon");
Why not to call of_node_put() in this function?
He returns an error and it is his responsibility to clean up
everything he used, right?
-Orson
> if (IS_ERR(regmap)) {
> pr_err("%s: failed to get syscon regmap\n", __func__);
> + of_node_put(np);
> 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, "syscon")) {
> + regmap = device_node_to_regmap(np);
> if (IS_ERR(regmap)) {
> + of_node_put(np);
> dev_err(dev, "failed to get regmap from its parent.\n");
> return PTR_ERR(regmap);
> }
> } else {
> base = devm_platform_ioremap_resource(pdev, 0);
> - if (IS_ERR(base))
> + if (IS_ERR(base)) {
> + of_node_put(np);
> return PTR_ERR(base);
> + }
>
> regmap = devm_regmap_init_mmio(&pdev->dev, base,
> &sprdclk_regmap_config);
> if (IS_ERR(regmap)) {
> + of_node_put(np);
> pr_err("failed to init regmap\n");
> return PTR_ERR(regmap);
> }
> }
>
> + of_node_put(np);
> sprd_clk_set_regmap(desc, regmap);
>
> return 0;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re:Re: [PATCH] clk/sprd: Hold reference returned by of_get_parent()
2022-06-25 8:36 ` Orson Zhai
@ 2022-06-25 15:32 ` Liang He
2022-06-26 3:32 ` Orson Zhai
0 siblings, 1 reply; 6+ messages in thread
From: Liang He @ 2022-06-25 15:32 UTC (permalink / raw)
To: Orson Zhai; +Cc: mturquette, Stephen Boyd, Baolin Wang, Lyra Zhang, linux-clk
At 2022-06-25 16:36:04, "Orson Zhai" <orsonzhai@gmail.com> wrote:
>Hi Liang,
>
>On Fri, Jun 24, 2022 at 6:39 PM Liang He <windhl@126.com> wrote:
>>
>> We should hold the reference returned by of_get_parent() and use it
>> to call of_node_out() for refcount balance.
>>
>> Fixes: f95e8c7923d1 ("clk: sprd: support to get regmap from parent node")
>> Signed-off-by: Liang He <windhl@126.com>
>> ---
>> these bugs are found in 5.19rc2
>> these bugs are confirmed not fixed by lore.kernel.org
>> these bugs are compiled tested in 5.19rc2 with at91_dt_defconfig
>>
>> drivers/clk/sprd/common.c | 16 +++++++++++-----
>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
>> index d620bbbcdfc8..08c1d7a9ec8b 100644
>> --- a/drivers/clk/sprd/common.c
>> +++ b/drivers/clk/sprd/common.c
>> @@ -41,35 +41,41 @@ 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;
>>
>> + np = of_get_parent(dev->of_node);
>> if (of_find_property(node, "sprd,syscon", NULL)) {
>> regmap = syscon_regmap_lookup_by_phandle(node, "sprd,syscon");
>
>Why not to call of_node_put() in this function?
>He returns an error and it is his responsibility to clean up
>everything he used, right?
>
>-Orson
>
Hi, Orson,
I only know we should use the pair of functions (of_get_parent(), of_node_put)
to keep refcount balance.
However, as I do not really know the semantic of other functions, so I cannot
give other kinds of patch.
By the way, if we chose to put of_node_put() in this function, I think it also need
to put of_node_put() into 'device_node_to_regmap' as it also return error code in
following path.
Anyway, thanks very much for your reply and review on my patch code.
Liang
>> if (IS_ERR(regmap)) {
>> pr_err("%s: failed to get syscon regmap\n", __func__);
>> + of_node_put(np);
>> 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, "syscon")) {
>> + regmap = device_node_to_regmap(np);
>> if (IS_ERR(regmap)) {
>> + of_node_put(np);
>> dev_err(dev, "failed to get regmap from its parent.\n");
>> return PTR_ERR(regmap);
>> }
>> } else {
>> base = devm_platform_ioremap_resource(pdev, 0);
>> - if (IS_ERR(base))
>> + if (IS_ERR(base)) {
>> + of_node_put(np);
>> return PTR_ERR(base);
>> + }
>>
>> regmap = devm_regmap_init_mmio(&pdev->dev, base,
>> &sprdclk_regmap_config);
>> if (IS_ERR(regmap)) {
>> + of_node_put(np);
>> pr_err("failed to init regmap\n");
>> return PTR_ERR(regmap);
>> }
>> }
>>
>> + of_node_put(np);
>> sprd_clk_set_regmap(desc, regmap);
>>
>> return 0;
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: [PATCH] clk/sprd: Hold reference returned by of_get_parent()
2022-06-25 15:32 ` Liang He
@ 2022-06-26 3:32 ` Orson Zhai
2022-06-26 4:03 ` Liang He
0 siblings, 1 reply; 6+ messages in thread
From: Orson Zhai @ 2022-06-26 3:32 UTC (permalink / raw)
To: Liang He; +Cc: mturquette, Stephen Boyd, Baolin Wang, Lyra Zhang, linux-clk
Hi Liang,
On Sat, Jun 25, 2022 at 11:34 PM Liang He <windhl@126.com> wrote:
>
>
> At 2022-06-25 16:36:04, "Orson Zhai" <orsonzhai@gmail.com> wrote:
> >Hi Liang,
> >
> >On Fri, Jun 24, 2022 at 6:39 PM Liang He <windhl@126.com> wrote:
> >>
> >> We should hold the reference returned by of_get_parent() and use it
> >> to call of_node_out() for refcount balance.
> >>
> >> Fixes: f95e8c7923d1 ("clk: sprd: support to get regmap from parent node")
> >> Signed-off-by: Liang He <windhl@126.com>
> >> ---
> >> these bugs are found in 5.19rc2
> >> these bugs are confirmed not fixed by lore.kernel.org
> >> these bugs are compiled tested in 5.19rc2 with at91_dt_defconfig
> >>
> >> drivers/clk/sprd/common.c | 16 +++++++++++-----
> >> 1 file changed, 11 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
> >> index d620bbbcdfc8..08c1d7a9ec8b 100644
> >> --- a/drivers/clk/sprd/common.c
> >> +++ b/drivers/clk/sprd/common.c
> >> @@ -41,35 +41,41 @@ 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;
> >>
> >> + np = of_get_parent(dev->of_node);
> >> if (of_find_property(node, "sprd,syscon", NULL)) {
> >> regmap = syscon_regmap_lookup_by_phandle(node, "sprd,syscon");
> >
> >Why not to call of_node_put() in this function?
> >He returns an error and it is his responsibility to clean up
> >everything he used, right?
> >
> >-Orson
> >
> Hi, Orson,
>
> I only know we should use the pair of functions (of_get_parent(), of_node_put)
> to keep refcount balance.
>
> However, as I do not really know the semantic of other functions, so I cannot
> give other kinds of patch.
Sorry, I didn't catch your point before.
You're right on this unbalance. But it has no direct relation to the
function here.
I think it is better to minimize the effective range of of_get_parent()
Could you refer to Pantelis's answer to the last question at [1].
Thanks.
-Orson
[1] https://www.spinics.net/lists/devicetree/msg122555.html
>
> By the way, if we chose to put of_node_put() in this function, I think it also need
> to put of_node_put() into 'device_node_to_regmap' as it also return error code in
> following path.
>
> Anyway, thanks very much for your reply and review on my patch code.
>
> Liang
>
>
> >> if (IS_ERR(regmap)) {
> >> pr_err("%s: failed to get syscon regmap\n", __func__);
> >> + of_node_put(np);
> >> 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, "syscon")) {
> >> + regmap = device_node_to_regmap(np);
> >> if (IS_ERR(regmap)) {
> >> + of_node_put(np);
> >> dev_err(dev, "failed to get regmap from its parent.\n");
> >> return PTR_ERR(regmap);
> >> }
> >> } else {
> >> base = devm_platform_ioremap_resource(pdev, 0);
> >> - if (IS_ERR(base))
> >> + if (IS_ERR(base)) {
> >> + of_node_put(np);
> >> return PTR_ERR(base);
> >> + }
> >>
> >> regmap = devm_regmap_init_mmio(&pdev->dev, base,
> >> &sprdclk_regmap_config);
> >> if (IS_ERR(regmap)) {
> >> + of_node_put(np);
> >> pr_err("failed to init regmap\n");
> >> return PTR_ERR(regmap);
> >> }
> >> }
> >>
> >> + of_node_put(np);
> >> sprd_clk_set_regmap(desc, regmap);
> >>
> >> return 0;
> >> --
> >> 2.25.1
> >>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re:Re: Re: [PATCH] clk/sprd: Hold reference returned by of_get_parent()
2022-06-26 3:32 ` Orson Zhai
@ 2022-06-26 4:03 ` Liang He
2022-06-27 7:55 ` Orson Zhai
0 siblings, 1 reply; 6+ messages in thread
From: Liang He @ 2022-06-26 4:03 UTC (permalink / raw)
To: Orson Zhai; +Cc: mturquette, Stephen Boyd, Baolin Wang, Lyra Zhang, linux-clk
At 2022-06-26 11:32:40, "Orson Zhai" <orsonzhai@gmail.com> wrote:
>Hi Liang,
>
>On Sat, Jun 25, 2022 at 11:34 PM Liang He <windhl@126.com> wrote:
>>
>>
>> At 2022-06-25 16:36:04, "Orson Zhai" <orsonzhai@gmail.com> wrote:
>> >Hi Liang,
>> >
>> >On Fri, Jun 24, 2022 at 6:39 PM Liang He <windhl@126.com> wrote:
>> >>
>> >> We should hold the reference returned by of_get_parent() and use it
>> >> to call of_node_out() for refcount balance.
>> >>
>> >> Fixes: f95e8c7923d1 ("clk: sprd: support to get regmap from parent node")
>> >> Signed-off-by: Liang He <windhl@126.com>
>> >> ---
>> >> these bugs are found in 5.19rc2
>> >> these bugs are confirmed not fixed by lore.kernel.org
>> >> these bugs are compiled tested in 5.19rc2 with at91_dt_defconfig
>> >>
>> >> drivers/clk/sprd/common.c | 16 +++++++++++-----
>> >> 1 file changed, 11 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
>> >> index d620bbbcdfc8..08c1d7a9ec8b 100644
>> >> --- a/drivers/clk/sprd/common.c
>> >> +++ b/drivers/clk/sprd/common.c
>> >> @@ -41,35 +41,41 @@ 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;
>> >>
>> >> + np = of_get_parent(dev->of_node);
>> >> if (of_find_property(node, "sprd,syscon", NULL)) {
>> >> regmap = syscon_regmap_lookup_by_phandle(node, "sprd,syscon");
>> >
>> >Why not to call of_node_put() in this function?
>> >He returns an error and it is his responsibility to clean up
>> >everything he used, right?
>> >
>> >-Orson
>> >
>> Hi, Orson,
>>
>> I only know we should use the pair of functions (of_get_parent(), of_node_put)
>> to keep refcount balance.
>>
>> However, as I do not really know the semantic of other functions, so I cannot
>> give other kinds of patch.
>
>Sorry, I didn't catch your point before.
>You're right on this unbalance. But it has no direct relation to the
>function here.
Thanks for your understanding. And yes, it has no direct relation
to the function here.
>I think it is better to minimize the effective range of of_get_parent()
>
>Could you refer to Pantelis's answer to the last question at [1].
>
>Thanks.
>-Orson
>
>[1] https://www.spinics.net/lists/devicetree/msg122555.html
>
Thanks, Orson.
Yes, I also wanted to make a better patch solution as the bug is only caused
by the 'else if' branch.
However, I cannot find a better solution to fix the bug in the middle of nested
'if-else if-else' brances unless we try to rewrite them, which is more difficult for me, now.
But if you think it is (semantic) safe to rewrite these branches, I can try it.
Thanks again for your relpy.
Liang
>>
>> By the way, if we chose to put of_node_put() in this function, I think it also need
>> to put of_node_put() into 'device_node_to_regmap' as it also return error code in
>> following path.
>>
>> Anyway, thanks very much for your reply and review on my patch code.
>>
>> Liang
>>
>>
>> >> if (IS_ERR(regmap)) {
>> >> pr_err("%s: failed to get syscon regmap\n", __func__);
>> >> + of_node_put(np);
>> >> 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, "syscon")) {
>> >> + regmap = device_node_to_regmap(np);
>> >> if (IS_ERR(regmap)) {
>> >> + of_node_put(np);
>> >> dev_err(dev, "failed to get regmap from its parent.\n");
>> >> return PTR_ERR(regmap);
>> >> }
>> >> } else {
>> >> base = devm_platform_ioremap_resource(pdev, 0);
>> >> - if (IS_ERR(base))
>> >> + if (IS_ERR(base)) {
>> >> + of_node_put(np);
>> >> return PTR_ERR(base);
>> >> + }
>> >>
>> >> regmap = devm_regmap_init_mmio(&pdev->dev, base,
>> >> &sprdclk_regmap_config);
>> >> if (IS_ERR(regmap)) {
>> >> + of_node_put(np);
>> >> pr_err("failed to init regmap\n");
>> >> return PTR_ERR(regmap);
>> >> }
>> >> }
>> >>
>> >> + of_node_put(np);
>> >> sprd_clk_set_regmap(desc, regmap);
>> >>
>> >> return 0;
>> >> --
>> >> 2.25.1
>> >>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: Re: [PATCH] clk/sprd: Hold reference returned by of_get_parent()
2022-06-26 4:03 ` Liang He
@ 2022-06-27 7:55 ` Orson Zhai
0 siblings, 0 replies; 6+ messages in thread
From: Orson Zhai @ 2022-06-27 7:55 UTC (permalink / raw)
To: Liang He
Cc: Michael Turquette, Stephen Boyd, Baolin Wang, Lyra Zhang, linux-clk
On Sun, Jun 26, 2022 at 12:03 PM Liang He <windhl@126.com> wrote:
>
>
>
> At 2022-06-26 11:32:40, "Orson Zhai" <orsonzhai@gmail.com> wrote:
> >Hi Liang,
> >
> >On Sat, Jun 25, 2022 at 11:34 PM Liang He <windhl@126.com> wrote:
> >>
> >>
> >> At 2022-06-25 16:36:04, "Orson Zhai" <orsonzhai@gmail.com> wrote:
> >> >Hi Liang,
> >> >
> >> >On Fri, Jun 24, 2022 at 6:39 PM Liang He <windhl@126.com> wrote:
> >> >>
> >> >> We should hold the reference returned by of_get_parent() and use it
> >> >> to call of_node_out() for refcount balance.
> >> >>
> >> >> Fixes: f95e8c7923d1 ("clk: sprd: support to get regmap from parent node")
> >> >> Signed-off-by: Liang He <windhl@126.com>
> >> >> ---
> >> >> these bugs are found in 5.19rc2
> >> >> these bugs are confirmed not fixed by lore.kernel.org
> >> >> these bugs are compiled tested in 5.19rc2 with at91_dt_defconfig
> >> >>
> >> >> drivers/clk/sprd/common.c | 16 +++++++++++-----
> >> >> 1 file changed, 11 insertions(+), 5 deletions(-)
> >> >>
> >> >> diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
> >> >> index d620bbbcdfc8..08c1d7a9ec8b 100644
> >> >> --- a/drivers/clk/sprd/common.c
> >> >> +++ b/drivers/clk/sprd/common.c
> >> >> @@ -41,35 +41,41 @@ 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;
> >> >>
> >> >> + np = of_get_parent(dev->of_node);
> >> >> if (of_find_property(node, "sprd,syscon", NULL)) {
> >> >> regmap = syscon_regmap_lookup_by_phandle(node, "sprd,syscon");
> >> >
> >> >Why not to call of_node_put() in this function?
> >> >He returns an error and it is his responsibility to clean up
> >> >everything he used, right?
> >> >
> >> >-Orson
> >> >
> >> Hi, Orson,
> >>
> >> I only know we should use the pair of functions (of_get_parent(), of_node_put)
> >> to keep refcount balance.
> >>
> >> However, as I do not really know the semantic of other functions, so I cannot
> >> give other kinds of patch.
> >
> >Sorry, I didn't catch your point before.
> >You're right on this unbalance. But it has no direct relation to the
> >function here.
>
> Thanks for your understanding. And yes, it has no direct relation
> to the function here.
>
> >I think it is better to minimize the effective range of of_get_parent()
> >
> >Could you refer to Pantelis's answer to the last question at [1].
> >
> >Thanks.
> >-Orson
> >
> >[1] https://www.spinics.net/lists/devicetree/msg122555.html
> >
>
> Thanks, Orson.
>
> Yes, I also wanted to make a better patch solution as the bug is only caused
> by the 'else if' branch.
>
> However, I cannot find a better solution to fix the bug in the middle of nested
> 'if-else if-else' brances unless we try to rewrite them, which is more difficult for me, now.
>
> But if you think it is (semantic) safe to rewrite these branches, I can try it.
No problem. Please do it as long as not to change its original logic.
Thanks.
-Orson
>
> Thanks again for your relpy.
>
> Liang
>
> >>
> >> By the way, if we chose to put of_node_put() in this function, I think it also need
> >> to put of_node_put() into 'device_node_to_regmap' as it also return error code in
> >> following path.
> >>
> >> Anyway, thanks very much for your reply and review on my patch code.
> >>
> >> Liang
> >>
> >>
> >> >> if (IS_ERR(regmap)) {
> >> >> pr_err("%s: failed to get syscon regmap\n", __func__);
> >> >> + of_node_put(np);
> >> >> 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, "syscon")) {
> >> >> + regmap = device_node_to_regmap(np);
> >> >> if (IS_ERR(regmap)) {
> >> >> + of_node_put(np);
> >> >> dev_err(dev, "failed to get regmap from its parent.\n");
> >> >> return PTR_ERR(regmap);
> >> >> }
> >> >> } else {
> >> >> base = devm_platform_ioremap_resource(pdev, 0);
> >> >> - if (IS_ERR(base))
> >> >> + if (IS_ERR(base)) {
> >> >> + of_node_put(np);
> >> >> return PTR_ERR(base);
> >> >> + }
> >> >>
> >> >> regmap = devm_regmap_init_mmio(&pdev->dev, base,
> >> >> &sprdclk_regmap_config);
> >> >> if (IS_ERR(regmap)) {
> >> >> + of_node_put(np);
> >> >> pr_err("failed to init regmap\n");
> >> >> return PTR_ERR(regmap);
> >> >> }
> >> >> }
> >> >>
> >> >> + of_node_put(np);
> >> >> sprd_clk_set_regmap(desc, regmap);
> >> >>
> >> >> return 0;
> >> >> --
> >> >> 2.25.1
> >> >>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-27 7:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-24 10:38 [PATCH] clk/sprd: Hold reference returned by of_get_parent() Liang He
2022-06-25 8:36 ` Orson Zhai
2022-06-25 15:32 ` Liang He
2022-06-26 3:32 ` Orson Zhai
2022-06-26 4:03 ` Liang He
2022-06-27 7:55 ` Orson Zhai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.