[alsa-devel] [PATCH v4 0/4] ASoC: simple-card: Add multi-CODEC support
This patch set adds multi-CODEC support to the simple card.
v4: (Mark Brown) - document the new core function - calculation of the number of codecs moved to the core function - split the patch about simple-card.c - less code changes - remove the cast of pointers - less changes in the DT binding example 2 v3: rebase on broonie git 2014/11/03 v2: accept list of CODECs in 'sound-dais' (Benoit Cousson)
Jean-Francois Moine (4): ASoC: core: add multi-codec support in DT ASoC: simple-card: Remove useless function argument ASoC: simple-card: add multi-CODECs in DT ASoC: simple-card: Remove useless check
.../devicetree/bindings/sound/simple-card.txt | 20 ++-- include/sound/soc.h | 3 + sound/soc/generic/simple-card.c | 87 ++++++++-------- sound/soc/soc-core.c | 113 ++++++++++++++++++--- 4 files changed, 154 insertions(+), 69 deletions(-)
SoC audio cards may have many CODECs per DAI link.
This patch exports a core function handling such links in the DT (as: "sound-dais = <&hdmi 0>, <&spdif_codec>;") and creating a CODEC component array in the DAI link.
Signed-off-by: Jean-Francois Moine moinejf@free.fr --- include/sound/soc.h | 3 ++ sound/soc/soc-core.c | 113 ++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 101 insertions(+), 15 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 405f967..0f4f895 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1451,6 +1451,9 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np, struct device_node **framemaster); int snd_soc_of_get_dai_name(struct device_node *of_node, const char **dai_name); +int snd_soc_of_get_dai_link_codecs(struct device *dev, + struct device_node *of_node, + struct snd_soc_dai_link *dai_link);
#include <sound/soc-dai.h>
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 3818cf3..65771a9 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3724,36 +3724,30 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np, } EXPORT_SYMBOL_GPL(snd_soc_of_parse_daifmt);
-int snd_soc_of_get_dai_name(struct device_node *of_node, - const char **dai_name) +static int snd_soc_get_dai_name(struct of_phandle_args *args, + const char **dai_name) { struct snd_soc_component *pos; - struct of_phandle_args args; - int ret; - - ret = of_parse_phandle_with_args(of_node, "sound-dai", - "#sound-dai-cells", 0, &args); - if (ret) - return ret; - - ret = -EPROBE_DEFER; + int ret = -EPROBE_DEFER;
mutex_lock(&client_mutex); list_for_each_entry(pos, &component_list, list) { - if (pos->dev->of_node != args.np) + if (pos->dev->of_node != args->np) continue;
if (pos->driver->of_xlate_dai_name) { - ret = pos->driver->of_xlate_dai_name(pos, &args, dai_name); + ret = pos->driver->of_xlate_dai_name(pos, + args, + dai_name); } else { int id = -1;
- switch (args.args_count) { + switch (args->args_count) { case 0: id = 0; /* same as dai_drv[0] */ break; case 1: - id = args.args[0]; + id = args->args[0]; break; default: /* not supported */ @@ -3775,6 +3769,21 @@ int snd_soc_of_get_dai_name(struct device_node *of_node, break; } mutex_unlock(&client_mutex); + return ret; +} + +int snd_soc_of_get_dai_name(struct device_node *of_node, + const char **dai_name) +{ + struct of_phandle_args args; + int ret; + + ret = of_parse_phandle_with_args(of_node, "sound-dai", + "#sound-dai-cells", 0, &args); + if (ret) + return ret; + + ret = snd_soc_get_dai_name(&args, dai_name);
of_node_put(args.np);
@@ -3782,6 +3791,80 @@ int snd_soc_of_get_dai_name(struct device_node *of_node, } EXPORT_SYMBOL_GPL(snd_soc_of_get_dai_name);
+/* + * snd_soc_of_get_dai_link_codecs - Parse a list of CODECs in the devicetree + * + * @dev: Card device + * @of_node: Device node + * @dai_link: DAI link + * + * Builds an array of CODEC DAI components from a DAI link property. + * The name of the property may be 'sound-dai' or 'sound-dais'. + * The array is set in the DAI link and the number of DAIs is set accordingly. + * The device nodes in the array (of_node) must be dereferenced by the caller. + * + * Returns 0 for success + */ +int snd_soc_of_get_dai_link_codecs(struct device *dev, + struct device_node *of_node, + struct snd_soc_dai_link *dai_link) +{ + struct of_phandle_args args; + struct snd_soc_dai_link_component *component; + char *name; + int index, num_codecs, ret; + + /* Count the number of CODECs */ + name = "sound-dai"; + num_codecs = of_count_phandle_with_args(of_node, name, + "#sound-dai-cells"); + if (num_codecs <= 0) { + name = "sound-dais"; + num_codecs = of_count_phandle_with_args(of_node, name, + "#sound-dai-cells"); + if (num_codecs <= 0) { + dev_err(dev, "No CODEC DAI phandle\n"); + return -EINVAL; + } + } + component = devm_kzalloc(dev, + sizeof *component * num_codecs, + GFP_KERNEL); + if (!component) + return -ENOMEM; + dai_link->codecs = component; + dai_link->num_codecs = num_codecs; + + /* Parse the list */ + for (index = 0, component = dai_link->codecs; + index < dai_link->num_codecs; + index++, component++) { + ret = of_parse_phandle_with_args(of_node, name, + "#sound-dai-cells", + index, &args); + if (ret) + goto err; + component->of_node = args.np; + ret = snd_soc_get_dai_name(&args, &component->dai_name); + if (ret < 0) + goto err; + } + return 0; +err: + for (index = 0, component = dai_link->codecs; + index < dai_link->num_codecs; + index++, component++) { + if (!component->of_node) + break; + of_node_put(component->of_node); + component->of_node = NULL; + } + dai_link->codecs = NULL; + dai_link->num_codecs = 0; + return ret; +} +EXPORT_SYMBOL_GPL(snd_soc_of_get_dai_link_codecs); + static int __init snd_soc_init(void) { #ifdef CONFIG_DEBUG_FS
The device node pointer 'cpu' is not used in the function asoc_simple_card_parse_daifmt().
Signed-off-by: Jean-Francois Moine moinejf@free.fr --- sound/soc/generic/simple-card.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 3e3fec4..ece22d5 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -232,7 +232,6 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
static int asoc_simple_card_parse_daifmt(struct device_node *node, struct simple_card_data *priv, - struct device_node *cpu, struct device_node *codec, char *prefix, int idx) { @@ -309,7 +308,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, }
ret = asoc_simple_card_parse_daifmt(node, priv, - cpu, codec, prefix, idx); + codec, prefix, idx); if (ret < 0) goto dai_link_of_err;
This patch allows many CODECs per link to be defined in the device tree.
Signed-off-by: Jean-Francois Moine moinejf@free.fr --- .../devicetree/bindings/sound/simple-card.txt | 20 ++---- sound/soc/generic/simple-card.c | 81 ++++++++++++---------- 2 files changed, 53 insertions(+), 48 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index c3cba60..debe35c 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -65,7 +65,10 @@ properties should also be placed in the codec node if needed.
Required CPU/CODEC subnodes properties:
-- sound-dai : phandle and port of CPU/CODEC +either + - sound-dai : phandle and port of CPU/CODEC +or + - sound-dais : list of phandle and port of CODECs
Optional CPU/CODEC subnodes properties:
@@ -119,7 +122,7 @@ 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"; @@ -135,21 +138,12 @@ sound { }; };
- simple-audio-card,dai-link@1 { /* S/PDIF - HDMI */ + simple-audio-card,dai-link@1 { /* S/PDIF - HDMI & S/PDIF */ cpu { sound-dai = <&audio1 1>; }; codec { - sound-dai = <&tda998x 1>; - }; - }; - - simple-audio-card,dai-link@2 { /* S/PDIF - S/PDIF */ - cpu { - sound-dai = <&audio1 1>; - }; - codec { - sound-dai = <&spdif_codec>; + sound-dais = <&tda998x 1>, <&spdif_codec>; }; }; }; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index ece22d5..61f9500 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -172,34 +172,12 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd) static int asoc_simple_card_sub_parse_of(struct device_node *np, struct asoc_simple_dai *dai, - struct device_node **p_node, - const char **name, - int *args_count) + struct device_node *dai_np) { - struct of_phandle_args args; struct clk *clk; u32 val; int ret;
- /* - * Get node via "sound-dai = <&phandle port>" - * it will be used as xxx_of_node on soc_bind_dai_link() - */ - ret = of_parse_phandle_with_args(np, "sound-dai", - "#sound-dai-cells", 0, &args); - if (ret) - return ret; - - *p_node = args.np; - - if (args_count) - *args_count = args.args_count; - - /* Get dai->name */ - ret = snd_soc_of_get_dai_name(np, name); - if (ret < 0) - return ret; - /* Parse TDM slot */ ret = snd_soc_of_parse_tdm_slot(np, &dai->slots, &dai->slot_width); if (ret) @@ -222,7 +200,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np, } else if (!of_property_read_u32(np, "system-clock-frequency", &val)) { dai->sysclk = val; } else { - clk = of_clk_get(args.np, 0); + clk = of_clk_get(dai_np, 0); if (!IS_ERR(clk)) dai->sysclk = clk_get_rate(clk); } @@ -286,10 +264,12 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx); struct device_node *cpu = NULL; struct device_node *codec = NULL; + struct snd_soc_dai_link_component *component; + struct of_phandle_args args; char *name; char prop[128]; char *prefix = ""; - int ret, cpu_args; + int ret, cpu_args, i;
/* For single DAI link & old style of DT node */ if (is_top_level_node) @@ -312,16 +292,30 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, if (ret < 0) goto dai_link_of_err;
+ /* Get the CPU node and name */ + ret = of_parse_phandle_with_args(cpu, "sound-dai", + "#sound-dai-cells", 0, &args); + if (ret) + goto dai_link_of_err; + dai_link->cpu_of_node = args.np; + cpu_args = args.args_count; + + ret = snd_soc_of_get_dai_name(cpu, &dai_link->cpu_dai_name); + if (ret < 0) + goto dai_link_of_err; + ret = asoc_simple_card_sub_parse_of(cpu, &dai_props->cpu_dai, - &dai_link->cpu_of_node, - &dai_link->cpu_dai_name, - &cpu_args); + dai_link->cpu_of_node); + if (ret < 0) + goto dai_link_of_err; + + /* Get the node and name of the CODEC(s) */ + ret = snd_soc_of_get_dai_link_codecs(dev, codec, dai_link); if (ret < 0) goto dai_link_of_err;
ret = asoc_simple_card_sub_parse_of(codec, &dai_props->codec_dai, - &dai_link->codec_of_node, - &dai_link->codec_dai_name, NULL); + dai_link->codecs[0].of_node); if (ret < 0) goto dai_link_of_err;
@@ -336,10 +330,10 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, /* DAI link 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, + strlen(dai_link->codecs[0].dai_name) + 2, GFP_KERNEL); sprintf(name, "%s-%s", dai_link->cpu_dai_name, - dai_link->codec_dai_name); + dai_link->codecs[0].dai_name); dai_link->name = dai_link->stream_name = name; dai_link->ops = &asoc_simple_card_ops; dai_link->init = asoc_simple_card_dai_init; @@ -350,7 +344,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, dai_props->cpu_dai.fmt, dai_props->cpu_dai.sysclk); dev_dbg(dev, "\tcodec : %s / %04x / %d\n", - dai_link->codec_dai_name, + dai_link->codecs[0].dai_name, dai_props->codec_dai.fmt, dai_props->codec_dai.sysclk);
@@ -365,8 +359,18 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, */ if (!cpu_args) dai_link->cpu_dai_name = NULL; + goto out;
dai_link_of_err: + for (i = 0, component = dai_link->codecs; + i < dai_link->num_codecs; + i++, component++) { + if (!component->of_node) + break; + of_node_put(component->of_node); + component->of_node = NULL; + } +out: of_node_put(cpu); of_node_put(codec);
@@ -456,13 +460,20 @@ static int asoc_simple_card_unref(struct platform_device *pdev) { struct snd_soc_card *card = platform_get_drvdata(pdev); struct snd_soc_dai_link *dai_link; - int num_links; + struct snd_soc_dai_link_component *component; + int num_links, i;
for (num_links = 0, dai_link = card->dai_link; num_links < card->num_links; num_links++, dai_link++) { of_node_put(dai_link->cpu_of_node); - of_node_put(dai_link->codec_of_node); + for (i = 0, component = dai_link->codecs; + i < dai_link->num_codecs; + i++, component++) { + if (!component->of_node) + break; + of_node_put(component->of_node); + } } return 0; }
On Sunday 09 November 2014 12:22:21 Jean-Francois Moine wrote:
-- sound-dai : phandle and port of CPU/CODEC +either
- sound-dai : phandle and port of CPU/CODEC
+or
- sound-dais : list of phandle and port of CODECs
Is it really necessary to change the property name here? I woudl think that you can make it all work more consistently with just the existing 'sound-dai' property, just extend it to allow multiple codecs
Arnd
On Mon, 10 Nov 2014 09:39:57 +0100 Arnd Bergmann arnd@arndb.de wrote:
On Sunday 09 November 2014 12:22:21 Jean-Francois Moine wrote:
-- sound-dai : phandle and port of CPU/CODEC +either
- sound-dai : phandle and port of CPU/CODEC
+or
- sound-dais : list of phandle and port of CODECs
Is it really necessary to change the property name here? I woudl think that you can make it all work more consistently with just the existing 'sound-dai' property, just extend it to allow multiple codecs
This was an idea from Benoit Cousson:
"Something like that:
sound-dais = <&spdif_codec 1>, <&hdmi 0>;
That being said, it will require changing the name with a plural form,"
As I coded it, both names are accepted for a single or many codecs.
On Monday 10 November 2014 10:05:26 Jean-Francois Moine wrote:
On Mon, 10 Nov 2014 09:39:57 +0100 Arnd Bergmann arnd@arndb.de wrote:
On Sunday 09 November 2014 12:22:21 Jean-Francois Moine wrote:
-- sound-dai : phandle and port of CPU/CODEC +either
- sound-dai : phandle and port of CPU/CODEC
+or
- sound-dais : list of phandle and port of CODECs
Is it really necessary to change the property name here? I woudl think that you can make it all work more consistently with just the existing 'sound-dai' property, just extend it to allow multiple codecs
This was an idea from Benoit Cousson:
"Something like that:
sound-dais = <&spdif_codec 1>, <&hdmi 0>;
That being said, it will require changing the name with a plural form,"
As I coded it, both names are accepted for a single or many codecs.
I don't see a strong reason for coming up with a plural form. If you really have to, then please list the old form as a deprecated alias and make the document talk about "sound-dais" as the preferred form independent of the number of arguments.
Arnd
The CPU and CODEC names are checked when getting the DAI names. There is no need to check them once more.
Signed-off-by: Jean-Francois Moine moinejf@free.fr --- sound/soc/generic/simple-card.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 61f9500..c1ba78b 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -319,11 +319,6 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, if (ret < 0) goto dai_link_of_err;
- if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name) { - ret = -EINVAL; - goto dai_link_of_err; - } - /* Simple Card assumes platform == cpu */ dai_link->platform_of_node = dai_link->cpu_of_node;
participants (2)
-
Arnd Bergmann
-
Jean-Francois Moine