All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ARM: OMAP2+: Fix device node reference counts
@ 2017-02-28 19:54 ` Guenter Roeck
  0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2017-02-28 19:54 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Tony Lindgren, Russell King, linux-omap, linux-arm-kernel,
	linux-kernel, Guenter Roeck, Qi Hou, Peter Rosin, Rob Herring

After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
the following error may be reported when running omap images.

OF: ERROR: Bad of_node_put() on /ocp@68000000
CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
Hardware name: Generic OMAP3-GP (Flattened Device Tree)
[<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
[<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
[<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
[<c05af1b0>] (kobject_release)
	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
[<c0ad1aa4>] (of_find_node_by_name)
	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
[<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
[<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
[<c1215d5c>] (omap3xxx_hwmod_init)
	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
[<c120faa8>] (omap3_init_early)
	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
[<c120fb2c>] (omap3430_init_early)
	from [<c1204710>] (setup_arch+0xc04/0xc34)
[<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
[<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)

of_find_node_by_name() drops the reference to the passed device node.
Use of_get_child_by_name() instead. Also, release references to
device nodes obtained with of_find_node_by_name() and
of_get_child_by_name() after they are no longer needed.

While at it, clean up the code and change the return type of
omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
and the return type of of_device_is_available().

Cc: Qi Hou <qi.hou@windriver.com>
Cc: Peter Rosin <peda@axentia.se>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Change subject ('Grab reference to device nodes where needed'
    didn't really cover all the changes made)
    Use of_get_child_by_name() instead of of_find_node_by_name()
    Drop references to device nodes as needed
    Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
    to bool

 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 56f917ec8621..042e460f4aa4 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -3111,16 +3111,20 @@ static struct omap_hwmod_ocp_if *omap3xxx_dss_hwmod_ocp_ifs[] __initdata = {
  * Return: 0 if device named @dev_name is not likely to be accessible,
  * or 1 if it is likely to be accessible.
  */
-static int __init omap3xxx_hwmod_is_hs_ip_block_usable(struct device_node *bus,
-						       const char *dev_name)
+static bool __init omap3xxx_hwmod_is_hs_ip_block_usable(struct device_node *bus,
+							const char *dev_name)
 {
+	struct device_node *node;
+	bool available;
+
 	if (!bus)
-		return (omap_type() == OMAP2_DEVICE_TYPE_GP) ? 1 : 0;
+		return omap_type() == OMAP2_DEVICE_TYPE_GP;
 
-	if (of_device_is_available(of_find_node_by_name(bus, dev_name)))
-		return 1;
+	node = of_get_child_by_name(bus, dev_name);
+	available = of_device_is_available(node);
+	of_node_put(node);
 
-	return 0;
+	return available;
 }
 
 int __init omap3xxx_hwmod_init(void)
@@ -3189,15 +3193,20 @@ int __init omap3xxx_hwmod_init(void)
 
 	if (h_sham && omap3xxx_hwmod_is_hs_ip_block_usable(bus, "sham")) {
 		r = omap_hwmod_register_links(h_sham);
-		if (r < 0)
+		if (r < 0) {
+			of_node_put(bus);
 			return r;
+		}
 	}
 
 	if (h_aes && omap3xxx_hwmod_is_hs_ip_block_usable(bus, "aes")) {
 		r = omap_hwmod_register_links(h_aes);
-		if (r < 0)
+		if (r < 0) {
+			of_node_put(bus);
 			return r;
+		}
 	}
+	of_node_put(bus);
 
 	/*
 	 * Register hwmod links specific to certain ES levels of a
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2] ARM: OMAP2+: Fix device node reference counts
@ 2017-02-28 19:54 ` Guenter Roeck
  0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2017-02-28 19:54 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Rob Herring, Tony Lindgren, Russell King, linux-kernel,
	linux-arm-kernel, linux-omap, Peter Rosin, Guenter Roeck, Qi Hou

After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
the following error may be reported when running omap images.

