[PATCH] ASoC: fix balance of of_node_get()/of_node_put()

[PATCH] ASoC: fix balance of of_node_get()/of_node_put()

[Antonio Borneo]

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like the following one
OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0/endpoint
each followed by stack dump.

Fixed by:
- removing of_node_put() in the body of of_for_each_phandle(){},
  since already provided at each iteration. Add it in case the
  loop is break out;
- adding of_node_get() before calling of_graph_get_port_parent()
  or asoc_graph_card_dai_link_of().

Tested with kernel v4.13-rc1 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
---
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
To: alsa-devel@alsa-project.org
Cc: linux-kernel@vger.kernel.org
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
---
 sound/soc/generic/audio-graph-card.c  | 14 +++++++++-----
 sound/soc/generic/simple-card-utils.c |  5 +++++
 sound/soc/soc-core.c                  |  5 +++++
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index c0f08a6..7574c5f 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -228,10 +228,16 @@ static int asoc_graph_card_parse_of(struct graph_card_data *priv)
 	 */
 
 	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
+		/*
+		 * asoc_graph_card_dai_link_of() will call
+		 * of_node_put(). So, call of_node_get() here
+		 */
+		of_node_get(it.node);
 		ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
-		of_node_put(it.node);
-		if (ret < 0)
+		if (ret < 0) {
+			of_node_put(it.node);
 			return ret;
+		}
 	}
 
 	return asoc_simple_card_parse_card_name(card, NULL);
@@ -244,10 +250,8 @@ static int asoc_graph_get_dais_count(struct device *dev)
 	int count = 0;
 	int rc;
 
-	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
+	of_for_each_phandle(&it, rc, node, "dais", NULL, 0)
 		count++;
-		of_node_put(it.node);
-	}
 
 	return count;
 }
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 26d64fa..144954b 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -250,6 +250,11 @@ static int asoc_simple_card_get_dai_id(struct device_node *ep)
 	if (ret != -ENOTSUPP)
 		return ret;
 
+	/*
+	 * of_graph_get_port_parent() will call
+	 * of_node_put(). So, call of_node_get() here
+	 */
+	of_node_get(ep);
 	node = of_graph_get_port_parent(ep);
 
 	/*
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 921622a..a0f39de 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -4087,6 +4087,11 @@ int snd_soc_get_dai_id(struct device_node *ep)
 	struct device_node *node;
 	int ret;
 
+	/*
+	 * of_graph_get_port_parent() will call
+	 * of_node_put(). So, call of_node_get() here
+	 */
+	of_node_get(ep);
 	node = of_graph_get_port_parent(ep);
 
 	/*
-- 
1.9.1

[PATCH] ASoC: fix balance of of_node_get()/of_node_put()

[Antonio Borneo]

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like the following one
OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0/endpoint
each followed by stack dump.

Fixed by:
- removing of_node_put() in the body of of_for_each_phandle(){},
  since already provided at each iteration. Add it in case the
  loop is break out;
- adding of_node_get() before calling of_graph_get_port_parent()
  or asoc_graph_card_dai_link_of().

Tested with kernel v4.13-rc1 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
---
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
To: alsa-devel at alsa-project.org
Cc: linux-kernel at vger.kernel.org
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-arm-kernel at lists.infradead.org
---
 sound/soc/generic/audio-graph-card.c  | 14 +++++++++-----
 sound/soc/generic/simple-card-utils.c |  5 +++++
 sound/soc/soc-core.c                  |  5 +++++
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index c0f08a6..7574c5f 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -228,10 +228,16 @@ static int asoc_graph_card_parse_of(struct graph_card_data *priv)
 	 */
 
 	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
+		/*
+		 * asoc_graph_card_dai_link_of() will call
+		 * of_node_put(). So, call of_node_get() here
+		 */
+		of_node_get(it.node);
 		ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
-		of_node_put(it.node);
-		if (ret < 0)
+		if (ret < 0) {
+			of_node_put(it.node);
 			return ret;
+		}
 	}
 
 	return asoc_simple_card_parse_card_name(card, NULL);
@@ -244,10 +250,8 @@ static int asoc_graph_get_dais_count(struct device *dev)
 	int count = 0;
 	int rc;
 
-	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
+	of_for_each_phandle(&it, rc, node, "dais", NULL, 0)
 		count++;
-		of_node_put(it.node);
-	}
 
 	return count;
 }
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 26d64fa..144954b 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -250,6 +250,11 @@ static int asoc_simple_card_get_dai_id(struct device_node *ep)
 	if (ret != -ENOTSUPP)
 		return ret;
 
+	/*
+	 * of_graph_get_port_parent() will call
+	 * of_node_put(). So, call of_node_get() here
+	 */
+	of_node_get(ep);
 	node = of_graph_get_port_parent(ep);
 
 	/*
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 921622a..a0f39de 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -4087,6 +4087,11 @@ int snd_soc_get_dai_id(struct device_node *ep)
 	struct device_node *node;
 	int ret;
 
+	/*
+	 * of_graph_get_port_parent() will call
+	 * of_node_put(). So, call of_node_get() here
+	 */
+	of_node_get(ep);
 	node = of_graph_get_port_parent(ep);
 
 	/*
-- 
1.9.1

Re: [alsa-devel] [PATCH] ASoC: fix balance of of_node_get()/of_node_put()

[Kuninori Morimoto]

Hi

> On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
> errors at kernel boot, like the following one
> OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0/endpoint
> each followed by stack dump.
> 
> Fixed by:
> - removing of_node_put() in the body of of_for_each_phandle(){},
>   since already provided at each iteration. Add it in case the
>   loop is break out;
> - adding of_node_get() before calling of_graph_get_port_parent()
>   or asoc_graph_card_dai_link_of().
> 
> Tested with kernel v4.13-rc1 with hikey_defconfig taken from
> https://git.linaro.org/people/john.stultz/android-dev.git
> branch dev/hikey-mainline-WIP
> 
> Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
> ---

I got kernel boot error on my board (= Renesas R-Car Salvator-X) + CONFIG_OF_DYNAMIC.
This patch solved this issue.

Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

> To: Liam Girdwood <lgirdwood@gmail.com>
> To: Mark Brown <broonie@kernel.org>
> To: Jaroslav Kysela <perex@perex.cz>
> To: Takashi Iwai <tiwai@suse.com>
> To: alsa-devel@alsa-project.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  sound/soc/generic/audio-graph-card.c  | 14 +++++++++-----
>  sound/soc/generic/simple-card-utils.c |  5 +++++
>  sound/soc/soc-core.c                  |  5 +++++
>  3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
> index c0f08a6..7574c5f 100644
> --- a/sound/soc/generic/audio-graph-card.c
> +++ b/sound/soc/generic/audio-graph-card.c
> @@ -228,10 +228,16 @@ static int asoc_graph_card_parse_of(struct graph_card_data *priv)
>  	 */
>  
>  	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
> +		/*
> +		 * asoc_graph_card_dai_link_of() will call
> +		 * of_node_put(). So, call of_node_get() here
> +		 */
> +		of_node_get(it.node);
>  		ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
> -		of_node_put(it.node);
> -		if (ret < 0)
> +		if (ret < 0) {
> +			of_node_put(it.node);
>  			return ret;
> +		}
>  	}
>  
>  	return asoc_simple_card_parse_card_name(card, NULL);
> @@ -244,10 +250,8 @@ static int asoc_graph_get_dais_count(struct device *dev)
>  	int count = 0;
>  	int rc;
>  
> -	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
> +	of_for_each_phandle(&it, rc, node, "dais", NULL, 0)
>  		count++;
> -		of_node_put(it.node);
> -	}
>  
>  	return count;
>  }
> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
> index 26d64fa..144954b 100644
> --- a/sound/soc/generic/simple-card-utils.c
> +++ b/sound/soc/generic/simple-card-utils.c
> @@ -250,6 +250,11 @@ static int asoc_simple_card_get_dai_id(struct device_node *ep)
>  	if (ret != -ENOTSUPP)
>  		return ret;
>  
> +	/*
> +	 * of_graph_get_port_parent() will call
> +	 * of_node_put(). So, call of_node_get() here
> +	 */
> +	of_node_get(ep);
>  	node = of_graph_get_port_parent(ep);
>  
>  	/*
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 921622a..a0f39de 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -4087,6 +4087,11 @@ int snd_soc_get_dai_id(struct device_node *ep)
>  	struct device_node *node;
>  	int ret;
>  
> +	/*
> +	 * of_graph_get_port_parent() will call
> +	 * of_node_put(). So, call of_node_get() here
> +	 */
> +	of_node_get(ep);
>  	node = of_graph_get_port_parent(ep);
>  
>  	/*
> -- 
> 1.9.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH] ASoC: fix balance of of_node_get()/of_node_put()

[Kuninori Morimoto]

Hi

> On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
> errors at kernel boot, like the following one
> OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0/endpoint
> each followed by stack dump.
> 
> Fixed by:
> - removing of_node_put() in the body of of_for_each_phandle(){},
>   since already provided at each iteration. Add it in case the
>   loop is break out;
> - adding of_node_get() before calling of_graph_get_port_parent()
>   or asoc_graph_card_dai_link_of().
> 
> Tested with kernel v4.13-rc1 with hikey_defconfig taken from
> https://git.linaro.org/people/john.stultz/android-dev.git
> branch dev/hikey-mainline-WIP
> 
> Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
> ---

I got kernel boot error on my board (= Renesas R-Car Salvator-X) + CONFIG_OF_DYNAMIC.
This patch solved this issue.

Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

> To: Liam Girdwood <lgirdwood@gmail.com>
> To: Mark Brown <broonie@kernel.org>
> To: Jaroslav Kysela <perex@perex.cz>
> To: Takashi Iwai <tiwai@suse.com>
> To: alsa-devel at alsa-project.org
> Cc: linux-kernel at vger.kernel.org
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: linux-arm-kernel at lists.infradead.org
> ---
>  sound/soc/generic/audio-graph-card.c  | 14 +++++++++-----
>  sound/soc/generic/simple-card-utils.c |  5 +++++
>  sound/soc/soc-core.c                  |  5 +++++
>  3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
> index c0f08a6..7574c5f 100644
> --- a/sound/soc/generic/audio-graph-card.c
> +++ b/sound/soc/generic/audio-graph-card.c
> @@ -228,10 +228,16 @@ static int asoc_graph_card_parse_of(struct graph_card_data *priv)
>  	 */
>  
>  	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
> +		/*
> +		 * asoc_graph_card_dai_link_of() will call
> +		 * of_node_put(). So, call of_node_get() here
> +		 */
> +		of_node_get(it.node);
>  		ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
> -		of_node_put(it.node);
> -		if (ret < 0)
> +		if (ret < 0) {
> +			of_node_put(it.node);
>  			return ret;
> +		}
>  	}
>  
>  	return asoc_simple_card_parse_card_name(card, NULL);
> @@ -244,10 +250,8 @@ static int asoc_graph_get_dais_count(struct device *dev)
>  	int count = 0;
>  	int rc;
>  
> -	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
> +	of_for_each_phandle(&it, rc, node, "dais", NULL, 0)
>  		count++;
> -		of_node_put(it.node);
> -	}
>  
>  	return count;
>  }
> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
> index 26d64fa..144954b 100644
> --- a/sound/soc/generic/simple-card-utils.c
> +++ b/sound/soc/generic/simple-card-utils.c
> @@ -250,6 +250,11 @@ static int asoc_simple_card_get_dai_id(struct device_node *ep)
>  	if (ret != -ENOTSUPP)
>  		return ret;
>  
> +	/*
> +	 * of_graph_get_port_parent() will call
> +	 * of_node_put(). So, call of_node_get() here
> +	 */
> +	of_node_get(ep);
>  	node = of_graph_get_port_parent(ep);
>  
>  	/*
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 921622a..a0f39de 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -4087,6 +4087,11 @@ int snd_soc_get_dai_id(struct device_node *ep)
>  	struct device_node *node;
>  	int ret;
>  
> +	/*
> +	 * of_graph_get_port_parent() will call
> +	 * of_node_put(). So, call of_node_get() here
> +	 */
> +	of_node_get(ep);
>  	node = of_graph_get_port_parent(ep);
>  
>  	/*
> -- 
> 1.9.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ASoC: fix balance of of_node_get()/of_node_put()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

On Tue, Jul 25, 2017 at 11:49:52PM +0200, Antonio Borneo wrote:

> Fixed by:
> - removing of_node_put() in the body of of_for_each_phandle(){},
>   since already provided at each iteration. Add it in case the
>   loop is break out;
> - adding of_node_get() before calling of_graph_get_port_parent()
>   or asoc_graph_card_dai_link_of().

>  sound/soc/generic/audio-graph-card.c  | 14 +++++++++-----
>  sound/soc/generic/simple-card-utils.c |  5 +++++
>  sound/soc/soc-core.c                  |  5 +++++
>  3 files changed, 19 insertions(+), 5 deletions(-)

This is a series of different changes to fix different (although
related) problems which should be being submitted individually.  Sending
multiple changes in one patch makes it harder to review things and for
fixes like this makes it harder to backport the fixes where not all the
code being fixed was introduced in a single kernel version.

>  	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
> +		/*
> +		 * asoc_graph_card_dai_link_of() will call
> +		 * of_node_put(). So, call of_node_get() here
> +		 */
> +		of_node_get(it.node);
>  		ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);

