Re: [PATCH 09/39 v2] ASoC: simple-card-utils: add asoc_simple_card_parse_clk()

[Kuninori Morimoto]

Hi Mark

> > Oops, of_clk_get() doesn't have devm_of_clk_get() ?
> 
> Perhaps add it? 
> 
> > (and no of_clk_put() ... )
> > I will keep above as-is in v3. We can fix it incrementally (?)
> 
> You can just use regular clk_put() with of_clk_get().

Ohh.. OK, but can I do it in additional patch ?
Because main purpose of this patch is cleanup by using util.c,
not adding new feature.
Maybe it can be
 0001 add devm_of_clk_get()
 0002 use devm_of_clk_get() on simple-card-utils.c

Re: [PATCH 09/39 v2] ASoC: simple-card-utils: add asoc_simple_card_parse_clk()

[Mark Brown]

On Thu, Jun 30, 2016 at 12:39:12AM +, Kuninori Morimoto wrote:

> Oops, of_clk_get() doesn't have devm_of_clk_get() ?

Perhaps add it? 

> (and no of_clk_put() ... )
> I will keep above as-is in v3. We can fix it incrementally (?)

You can just use regular clk_put() with of_clk_get().


signature.asc
Description: PGP signature

Re: [PATCH 09/39 v2] ASoC: simple-card-utils: add asoc_simple_card_parse_clk()

[Kuninori Morimoto]

Hi Mark, again

> > > + struct clk *clk;
> > > + u32 val;
> > > +
> > > + /*
> > > +  * Parse dai->sysclk come from "clocks = <>"
> > > +  * (if system has common clock)
> > > +  *  or "system-clock-frequency = "
> > > +  *  or device's module clock.
> > > +  */
> > > + clk = of_clk_get(port_np, 0);
> > > + if (!IS_ERR(clk)) {
> > > + simple_dai->sysclk = clk_get_rate(clk);
> > > + simple_dai->clk = clk;
> > > + } else if (!of_property_read_u32(port_np, "system-clock-frequency", 
> > > )) {
> > > + simple_dai->sysclk = val;
> > > + } else {
> > > + clk = of_clk_get(endpoint_np, 0);
> > > + if (!IS_ERR(clk))
> > > + simple_dai->sysclk = clk_get_rate(clk);
> > > + }
> > 
> > This looks like we're leaking the clocks - devm_ might help here
> > perhaps?
> 
> Good catch.
> This came from original simple-card, but yes, we should use devm_*
> I will fix it on v3

Oops, of_clk_get() doesn't have devm_of_clk_get() ?
(and no of_clk_put() ... )
I will keep above as-is in v3. We can fix it incrementally (?)

Re: [PATCH 09/39 v2] ASoC: simple-card-utils: add asoc_simple_card_parse_clk()

[Kuninori Morimoto]

Hi Mark

> > +   struct clk *clk;
> > +   u32 val;
> > +
> > +   /*
> > +* Parse dai->sysclk come from "clocks = <>"
> > +* (if system has common clock)
> > +*  or "system-clock-frequency = "
> > +*  or device's module clock.
> > +*/
> > +   clk = of_clk_get(port_np, 0);
> > +   if (!IS_ERR(clk)) {
> > +   simple_dai->sysclk = clk_get_rate(clk);
> > +   simple_dai->clk = clk;
> > +   } else if (!of_property_read_u32(port_np, "system-clock-frequency", 
> > )) {
> > +   simple_dai->sysclk = val;
> > +   } else {
> > +   clk = of_clk_get(endpoint_np, 0);
> > +   if (!IS_ERR(clk))
> > +   simple_dai->sysclk = clk_get_rate(clk);
> > +   }
> 
> This looks like we're leaking the clocks - devm_ might help here
> perhaps?

Good catch.
This came from original simple-card, but yes, we should use devm_*
I will fix it on v3

Re: [PATCH 09/39 v2] ASoC: simple-card-utils: add asoc_simple_card_parse_clk()

