* [PATCH 2/4] ASoC: simple-card: dynamically allocate the DAI link array
2014-02-21 15:12 [PATCH 0/4] ASoC: simple-card: DT fix and multi DAI links extension Jean-Francois Moine
@ 2014-02-19 18:07 ` Jean-Francois Moine
2014-02-24 3:32 ` Li.Xiubo
2014-02-20 17:25 ` [PATCH 1/4] ASoC: simple-card: Fix device node locks Jean-Francois Moine
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Jean-Francois Moine @ 2014-02-19 18:07 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel, Kuninori Morimoto, linux-kernel, Xiubo Li
The DAI link array is hard-coded as a single CPU / CODEC DAIs link.
This patch allocates this array with the card definition and facilitates
handling more links.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
sound/soc/generic/simple-card.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 8809ab4..a75a8bb 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -20,7 +20,6 @@ struct simple_card_data {
unsigned int daifmt;
struct asoc_simple_dai cpu_dai;
struct asoc_simple_dai codec_dai;
- struct snd_soc_dai_link snd_link;
};
static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
@@ -246,7 +245,9 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
int ret;
- priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ priv = devm_kzalloc(dev,
+ sizeof(*priv) + sizeof(*dai_link),
+ GFP_KERNEL);
if (!priv)
return -ENOMEM;
@@ -255,7 +256,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
*/
priv->snd_card.owner = THIS_MODULE;
priv->snd_card.dev = dev;
- dai_link = &priv->snd_link;
+ dai_link = (struct snd_soc_dai_link *) (priv + 1);
priv->snd_card.dai_link = dai_link;
priv->snd_card.num_links = 1;
--
1.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] ASoC: simple-card: dynamically allocate the DAI link array
2014-02-19 18:07 ` [PATCH 2/4] ASoC: simple-card: dynamically allocate the DAI link array Jean-Francois Moine
@ 2014-02-24 3:32 ` Li.Xiubo
2014-02-25 8:02 ` [alsa-devel] " Jean-Francois Moine
0 siblings, 1 reply; 10+ messages in thread
From: Li.Xiubo @ 2014-02-24 3:32 UTC (permalink / raw)
To: Jean-Francois Moine, Mark Brown
Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
Kuninori Morimoto
> @@ -20,7 +20,6 @@ struct simple_card_data {
> unsigned int daifmt;
> struct asoc_simple_dai cpu_dai;
> struct asoc_simple_dai codec_dai;
> - struct snd_soc_dai_link snd_link;
> };
>
> static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
> @@ -246,7 +245,9 @@ static int asoc_simple_card_probe(struct platform_device
> *pdev)
> struct device *dev = &pdev->dev;
> int ret;
>
> - priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + priv = devm_kzalloc(dev,
> + sizeof(*priv) + sizeof(*dai_link),
This is okey for me.
Well, how about splitting the *priv and *dai_link into two separated
memory blocks? As we can get the dai-link pointer via priv->snd_card.dai_link
in other places.
IMHO, then the code will be much more simplifier and readable.
Just for one suggestion.
Thanks,
--
Best regards,
Xiubo
> + GFP_KERNEL);
> if (!priv)
> return -ENOMEM;
>
> @@ -255,7 +256,7 @@ static int asoc_simple_card_probe(struct platform_device
> *pdev)
> */
> priv->snd_card.owner = THIS_MODULE;
> priv->snd_card.dev = dev;
> - dai_link = &priv->snd_link;
> + dai_link = (struct snd_soc_dai_link *) (priv + 1);
> priv->snd_card.dai_link = dai_link;
> priv->snd_card.num_links = 1;
>
> --
> 1.9.0
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [alsa-devel] [PATCH 2/4] ASoC: simple-card: dynamically allocate the DAI link array
2014-02-24 3:32 ` Li.Xiubo
@ 2014-02-25 8:02 ` Jean-Francois Moine
0 siblings, 0 replies; 10+ messages in thread
From: Jean-Francois Moine @ 2014-02-25 8:02 UTC (permalink / raw)
To: Li.Xiubo@freescale.com
Cc: Mark Brown, alsa-devel@alsa-project.org,
linux-kernel@vger.kernel.org, Kuninori Morimoto
On Mon, 24 Feb 2014 03:32:02 +0000
"Li.Xiubo@freescale.com" <Li.Xiubo@freescale.com> wrote:
>
> > @@ -20,7 +20,6 @@ struct simple_card_data {
> > unsigned int daifmt;
> > struct asoc_simple_dai cpu_dai;
> > struct asoc_simple_dai codec_dai;
> > - struct snd_soc_dai_link snd_link;
> > };
> >
> > static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
> > @@ -246,7 +245,9 @@ static int asoc_simple_card_probe(struct platform_device
> > *pdev)
> > struct device *dev = &pdev->dev;
> > int ret;
> >
> > - priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + priv = devm_kzalloc(dev,
> > + sizeof(*priv) + sizeof(*dai_link),
>
> This is okey for me.
>
> Well, how about splitting the *priv and *dai_link into two separated
> memory blocks? As we can get the dai-link pointer via priv->snd_card.dai_link
> in other places.
>
> IMHO, then the code will be much more simplifier and readable.
It is just a simple optimization: less calls to memory allocation and
less code (also, less TLB reload?). I will add more comments.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] ASoC: simple-card: Fix device node locks
2014-02-21 15:12 [PATCH 0/4] ASoC: simple-card: DT fix and multi DAI links extension Jean-Francois Moine
2014-02-19 18:07 ` [PATCH 2/4] ASoC: simple-card: dynamically allocate the DAI link array Jean-Francois Moine
@ 2014-02-20 17:25 ` Jean-Francois Moine
[not found] ` <4dca81f45b67a4dcb21271e57409ba114c3b59cb.1392995566.git.moinejf-GANU6spQydw@public.gmane.org>
2014-02-21 8:56 ` [PATCH 3/4] ASoC: simple-card: accept many DAI links Jean-Francois Moine
2014-02-21 11:43 ` [PATCH 4/4] ASoC: simple-card: add DT documentation for multi-DAI links Jean-Francois Moine
3 siblings, 1 reply; 10+ messages in thread
From: Jean-Francois Moine @ 2014-02-20 17:25 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, Kuninori Morimoto, linux-kernel, Xiubo Li, devicetree
Some device nodes stay locked and some other ones are not locked
while being used during the card lifetime.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
sound/soc/generic/simple-card.c | 51 +++++++++++++++++++++++++++++++----------
1 file changed, 39 insertions(+), 12 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 4fe8abc..8809ab4 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -92,7 +92,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
/* get dai->name */
ret = snd_soc_of_get_dai_name(np, name);
if (ret < 0)
- goto parse_error;
+ return ret;
/*
* bitclock-inversion, frame-inversion
@@ -112,7 +112,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
clk = of_clk_get(np, 0);
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
- goto parse_error;
+ return ret;
}
dai->sysclk = clk_get_rate(clk);
@@ -126,12 +126,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
dai->sysclk = clk_get_rate(clk);
}
- ret = 0;
-
-parse_error:
- of_node_put(node);
-
- return ret;
+ return 0;
}
static int asoc_simple_card_parse_of(struct device_node *node,
@@ -169,22 +164,26 @@ static int asoc_simple_card_parse_of(struct device_node *node,
/* CPU sub-node */
ret = -EINVAL;
np = of_get_child_by_name(node, "simple-audio-card,cpu");
- if (np)
+ if (np) {
ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
&priv->cpu_dai,
&dai_link->cpu_of_node,
&dai_link->cpu_dai_name);
+ of_node_put(np);
+ }
if (ret < 0)
return ret;
/* CODEC sub-node */
ret = -EINVAL;
np = of_get_child_by_name(node, "simple-audio-card,codec");
- if (np)
+ if (np) {
ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
&priv->codec_dai,
&dai_link->codec_of_node,
&dai_link->codec_dai_name);
+ of_node_put(np);
+ }
if (ret < 0)
return ret;
@@ -219,6 +218,26 @@ static int asoc_simple_card_parse_of(struct device_node *node,
return 0;
}
+static int asoc_simple_card_purge(struct platform_device *pdev)
+{
+ struct snd_soc_card *card = platform_get_drvdata(pdev);
+ struct snd_soc_dai_link *dai_link;
+ struct device_node *np;
+ int num_links;
+
+ for (num_links = 0, dai_link = card->dai_link;
+ num_links < card->num_links;
+ num_links++, dai_link++) {
+ np = (struct device_node *) dai_link->cpu_of_node;
+ if (np)
+ of_node_put(np);
+ np = (struct device_node *) dai_link->codec_of_node;
+ if (np)
+ of_node_put(np);
+ }
+ return 0;
+}
+
static int asoc_simple_card_probe(struct platform_device *pdev)
{
struct simple_card_data *priv;
@@ -246,7 +265,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
if (ret < 0) {
if (ret != -EPROBE_DEFER)
dev_err(dev, "parse error %d\n", ret);
- return ret;
+ goto err;
}
} else {
struct asoc_simple_card_info *cinfo;
@@ -287,7 +306,14 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
snd_soc_card_set_drvdata(&priv->snd_card, priv);
- return devm_snd_soc_register_card(&pdev->dev, &priv->snd_card);
+ ret = devm_snd_soc_register_card(&pdev->dev, &priv->snd_card);
+ if (ret < 0)
+ goto err;
+ return 0;
+
+err:
+ asoc_simple_card_purge(pdev);
+ return ret;
}
static const struct of_device_id asoc_simple_of_match[] = {
@@ -303,6 +329,7 @@ static struct platform_driver asoc_simple_card = {
.of_match_table = asoc_simple_of_match,
},
.probe = asoc_simple_card_probe,
+ .remove = asoc_simple_card_purge,
};
module_platform_driver(asoc_simple_card);
--
1.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/4] ASoC: simple-card: accept many DAI links
2014-02-21 15:12 [PATCH 0/4] ASoC: simple-card: DT fix and multi DAI links extension Jean-Francois Moine
2014-02-19 18:07 ` [PATCH 2/4] ASoC: simple-card: dynamically allocate the DAI link array Jean-Francois Moine
2014-02-20 17:25 ` [PATCH 1/4] ASoC: simple-card: Fix device node locks Jean-Francois Moine
@ 2014-02-21 8:56 ` Jean-Francois Moine
2014-02-21 11:43 ` [PATCH 4/4] ASoC: simple-card: add DT documentation for multi-DAI links Jean-Francois Moine
3 siblings, 0 replies; 10+ messages in thread
From: Jean-Francois Moine @ 2014-02-21 8:56 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel, Kuninori Morimoto, linux-kernel, Xiubo Li
Some simple audio cards may have many DAI links.
This patch makes them handled by the simple-card driver.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
sound/soc/generic/simple-card.c | 100 ++++++++++++++++++++++++++--------------
1 file changed, 66 insertions(+), 34 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index a75a8bb..6e0e580 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -93,6 +93,9 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
if (ret < 0)
return ret;
+ if (!dai)
+ return 0;
+
/*
* bitclock-inversion, frame-inversion
* bitclock-master, frame-master
@@ -135,7 +138,7 @@ static int asoc_simple_card_parse_of(struct device_node *node,
struct snd_soc_dai_link *dai_link = priv->snd_card.dai_link;
struct device_node *np;
char *name;
- int ret;
+ int first_link, ret;
/* parsing the card name from DT */
snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
@@ -160,50 +163,67 @@ static int asoc_simple_card_parse_of(struct device_node *node,
return ret;
}
- /* CPU sub-node */
- ret = -EINVAL;
- np = of_get_child_by_name(node, "simple-audio-card,cpu");
- if (np) {
+ /* loop on the DAI links */
+ np = NULL;
+ first_link = 1;
+ for (;;) {
+ np = of_get_next_child(node, np);
+ if (!np)
+ break;
+
+ /* CPU sub-node */
+ if (strcmp(np->name, "simple-audio-card,cpu") != 0) {
+ dev_err(dev, "Bad CPU DAI\n");
+ of_node_put(np);
+ return -EINVAL;
+ }
ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
- &priv->cpu_dai,
+ first_link ? &priv->cpu_dai : NULL,
&dai_link->cpu_of_node,
&dai_link->cpu_dai_name);
of_node_put(np);
- }
- if (ret < 0)
- return ret;
+ if (ret < 0)
+ return ret;
- /* CODEC sub-node */
- ret = -EINVAL;
- np = of_get_child_by_name(node, "simple-audio-card,codec");
- if (np) {
+ /* CODEC sub-node */
+ np = of_get_next_child(node, np);
+ if (strcmp(np->name, "simple-audio-card,codec") != 0) {
+ dev_err(dev, "Bad CODEC DAI\n");
+ of_node_put(np);
+ return -EINVAL;
+ }
ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
- &priv->codec_dai,
+ first_link ? &priv->codec_dai : NULL,
&dai_link->codec_of_node,
&dai_link->codec_dai_name);
of_node_put(np);
- }
- if (ret < 0)
- return ret;
+ if (ret < 0)
+ return ret;
+
+ if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name)
+ return -EINVAL;
- if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name)
- return -EINVAL;
+ /* simple-card assumes platform == cpu */
+ dai_link->platform_of_node = dai_link->cpu_of_node;
+
+ name = devm_kzalloc(dev,
+ strlen(dai_link->cpu_dai_name) +
+ strlen(dai_link->codec_dai_name) + 2,
+ GFP_KERNEL);
+ sprintf(name, "%s-%s", dai_link->cpu_dai_name,
+ dai_link->codec_dai_name);
+ dai_link->name = dai_link->stream_name = name;
+
+ dai_link++;
+ first_link = 0;
+ }
/* card name is created from CPU/CODEC dai name */
- name = devm_kzalloc(dev,
- strlen(dai_link->cpu_dai_name) +
- strlen(dai_link->codec_dai_name) + 2,
- GFP_KERNEL);
- sprintf(name, "%s-%s", dai_link->cpu_dai_name,
- dai_link->codec_dai_name);
+ dai_link = priv->snd_card.dai_link;
if (!priv->snd_card.name)
- priv->snd_card.name = name;
- dai_link->name = dai_link->stream_name = name;
-
- /* simple-card assumes platform == cpu */
- dai_link->platform_of_node = dai_link->cpu_of_node;
+ priv->snd_card.name = dai_link->name;
- dev_dbg(dev, "card-name : %s\n", name);
+ dev_dbg(dev, "card-name : %s\n", priv->snd_card.name);
dev_dbg(dev, "platform : %04x\n", priv->daifmt);
dev_dbg(dev, "cpu : %s / %04x / %d\n",
dai_link->cpu_dai_name,
@@ -243,10 +263,22 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
struct snd_soc_dai_link *dai_link;
struct device_node *np = pdev->dev.of_node;
struct device *dev = &pdev->dev;
- int ret;
+ int num_links, ret;
+
+ /* get the number of DAI links */
+ if (np) {
+ num_links = of_get_child_count(np);
+ if (num_links == 0 || (num_links & 1)) {
+ dev_err(&pdev->dev, "Bad number of DAI links\n");
+ return -EINVAL;
+ }
+ num_links /= 2;
+ } else {
+ num_links = 1;
+ }
priv = devm_kzalloc(dev,
- sizeof(*priv) + sizeof(*dai_link),
+ sizeof(*priv) + sizeof(*dai_link) * num_links,
GFP_KERNEL);
if (!priv)
return -ENOMEM;
@@ -258,7 +290,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
priv->snd_card.dev = dev;
dai_link = (struct snd_soc_dai_link *) (priv + 1);
priv->snd_card.dai_link = dai_link;
- priv->snd_card.num_links = 1;
+ priv->snd_card.num_links = num_links;
if (np && of_device_is_available(np)) {
--
1.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] ASoC: simple-card: add DT documentation for multi-DAI links
2014-02-21 15:12 [PATCH 0/4] ASoC: simple-card: DT fix and multi DAI links extension Jean-Francois Moine
` (2 preceding siblings ...)
2014-02-21 8:56 ` [PATCH 3/4] ASoC: simple-card: accept many DAI links Jean-Francois Moine
@ 2014-02-21 11:43 ` Jean-Francois Moine
3 siblings, 0 replies; 10+ messages in thread
From: Jean-Francois Moine @ 2014-02-21 11:43 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, Kuninori Morimoto, linux-kernel, Xiubo Li, devicetree
There may be many couples of CPU/CODEC DAI links.
The example 2 is extracted from the Cubox DT.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
.../devicetree/bindings/sound/simple-card.txt | 34 +++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index 0527358..60306bf 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -24,6 +24,9 @@ Required subnodes:
- simple-audio-card,cpu : CPU sub-node
- simple-audio-card,codec : CODEC sub-node
+ There may be one or many couples (simple-audio-card,cpu, simple-audio-card,codec)
+ (see example 2).
+
Required CPU/CODEC subnodes properties:
- sound-dai : phandle and port of CPU/CODEC
@@ -41,7 +44,7 @@ Optional CPU/CODEC subnodes properties:
clock node (= common clock), or "system-clock-frequency"
(if system doens't support common clock)
-Example:
+Example 1:
sound {
compatible = "simple-audio-card";
@@ -83,3 +86,32 @@ sh_fsi2: sh_fsi2@ec230000 {
interrupt-parent = <&gic>;
interrupts = <0 146 0x4>;
};
+
+Example 2:
+
+sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,name = "Cubox Audio";
+
+ simple-audio-card,cpu@0 { /* I2S - HDMI */
+ sound-dai = <&audio1 0>;
+ format = "i2s";
+ };
+ simple-audio-card,codec@0 {
+ sound-dai = <&tda998x 0>;
+ };
+
+ simple-audio-card,cpu@1 { /* S/PDIF - HDMI */
+ sound-dai = <&audio1 1>;
+ };
+ simple-audio-card,codec@1 {
+ sound-dai = <&tda998x 1>;
+ };
+
+ simple-audio-card,cpu@2 { /* S/PDIF - S/PDIF */
+ sound-dai = <&audio1 1>;
+ };
+ simple-audio-card,codec@2 {
+ sound-dai = <&spdif_codec>;
+ };
+};
--
1.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread