diff mbox

[v4,6/9] ASoC: add snd_soc_get_dai_id()

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

Commit Message

Kuninori Morimoto March 13, 2017, 5:53 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

ALSA SoC needs to know connected DAI ID for probing.
On OF-graph case, basically we can check DT port location.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v3 -> v4

 - no change

 include/sound/soc.h  |  1 +
 sound/soc/soc-core.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

Comments

Rob Herring March 20, 2017, 8:20 p.m. UTC | #1
On Mon, Mar 13, 2017 at 12:53 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> ALSA SoC needs to know connected DAI ID for probing.
> On OF-graph case, basically we can check DT port location.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v3 -> v4
>
>  - no change
>
>  include/sound/soc.h  |  1 +
>  sound/soc/soc-core.c | 23 +++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index cdfb55f..ab4639e 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -1664,6 +1664,7 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
>                                      const char *prefix,
>                                      struct device_node **bitclkmaster,
>                                      struct device_node **framemaster);
> +int snd_soc_get_dai_id(struct device_node *ep);
>  int snd_soc_get_dai_name(struct of_phandle_args *args,
>                          const char **dai_name);
>  int snd_soc_of_get_dai_name(struct device_node *of_node,
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 175ade0..c91010d 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -34,6 +34,7 @@
>  #include <linux/ctype.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
> +#include <linux/of_graph.h>
>  #include <linux/dmi.h>
>  #include <sound/core.h>
>  #include <sound/jack.h>
> @@ -4040,6 +4041,28 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
>  }
>  EXPORT_SYMBOL_GPL(snd_soc_of_parse_daifmt);
>
> +int snd_soc_get_dai_id(struct device_node *ep)

Shouldn't the DAI id be the index of the "dais" property?

> +{
> +       struct device_node *node;
> +       struct device_node *endpoint;
> +       int i, id;
> +
> +       node = of_graph_get_port_parent(ep);
> +
> +       i = 0;
> +       id = -1;
> +       for_each_endpoint_of_node(node, endpoint) {
> +               if (endpoint == ep)
> +                       id = i;

I don't see how this works when you have 1 DAI controller with
multiple endpoints versus multiple DAI controllers with a single
endpoint each. All the IDs will be 0 in the latter case.

> +               i++;
> +       }
> +       if (id < 0)
> +               return -ENODEV;
> +
> +       return id;
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_get_dai_id);
> +
>  int snd_soc_get_dai_name(struct of_phandle_args *args,
>                                 const char **dai_name)
>  {
> --
> 1.9.1
>
Kuninori Morimoto March 21, 2017, 1:59 a.m. UTC | #2
Hi Rob

Thank you for your review

> > +{
> > +       struct device_node *node;
> > +       struct device_node *endpoint;
> > +       int i, id;
> > +
> > +       node = of_graph_get_port_parent(ep);
> > +
> > +       i = 0;
> > +       id = -1;
> > +       for_each_endpoint_of_node(node, endpoint) {
> > +               if (endpoint == ep)
> > +                       id = i;
> 
> I don't see how this works when you have 1 DAI controller with
> multiple endpoints versus multiple DAI controllers with a single
> endpoint each. All the IDs will be 0 in the latter case.

It support 1:1 endpoint pattern only.

Best regards
---
Kuninori Morimoto
Rob Herring April 7, 2017, 7:16 p.m. UTC | #3
On Mon, Mar 20, 2017 at 8:59 PM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
> Hi Rob
>
> Thank you for your review
>
>> > +{
>> > +       struct device_node *node;
>> > +       struct device_node *endpoint;
>> > +       int i, id;
>> > +
>> > +       node = of_graph_get_port_parent(ep);
>> > +
>> > +       i = 0;
>> > +       id = -1;
>> > +       for_each_endpoint_of_node(node, endpoint) {
>> > +               if (endpoint == ep)
>> > +                       id = i;
>>
>> I don't see how this works when you have 1 DAI controller with
>> multiple endpoints versus multiple DAI controllers with a single
>> endpoint each. All the IDs will be 0 in the latter case.
>
> It support 1:1 endpoint pattern only.

Then the endpoint id is always 0 and this function is pointless.

Rob
Kuninori Morimoto April 10, 2017, 12:17 a.m. UTC | #4
Hi Rob

Thank you for your review.

> >> > +{
> >> > +       struct device_node *node;
> >> > +       struct device_node *endpoint;
> >> > +       int i, id;
> >> > +
> >> > +       node = of_graph_get_port_parent(ep);
> >> > +
> >> > +       i = 0;
> >> > +       id = -1;
> >> > +       for_each_endpoint_of_node(node, endpoint) {
> >> > +               if (endpoint == ep)
> >> > +                       id = i;
> >>
> >> I don't see how this works when you have 1 DAI controller with
> >> multiple endpoints versus multiple DAI controllers with a single
> >> endpoint each. All the IDs will be 0 in the latter case.
> >
> > It support 1:1 endpoint pattern only.
> 
> Then the endpoint id is always 0 and this function is pointless.

Sorry, I checked my patch-list, and I noticed that
this function will be expand to use callback function in next
patch-set (= HDMI support).
Thus, inded current function is pointless at this point.
I will merge this expansion patch in v5
diff mbox

Patch

diff --git a/include/sound/soc.h b/include/sound/soc.h
index cdfb55f..ab4639e 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1664,6 +1664,7 @@  unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 				     const char *prefix,
 				     struct device_node **bitclkmaster,
 				     struct device_node **framemaster);
+int snd_soc_get_dai_id(struct device_node *ep);
 int snd_soc_get_dai_name(struct of_phandle_args *args,
 			 const char **dai_name);
 int snd_soc_of_get_dai_name(struct device_node *of_node,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 175ade0..c91010d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -34,6 +34,7 @@ 
 #include <linux/ctype.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/dmi.h>
 #include <sound/core.h>
 #include <sound/jack.h>
@@ -4040,6 +4041,28 @@  unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(snd_soc_of_parse_daifmt);
 
+int snd_soc_get_dai_id(struct device_node *ep)
+{
+	struct device_node *node;
+	struct device_node *endpoint;
+	int i, id;
+
+	node = of_graph_get_port_parent(ep);
+
+	i = 0;
+	id = -1;
+	for_each_endpoint_of_node(node, endpoint) {
+		if (endpoint == ep)
+			id = i;
+		i++;
+	}
+	if (id < 0)
+		return -ENODEV;
+
+	return id;
+}
+EXPORT_SYMBOL_GPL(snd_soc_get_dai_id);
+
 int snd_soc_get_dai_name(struct of_phandle_args *args,
 				const char **dai_name)
 {