[Mark Brown]

On Tue, May 31, 2016 at 09:02:22AM +, Kuninori Morimoto wrote:

> + struct clk *clk;
> + u32 val;
> +
> + /*
> +  * Parse dai->sysclk come from "clocks = <>"
> +  * (if system has common clock)
> +  *  or "system-clock-frequency = "
> +  *  or device's module clock.
> +  */
> + clk = of_clk_get(port_np, 0);
> + if (!IS_ERR(clk)) {
> + simple_dai->sysclk = clk_get_rate(clk);
> + simple_dai->clk = clk;
> + } else if (!of_property_read_u32(port_np, "system-clock-frequency", 
> )) {
> + simple_dai->sysclk = val;
> + } else {
> + clk = of_clk_get(endpoint_np, 0);
> + if (!IS_ERR(clk))
> + simple_dai->sysclk = clk_get_rate(clk);
> + }

This looks like we're leaking the clocks - devm_ might help here
perhaps?


signature.asc
Description: PGP signature

[PATCH 09/39 v2] ASoC: simple-card-utils: add asoc_simple_card_parse_clk()

[Kuninori Morimoto]

From: Kuninori Morimoto 

Current simple-card can get clock via DT clocks or
"system-clock-frequency" property.
This patch makes it simple style standard

Signed-off-by: Kuninori Morimoto 
---
 include/sound/simple_card_utils.h |  8 
 sound/soc/generic/simple-card-utils.c | 30 ++
 2 files changed, 38 insertions(+)

diff --git a/include/sound/simple_card_utils.h 
b/include/sound/simple_card_utils.h
index 89172aa..b8a8649 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -38,4 +38,12 @@ int asoc_simple_card_parse_card_prefix(struct snd_soc_card 
*card,
   struct snd_soc_codec_conf *codec_conf,
   char *prefix);
 
+#define asoc_simple_card_parse_clk_cpu(port_np, dai_link, simple_dai)\
+   asoc_simple_card_parse_clk(port_np, dai_link->cpu_of_node, simple_dai)
+#define asoc_simple_card_parse_clk_codec(port_np, dai_link, simple_dai)  \
+   asoc_simple_card_parse_clk(port_np, dai_link->codec_of_node, simple_dai)
+int asoc_simple_card_parse_clk(struct device_node *port_np,
+  struct device_node *endpoint_np,
+  struct asoc_simple_dai *simple_dai);
+
 #endif /* __SIMPLE_CARD_CORE_H */
diff --git a/sound/soc/generic/simple-card-utils.c 
b/sound/soc/generic/simple-card-utils.c
index 439fc01..dbf4b00 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -7,6 +7,7 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include 
 #include 
 #include 
 
@@ -142,3 +143,32 @@ int asoc_simple_card_parse_card_prefix(struct snd_soc_card 
*card,
return 0;
 }
 EXPORT_SYMBOL_GPL(asoc_simple_card_parse_card_prefix);
+
+int asoc_simple_card_parse_clk(struct device_node *port_np,
+  struct device_node *endpoint_np,
+  struct asoc_simple_dai *simple_dai)
+{
+   struct clk *clk;
+   u32 val;
+
+   /*
+* Parse dai->sysclk come from "clocks = <>"
+* (if system has common clock)
+*  or "system-clock-frequency = "
+*  or device's module clock.
+*/
+   clk = of_clk_get(port_np, 0);
+   if (!IS_ERR(clk)) {
+   simple_dai->sysclk = clk_get_rate(clk);
+   simple_dai->clk = clk;
+   } else if (!of_property_read_u32(port_np, "system-clock-frequency", 
)) {
+   simple_dai->sysclk = val;
+   } else {
+   clk = of_clk_get(endpoint_np, 0);
+   if (!IS_ERR(clk))
+   simple_dai->sysclk = clk_get_rate(clk);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_simple_card_parse_clk);
-- 
1.9.1