diff mbox

[2/3] ASoC: simple-card: use devm_get_clk_from_child()

Message ID 871sxnvtkp.wl%kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kuninori Morimoto Dec. 5, 2016, 5:23 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current simple-card-utils is getting clk by of_clk_get(), but didn't call
clk_free(). Now we can use devm_get_clk_from_child() for this purpose.
Let's use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/simple_card_utils.h     | 11 ++++++-----
 sound/soc/generic/simple-card-utils.c |  8 ++++----
 sound/soc/generic/simple-card.c       |  4 ++--
 sound/soc/generic/simple-scu-card.c   |  4 ++--
 4 files changed, 14 insertions(+), 13 deletions(-)

Comments

Stephen Boyd Dec. 8, 2016, 10:09 p.m. UTC | #1
On 12/05, Kuninori Morimoto wrote:
> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
> index cf02625..4924575 100644
> --- a/sound/soc/generic/simple-card-utils.c
> +++ b/sound/soc/generic/simple-card-utils.c
> @@ -98,7 +98,8 @@ int asoc_simple_card_parse_card_name(struct snd_soc_card *card,
>  }
>  EXPORT_SYMBOL_GPL(asoc_simple_card_parse_card_name);
>  
> -int asoc_simple_card_parse_clk(struct device_node *node,
> +int asoc_simple_card_parse_clk(struct device *dev,
> +			       struct device_node *node,
>  			       struct device_node *dai_of_node,
>  			       struct asoc_simple_dai *simple_dai)
>  {
> @@ -111,14 +112,13 @@ int asoc_simple_card_parse_clk(struct device_node *node,
>  	 *  or "system-clock-frequency = <xxx>"
>  	 *  or device's module clock.
>  	 */
> -	clk = of_clk_get(node, 0);
> +	clk = devm_get_clk_from_child(dev, node, NULL);
>  	if (!IS_ERR(clk)) {
>  		simple_dai->sysclk = clk_get_rate(clk);
> -		simple_dai->clk = clk;
>  	} else if (!of_property_read_u32(node, "system-clock-frequency", &val)) {
>  		simple_dai->sysclk = val;
>  	} else {
> -		clk = of_clk_get(dai_of_node, 0);
> +		clk = devm_get_clk_from_child(dev, dai_of_node, NULL);


I was confused for a minute about how the second of_clk_get()
call with the dai_link node could work. Is that documented
anywhere or used by anyone? It seems like it's at least another
child node of the sound node (which is dev here) so it seems ok.


>  		if (!IS_ERR(clk))
>  			simple_dai->sysclk = clk_get_rate(clk);
>  	}
Kuninori Morimoto Dec. 9, 2016, 12:20 a.m. UTC | #2
Hi Stephen

> > @@ -111,14 +112,13 @@ int asoc_simple_card_parse_clk(struct device_node *node,
> >  	 *  or "system-clock-frequency = <xxx>"
> >  	 *  or device's module clock.
> >  	 */
> > -	clk = of_clk_get(node, 0);
> > +	clk = devm_get_clk_from_child(dev, node, NULL);
> >  	if (!IS_ERR(clk)) {
> >  		simple_dai->sysclk = clk_get_rate(clk);
> > -		simple_dai->clk = clk;
> >  	} else if (!of_property_read_u32(node, "system-clock-frequency", &val)) {
> >  		simple_dai->sysclk = val;
> >  	} else {
> > -		clk = of_clk_get(dai_of_node, 0);
> > +		clk = devm_get_clk_from_child(dev, dai_of_node, NULL);
> 
> 
> I was confused for a minute about how the second of_clk_get()
> call with the dai_link node could work. Is that documented
> anywhere or used by anyone? It seems like it's at least another
> child node of the sound node (which is dev here) so it seems ok.

Documentation/devicetree/bindings/sound/simple-card.txt
explains 1st of_clk_get will be used as "if needed",
2nd of_clk_get will be used as "not needed pattern".
1st pattern will use specific clock, 2nd pattern will use
"cpu" or "codec" clock.
2nd one was added by someone (I forgot), and many driver is
based on this feature.

Best regards
---
Kuninori Morimoto
Stephen Boyd Dec. 9, 2016, 12:28 a.m. UTC | #3
On 12/09, Kuninori Morimoto wrote:
> 
> Hi Stephen
> 
> > > @@ -111,14 +112,13 @@ int asoc_simple_card_parse_clk(struct device_node *node,
> > >  	 *  or "system-clock-frequency = <xxx>"
> > >  	 *  or device's module clock.
> > >  	 */
> > > -	clk = of_clk_get(node, 0);
> > > +	clk = devm_get_clk_from_child(dev, node, NULL);
> > >  	if (!IS_ERR(clk)) {
> > >  		simple_dai->sysclk = clk_get_rate(clk);
> > > -		simple_dai->clk = clk;
> > >  	} else if (!of_property_read_u32(node, "system-clock-frequency", &val)) {
> > >  		simple_dai->sysclk = val;
> > >  	} else {
> > > -		clk = of_clk_get(dai_of_node, 0);
> > > +		clk = devm_get_clk_from_child(dev, dai_of_node, NULL);
> > 
> > 
> > I was confused for a minute about how the second of_clk_get()
> > call with the dai_link node could work. Is that documented
> > anywhere or used by anyone? It seems like it's at least another
> > child node of the sound node (which is dev here) so it seems ok.
> 
> Documentation/devicetree/bindings/sound/simple-card.txt
> explains 1st of_clk_get will be used as "if needed",
> 2nd of_clk_get will be used as "not needed pattern".
> 1st pattern will use specific clock, 2nd pattern will use
> "cpu" or "codec" clock.
> 2nd one was added by someone (I forgot), and many driver is
> based on this feature.
> 

Can you point to some dts file in the kernel that falls into the
devm_get_clk_from_child(dev, dai_of_node, NULL) part?
Kuninori Morimoto Dec. 9, 2016, 12:33 a.m. UTC | #4
Hi Stephen

> > Documentation/devicetree/bindings/sound/simple-card.txt
> > explains 1st of_clk_get will be used as "if needed",
> > 2nd of_clk_get will be used as "not needed pattern".
> > 1st pattern will use specific clock, 2nd pattern will use
> > "cpu" or "codec" clock.
> > 2nd one was added by someone (I forgot), and many driver is
> > based on this feature.
> > 
> 
> Can you point to some dts file in the kernel that falls into the
> devm_get_clk_from_child(dev, dai_of_node, NULL) part?

How about this ?

linux/arch/arm/boot/dts/r8a7790-lager.dts :: rsnd_ak4643
diff mbox

Patch

diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index 64e90ca..af58d23 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -34,11 +34,12 @@  int asoc_simple_card_set_dailink_name(struct device *dev,
 int asoc_simple_card_parse_card_name(struct snd_soc_card *card,
 				     char *prefix);
 
-#define asoc_simple_card_parse_clk_cpu(node, dai_link, simple_dai)		\
-	asoc_simple_card_parse_clk(node, dai_link->cpu_of_node, simple_dai)
-#define asoc_simple_card_parse_clk_codec(node, dai_link, simple_dai)		\
-	asoc_simple_card_parse_clk(node, dai_link->codec_of_node, simple_dai)
-int asoc_simple_card_parse_clk(struct device_node *node,
+#define asoc_simple_card_parse_clk_cpu(dev, node, dai_link, simple_dai)		\
+	asoc_simple_card_parse_clk(dev, node, dai_link->cpu_of_node, simple_dai)
+#define asoc_simple_card_parse_clk_codec(dev, node, dai_link, simple_dai)	\
+	asoc_simple_card_parse_clk(dev, node, dai_link->codec_of_node, simple_dai)
+int asoc_simple_card_parse_clk(struct device *dev,
+			       struct device_node *node,
 			       struct device_node *dai_of_node,
 			       struct asoc_simple_dai *simple_dai);
 
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index cf02625..4924575 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -98,7 +98,8 @@  int asoc_simple_card_parse_card_name(struct snd_soc_card *card,
 }
 EXPORT_SYMBOL_GPL(asoc_simple_card_parse_card_name);
 
-int asoc_simple_card_parse_clk(struct device_node *node,
+int asoc_simple_card_parse_clk(struct device *dev,
+			       struct device_node *node,
 			       struct device_node *dai_of_node,
 			       struct asoc_simple_dai *simple_dai)
 {
@@ -111,14 +112,13 @@  int asoc_simple_card_parse_clk(struct device_node *node,
 	 *  or "system-clock-frequency = <xxx>"
 	 *  or device's module clock.
 	 */
-	clk = of_clk_get(node, 0);
+	clk = devm_get_clk_from_child(dev, node, NULL);
 	if (!IS_ERR(clk)) {
 		simple_dai->sysclk = clk_get_rate(clk);
-		simple_dai->clk = clk;
 	} else if (!of_property_read_u32(node, "system-clock-frequency", &val)) {
 		simple_dai->sysclk = val;
 	} else {
-		clk = of_clk_get(dai_of_node, 0);
+		clk = devm_get_clk_from_child(dev, dai_of_node, NULL);
 		if (!IS_ERR(clk))
 			simple_dai->sysclk = clk_get_rate(clk);
 	}
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index a385ff6..85b4f18 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -278,11 +278,11 @@  static int asoc_simple_card_dai_link_of(struct device_node *node,
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	ret = asoc_simple_card_parse_clk_cpu(cpu, dai_link, cpu_dai);
+	ret = asoc_simple_card_parse_clk_cpu(dev, cpu, dai_link, cpu_dai);
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	ret = asoc_simple_card_parse_clk_codec(codec, dai_link, codec_dai);
+	ret = asoc_simple_card_parse_clk_codec(dev, codec, dai_link, codec_dai);
 	if (ret < 0)
 		goto dai_link_of_err;
 
diff --git a/sound/soc/generic/simple-scu-card.c b/sound/soc/generic/simple-scu-card.c
index bb86ee0..308ff4c 100644
--- a/sound/soc/generic/simple-scu-card.c
+++ b/sound/soc/generic/simple-scu-card.c
@@ -128,7 +128,7 @@  static int asoc_simple_card_dai_link_of(struct device_node *np,
 		if (ret)
 			return ret;
 
-		ret = asoc_simple_card_parse_clk_cpu(np, dai_link, dai_props);
+		ret = asoc_simple_card_parse_clk_cpu(dev, np, dai_link, dai_props);
 		if (ret < 0)
 			return ret;
 
@@ -153,7 +153,7 @@  static int asoc_simple_card_dai_link_of(struct device_node *np,
 		if (ret < 0)
 			return ret;
 
-		ret = asoc_simple_card_parse_clk_codec(np, dai_link, dai_props);
+		ret = asoc_simple_card_parse_clk_codec(dev, np, dai_link, dai_props);
 		if (ret < 0)
 			return ret;