Why is this the most sensible fix?  It is really not at all obvious why
asoc_graph_card_dai_link_of() would drop a reference, or in what
situations callers might have a reference they're OK with dropping.

> +	/*
> +	 * of_graph_get_port_parent() will call
> +	 * of_node_put(). So, call of_node_get() here
> +	 */
> +	of_node_get(ep);
>  	node = of_graph_get_port_parent(ep);

Same here, why does this make sense?  It is not in the least bit obvious
to me why looking up the parent of a node would cause us to drop the
reference to the current node, this seems like an error prone and
confusing API which would be better fixed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH] ASoC: fix balance of of_node_get()/of_node_put()

[Mark Brown]

On Tue, Jul 25, 2017 at 11:49:52PM +0200, Antonio Borneo wrote:

> Fixed by:
> - removing of_node_put() in the body of of_for_each_phandle(){},
>   since already provided at each iteration. Add it in case the
>   loop is break out;
> - adding of_node_get() before calling of_graph_get_port_parent()
>   or asoc_graph_card_dai_link_of().

>  sound/soc/generic/audio-graph-card.c  | 14 +++++++++-----
>  sound/soc/generic/simple-card-utils.c |  5 +++++
>  sound/soc/soc-core.c                  |  5 +++++
>  3 files changed, 19 insertions(+), 5 deletions(-)

This is a series of different changes to fix different (although
related) problems which should be being submitted individually.  Sending
multiple changes in one patch makes it harder to review things and for
fixes like this makes it harder to backport the fixes where not all the
code being fixed was introduced in a single kernel version.

