Use of of_parse_phandle()/of_node_put()

Commit Message

Anil Kumar Feb. 13, 2013, 4:40 p.m. UTC

Hi,

On Wed, Feb 13, 2013 at 9:22 PM, Peter Ujfalusi <[e-mail]> wrote:
> On 02/13/2013 07:04 AM, Kumar, Anil wrote:
>> Hi  Peter,
>>
>> Just trying to understand.
>>
>> In omap-twl4030.c file probe function :-
>>
>> dai_node = of_parse_phandle(node, "ti,mcbsp", 0);
>>                 if (!dai_node) {
>>                         dev_err(&pdev->dev, "McBSP node is not provided\n");
>>                         return -EINVAL;
>>                 }
>>
>> Here  “of_parse_phandle()”  is used to get  “of_device” node pointer.
>> of_parse_phandle() suggest to use of_node_put()  on it when done.
>>
>> It looks when code request for an “of_device” node, kernel  maintains
>> “refcount” for this.
>> It check “refcount”  before giving pointer of of_device node and  WARN_ON()
>> in case of refcount  > 0 and increase it on success.
>>
>> Should this code need to use of_node_put() on the requested “of_device” when done
>> so that this can be get  again ?
>
> Hrm, one thing or sure we should not call it of_node_put() while we have the
> card loaded since the node is used runtime by the core.
>
> However when we unload the machine driver it might be needed, but not sure
> about this. None of the existing machine drivers doing it (tegra, samsung,
> omap, etc).
> But if it is needed it might be better to be done by the core?
>

It looks issue is different here.
I have done some testing here with patch[1]. In this patch i tried to
get same "of_device" node pointer
again and found refcount for this dt node is 1 [Result].

As of_parse_phandle() says it "returns the device_node pointer with
refcount incremented".
But why refcount  value is 1 [Result] again ?

-----------------------------8---------------------
Patch[1]:-
                omap_twl4030_dai_links[0].cpu_of_node = dai_node;

[Result]:-
root@DevKit8000:/# insmod snd-soc-omap-twl4030.ko
[   95.718109] refcount 0x1
[   95.720611] refcount 0x1 [   95.818054] omap-twl4030 sound.20:
twl4030-hifi <-> 49022000.mcbsp mapping ok


Thanks,
Anil
[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Peter Ujfalusi Feb. 14, 2013, 9:21 a.m. UTC | #1

On 02/13/2013 05:40 PM, Anil Kumar wrote:
> It looks issue is different here.
> I have done some testing here with patch[1]. In this patch i tried to
> get same "of_device" node pointer
> again and found refcount for this dt node is 1 [Result].
> 
> As of_parse_phandle() says it "returns the device_node pointer with
> refcount incremented".
> But why refcount  value is 1 [Result] again ?

Documentation/kref.txt

> 
> -----------------------------8---------------------
> Patch[1]:-
> diff --git a/sound/soc/omap/omap-twl4030.c b/sound/soc/omap/omap-twl4030.c
> index fd98509..0828a5c 100644
> --- a/sound/soc/omap/omap-twl4030.c
> +++ b/sound/soc/omap/omap-twl4030.c
> @@ -297,6 +297,16 @@ static int omap_twl4030_probe(struct platform_device *pdev)
>                         dev_err(&pdev->dev, "McBSP node is not provided\n");
>                         return -EINVAL;
>                 }
> +               printk(KERN_ERR"refcount 0x%x",
> atomic_read(&dai_node->kref.refcount));
> +
> +               dai_node = of_parse_phandle(node, "ti,mcbsp", 0);
> +                if (!dai_node) {
> +                        dev_err(&pdev->dev, "McBSP node is not provided\n");
> +                        return -EINVAL;
> +                }
> +               printk(KERN_ERR"refcount 0x%x",
> atomic_read(&dai_node->kref.refcount));
> +
> +
>                 omap_twl4030_dai_links[0].cpu_dai_name  = NULL;
>                 omap_twl4030_dai_links[0].cpu_of_node = dai_node;
> 
> [Result]:-
> root@DevKit8000:/# insmod snd-soc-omap-twl4030.ko
> [   95.718109] refcount 0x1
> [   95.720611] refcount 0x1 [   95.818054] omap-twl4030 sound.20:
> twl4030-hifi <-> 49022000.mcbsp mapping ok
> 
> 
> Thanks,
> Anil
> [...]
>

Patch

diff --git a/sound/soc/omap/omap-twl4030.c b/sound/soc/omap/omap-twl4030.c
index fd98509..0828a5c 100644
--- a/sound/soc/omap/omap-twl4030.c
+++ b/sound/soc/omap/omap-twl4030.c
@@ -297,6 +297,16 @@  static int omap_twl4030_probe(struct platform_device *pdev)
                        dev_err(&pdev->dev, "McBSP node is not provided\n");
                        return -EINVAL;
                }
+               printk(KERN_ERR"refcount 0x%x",
atomic_read(&dai_node->kref.refcount));
+
+               dai_node = of_parse_phandle(node, "ti,mcbsp", 0);
+                if (!dai_node) {
+                        dev_err(&pdev->dev, "McBSP node is not provided\n");
+                        return -EINVAL;
+                }
+               printk(KERN_ERR"refcount 0x%x",
atomic_read(&dai_node->kref.refcount));
+
+
                omap_twl4030_dai_links[0].cpu_dai_name  = NULL;