From: Tony Lindgren <tony@atomide.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Paul Walmsley <paul@pwsan.com>, Rob Herring <robh@kernel.org>,
Russell King <linux@armlinux.org.uk>,
linux-kernel@vger.kernel.org, Qi Hou <qi.hou@windriver.com>,
linux-omap@vger.kernel.org, Peter Rosin <peda@axentia.se>,
linux-arm-kernel@lists.infradead.org
Subject: Re: ARM: OMAP2+: Grab reference to device nodes where needed
Date: Tue, 28 Feb 2017 09:52:47 -0800 [thread overview]
Message-ID: <20170228175246.GE20572@atomide.com> (raw)
In-Reply-To: <20170228172637.GA13455@roeck-us.net>
* Guenter Roeck <linux@roeck-us.net> [170228 09:28]:
> On Tue, Feb 28, 2017 at 08:23:16AM -0800, Tony Lindgren wrote:
> > * Guenter Roeck <linux@roeck-us.net> [170227 15:36]:
> > > ping ... this problem is now seen in mainline.
> >
> > Oops sorry I had accidentally tagged this with "next" instead of
> > "fixes".
> >
> > Looking at the patch though, can't we make of_node_get/put local to
> > omap3xxx_hwmod_is_hs_ip_block_usable()?
> >
> Not really.
>
> Strictly speaking the patch fixes two problems:
> 1) of_find_node_by_name() gets a reference on "bus", thus of_node_put()
> is necessary to release it. This is the change in omap3xxx_hwmod_init().
> 2) of_find_node_by_name() drops a reference to the passed device node,
> thus it is necessary to call of_node_get(bus) in
> omap3xxx_hwmod_is_hs_ip_block_usable().
>
> I should have stated that more clearly in the description.
OK
> We could ignore 1) to get rid of the warning, but that would leave us with
> a dangling reference to the bus node. I could also improve the description,
> or split the patch in two. Let me know what you prefer.
Once patch is just fine with me.
> Another possible change would be to use of_get_child_by_name() instead of
> of_find_node_by_name(). That would, however, only search immediate children
> of the 'ocp' node, not all nodes. I think that is what is actually intended
> here, but I am not entirely sure. Thoughts ?
Yes that would be better.
> Either case, turns out I'll have to re-submit the patch because, as mentioned
> above, of_find_node_by_name() gets a reference to the device node it returns,
> so a put on that node is necessary in omap3xxx_hwmod_is_hs_ip_block_usable().
OK
Thanks,
Tony
WARNING: multiple messages have this Message-ID (diff)
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: ARM: OMAP2+: Grab reference to device nodes where needed
Date: Tue, 28 Feb 2017 09:52:47 -0800 [thread overview]
Message-ID: <20170228175246.GE20572@atomide.com> (raw)
In-Reply-To: <20170228172637.GA13455@roeck-us.net>
* Guenter Roeck <linux@roeck-us.net> [170228 09:28]:
> On Tue, Feb 28, 2017 at 08:23:16AM -0800, Tony Lindgren wrote:
> > * Guenter Roeck <linux@roeck-us.net> [170227 15:36]:
> > > ping ... this problem is now seen in mainline.
> >
> > Oops sorry I had accidentally tagged this with "next" instead of
> > "fixes".
> >
> > Looking at the patch though, can't we make of_node_get/put local to
> > omap3xxx_hwmod_is_hs_ip_block_usable()?
> >
> Not really.
>
> Strictly speaking the patch fixes two problems:
> 1) of_find_node_by_name() gets a reference on "bus", thus of_node_put()
> is necessary to release it. This is the change in omap3xxx_hwmod_init().
> 2) of_find_node_by_name() drops a reference to the passed device node,
> thus it is necessary to call of_node_get(bus) in
> omap3xxx_hwmod_is_hs_ip_block_usable().
>
> I should have stated that more clearly in the description.
OK
> We could ignore 1) to get rid of the warning, but that would leave us with
> a dangling reference to the bus node. I could also improve the description,
> or split the patch in two. Let me know what you prefer.
Once patch is just fine with me.
> Another possible change would be to use of_get_child_by_name() instead of
> of_find_node_by_name(). That would, however, only search immediate children
> of the 'ocp' node, not all nodes. I think that is what is actually intended
> here, but I am not entirely sure. Thoughts ?
Yes that would be better.
> Either case, turns out I'll have to re-submit the patch because, as mentioned
> above, of_find_node_by_name() gets a reference to the device node it returns,
> so a put on that node is necessary in omap3xxx_hwmod_is_hs_ip_block_usable().
OK
Thanks,
Tony
next prev parent reply other threads:[~2017-02-28 17:52 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-11 22:59 [PATCH] ARM: OMAP2+: Grab reference to device nodes where needed Guenter Roeck
2017-02-11 22:59 ` Guenter Roeck
2017-02-27 23:34 ` Guenter Roeck
2017-02-27 23:34 ` Guenter Roeck
2017-02-27 23:34 ` Guenter Roeck
2017-02-28 16:23 ` Tony Lindgren
2017-02-28 16:23 ` Tony Lindgren
2017-02-28 16:23 ` Tony Lindgren
2017-02-28 17:26 ` Guenter Roeck
2017-02-28 17:26 ` Guenter Roeck
2017-02-28 17:26 ` Guenter Roeck
2017-02-28 17:52 ` Tony Lindgren [this message]
2017-02-28 17:52 ` Tony Lindgren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170228175246.GE20572@atomide.com \
--to=tony@atomide.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=linux@roeck-us.net \
--cc=paul@pwsan.com \
--cc=peda@axentia.se \
--cc=qi.hou@windriver.com \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.