>  	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
> +		/*
> +		 * asoc_graph_card_dai_link_of() will call
> +		 * of_node_put(). So, call of_node_get() here
> +		 */
> +		of_node_get(it.node);
>  		ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);

Why is this the most sensible fix?  It is really not at all obvious why
asoc_graph_card_dai_link_of() would drop a reference, or in what
situations callers might have a reference they're OK with dropping.

> +	/*
> +	 * of_graph_get_port_parent() will call
> +	 * of_node_put(). So, call of_node_get() here
> +	 */
> +	of_node_get(ep);
>  	node = of_graph_get_port_parent(ep);

Same here, why does this make sense?  It is not in the least bit obvious
to me why looking up the parent of a node would cause us to drop the
reference to the current node, this seems like an error prone and
confusing API which would be better fixed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170726/8cd234f7/attachment.sig>

Re: [PATCH] ASoC: fix balance of of_node_get()/of_node_put()

[Antonio Borneo]

Hi Mark,
thanks for the review.

On Wed, Jul 26, 2017 at 1:37 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jul 25, 2017 at 11:49:52PM +0200, Antonio Borneo wrote:
>
>> Fixed by:
>> - removing of_node_put() in the body of of_for_each_phandle(){},
>>   since already provided at each iteration. Add it in case the
>>   loop is break out;
>> - adding of_node_get() before calling of_graph_get_port_parent()
>>   or asoc_graph_card_dai_link_of().
>
>>  sound/soc/generic/audio-graph-card.c  | 14 +++++++++-----
>>  sound/soc/generic/simple-card-utils.c |  5 +++++
>>  sound/soc/soc-core.c                  |  5 +++++
>>  3 files changed, 19 insertions(+), 5 deletions(-)
>
> This is a series of different changes to fix different (although
> related) problems which should be being submitted individually.  Sending
> multiple changes in one patch makes it harder to review things and for
> fixes like this makes it harder to backport the fixes where not all the
> code being fixed was introduced in a single kernel version.

Agree! Will split it and send a v2.

>>       of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
>> +             /*
>> +              * asoc_graph_card_dai_link_of() will call
>> +              * of_node_put(). So, call of_node_get() here
>> +              */
>> +             of_node_get(it.node);
>>               ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
>
> Why is this the most sensible fix?  It is really not at all obvious why
> asoc_graph_card_dai_link_of() would drop a reference, or in what
> situations callers might have a reference they're OK with dropping.

Actually this part is wrong. Splitting for v2 and rechecking every
step I get that it's not this function that drops the reference; the
fix is already covered by the rest of the patch. Sorry for the
mistake.

>> +     /*
>> +      * of_graph_get_port_parent() will call
>> +      * of_node_put(). So, call of_node_get() here
>> +      */
>> +     of_node_get(ep);
>>       node = of_graph_get_port_parent(ep);
>
> Same here, why does this make sense?  It is not in the least bit obvious
> to me why looking up the parent of a node would cause us to drop the
> reference to the current node, this seems like an error prone and
> confusing API which would be better fixed.

I tend to agree with you, quite error prone and confusing. I spent
some time to identify who is dropping what.
Actually there is a bunch of functions like this in driver/of/; they
take a node as parameter and return a node, while doing a put of the
parameter. While questionable, looks like there is at least some
consistency.
Maybe the "get" in the API name should match the implicit
of_node_get() of the returned node (not sure it is the case on all
such API). Something else in the name should indicate if the input
node will be of_node_put(). Suggestions are welcome, but I think the
discussion goes beyond the scope of this fix.
In this specific case, I lazily reused some code already upstream here
http://elixir.free-electrons.com/linux/v4.13-rc2/source/sound/soc/generic/simple-card-utils.c#L285
I added in copy Kuninori Morimoto, the developer of the lines above,
in case he has any hint.

Antonio

Re: [PATCH] ASoC: fix balance of of_node_get()/of_node_put()

[Antonio Borneo]

Hi Mark,
thanks for the review.

On Wed, Jul 26, 2017 at 1:37 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jul 25, 2017 at 11:49:52PM +0200, Antonio Borneo wrote:
>
>> Fixed by:
>> - removing of_node_put() in the body of of_for_each_phandle(){},
>>   since already provided at each iteration. Add it in case the
>>   loop is break out;
>> - adding of_node_get() before calling of_graph_get_port_parent()
>>   or asoc_graph_card_dai_link_of().
>
>>  sound/soc/generic/audio-graph-card.c  | 14 +++++++++-----
>>  sound/soc/generic/simple-card-utils.c |  5 +++++
>>  sound/soc/soc-core.c                  |  5 +++++
>>  3 files changed, 19 insertions(+), 5 deletions(-)
>
> This is a series of different changes to fix different (although
> related) problems which should be being submitted individually.  Sending
> multiple changes in one patch makes it harder to review things and for
> fixes like this makes it harder to backport the fixes where not all the
> code being fixed was introduced in a single kernel version.

Agree! Will split it and send a v2.

