Re: [alsa-devel] [PATCH] ASoC:simple-card: Add multi-CODEC support
Hi Jean-Francois,
On 10/09/2014 13:28, Jean-Francois Moine wrote:
This patch adds multi-CODEC support to the simple-card.
Signed-off-by: Jean-Francois Moine moinejf@free.fr
.../devicetree/bindings/sound/simple-card.txt | 28 +++++++-------- sound/soc/generic/simple-card.c | 41 +++++++++++++++++++++- 2 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index c2e9841..c217687 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -37,6 +37,8 @@ Required dai-link subnodes:
- cpu : CPU sub-node
- codec : CODEC sub-node
In case of multi-CODECs, there maybe many of such sub-nodes.Optional dai-link subnode properties:
@@ -115,37 +117,31 @@ sh_fsi2: sh_fsi2@ec230000 { interrupts = <0 146 0x4>; };
-Example 2 - many DAI links: +Example 2 - many DAI links and multi-CODECs:
sound { compatible = "simple-audio-card"; simple-audio-card,name = "Cubox Audio";
- simple-audio-card,dai-link@0 { /* I2S - HDMI */
- simple-audio-card,dai-link@0 { /* S/PDIF - HDMI & S/PDIF */ format = "i2s"; cpu {
sound-dai = <&audio1 0>;};codec {sound-dai = <&tda998x 0>;};- };
- simple-audio-card,dai-link@1 { /* S/PDIF - HDMI */
cpu { sound-dai = <&audio1 1>;codec {sound-dai = <&tda998x 1>;
codec@0 {sound-dai = <&hdmi 0>;};codec@1 {sound-dai = <&spdif_codec>;
I don't have strong opinion on that, but in my case, I was considering using a simple list instead of several nodes. I don't like having to add fake address just to ensure uniqueness.
Something like that:
sound-dais = <&spdif_codec 1>, <&hdmi 0>;
That being said, it will require changing the name with a plural form, and ensuring we have the same number of parameters for each codec.
That was just my .2 cents.
Regards, Benoit
};
- simple-audio-card,dai-link@2 { /* S/PDIF - S/PDIF */
- simple-audio-card,dai-link@1 { /* I2S - HDMI */ cpu {
sound-dai = <&audio1 1>;
sound-dai = <&audio1 0>;
sound-dai = <&spdif_codec>;
sound-dai = <&hdmi 1>;diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 4053152..bf0ce08 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -179,11 +179,12 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, struct device_node *np = NULL; struct device_node *bitclkmaster = NULL; struct device_node *framemaster = NULL;
- struct snd_soc_dai_link_component *component; unsigned int daifmt; char *name; char prop[128]; char *prefix = "";
- int ret, cpu_args;
int ret, cpu_args, num_codec_dais;
/* For single DAI link & old style of DT node */ if (is_top_level_node)
@@ -225,7 +226,16 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, }
of_node_put(np);
- /* count the number of codec DAIs */ snprintf(prop, sizeof(prop), "%scodec", prefix);
- num_codec_dais = 0;
- for_each_child_of_node(node, np) {
if (strcmp(np->name, prop) == 0)num_codec_dais++;- }
- /* treat the first DAI */ np = of_get_child_by_name(node, prop); if (!np) { ret = -EINVAL;
@@ -307,6 +317,35 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, if (!cpu_args) dai_link->cpu_dai_name = NULL;
- /* handle multi-codec DAIs */
- if (num_codec_dais == 1)
goto out;- dai_link->codecs = component =
devm_kzalloc(dev,sizeof *component * num_codec_dais,GFP_KERNEL);- dai_link->num_codecs = num_codec_dais;
- component->of_node = dai_link->codec_of_node;
- dai_link->codec_of_node = NULL;
- component->dai_name = dai_link->codec_dai_name;
- dai_link->codec_dai_name = NULL;
- for (;;) {
np = of_get_next_child(node, np);if (!np)break;component++;component->of_node = of_parse_phandle(np, "sound-dai", 0);if (!component->of_node) {ret = -ENODEV;dev_err(dev, "Bad sound-dai\n");goto dai_link_of_err;}ret = snd_soc_of_get_dai_name(np, &component->dai_name);if (ret < 0)goto dai_link_of_err;- }
+out: dai_link_of_err: if (np) of_node_put(np);
On Wed, 10 Sep 2014 13:43:06 +0200 Benoit Cousson bcousson@baylibre.com wrote:
codec@0 {sound-dai = <&hdmi 0>;};codec@1 {sound-dai = <&spdif_codec>;I don't have strong opinion on that, but in my case, I was considering using a simple list instead of several nodes. I don't like having to add fake address just to ensure uniqueness.
Something like that:
sound-dais = <&spdif_codec 1>, <&hdmi 0>;
You are right, this would be simpler (I did not see that there could be phandle's with different cell sizes in a list).
That being said, it will require changing the name with a plural form, and ensuring we have the same number of parameters for each codec.
But I don't think that changing the name could be useful: the treatment is exactly the same and, as a result, all CODEC DAIs go to the DAI list of the DAI link.
On 10/09/2014 18:06, Jean-Francois Moine wrote:
On Wed, 10 Sep 2014 13:43:06 +0200 Benoit Cousson bcousson@baylibre.com wrote:
codec@0 {sound-dai = <&hdmi 0>;};codec@1 {sound-dai = <&spdif_codec>; };I don't have strong opinion on that, but in my case, I was considering using a simple list instead of several nodes. I don't like having to add fake address just to ensure uniqueness.
Something like that:
sound-dais = <&spdif_codec 1>, <&hdmi 0>;
You are right, this would be simpler (I did not see that there could be phandle's with different cell sizes in a list).
In fact you cannot, that why I added the same number of parameters for both.
For DT, it is just a list of elements, it could be encoded like that: <&spdif_codec 1 &hdmi 0>;
At the end it will be a list of u32 inside the dtb.
BTW, what is that extra parameter after the phandle? I cannot find any reference of that inside the Documentation.
If we need to have a random number of parameters after the code-dai phandle, we couldn't use that list.
That being said, it will require changing the name with a plural form, and ensuring we have the same number of parameters for each codec.
But I don't think that changing the name could be useful: the treatment is exactly the same and, as a result, all CODEC DAIs go to the DAI list of the DAI link.
Yeah, maybe not that useful, but that was just to be aligned with similar cases in DT bindings. Like gpios, interrupts...
Regards, Benoit
On Wed, 10 Sep 2014 18:27:26 +0200 Benoit Cousson bcousson@baylibre.com wrote:
I don't have strong opinion on that, but in my case, I was considering using a simple list instead of several nodes. I don't like having to add fake address just to ensure uniqueness.
Something like that:
sound-dais = <&spdif_codec 1>, <&hdmi 0>;
You are right, this would be simpler (I did not see that there could be phandle's with different cell sizes in a list).
In fact you cannot, that why I added the same number of parameters for both.
For DT, it is just a list of elements, it could be encoded like that: <&spdif_codec 1 &hdmi 0>;
At the end it will be a list of u32 inside the dtb.
Yes, but __of_parse_phandle_with_args() handles the number of cells of the phandle.
BTW, what is that extra parameter after the phandle? I cannot find any reference of that inside the Documentation.
It is the DAI number inside the CODEC. The TDA998x CODEC has two DAIs, S/PDIF and I2S.
If we need to have a random number of parameters after the code-dai phandle, we couldn't use that list.
That being said, it will require changing the name with a plural form, and ensuring we have the same number of parameters for each codec.
But I don't think that changing the name could be useful: the treatment is exactly the same and, as a result, all CODEC DAIs go to the DAI list of the DAI link.
Yeah, maybe not that useful, but that was just to be aligned with similar cases in DT bindings. Like gpios, interrupts...
Not easy! "sound-dai" is used for both the CPU and CODEC DAIs. Adding "sound-dais" for just the CODEC DAIs would complexify the simple-card logic...
participants (2)
-
Benoit Cousson -
Jean-Francois Moine