[PATCH] *** ASoC: core: refine of node id parse of cpu/codec dai ***

[PATCH] *** ASoC: core: refine of node id parse of cpu/codec dai ***

[Qiao Zhou]

Hi Mark, Liam

This patch is to add another check besides cpu_dai/codec_dai name during
dai_link bind. currently if the cpu_dai/codec_dai name match corresponding
dai_link cpu/codec name, then a match is found. in this patch, it also checks
whether cpu/codec dai id match dailink cpu_dai_id/codec_dai_id. Still check
name first.
1. if it doesn't match, it will keep checking whether cpu_id/codec_id match
corresponding dai_link cpu_dai_id/codec_dai_id. if the ids are equal, then
a match is found.
2. if it does match, then a match is already found. no need to further check.

By this way, it's convenient to maintain dai_link in DT files, and machine
driver can parse related node point to get dai id info. Then there is no need
to parse cpu_dai/codec_dai name for each dai. Like below:

In DT files:
sound {
	compatible = "sound card name xx";
	dai_link_1:dai_link_1 {
		compatible = "xx-dailink-1";
		cpu-dai = <&cpu-dai-node CPU_DAI_ID_1>;
		codec-dai = <&codec-dai-node CODEC_DAI_ID_1>;
		......
	}

	dai_link_2:dai_link_2 {
		compatible = "xx-dailink-2";
		cpu-dai = <&cpu-dai-node CPU_DAI_ID_2>;
		codec-dai = <&codec-dai-node CODEC_DAI_ID_2>;
		......
	}
	......
}

In machine driver:
static xx_sound_card_probe(struct platform_device *pdev)
{
	struct device_node *np = pdev->dev.of_node;

	i = 0;
	for_each_child_of_node(np, dailink_np) {
		of_parse_phandle_with_args(dailink_np, "cpu-dai", "#dailink-cells", 0,
&out_args);
		dailink[i].cpu_of_node = out_args.np;
		dailink[i].cpu_dai_id = out_args.args[0];
		dailink[i].platform_of_node = out_args.np;

		of_parse_phandle_with_args(dailink_np, "codec-dai", "#dailink-cells", 0,
&out_args);
		dailink[i].codec_of_node = out_args.np;
		dailink[i].codec_dai_id = out_args.args[0];
		......
		i++
	}
	......
	ret = snd_soc_register_card(card);
	......
}

If some drivers don't have DT support, then we have no choice but to add correct
cpu/codec(dai) name in DT and parse name info in machine driver.

Could you help share your opinions? Thanks in advance.

Qiao Zhou (1):
  ASoC: core: refine of node id parse of cpu/codec dai

 include/sound/soc.h  |    2 ++
 sound/soc/soc-core.c |   32 +++++++++++++++++++++++---------
 2 files changed, 25 insertions(+), 9 deletions(-)

-- 
1.7.9.5

[PATCH] ASoC: core: refine of node id parse of cpu/codec dai

[Qiao Zhou]

refine cpu_dai/codec_dai of_node checking by checking either name
or dai_id matches.

Signed-off-by: Qiao Zhou <zhouqiao@marvell.com>
---
 include/sound/soc.h  |    2 ++
 sound/soc/soc-core.c |   32 +++++++++++++++++++++++---------
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index ed9e2d7..782f7a0 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -888,12 +888,14 @@ struct snd_soc_dai_link {
 	 * only, which only works well when that device exposes a single DAI.
 	 */
 	const char *cpu_dai_name;
+	unsigned int cpu_dai_id;
 	/*
 	 * You MUST specify the link's codec, either by device name, or by
 	 * DT/OF node, but not both.
 	 */
 	const char *codec_name;
 	const struct device_node *codec_of_node;
+	unsigned int codec_dai_id;
 	/* You MUST specify the DAI name within the codec */
 	const char *codec_dai_name;
 	/*
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b87d7d8..777237d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -868,14 +868,16 @@ static struct snd_soc_codec *soc_find_codec(const struct device_node *codec_of_n
 }
 
 static struct snd_soc_dai *soc_find_codec_dai(struct snd_soc_codec *codec,
-					      const char *codec_dai_name)
+					      struct snd_soc_dai_link *dai_link)
 {
 	struct snd_soc_dai *codec_dai;
 
 	list_for_each_entry(codec_dai, &codec->component.dai_list, list) {
-		if (!strcmp(codec_dai->name, codec_dai_name)) {
+		if (dai_link->codec_dai_name
+		    && !strcmp(codec_dai->name, dai_link->codec_dai_name))
+			return codec_dai;
+		else if (codec_dai->id == dai_link->codec_dai_id)
 			return codec_dai;
-		}
 	}
 
 	return NULL;
@@ -904,6 +906,9 @@ static int soc_bind_dai_link(struct snd_soc_card *card, int num)
 			if (dai_link->cpu_dai_name &&
 				strcmp(cpu_dai->name, dai_link->cpu_dai_name))
 				continue;
+			if (!dai_link->cpu_dai_name &&
+				cpu_dai->id != dai_link->cpu_dai_id)
+				continue;
 
 			rtd->cpu_dai = cpu_dai;
 		}
@@ -919,17 +924,26 @@ static int soc_bind_dai_link(struct snd_soc_card *card, int num)
 	rtd->codec = soc_find_codec(dai_link->codec_of_node,
 				    dai_link->codec_name);
 	if (!rtd->codec) {
-		dev_err(card->dev, "ASoC: CODEC %s not registered\n",
-			dai_link->codec_name);
+		if (dai_link->codec_name)
+			dev_err(card->dev, "ASoC: CODEC %s not registered\n",
+				dai_link->codec_name);
+		else if (dai_link->codec_of_node)
+			dev_err(card->dev, "ASoC: CODEC %s not registered\n",
+				dai_link->codec_of_node->name);
 		return -EPROBE_DEFER;
 	}
 
 	/* Find CODEC DAI from registered list */
-	rtd->codec_dai = soc_find_codec_dai(rtd->codec,
-					    dai_link->codec_dai_name);
+	rtd->codec_dai = soc_find_codec_dai(rtd->codec, dai_link);
 	if (!rtd->codec_dai) {
-		dev_err(card->dev, "ASoC: CODEC DAI %s not registered\n",
-			dai_link->codec_dai_name);
+		if (dai_link->codec_dai_name)
+			dev_err(card->dev,
+				"ASoC: CODEC DAI %s not registered\n",
+				dai_link->codec_dai_name);
+		else
+			dev_err(card->dev,
+				"ASoC: CODEC DAI %d not registered\n",
+				dai_link->codec_dai_id);
 		return -EPROBE_DEFER;
 	}
 
-- 
1.7.9.5

Re: [PATCH] *** ASoC: core: refine of node id parse of cpu/codec dai ***

[Lars-Peter Clausen]

On 06/18/2014 01:01 PM, Qiao Zhou wrote:
> Hi Mark, Liam
>
> This patch is to add another check besides cpu_dai/codec_dai name during
> dai_link bind. currently if the cpu_dai/codec_dai name match corresponding
> dai_link cpu/codec name, then a match is found. in this patch, it also checks
> whether cpu/codec dai id match dailink cpu_dai_id/codec_dai_id. Still check
> name first.
> 1. if it doesn't match, it will keep checking whether cpu_id/codec_id match
> corresponding dai_link cpu_dai_id/codec_dai_id. if the ids are equal, then
> a match is found.
> 2. if it does match, then a match is already found. no need to further check.
>
[...]
> Could you help share your opinions? Thanks in advance.

Hi,

There is already snd_soc_of_get_dai_name() which will translate a phandle + 
specifier to a DAI name. By default it will use the DAI id for the 
specifier. Alternatively the driver can implement a of_xlate_dai_name 
callback that does the translation from specifier to name. The advantage of 
this approach is that the board driver does not need to know about the 
specific format of the DAI specifier.

- Lars

Re: [PATCH] *** ASoC: core: refine of node id parse of cpu/codec dai ***

[Qiao Zhou]

On 06/18/2014 08:13 PM, Lars-Peter Clausen wrote:
> On 06/18/2014 01:01 PM, Qiao Zhou wrote:
>> Hi Mark, Liam
>>
>> This patch is to add another check besides cpu_dai/codec_dai name during
>> dai_link bind. currently if the cpu_dai/codec_dai name match corresponding
>> dai_link cpu/codec name, then a match is found. in this patch, it also checks
>> whether cpu/codec dai id match dailink cpu_dai_id/codec_dai_id. Still check
>> name first.
>> 1. if it doesn't match, it will keep checking whether cpu_id/codec_id match
>> corresponding dai_link cpu_dai_id/codec_dai_id. if the ids are equal, then
>> a match is found.
>> 2. if it does match, then a match is already found. no need to further check.
>>
> [...]
>> Could you help share your opinions? Thanks in advance.
>
> Hi,
>
> There is already snd_soc_of_get_dai_name() which will translate a phandle +
> specifier to a DAI name. By default it will use the DAI id for the
> specifier. Alternatively the driver can implement a of_xlate_dai_name
> callback that does the translation from specifier to name. The advantage of
> this approach is that the board driver does not need to know about the
> specific format of the DAI specifier.
>
> - Lars
>
Hi Lars,

This API is powerful and meets my requirement. Thanks a lot.

I still have a small question. why don't we consider to also use DAI id 
to match DAIs & dai_link? It seems to be a more direct alternative. 
Please correct/instruct me if anything is wrong.

-- 

Best Regards
Qiao

Re: [PATCH] *** ASoC: core: refine of node id parse of cpu/codec dai ***

[Lars-Peter Clausen]

On 06/19/2014 03:10 AM, Qiao Zhou wrote:
> On 06/18/2014 08:13 PM, Lars-Peter Clausen wrote:
>> On 06/18/2014 01:01 PM, Qiao Zhou wrote:
>>> Hi Mark, Liam
>>>
>>> This patch is to add another check besides cpu_dai/codec_dai name during
>>> dai_link bind. currently if the cpu_dai/codec_dai name match corresponding
>>> dai_link cpu/codec name, then a match is found. in this patch, it also
>>> checks
>>> whether cpu/codec dai id match dailink cpu_dai_id/codec_dai_id. Still check
>>> name first.
>>> 1. if it doesn't match, it will keep checking whether cpu_id/codec_id match
>>> corresponding dai_link cpu_dai_id/codec_dai_id. if the ids are equal, then
>>> a match is found.
>>> 2. if it does match, then a match is already found. no need to further
>>> check.
>>>
>> [...]
>>> Could you help share your opinions? Thanks in advance.
>>
>> Hi,
>>
>> There is already snd_soc_of_get_dai_name() which will translate a phandle +
>> specifier to a DAI name. By default it will use the DAI id for the
>> specifier. Alternatively the driver can implement a of_xlate_dai_name
>> callback that does the translation from specifier to name. The advantage of
>> this approach is that the board driver does not need to know about the
>> specific format of the DAI specifier.
>>
>> - Lars
>>
> Hi Lars,
>
> This API is powerful and meets my requirement. Thanks a lot.
>
> I still have a small question. why don't we consider to also use DAI id to
> match DAIs & dai_link? It seems to be a more direct alternative. Please
> correct/instruct me if anything is wrong.

Matching by id would also work, I guess. But it doesn't really matter if 
name or id is used since both should be unique per CODEC. But what you 
shouldn't do is manually parse the id from the devicetree property in the 
machine driver since the specifier layout is CODEC specific and hence has to 
be parsed by the CODEC.

I think ideally the machine driver would pass the devicetree node and the 
name of the property that specifies the DAI to the core and the core would 
take care of everything else. As it is right now we first lookup the DAI by 
the of node and return the name of the DAI, then later on we use that name 
to lookup the DAI again. The second lookup is kind of superfluous since we 
already had a pointer to the DAI during the first lookup, so this is 
something that could be optimized if somebody was interested in optimizing this.

- Lars

Re: [PATCH] *** ASoC: core: refine of node id parse of cpu/codec dai ***

[Qiao Zhou]

On 06/19/2014 01:29 PM, Lars-Peter Clausen wrote:
> On 06/19/2014 03:10 AM, Qiao Zhou wrote:
>> On 06/18/2014 08:13 PM, Lars-Peter Clausen wrote:
>>> On 06/18/2014 01:01 PM, Qiao Zhou wrote:
>>>> Hi Mark, Liam
>>>>
>>>> This patch is to add another check besides cpu_dai/codec_dai name during
>>>> dai_link bind. currently if the cpu_dai/codec_dai name match corresponding
>>>> dai_link cpu/codec name, then a match is found. in this patch, it also
>>>> checks
>>>> whether cpu/codec dai id match dailink cpu_dai_id/codec_dai_id. Still check
>>>> name first.
>>>> 1. if it doesn't match, it will keep checking whether cpu_id/codec_id match
>>>> corresponding dai_link cpu_dai_id/codec_dai_id. if the ids are equal, then
>>>> a match is found.
>>>> 2. if it does match, then a match is already found. no need to further
>>>> check.
>>>>
>>> [...]
>>>> Could you help share your opinions? Thanks in advance.
>>>
>>> Hi,
>>>
>>> There is already snd_soc_of_get_dai_name() which will translate a phandle +
>>> specifier to a DAI name. By default it will use the DAI id for the
>>> specifier. Alternatively the driver can implement a of_xlate_dai_name
>>> callback that does the translation from specifier to name. The advantage of
>>> this approach is that the board driver does not need to know about the
>>> specific format of the DAI specifier.
>>>
>>> - Lars
>>>
>> Hi Lars,
>>
>> This API is powerful and meets my requirement. Thanks a lot.
>>
>> I still have a small question. why don't we consider to also use DAI id to
>> match DAIs & dai_link? It seems to be a more direct alternative. Please
>> correct/instruct me if anything is wrong.
>
> Matching by id would also work, I guess. But it doesn't really matter if
> name or id is used since both should be unique per CODEC. But what you
> shouldn't do is manually parse the id from the devicetree property in the
> machine driver since the specifier layout is CODEC specific and hence has to
> be parsed by the CODEC.
For DPCM, CPU dai of BE also needs to handle id matching specifically 
just as codec dai does. In this case, we need to also consider cpu dai 
id parsing, right?
>
> I think ideally the machine driver would pass the devicetree node and the
> name of the property that specifies the DAI to the core and the core would
> take care of everything else. As it is right now we first lookup the DAI by
Totally agree. A device node & a specifier together should be enough.
> the of node and return the name of the DAI, then later on we use that name
> to lookup the DAI again. The second lookup is kind of superfluous since we
> already had a pointer to the DAI during the first lookup, so this is
> something that could be optimized if somebody was interested in optimizing this.
>
> - Lars
>


-- 

Best Regards
Qiao