>>       of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
>> +             /*
>> +              * asoc_graph_card_dai_link_of() will call
>> +              * of_node_put(). So, call of_node_get() here
>> +              */
>> +             of_node_get(it.node);
>>               ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
>
> Why is this the most sensible fix?  It is really not at all obvious why
> asoc_graph_card_dai_link_of() would drop a reference, or in what
> situations callers might have a reference they're OK with dropping.

Actually this part is wrong. Splitting for v2 and rechecking every
step I get that it's not this function that drops the reference; the
fix is already covered by the rest of the patch. Sorry for the
mistake.

>> +     /*
>> +      * of_graph_get_port_parent() will call
>> +      * of_node_put(). So, call of_node_get() here
>> +      */
>> +     of_node_get(ep);
>>       node = of_graph_get_port_parent(ep);
>
> Same here, why does this make sense?  It is not in the least bit obvious
> to me why looking up the parent of a node would cause us to drop the
> reference to the current node, this seems like an error prone and
> confusing API which would be better fixed.

I tend to agree with you, quite error prone and confusing. I spent
some time to identify who is dropping what.
Actually there is a bunch of functions like this in driver/of/; they
take a node as parameter and return a node, while doing a put of the
parameter. While questionable, looks like there is at least some
consistency.
Maybe the "get" in the API name should match the implicit
of_node_get() of the returned node (not sure it is the case on all
such API). Something else in the name should indicate if the input
node will be of_node_put(). Suggestions are welcome, but I think the
discussion goes beyond the scope of this fix.
In this specific case, I lazily reused some code already upstream here
http://elixir.free-electrons.com/linux/v4.13-rc2/source/sound/soc/generic/simple-card-utils.c#L285
I added in copy Kuninori Morimoto, the developer of the lines above,
in case he has any hint.

Antonio

[PATCH] ASoC: fix balance of of_node_get()/of_node_put()

[Antonio Borneo]

Hi Mark,
thanks for the review.

On Wed, Jul 26, 2017 at 1:37 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jul 25, 2017 at 11:49:52PM +0200, Antonio Borneo wrote:
>
>> Fixed by:
>> - removing of_node_put() in the body of of_for_each_phandle(){},
>>   since already provided at each iteration. Add it in case the
>>   loop is break out;
>> - adding of_node_get() before calling of_graph_get_port_parent()
>>   or asoc_graph_card_dai_link_of().
>
>>  sound/soc/generic/audio-graph-card.c  | 14 +++++++++-----
>>  sound/soc/generic/simple-card-utils.c |  5 +++++
>>  sound/soc/soc-core.c                  |  5 +++++
>>  3 files changed, 19 insertions(+), 5 deletions(-)
>
> This is a series of different changes to fix different (although
> related) problems which should be being submitted individually.  Sending
> multiple changes in one patch makes it harder to review things and for
> fixes like this makes it harder to backport the fixes where not all the
> code being fixed was introduced in a single kernel version.

Agree! Will split it and send a v2.

>>       of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
>> +             /*
>> +              * asoc_graph_card_dai_link_of() will call
>> +              * of_node_put(). So, call of_node_get() here
>> +              */
>> +             of_node_get(it.node);
>>               ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
>
> Why is this the most sensible fix?  It is really not at all obvious why
> asoc_graph_card_dai_link_of() would drop a reference, or in what
> situations callers might have a reference they're OK with dropping.

Actually this part is wrong. Splitting for v2 and rechecking every
step I get that it's not this function that drops the reference; the
fix is already covered by the rest of the patch. Sorry for the
mistake.

>> +     /*
>> +      * of_graph_get_port_parent() will call
>> +      * of_node_put(). So, call of_node_get() here
>> +      */
>> +     of_node_get(ep);
>>       node = of_graph_get_port_parent(ep);
>
> Same here, why does this make sense?  It is not in the least bit obvious
> to me why looking up the parent of a node would cause us to drop the
> reference to the current node, this seems like an error prone and
> confusing API which would be better fixed.

I tend to agree with you, quite error prone and confusing. I spent
some time to identify who is dropping what.
Actually there is a bunch of functions like this in driver/of/; they
take a node as parameter and return a node, while doing a put of the
parameter. While questionable, looks like there is at least some
consistency.
Maybe the "get" in the API name should match the implicit
of_node_get() of the returned node (not sure it is the case on all
such API). Something else in the name should indicate if the input
node will be of_node_put(). Suggestions are welcome, but I think the
discussion goes beyond the scope of this fix.
In this specific case, I lazily reused some code already upstream here
http://elixir.free-electrons.com/linux/v4.13-rc2/source/sound/soc/generic/simple-card-utils.c#L285
I added in copy Kuninori Morimoto, the developer of the lines above,
in case he has any hint.

Antonio

[PATCH v2 0/3] ASoC: fix unbalanced of_node_get()/of_node_put()

[Antonio Borneo]

From: Antonio Borneo <borneo.antonio@gmail.com>

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like
OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0
OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0/endpoint
each followed by stack dump.