OF: ERROR: Bad of_node_put() on /ocp@68000000
CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
Hardware name: Generic OMAP3-GP (Flattened Device Tree)
[<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
[<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
[<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
[<c05af1b0>] (kobject_release)
	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
[<c0ad1aa4>] (of_find_node_by_name)
	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
[<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
[<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
[<c1215d5c>] (omap3xxx_hwmod_init)
	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
[<c120faa8>] (omap3_init_early)
	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
[<c120fb2c>] (omap3430_init_early)
	from [<c1204710>] (setup_arch+0xc04/0xc34)
[<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
[<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)

of_find_node_by_name() drops the reference to the passed device node.
Use of_get_child_by_name() instead. Also, release references to
device nodes obtained with of_find_node_by_name() and
of_get_child_by_name() after they are no longer needed.

While at it, clean up the code and change the return type of
omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
and the return type of of_device_is_available().

Cc: Qi Hou <qi.hou@windriver.com>
Cc: Peter Rosin <peda@axentia.se>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Change subject ('Grab reference to device nodes where needed'
    didn't really cover all the changes made)
    Use of_get_child_by_name() instead of of_find_node_by_name()
    Drop references to device nodes as needed
    Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
    to bool

 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 56f917ec8621..042e460f4aa4 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -3111,16 +3111,20 @@ static struct omap_hwmod_ocp_if *omap3xxx_dss_hwmod_ocp_ifs[] __initdata = {
  * Return: 0 if device named @dev_name is not likely to be accessible,
  * or 1 if it is likely to be accessible.
  */
-static int __init omap3xxx_hwmod_is_hs_ip_block_usable(struct device_node *bus,
-						       const char *dev_name)
+static bool __init omap3xxx_hwmod_is_hs_ip_block_usable(struct device_node *bus,
+							const char *dev_name)
 {
+	struct device_node *node;
+	bool available;
+
 	if (!bus)
-		return (omap_type() == OMAP2_DEVICE_TYPE_GP) ? 1 : 0;
+		return omap_type() == OMAP2_DEVICE_TYPE_GP;
 
-	if (of_device_is_available(of_find_node_by_name(bus, dev_name)))
-		return 1;
+	node = of_get_child_by_name(bus, dev_name);
+	available = of_device_is_available(node);
+	of_node_put(node);
 
-	return 0;
+	return available;
 }
 
 int __init omap3xxx_hwmod_init(void)
@@ -3189,15 +3193,20 @@ int __init omap3xxx_hwmod_init(void)
 
 	if (h_sham && omap3xxx_hwmod_is_hs_ip_block_usable(bus, "sham")) {
 		r = omap_hwmod_register_links(h_sham);
-		if (r < 0)
+		if (r < 0) {
+			of_node_put(bus);
 			return r;
+		}
 	}
 
 	if (h_aes && omap3xxx_hwmod_is_hs_ip_block_usable(bus, "aes")) {
 		r = omap_hwmod_register_links(h_aes);
-		if (r < 0)
+		if (r < 0) {
+			of_node_put(bus);
 			return r;
+		}
 	}
+	of_node_put(bus);
 
 	/*
 	 * Register hwmod links specific to certain ES levels of a
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2] ARM: OMAP2+: Fix device node reference counts
@ 2017-02-28 19:54 ` Guenter Roeck
  0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2017-02-28 19:54 UTC (permalink / raw)
  To: linux-arm-kernel

After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
the following error may be reported when running omap images.

OF: ERROR: Bad of_node_put() on /ocp at 68000000
CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
Hardware name: Generic OMAP3-GP (Flattened Device Tree)
[<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
[<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
[<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
[<c05af1b0>] (kobject_release)
	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
[<c0ad1aa4>] (of_find_node_by_name)
	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
[<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
[<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
[<c1215d5c>] (omap3xxx_hwmod_init)
	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
[<c120faa8>] (omap3_init_early)
	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
[<c120fb2c>] (omap3430_init_early)
	from [<c1204710>] (setup_arch+0xc04/0xc34)
[<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
[<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)

of_find_node_by_name() drops the reference to the passed device node.
Use of_get_child_by_name() instead. Also, release references to
device nodes obtained with of_find_node_by_name() and
of_get_child_by_name() after they are no longer needed.

While at it, clean up the code and change the return type of
omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
and the return type of of_device_is_available().

Cc: Qi Hou <qi.hou@windriver.com>
Cc: Peter Rosin <peda@axentia.se>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Change subject ('Grab reference to device nodes where needed'
    didn't really cover all the changes made)
    Use of_get_child_by_name() instead of of_find_node_by_name()
    Drop references to device nodes as needed
    Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
    to bool

 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 56f917ec8621..042e460f4aa4 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -3111,16 +3111,20 @@ static struct omap_hwmod_ocp_if *omap3xxx_dss_hwmod_ocp_ifs[] __initdata = {
  * Return: 0 if device named @dev_name is not likely to be accessible,
  * or 1 if it is likely to be accessible.
  */
-static int __init omap3xxx_hwmod_is_hs_ip_block_usable(struct device_node *bus,
-						       const char *dev_name)
+static bool __init omap3xxx_hwmod_is_hs_ip_block_usable(struct device_node *bus,
+							const char *dev_name)
 {
+	struct device_node *node;
+	bool available;
+
 	if (!bus)
-		return (omap_type() == OMAP2_DEVICE_TYPE_GP) ? 1 : 0;
+		return omap_type() == OMAP2_DEVICE_TYPE_GP;
 
-	if (of_device_is_available(of_find_node_by_name(bus, dev_name)))
-		return 1;
+	node = of_get_child_by_name(bus, dev_name);
+	available = of_device_is_available(node);
+	of_node_put(node);
 
-	return 0;
+	return available;
 }
 
 int __init omap3xxx_hwmod_init(void)
@@ -3189,15 +3193,20 @@ int __init omap3xxx_hwmod_init(void)
 
 	if (h_sham && omap3xxx_hwmod_is_hs_ip_block_usable(bus, "sham")) {
 		r = omap_hwmod_register_links(h_sham);
-		if (r < 0)
+		if (r < 0) {
+			of_node_put(bus);
 			return r;
+		}
 	}
 
 	if (h_aes && omap3xxx_hwmod_is_hs_ip_block_usable(bus, "aes")) {
 		r = omap_hwmod_register_links(h_aes);
-		if (r < 0)
+		if (r < 0) {
+			of_node_put(bus);
 			return r;
+		}
 	}
+	of_node_put(bus);
 
 	/*
 	 * Register hwmod links specific to certain ES levels of a
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: OMAP2+: Fix device node reference counts
  2017-02-28 19:54 ` Guenter Roeck
  (?)
@ 2017-03-01 18:04   ` Tony Lindgren
  -1 siblings, 0 replies; 11+ messages in thread
From: Tony Lindgren @ 2017-03-01 18:04 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Paul Walmsley, Russell King, linux-omap, linux-arm-kernel,
	linux-kernel, Qi Hou, Peter Rosin, Rob Herring

* Guenter Roeck <linux@roeck-us.net> [170228 11:55]:
> After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
> the following error may be reported when running omap images.
> 
> OF: ERROR: Bad of_node_put() on /ocp@68000000
> CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
> Hardware name: Generic OMAP3-GP (Flattened Device Tree)
> [<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
> [<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
> [<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
> [<c05af1b0>] (kobject_release)
> 	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
> [<c0ad1aa4>] (of_find_node_by_name)
> 	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
> [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
> [<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
> [<c1215d5c>] (omap3xxx_hwmod_init)
> 	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
> [<c120faa8>] (omap3_init_early)
> 	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
> [<c120fb2c>] (omap3430_init_early)
> 	from [<c1204710>] (setup_arch+0xc04/0xc34)
> [<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
> [<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)
> 
> of_find_node_by_name() drops the reference to the passed device node.
> Use of_get_child_by_name() instead. Also, release references to
> device nodes obtained with of_find_node_by_name() and
> of_get_child_by_name() after they are no longer needed.
> 
> While at it, clean up the code and change the return type of
> omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
> and the return type of of_device_is_available().
> 
> Cc: Qi Hou <qi.hou@windriver.com>
> Cc: Peter Rosin <peda@axentia.se>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Change subject ('Grab reference to device nodes where needed'
>     didn't really cover all the changes made)
>     Use of_get_child_by_name() instead of of_find_node_by_name()
>     Drop references to device nodes as needed
>     Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
>     to bool

OK so now we have a commit id for it and should have:

Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in
of_find_node_opts_by_path")

What about other leaky users of of_find_node_by_name() and
of_get_child_by_name()?

We have those in:

control.c
display.c
omap_device.c
omap_hwmod.c

So probably it really should be two fixes, one for the regression
causing the error. Then another one for fixing the leaky np use.

Sorry for going back and forth here, I guess I did not full understand
the second part of the change in your earlier version of the patch
when you asked if it should be two patches.

Regards,

Tony

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: OMAP2+: Fix device node reference counts
@ 2017-03-01 18:04   ` Tony Lindgren
  0 siblings, 0 replies; 11+ messages in thread
From: Tony Lindgren @ 2017-03-01 18:04 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Rob Herring, Paul Walmsley, Russell King, linux-kernel, Qi Hou,
	linux-omap, Peter Rosin, linux-arm-kernel

* Guenter Roeck <linux@roeck-us.net> [170228 11:55]:
> After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
> the following error may be reported when running omap images.
> 
> OF: ERROR: Bad of_node_put() on /ocp@68000000
> CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
> Hardware name: Generic OMAP3-GP (Flattened Device Tree)
> [<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
> [<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
> [<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
> [<c05af1b0>] (kobject_release)
> 	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
> [<c0ad1aa4>] (of_find_node_by_name)
> 	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
> [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
> [<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
> [<c1215d5c>] (omap3xxx_hwmod_init)
> 	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
> [<c120faa8>] (omap3_init_early)
> 	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
> [<c120fb2c>] (omap3430_init_early)
> 	from [<c1204710>] (setup_arch+0xc04/0xc34)
> [<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
> [<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)
> 
> of_find_node_by_name() drops the reference to the passed device node.
> Use of_get_child_by_name() instead. Also, release references to
> device nodes obtained with of_find_node_by_name() and
> of_get_child_by_name() after they are no longer needed.
> 
> While at it, clean up the code and change the return type of
> omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
> and the return type of of_device_is_available().
> 
> Cc: Qi Hou <qi.hou@windriver.com>
> Cc: Peter Rosin <peda@axentia.se>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Change subject ('Grab reference to device nodes where needed'
>     didn't really cover all the changes made)
>     Use of_get_child_by_name() instead of of_find_node_by_name()
>     Drop references to device nodes as needed
>     Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
>     to bool

OK so now we have a commit id for it and should have:

Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in
of_find_node_opts_by_path")

What about other leaky users of of_find_node_by_name() and
of_get_child_by_name()?

We have those in:

control.c
display.c
omap_device.c
omap_hwmod.c

So probably it really should be two fixes, one for the regression
causing the error. Then another one for fixing the leaky np use.

Sorry for going back and forth here, I guess I did not full understand
the second part of the change in your earlier version of the patch
when you asked if it should be two patches.

Regards,

Tony

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] ARM: OMAP2+: Fix device node reference counts
@ 2017-03-01 18:04   ` Tony Lindgren
  0 siblings, 0 replies; 11+ messages in thread
From: Tony Lindgren @ 2017-03-01 18:04 UTC (permalink / raw)
  To: linux-arm-kernel

* Guenter Roeck <linux@roeck-us.net> [170228 11:55]:
> After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
> the following error may be reported when running omap images.
> 
> OF: ERROR: Bad of_node_put() on /ocp at 68000000
> CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
> Hardware name: Generic OMAP3-GP (Flattened Device Tree)
> [<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
> [<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
> [<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
> [<c05af1b0>] (kobject_release)
> 	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
> [<c0ad1aa4>] (of_find_node_by_name)
> 	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
> [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
> [<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
> [<c1215d5c>] (omap3xxx_hwmod_init)
> 	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
> [<c120faa8>] (omap3_init_early)
> 	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
> [<c120fb2c>] (omap3430_init_early)
> 	from [<c1204710>] (setup_arch+0xc04/0xc34)
> [<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
> [<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)
> 
> of_find_node_by_name() drops the reference to the passed device node.
> Use of_get_child_by_name() instead. Also, release references to
> device nodes obtained with of_find_node_by_name() and
> of_get_child_by_name() after they are no longer needed.
> 
> While at it, clean up the code and change the return type of
> omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
> and the return type of of_device_is_available().
> 
> Cc: Qi Hou <qi.hou@windriver.com>
> Cc: Peter Rosin <peda@axentia.se>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Change subject ('Grab reference to device nodes where needed'
>     didn't really cover all the changes made)
>     Use of_get_child_by_name() instead of of_find_node_by_name()
>     Drop references to device nodes as needed
>     Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
>     to bool

OK so now we have a commit id for it and should have:

Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in
of_find_node_opts_by_path")

What about other leaky users of of_find_node_by_name() and
of_get_child_by_name()?

We have those in:

control.c
display.c
omap_device.c
omap_hwmod.c

So probably it really should be two fixes, one for the regression
causing the error. Then another one for fixing the leaky np use.

Sorry for going back and forth here, I guess I did not full understand
the second part of the change in your earlier version of the patch
when you asked if it should be two patches.

Regards,

Tony

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: OMAP2+: Fix device node reference counts
  2017-03-01 18:04   ` Tony Lindgren
  (?)
@ 2017-03-01 20:15     ` Guenter Roeck
  -1 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2017-03-01 20:15 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Paul Walmsley, Russell King, linux-omap, linux-arm-kernel,
	linux-kernel, Qi Hou, Peter Rosin, Rob Herring

On Wed, Mar 01, 2017 at 10:04:39AM -0800, Tony Lindgren wrote:
> * Guenter Roeck <linux@roeck-us.net> [170228 11:55]:
> > After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
> > the following error may be reported when running omap images.
> > 
> > OF: ERROR: Bad of_node_put() on /ocp@68000000
> > CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
> > Hardware name: Generic OMAP3-GP (Flattened Device Tree)
> > [<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
> > [<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
> > [<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
> > [<c05af1b0>] (kobject_release)
> > 	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
> > [<c0ad1aa4>] (of_find_node_by_name)
> > 	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
> > [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
> > [<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
> > [<c1215d5c>] (omap3xxx_hwmod_init)
> > 	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
> > [<c120faa8>] (omap3_init_early)
> > 	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
> > [<c120fb2c>] (omap3430_init_early)
> > 	from [<c1204710>] (setup_arch+0xc04/0xc34)
> > [<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
> > [<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)
> > 
> > of_find_node_by_name() drops the reference to the passed device node.
> > Use of_get_child_by_name() instead. Also, release references to
> > device nodes obtained with of_find_node_by_name() and
> > of_get_child_by_name() after they are no longer needed.
> > 
> > While at it, clean up the code and change the return type of
> > omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
> > and the return type of of_device_is_available().
> > 
> > Cc: Qi Hou <qi.hou@windriver.com>
> > Cc: Peter Rosin <peda@axentia.se>
> > Cc: Rob Herring <robh@kernel.org>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > v2: Change subject ('Grab reference to device nodes where needed'
> >     didn't really cover all the changes made)
> >     Use of_get_child_by_name() instead of of_find_node_by_name()
> >     Drop references to device nodes as needed
> >     Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
> >     to bool
> 
> OK so now we have a commit id for it and should have:
> 
> Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in
> of_find_node_opts_by_path")
> 
Not really; 0549bde0fcb1 fixes a bug and hides _this_ bug.
More appropriate, if that would exist, would be something like

Exposed-by: 0549bde0fcb1 ("of: fix of_node leak caused in ...")

> What about other leaky users of of_find_node_by_name() and
> of_get_child_by_name()?
> 
> We have those in:
> 
> control.c
> display.c
> omap_device.c
> omap_hwmod.c
> 

I am sure there are many more. For example, the bug solved in my patch
"disappears" if one adds some delays (or just log messages) at strategic
areas in the code. Such delays result in node leaks elsewhere, which
again hides the bug I am trying to fix with my patch.

> So probably it really should be two fixes, one for the regression
> causing the error. Then another one for fixing the leaky np use.
> 
No problem; I 'll split into two patches.

Guenter

> Sorry for going back and forth here, I guess I did not full understand
> the second part of the change in your earlier version of the patch
> when you asked if it should be two patches.
> 

> Regards,
> 
> Tony

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: OMAP2+: Fix device node reference counts
@ 2017-03-01 20:15     ` Guenter Roeck
  0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2017-03-01 20:15 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Rob Herring, Paul Walmsley, Russell King, linux-kernel, Qi Hou,
	linux-omap, Peter Rosin, linux-arm-kernel

On Wed, Mar 01, 2017 at 10:04:39AM -0800, Tony Lindgren wrote:
> * Guenter Roeck <linux@roeck-us.net> [170228 11:55]:
> > After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
> > the following error may be reported when running omap images.
> > 
> > OF: ERROR: Bad of_node_put() on /ocp@68000000
> > CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
> > Hardware name: Generic OMAP3-GP (Flattened Device Tree)
> > [<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
> > [<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
> > [<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
> > [<c05af1b0>] (kobject_release)
> > 	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
> > [<c0ad1aa4>] (of_find_node_by_name)
> > 	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
> > [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
> > [<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
> > [<c1215d5c>] (omap3xxx_hwmod_init)
> > 	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
> > [<c120faa8>] (omap3_init_early)
> > 	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
> > [<c120fb2c>] (omap3430_init_early)
> > 	from [<c1204710>] (setup_arch+0xc04/0xc34)
> > [<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
> > [<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)
> > 
> > of_find_node_by_name() drops the reference to the passed device node.
> > Use of_get_child_by_name() instead. Also, release references to
> > device nodes obtained with of_find_node_by_name() and
> > of_get_child_by_name() after they are no longer needed.
> > 
> > While at it, clean up the code and change the return type of
> > omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
> > and the return type of of_device_is_available().
> > 
> > Cc: Qi Hou <qi.hou@windriver.com>
> > Cc: Peter Rosin <peda@axentia.se>
> > Cc: Rob Herring <robh@kernel.org>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > v2: Change subject ('Grab reference to device nodes where needed'
> >     didn't really cover all the changes made)
> >     Use of_get_child_by_name() instead of of_find_node_by_name()
> >     Drop references to device nodes as needed
> >     Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
> >     to bool
> 
> OK so now we have a commit id for it and should have:
> 
> Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in
> of_find_node_opts_by_path")
> 
Not really; 0549bde0fcb1 fixes a bug and hides _this_ bug.
More appropriate, if that would exist, would be something like

Exposed-by: 0549bde0fcb1 ("of: fix of_node leak caused in ...")

> What about other leaky users of of_find_node_by_name() and
> of_get_child_by_name()?
> 
> We have those in:
> 
> control.c
> display.c
> omap_device.c
> omap_hwmod.c
> 

I am sure there are many more. For example, the bug solved in my patch
"disappears" if one adds some delays (or just log messages) at strategic
areas in the code. Such delays result in node leaks elsewhere, which
again hides the bug I am trying to fix with my patch.

> So probably it really should be two fixes, one for the regression
> causing the error. Then another one for fixing the leaky np use.
> 
No problem; I 'll split into two patches.

Guenter

> Sorry for going back and forth here, I guess I did not full understand
> the second part of the change in your earlier version of the patch
> when you asked if it should be two patches.
> 

> Regards,
> 
> Tony

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] ARM: OMAP2+: Fix device node reference counts
@ 2017-03-01 20:15     ` Guenter Roeck
  0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2017-03-01 20:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 01, 2017 at 10:04:39AM -0800, Tony Lindgren wrote:
> * Guenter Roeck <linux@roeck-us.net> [170228 11:55]:
> > After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
> > the following error may be reported when running omap images.
> > 
> > OF: ERROR: Bad of_node_put() on /ocp at 68000000
> > CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
> > Hardware name: Generic OMAP3-GP (Flattened Device Tree)
> > [<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
> > [<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
> > [<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
> > [<c05af1b0>] (kobject_release)
> > 	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
> > [<c0ad1aa4>] (of_find_node_by_name)
> > 	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
> > [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
> > [<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
> > [<c1215d5c>] (omap3xxx_hwmod_init)
> > 	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
> > [<c120faa8>] (omap3_init_early)
> > 	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
> > [<c120fb2c>] (omap3430_init_early)
> > 	from [<c1204710>] (setup_arch+0xc04/0xc34)
> > [<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
> > [<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)
> > 
> > of_find_node_by_name() drops the reference to the passed device node.
> > Use of_get_child_by_name() instead. Also, release references to
> > device nodes obtained with of_find_node_by_name() and
> > of_get_child_by_name() after they are no longer needed.
> > 
> > While at it, clean up the code and change the return type of
> > omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
> > and the return type of of_device_is_available().
> > 
> > Cc: Qi Hou <qi.hou@windriver.com>
> > Cc: Peter Rosin <peda@axentia.se>
> > Cc: Rob Herring <robh@kernel.org>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > v2: Change subject ('Grab reference to device nodes where needed'
> >     didn't really cover all the changes made)
> >     Use of_get_child_by_name() instead of of_find_node_by_name()
> >     Drop references to device nodes as needed
> >     Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
> >     to bool
> 
> OK so now we have a commit id for it and should have:
> 
> Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in
> of_find_node_opts_by_path")
> 
Not really; 0549bde0fcb1 fixes a bug and hides _this_ bug.
More appropriate, if that would exist, would be something like

Exposed-by: 0549bde0fcb1 ("of: fix of_node leak caused in ...")

> What about other leaky users of of_find_node_by_name() and
> of_get_child_by_name()?
> 
> We have those in:
> 
> control.c
> display.c
> omap_device.c
> omap_hwmod.c
> 

I am sure there are many more. For example, the bug solved in my patch
"disappears" if one adds some delays (or just log messages) at strategic
areas in the code. Such delays result in node leaks elsewhere, which
again hides the bug I am trying to fix with my patch.

> So probably it really should be two fixes, one for the regression
> causing the error. Then another one for fixing the leaky np use.
> 
No problem; I 'll split into two patches.

Guenter

> Sorry for going back and forth here, I guess I did not full understand
> the second part of the change in your earlier version of the patch
> when you asked if it should be two patches.
> 

> Regards,
> 
> Tony

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: OMAP2+: Fix device node reference counts
  2017-03-01 20:15     ` Guenter Roeck
@ 2017-03-01 23:29       ` Tony Lindgren
  -1 siblings, 0 replies; 11+ messages in thread
From: Tony Lindgren @ 2017-03-01 23:29 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Paul Walmsley, Russell King, linux-omap, linux-arm-kernel,
	linux-kernel, Qi Hou, Peter Rosin, Rob Herring

* Guenter Roeck <linux@roeck-us.net> [170301 12:16]:
> On Wed, Mar 01, 2017 at 10:04:39AM -0800, Tony Lindgren wrote:
> > * Guenter Roeck <linux@roeck-us.net> [170228 11:55]:
> > > After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
> > > the following error may be reported when running omap images.
> > > 
> > > OF: ERROR: Bad of_node_put() on /ocp@68000000
> > > CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
> > > Hardware name: Generic OMAP3-GP (Flattened Device Tree)
> > > [<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
> > > [<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
> > > [<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
> > > [<c05af1b0>] (kobject_release)
> > > 	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
> > > [<c0ad1aa4>] (of_find_node_by_name)
> > > 	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
> > > [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
> > > [<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
> > > [<c1215d5c>] (omap3xxx_hwmod_init)
> > > 	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
> > > [<c120faa8>] (omap3_init_early)
> > > 	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
> > > [<c120fb2c>] (omap3430_init_early)
> > > 	from [<c1204710>] (setup_arch+0xc04/0xc34)
> > > [<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
> > > [<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)
> > > 
> > > of_find_node_by_name() drops the reference to the passed device node.
> > > Use of_get_child_by_name() instead. Also, release references to
> > > device nodes obtained with of_find_node_by_name() and
> > > of_get_child_by_name() after they are no longer needed.
> > > 
> > > While at it, clean up the code and change the return type of
> > > omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
> > > and the return type of of_device_is_available().
> > > 
> > > Cc: Qi Hou <qi.hou@windriver.com>
> > > Cc: Peter Rosin <peda@axentia.se>
> > > Cc: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > ---
> > > v2: Change subject ('Grab reference to device nodes where needed'
> > >     didn't really cover all the changes made)
> > >     Use of_get_child_by_name() instead of of_find_node_by_name()
> > >     Drop references to device nodes as needed
> > >     Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
> > >     to bool
> > 
> > OK so now we have a commit id for it and should have:
> > 
> > Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in
> > of_find_node_opts_by_path")
> > 
> Not really; 0549bde0fcb1 fixes a bug and hides _this_ bug.
> More appropriate, if that would exist, would be something like
> 
> Exposed-by: 0549bde0fcb1 ("of: fix of_node leak caused in ...")

OK

> > What about other leaky users of of_find_node_by_name() and
> > of_get_child_by_name()?
> > 
> > We have those in:
> > 
> > control.c
> > display.c
> > omap_device.c
> > omap_hwmod.c
> > 
> 
> I am sure there are many more. For example, the bug solved in my patch
> "disappears" if one adds some delays (or just log messages) at strategic
> areas in the code. Such delays result in node leaks elsewhere, which
> again hides the bug I am trying to fix with my patch.
> 
> > So probably it really should be two fixes, one for the regression
> > causing the error. Then another one for fixing the leaky np use.
> > 
> No problem; I 'll split into two patches.

OK thanks.

Tony

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] ARM: OMAP2+: Fix device node reference counts
@ 2017-03-01 23:29       ` Tony Lindgren
  0 siblings, 0 replies; 11+ messages in thread
From: Tony Lindgren @ 2017-03-01 23:29 UTC (permalink / raw)
  To: linux-arm-kernel

* Guenter Roeck <linux@roeck-us.net> [170301 12:16]:
> On Wed, Mar 01, 2017 at 10:04:39AM -0800, Tony Lindgren wrote:
> > * Guenter Roeck <linux@roeck-us.net> [170228 11:55]:
> > > After commit 'of: fix of_node leak caused in of_find_node_opts_by_path',
> > > the following error may be reported when running omap images.
> > > 
> > > OF: ERROR: Bad of_node_put() on /ocp at 68000000
> > > CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1
> > > Hardware name: Generic OMAP3-GP (Flattened Device Tree)
> > > [<c0310604>] (unwind_backtrace) from [<c030bbf4>] (show_stack+0x10/0x14)
> > > [<c030bbf4>] (show_stack) from [<c05add8c>] (dump_stack+0x98/0xac)
> > > [<c05add8c>] (dump_stack) from [<c05af1b0>] (kobject_release+0x48/0x7c)
> > > [<c05af1b0>] (kobject_release)
> > > 	from [<c0ad1aa4>] (of_find_node_by_name+0x74/0x94)
> > > [<c0ad1aa4>] (of_find_node_by_name)
> > > 	from [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c)
> > > [<c1215bd4>] (omap3xxx_hwmod_is_hs_ip_block_usable) from
> > > [<c1215d5c>] (omap3xxx_hwmod_init+0x180/0x274)
> > > [<c1215d5c>] (omap3xxx_hwmod_init)
> > > 	from [<c120faa8>] (omap3_init_early+0xa0/0x11c)
> > > [<c120faa8>] (omap3_init_early)
> > > 	from [<c120fb2c>] (omap3430_init_early+0x8/0x30)
> > > [<c120fb2c>] (omap3430_init_early)
> > > 	from [<c1204710>] (setup_arch+0xc04/0xc34)
> > > [<c1204710>] (setup_arch) from [<c1200948>] (start_kernel+0x68/0x38c)
> > > [<c1200948>] (start_kernel) from [<8020807c>] (0x8020807c)
> > > 
> > > of_find_node_by_name() drops the reference to the passed device node.
> > > Use of_get_child_by_name() instead. Also, release references to
> > > device nodes obtained with of_find_node_by_name() and
> > > of_get_child_by_name() after they are no longer needed.
> > > 
> > > While at it, clean up the code and change the return type of
> > > omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use
> > > and the return type of of_device_is_available().
> > > 
> > > Cc: Qi Hou <qi.hou@windriver.com>
> > > Cc: Peter Rosin <peda@axentia.se>
> > > Cc: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > ---
> > > v2: Change subject ('Grab reference to device nodes where needed'
> > >     didn't really cover all the changes made)
> > >     Use of_get_child_by_name() instead of of_find_node_by_name()
> > >     Drop references to device nodes as needed
> > >     Change return type of omap3xxx_hwmod_is_hs_ip_block_usable()
> > >     to bool
> > 
> > OK so now we have a commit id for it and should have:
> > 
> > Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in
> > of_find_node_opts_by_path")
> > 
> Not really; 0549bde0fcb1 fixes a bug and hides _this_ bug.
> More appropriate, if that would exist, would be something like
> 
> Exposed-by: 0549bde0fcb1 ("of: fix of_node leak caused in ...")

OK

> > What about other leaky users of of_find_node_by_name() and
> > of_get_child_by_name()?
> > 
> > We have those in:
> > 
> > control.c
> > display.c
> > omap_device.c
> > omap_hwmod.c
> > 
> 
> I am sure there are many more. For example, the bug solved in my patch
> "disappears" if one adds some delays (or just log messages) at strategic
> areas in the code. Such delays result in node leaks elsewhere, which
> again hides the bug I am trying to fix with my patch.
> 
> > So probably it really should be two fixes, one for the regression
> > causing the error. Then another one for fixing the leaky np use.
> > 
> No problem; I 'll split into two patches.

OK thanks.

Tony

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-03-01 23:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-28 19:54 [PATCH v2] ARM: OMAP2+: Fix device node reference counts Guenter Roeck
2017-02-28 19:54 ` Guenter Roeck
2017-02-28 19:54 ` Guenter Roeck
2017-03-01 18:04 ` Tony Lindgren
2017-03-01 18:04   ` Tony Lindgren
2017-03-01 18:04   ` Tony Lindgren
2017-03-01 20:15   ` Guenter Roeck
2017-03-01 20:15     ` Guenter Roeck
2017-03-01 20:15     ` Guenter Roeck
2017-03-01 23:29     ` Tony Lindgren
2017-03-01 23:29       ` Tony Lindgren

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.