Fixed by:
- removing of_node_put() in the body of of_for_each_phandle(){},
  since already provided at each iteration. Add it in case the
  loop is break out;
- adding of_node_get() before calling of_graph_get_port_parent().

Tested with kernel v4.13-rc2 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

v1 -> v2:
- modify subject "s/fix balance of/fix unbalanced/";
- split the patch per each individual issue. It also simplify the
  backport;
- add ref to the commit being fixed;
- drop one fix not needed.

Antonio Borneo (3):
  ASoC: fix use of of_node_put() in of_for_each_phandle() loops
  ASoC: soc-core: fix unbalanced of_node_get()/of_node_put()
  ASoC: simple-card-utils: fix unbalanced of_node_get()/of_node_put()

 sound/soc/generic/audio-graph-card.c  | 9 ++++-----
 sound/soc/generic/simple-card-utils.c | 5 +++++
 sound/soc/soc-core.c                  | 5 +++++
 3 files changed, 14 insertions(+), 5 deletions(-)

-- 
1.9.1

[PATCH v2 0/3] ASoC: fix unbalanced of_node_get()/of_node_put()

[Antonio Borneo]

From: Antonio Borneo <borneo.antonio@gmail.com>

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like
OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0
OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0/endpoint
each followed by stack dump.

Fixed by:
- removing of_node_put() in the body of of_for_each_phandle(){},
  since already provided at each iteration. Add it in case the
  loop is break out;
- adding of_node_get() before calling of_graph_get_port_parent().

Tested with kernel v4.13-rc2 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

v1 -> v2:
- modify subject "s/fix balance of/fix unbalanced/";
- split the patch per each individual issue. It also simplify the
  backport;
- add ref to the commit being fixed;
- drop one fix not needed.

Antonio Borneo (3):
  ASoC: fix use of of_node_put() in of_for_each_phandle() loops
  ASoC: soc-core: fix unbalanced of_node_get()/of_node_put()
  ASoC: simple-card-utils: fix unbalanced of_node_get()/of_node_put()

 sound/soc/generic/audio-graph-card.c  | 9 ++++-----
 sound/soc/generic/simple-card-utils.c | 5 +++++
 sound/soc/soc-core.c                  | 5 +++++
 3 files changed, 14 insertions(+), 5 deletions(-)

-- 
1.9.1

[PATCH v2 0/3] ASoC: fix unbalanced of_node_get()/of_node_put()

[Antonio Borneo]

From: Antonio Borneo <borneo.antonio@gmail.com>

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like
OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0
OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0/endpoint
each followed by stack dump.

Fixed by:
- removing of_node_put() in the body of of_for_each_phandle(){},
  since already provided at each iteration. Add it in case the
  loop is break out;
- adding of_node_get() before calling of_graph_get_port_parent().

Tested with kernel v4.13-rc2 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

v1 -> v2:
- modify subject "s/fix balance of/fix unbalanced/";
- split the patch per each individual issue. It also simplify the
  backport;
- add ref to the commit being fixed;
- drop one fix not needed.

Antonio Borneo (3):
  ASoC: fix use of of_node_put() in of_for_each_phandle() loops
  ASoC: soc-core: fix unbalanced of_node_get()/of_node_put()
  ASoC: simple-card-utils: fix unbalanced of_node_get()/of_node_put()

 sound/soc/generic/audio-graph-card.c  | 9 ++++-----
 sound/soc/generic/simple-card-utils.c | 5 +++++
 sound/soc/soc-core.c                  | 5 +++++
 3 files changed, 14 insertions(+), 5 deletions(-)

-- 
1.9.1

[PATCH v2 1/3] ASoC: fix use of of_node_put() in of_for_each_phandle() loops

[Antonio Borneo]

From: Antonio Borneo <borneo.antonio@gmail.com>

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like the following one
OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0
each followed by stack dump.

Each iteration of of_for_each_phandle(){} already provides the
needed of_node_put(); adding one more in the body of the loop
triggers the error.
Fixed by removing the wrong of_node_put(), but adding it when the
loop is break out.

Tested with kernel v4.13-rc2 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

This fixes commit 2692c1c63c29 ("ASoC: add audio-graph-card
support").

Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
---
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
To: alsa-devel@alsa-project.org
Cc: linux-kernel@vger.kernel.org
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 105ec3a..20c2029 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -224,9 +224,10 @@ static int asoc_graph_card_parse_of(struct graph_card_data *priv)
 
 	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
 		ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
-		of_node_put(it.node);
-		if (ret < 0)
+		if (ret < 0) {
+			of_node_put(it.node);
 			return ret;
+		}
 	}
 
 	return asoc_simple_card_parse_card_name(card, NULL);
@@ -239,10 +240,8 @@ static int asoc_graph_get_dais_count(struct device *dev)
 	int count = 0;
 	int rc;
 
-	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
+	of_for_each_phandle(&it, rc, node, "dais", NULL, 0)
 		count++;
-		of_node_put(it.node);
-	}
 
 	return count;
 }
-- 
1.9.1

[PATCH v2 1/3] ASoC: fix use of of_node_put() in of_for_each_phandle() loops

[Antonio Borneo]

From: Antonio Borneo <borneo.antonio@gmail.com>

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like the following one
OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0
each followed by stack dump.

Each iteration of of_for_each_phandle(){} already provides the
needed of_node_put(); adding one more in the body of the loop
triggers the error.
Fixed by removing the wrong of_node_put(), but adding it when the
loop is break out.

Tested with kernel v4.13-rc2 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

This fixes commit 2692c1c63c29 ("ASoC: add audio-graph-card
support").

Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
---
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
To: alsa-devel at alsa-project.org
Cc: linux-kernel at vger.kernel.org
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-arm-kernel at lists.infradead.org
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 105ec3a..20c2029 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -224,9 +224,10 @@ static int asoc_graph_card_parse_of(struct graph_card_data *priv)
 
 	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
 		ret = asoc_graph_card_dai_link_of(it.node, priv, idx++);
-		of_node_put(it.node);
-		if (ret < 0)
+		if (ret < 0) {
+			of_node_put(it.node);
 			return ret;
+		}
 	}
 
 	return asoc_simple_card_parse_card_name(card, NULL);
@@ -239,10 +240,8 @@ static int asoc_graph_get_dais_count(struct device *dev)
 	int count = 0;
 	int rc;
 
-	of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
+	of_for_each_phandle(&it, rc, node, "dais", NULL, 0)
 		count++;
-		of_node_put(it.node);
-	}
 
 	return count;
 }
-- 
1.9.1

[PATCH v2 2/3] ASoC: soc-core: fix unbalanced of_node_get()/of_node_put()

[Antonio Borneo]

From: Antonio Borneo <borneo.antonio@gmail.com>

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like the following one
OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0/endpoint
each followed by stack dump.

of_graph_get_port_parent() walks through the parents looking for a
node named "ports". At each step it uses of_get_next_parent() that
drops the current node with of_node_put().
Avoid dropping the initial node by calling of_node_get() before
of_graph_get_port_parent().

Tested with kernel v4.13-rc2 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

This fixes commit a180e8b98843 ("ASoC: add snd_soc_get_dai_id()
function").

Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
---
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
To: alsa-devel@alsa-project.org
Cc: linux-kernel@vger.kernel.org
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 921622a..a0f39de 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -4087,6 +4087,11 @@ int snd_soc_get_dai_id(struct device_node *ep)
 	struct device_node *node;
 	int ret;
 
+	/*
+	 * of_graph_get_port_parent() will call
+	 * of_node_put(). So, call of_node_get() here
+	 */
+	of_node_get(ep);
 	node = of_graph_get_port_parent(ep);
 
 	/*
-- 
1.9.1

[PATCH v2 2/3] ASoC: soc-core: fix unbalanced of_node_get()/of_node_put()

[Antonio Borneo]

From: Antonio Borneo <borneo.antonio@gmail.com>

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like the following one
OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0/endpoint
each followed by stack dump.

of_graph_get_port_parent() walks through the parents looking for a
node named "ports". At each step it uses of_get_next_parent() that
drops the current node with of_node_put().
Avoid dropping the initial node by calling of_node_get() before
of_graph_get_port_parent().

Tested with kernel v4.13-rc2 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

This fixes commit a180e8b98843 ("ASoC: add snd_soc_get_dai_id()
function").

Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
---
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
To: alsa-devel at alsa-project.org
Cc: linux-kernel at vger.kernel.org
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-arm-kernel at lists.infradead.org
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 921622a..a0f39de 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -4087,6 +4087,11 @@ int snd_soc_get_dai_id(struct device_node *ep)
 	struct device_node *node;
 	int ret;
 
+	/*
+	 * of_graph_get_port_parent() will call
+	 * of_node_put(). So, call of_node_get() here
+	 */
+	of_node_get(ep);
 	node = of_graph_get_port_parent(ep);
 
 	/*
-- 
1.9.1

[PATCH v2 3/3] ASoC: simple-card-utils: fix unbalanced of_node_get()/of_node_put()

[Antonio Borneo]

From: Antonio Borneo <borneo.antonio@gmail.com>

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like the following one
OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0/endpoint
each followed by stack dump.

of_graph_get_port_parent() walks through the parents looking for a
node named "ports". At each step it uses of_get_next_parent() that
drops the current node with of_node_put().
Avoid dropping the initial node by calling of_node_get() before
of_graph_get_port_parent().

Tested with kernel v4.13-rc2 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

This fixes commit 1689333f8311 ("ASoC: simple-card-utils: add
asoc_simple_card_parse_graph_dai()").

Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
---
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
To: alsa-devel@alsa-project.org
Cc: linux-kernel@vger.kernel.org
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/simple-card-utils.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 26d64fa..144954b 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -250,6 +250,11 @@ static int asoc_simple_card_get_dai_id(struct device_node *ep)
 	if (ret != -ENOTSUPP)
 		return ret;
 
+	/*
+	 * of_graph_get_port_parent() will call
+	 * of_node_put(). So, call of_node_get() here
+	 */
+	of_node_get(ep);
 	node = of_graph_get_port_parent(ep);
 
 	/*
-- 
1.9.1

[PATCH v2 3/3] ASoC: simple-card-utils: fix unbalanced of_node_get()/of_node_put()

[Antonio Borneo]

From: Antonio Borneo <borneo.antonio@gmail.com>

On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
errors at kernel boot, like the following one
OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0/endpoint
each followed by stack dump.

of_graph_get_port_parent() walks through the parents looking for a
node named "ports". At each step it uses of_get_next_parent() that
drops the current node with of_node_put().
Avoid dropping the initial node by calling of_node_get() before
of_graph_get_port_parent().

Tested with kernel v4.13-rc2 with hikey_defconfig taken from
https://git.linaro.org/people/john.stultz/android-dev.git
branch dev/hikey-mainline-WIP

This fixes commit 1689333f8311 ("ASoC: simple-card-utils: add
asoc_simple_card_parse_graph_dai()").

Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
---
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
To: alsa-devel at alsa-project.org
Cc: linux-kernel at vger.kernel.org
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-arm-kernel at lists.infradead.org
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/simple-card-utils.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 26d64fa..144954b 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -250,6 +250,11 @@ static int asoc_simple_card_get_dai_id(struct device_node *ep)
 	if (ret != -ENOTSUPP)
 		return ret;
 
+	/*
+	 * of_graph_get_port_parent() will call
+	 * of_node_put(). So, call of_node_get() here
+	 */
+	of_node_get(ep);
 	node = of_graph_get_port_parent(ep);
 
 	/*
-- 
1.9.1

Re: [PATCH v2 0/3] ASoC: fix unbalanced of_node_get()/of_node_put()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

On Fri, Jul 28, 2017 at 01:26:09AM +0200, Antonio Borneo wrote:
> From: Antonio Borneo <borneo.antonio@gmail.com>
> 
> On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
> errors at kernel boot, like
> OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0
> OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0/endpoint
> each followed by stack dump.

Please take a look at the patches that Tony Lindgren has posted in the
past day for this issue which go into the of_graph API to make it less
error prone.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 0/3] ASoC: fix unbalanced of_node_get()/of_node_put()

[Mark Brown]

On Fri, Jul 28, 2017 at 01:26:09AM +0200, Antonio Borneo wrote:
> From: Antonio Borneo <borneo.antonio@gmail.com>
> 
> On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
> errors at kernel boot, like
> OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0
> OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0/endpoint
> each followed by stack dump.

Please take a look at the patches that Tony Lindgren has posted in the
past day for this issue which go into the of_graph API to make it less
error prone.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170728/2f9d19fa/attachment-0001.sig>

Re: [PATCH v2 0/3] ASoC: fix unbalanced of_node_get()/of_node_put()

[Antonio Borneo]

On Fri, Jul 28, 2017 at 12:07 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 28, 2017 at 01:26:09AM +0200, Antonio Borneo wrote:
>> From: Antonio Borneo <borneo.antonio@gmail.com>
>>
>> On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
>> errors at kernel boot, like
>> OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0
>> OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0/endpoint
>> each followed by stack dump.
>
> Please take a look at the patches that Tony Lindgren has posted in the
> past day for this issue which go into the of_graph API to make it less
> error prone.

Great, it fixes the issue!
Please drop this patchset, the issue is completely covered by Tony's work.

Best Regards
Antonio

Re: [PATCH v2 0/3] ASoC: fix unbalanced of_node_get()/of_node_put()

[Antonio Borneo]

On Fri, Jul 28, 2017 at 12:07 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 28, 2017 at 01:26:09AM +0200, Antonio Borneo wrote:
>> From: Antonio Borneo <borneo.antonio@gmail.com>
>>
>> On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
>> errors at kernel boot, like
>> OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0
>> OF: ERROR: Bad of_node_put() on /soc/i2s@f7118000/ports/port@0/endpoint
>> each followed by stack dump.
>
> Please take a look at the patches that Tony Lindgren has posted in the
> past day for this issue which go into the of_graph API to make it less
> error prone.

Great, it fixes the issue!
Please drop this patchset, the issue is completely covered by Tony's work.

Best Regards
Antonio

[PATCH v2 0/3] ASoC: fix unbalanced of_node_get()/of_node_put()

[Antonio Borneo]

On Fri, Jul 28, 2017 at 12:07 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 28, 2017 at 01:26:09AM +0200, Antonio Borneo wrote:
>> From: Antonio Borneo <borneo.antonio@gmail.com>
>>
>> On Hikey target board, enabling CONFIG_OF_DYNAMIC triggers several
>> errors at kernel boot, like
>> OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0
>> OF: ERROR: Bad of_node_put() on /soc/i2s at f7118000/ports/port at 0/endpoint
>> each followed by stack dump.
>
> Please take a look at the patches that Tony Lindgren has posted in the
> past day for this issue which go into the of_graph API to make it less
> error prone.

Great, it fixes the issue!
Please drop this patchset, the issue is completely covered by Tony's work.

Best Regards
Antonio