[alsa-devel] [PATCH RFC v3 0/2] Move dai-link level properties away from dai subnodes
This patch is implemented on top of late patches from Jean-Francois Moine [1].
These patches implement the main part of the simple-card changes discussed in alsa-devel mailing list [2].
Since RFC v2 version of the patches, address remaining comments from Jean-Francois Moine [4]: - ASoC: simple-card: Move dai-link level properties away from dai subnodes - Only allow legacy parsing if there is no dai-link node. - Fix double unref in of_card parsing loop.
Since RFC version of the patches: - ASoC: core: Update snd_soc_of_parse_daifmt() interface - Make minimal changes to simple-card for it to work with the updated snd_soc_of_parse_daifmt(). (Kuninori Morimoto's comment [3]). - ASoC: simple-card: Move dai-link level properties away from dai subnodes - Fix typo in DT binding document (Jean-Francois Moine's comment [4]). - Remove #define DEBUG - Remove accidental line wrap at simple-audio-card,name property descripition in the DT binding document
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074592.html [2] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074388.html [3] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074699.html [4] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074697.html
Best regards, Jyri
Jyri Sarha (2): ASoC: core: Update snd_soc_of_parse_daifmt() interface ASoC: simple-card: Move dai-link level properties away from dai subnodes
.../devicetree/bindings/sound/simple-card.txt | 88 ++++---- include/sound/soc.h | 4 +- sound/soc/generic/simple-card.c | 238 ++++++++++++-------- sound/soc/soc-core.c | 8 +- 4 files changed, 200 insertions(+), 138 deletions(-)
Adds struct device_node **bitclkmaster and struct device_node **framemaster function parameters. With the new syntax bitclock-master and frame-master properties can explicitly indicate the dai-link bit-clock and frame masters with a phandle. This patch also makes the minimal changes to simple-card for it to work with the updated snd_soc_of_parse_daifmt(). Simple-card appears to be the only user of snd_soc_of_parse_daifmt() for now.
Signed-off-by: Jyri Sarha jsarha@ti.com --- include/sound/soc.h | 4 +++- sound/soc/generic/simple-card.c | 5 +++-- sound/soc/soc-core.c | 8 +++++++- 3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index f7de629..e40713a 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1228,7 +1228,9 @@ int snd_soc_of_parse_tdm_slot(struct device_node *np, int snd_soc_of_parse_audio_routing(struct snd_soc_card *card, const char *propname); unsigned int snd_soc_of_parse_daifmt(struct device_node *np, - const char *prefix); + const char *prefix, + struct device_node **bitclkmaster, + struct device_node **framemaster); int snd_soc_of_get_dai_name(struct device_node *of_node, const char **dai_name);
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 21f1ccb..835fd02 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -121,7 +121,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np, * bitclock-master, frame-master * and specific "format" if it has */ - dai->fmt = snd_soc_of_parse_daifmt(np, NULL); + dai->fmt = snd_soc_of_parse_daifmt(np, NULL, NULL, NULL); dai->fmt |= daifmt;
/* @@ -201,7 +201,8 @@ static int asoc_simple_card_parse_of(struct device_node *node, snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
/* get CPU/CODEC common format via simple-audio-card,format */ - daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,") & + daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,", NULL, + NULL) & (SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);
/* off-codec widgets */ diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index b322cf2..7979dcc 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -4560,7 +4560,9 @@ int snd_soc_of_parse_audio_routing(struct snd_soc_card *card, EXPORT_SYMBOL_GPL(snd_soc_of_parse_audio_routing);
unsigned int snd_soc_of_parse_daifmt(struct device_node *np, - const char *prefix) + const char *prefix, + struct device_node **bitclkmaster, + struct device_node **framemaster) { int ret, i; char prop[128]; @@ -4643,9 +4645,13 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np, */ snprintf(prop, sizeof(prop), "%sbitclock-master", prefix); bit = !!of_get_property(np, prop, NULL); + if (bit && bitclkmaster) + *bitclkmaster = of_parse_phandle(np, prop, 0);
snprintf(prop, sizeof(prop), "%sframe-master", prefix); frame = !!of_get_property(np, prop, NULL); + if (frame && framemaster) + *framemaster = of_parse_phandle(np, prop, 0);
switch ((bit << 4) + frame) { case 0x11:
The properties like format, bitclock-master, frame-master, bitclock-inversion, and frame-inversion should be common to the dais connected with a dai-link. For bitclock-master and frame-master properties to be unambiguous they need to indicate the mastering dai node with a phandle.
Signed-off-by: Jyri Sarha jsarha@ti.com --- .../devicetree/bindings/sound/simple-card.txt | 88 +++---- sound/soc/generic/simple-card.c | 239 ++++++++++++-------- 2 files changed, 190 insertions(+), 137 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index 131aa2a..9b9df14 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -1,6 +1,6 @@ Simple-Card:
-Simple-Card specifies audio DAI connection of SoC <-> codec. +Simple-Card specifies audio DAI connections of SoC <-> codec.
Required properties:
@@ -10,26 +10,51 @@ Optional properties:
- simple-audio-card,name : User specified audio sound card name, one string property. -- simple-audio-card,format : CPU/CODEC common audio format. - "i2s", "right_j", "left_j" , "dsp_a" - "dsp_b", "ac97", "pdm", "msb", "lsb" - simple-audio-card,widgets : Please refer to widgets.txt. - simple-audio-card,routing : A list of the connections between audio components. Each entry is a pair of strings, the first being the connection's sink, the second being the connection's source. -- dai-tdm-slot-num : Please refer to tdm-slot.txt. -- dai-tdm-slot-width : Please refer to tdm-slot.txt. +Optional subnodes: + +- simple-audio-card,dai-link : Container for dai-link level + properties and the CPU and CODEC + sub-nodes. This container may be + omitted when the card has only one + DAI link. See the examples and the + section bellow. + +Dai-link subnode properties and subnodes: + +If dai-link subnode is omitted and the subnode properties are directly +under "sound"-node the subnode property and subnode names have to be +prefixed with "simple-audio-card,"-prefix.
-Required subnodes: +Required dai-link subnodes:
-- simple-audio-card,dai-link : container for the CPU and CODEC sub-nodes - This container may be omitted when the - card has only one DAI link. - See the examples. +- cpu : CPU sub-node +- codec : CODEC sub-node
-- simple-audio-card,cpu : CPU sub-node -- simple-audio-card,codec : CODEC sub-node +Optional dai-link subnode properties: + +- format : CPU/CODEC common audio format. + "i2s", "right_j", "left_j" , "dsp_a" + "dsp_b", "ac97", "pdm", "msb", "lsb" +- frame-master : Indicates dai-link frame master. + phandle to a cpu or codec subnode. +- bitclock-master : Indicates dai-link bit clock master. + phandle to a cpu or codec subnode. +- bitclock-inversion : bool property. Add this if the + dai-link uses bit clock inversion. +- frame-inversion : bool property. Add this if the + dai-link uses frame clock inversion. + +For backward compatibility the frame-master and bitclock-master +properties can be used as booleans in codec subnode to indicate if the +codec is the dai-link frame or bit clock master. In this case there +should be no dai-link node, the same properties should not be present +at sound-node level, and the bitclock-inversion and frame-inversion +properties should also be placed in the codec node if needed.
Required CPU/CODEC subnodes properties:
@@ -37,29 +62,21 @@ Required CPU/CODEC subnodes properties:
Optional CPU/CODEC subnodes properties:
-- format : CPU/CODEC specific audio format if needed. - see simple-audio-card,format -- frame-master : bool property. add this if subnode is frame master -- bitclock-master : bool property. add this if subnode is bitclock master -- bitclock-inversion : bool property. add this if subnode has clock inversion -- frame-inversion : bool property. add this if subnode has frame inversion +- dai-tdm-slot-num : Please refer to tdm-slot.txt. +- dai-tdm-slot-width : Please refer to tdm-slot.txt. - clocks / system-clock-frequency : specify subnode's clock if needed. it can be specified via "clocks" if system has clock node (= common clock), or "system-clock-frequency" (if system doens't support common clock)
-Note: - * For 'format', 'frame-master', 'bitclock-master', 'bitclock-inversion' and - 'frame-inversion', the simple card will use the settings of CODEC for both - CPU and CODEC sides as we need to keep the settings identical for both ends - of the link. - Example 1 - single DAI link:
sound { compatible = "simple-audio-card"; simple-audio-card,name = "VF610-Tower-Sound-Card"; simple-audio-card,format = "left_j"; + simple-audio-card,bitclock-master = <&dailink0_master>; + simple-audio-card,frame-master = <&dailink0_master>; simple-audio-card,widgets = "Microphone", "Microphone Jack", "Headphone", "Headphone Jack", @@ -69,17 +86,12 @@ sound { "Headphone Jack", "HP_OUT", "External Speaker", "LINE_OUT";
- dai-tdm-slot-num = <2>; - dai-tdm-slot-width = <8>; - simple-audio-card,cpu { sound-dai = <&sh_fsi2 0>; };
- simple-audio-card,codec { + dailink0_master: simple-audio-card,codec { sound-dai = <&ak4648>; - bitclock-master; - frame-master; clocks = <&osc>; }; }; @@ -105,31 +117,31 @@ Example 2 - many DAI links: sound { compatible = "simple-audio-card"; simple-audio-card,name = "Cubox Audio"; - simple-audio-card,format = "i2s";
simple-audio-card,dai-link@0 { /* I2S - HDMI */ - simple-audio-card,cpu { + format = "i2s"; + cpu { sound-dai = <&audio1 0>; }; - simple-audio-card,codec { + codec { sound-dai = <&tda998x 0>; }; };
simple-audio-card,dai-link@1 { /* S/PDIF - HDMI */ - simple-audio-card,cpu { + cpu { sound-dai = <&audio1 1>; }; - simple-audio-card,codec { + codec { sound-dai = <&tda998x 1>; }; };
simple-audio-card,dai-link@2 { /* S/PDIF - S/PDIF */ - simple-audio-card,cpu { + cpu { sound-dai = <&audio1 1>; }; - simple-audio-card,codec { + codec { sound-dai = <&spdif_codec>; }; }; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 835fd02..3f2e580 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -88,7 +88,6 @@ 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, - unsigned int daifmt, struct asoc_simple_dai *dai, const struct device_node **p_node, const char **name) @@ -117,14 +116,6 @@ asoc_simple_card_sub_parse_of(struct device_node *np, return ret;
/* - * bitclock-inversion, frame-inversion - * bitclock-master, frame-master - * and specific "format" if it has - */ - dai->fmt = snd_soc_of_parse_daifmt(np, NULL, NULL, NULL); - dai->fmt |= daifmt; - - /* * dai->sysclk come from * "clocks = <&xxx>" (if system has common clock) * or "system-clock-frequency = <xxx>" @@ -151,37 +142,135 @@ asoc_simple_card_sub_parse_of(struct device_node *np, return 0; }
-static int simple_card_cpu_codec_of(struct device_node *node, - int daifmt, - struct snd_soc_dai_link *dai_link, - struct simple_dai_props *dai_props) +static int simple_card_dai_link_of(struct device_node *node, + struct device *dev, + struct snd_soc_dai_link *dai_link, + struct simple_dai_props *dai_props) { - struct device_node *np; + struct device_node *np = NULL; + struct device_node *bitclkmaster = NULL; + struct device_node *framemaster = NULL; + unsigned int daifmt; + char *name; + char prop[128]; + char *prefix = ""; int ret;
- /* CPU sub-node */ - ret = -EINVAL; - np = of_get_child_by_name(node, "simple-audio-card,cpu"); - if (np) { - ret = asoc_simple_card_sub_parse_of(np, daifmt, - &dai_props->cpu_dai, - &dai_link->cpu_of_node, - &dai_link->cpu_dai_name); - of_node_put(np); + if (!strcmp("sound", node->name)) + prefix = "simple-audio-card,"; + + daifmt = snd_soc_of_parse_daifmt(node, prefix, + &bitclkmaster, &framemaster); + daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK; + + snprintf(prop, sizeof(prop), "%scpu", prefix); + np = of_get_child_by_name(node, prop); + if (!np) { + ret = -EINVAL; + dev_err(dev, "%s: Can't find simple-audio-card,cpu DT node\n", + __func__); + goto dai_link_of_err; } + + ret = asoc_simple_card_sub_parse_of(np, &dai_props->cpu_dai, + &dai_link->cpu_of_node, + &dai_link->cpu_dai_name); if (ret < 0) - return ret; + goto dai_link_of_err; + + dai_props->cpu_dai.fmt = daifmt; + switch (((np == bitclkmaster)<<4)|(np == framemaster)) { + case 0x11: + dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBS_CFS; + break; + case 0x10: + dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBS_CFM; + break; + case 0x01: + dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBM_CFS; + break; + default: + dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBM_CFM; + break; + }
- /* CODEC sub-node */ - ret = -EINVAL; - np = of_get_child_by_name(node, "simple-audio-card,codec"); - if (np) { - ret = asoc_simple_card_sub_parse_of(np, daifmt, - &dai_props->codec_dai, - &dai_link->codec_of_node, - &dai_link->codec_dai_name); - of_node_put(np); + of_node_put(np); + snprintf(prop, sizeof(prop), "%scodec", prefix); + np = of_get_child_by_name(node, prop); + if (!np) { + ret = -EINVAL; + dev_err(dev, "%s: Can't find simple-audio-card,codec DT node\n", + __func__); + goto dai_link_of_err; + } + + ret = asoc_simple_card_sub_parse_of(np, &dai_props->codec_dai, + &dai_link->codec_of_node, + &dai_link->codec_dai_name); + if (ret < 0) + goto dai_link_of_err; + + if (strlen(prefix) && !bitclkmaster && !framemaster) { + /* No dai-link level and master setting was not found from + sound node level, revert back to legacy DT parsing and + take the settings from codec node. */ + dev_dbg(dev, "%s: Revert to legacy daifmt parsing\n", + __func__); + dai_props->cpu_dai.fmt = dai_props->codec_dai.fmt = + snd_soc_of_parse_daifmt(np, NULL, NULL, NULL) | + (daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK); + } else { + dai_props->codec_dai.fmt = daifmt; + switch (((np == bitclkmaster)<<4)|(np == framemaster)) { + case 0x11: + dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBM_CFM; + break; + case 0x10: + dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBM_CFS; + break; + case 0x01: + dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBS_CFM; + break; + default: + dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBS_CFS; + break; + } + } + + 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; + + /* 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, + GFP_KERNEL); + sprintf(name, "%s-%s", dai_link->cpu_dai_name, + dai_link->codec_dai_name); + dai_link->name = dai_link->stream_name = name; + + dev_dbg(dev, "\tname : %s\n", dai_link->stream_name); + dev_dbg(dev, "\tcpu : %s / %04x / %d\n", + dai_link->cpu_dai_name, + dai_props->cpu_dai.fmt, + dai_props->cpu_dai.sysclk); + dev_dbg(dev, "\tcodec : %s / %04x / %d\n", + dai_link->codec_dai_name, + dai_props->codec_dai.fmt, + dai_props->codec_dai.sysclk); + +dai_link_of_err: + if (np) + of_node_put(np); + if (bitclkmaster) + of_node_put(bitclkmaster); + if (framemaster) + of_node_put(framemaster); return ret; }
@@ -192,19 +281,11 @@ static int asoc_simple_card_parse_of(struct device_node *node, { struct snd_soc_dai_link *dai_link = priv->snd_card.dai_link; struct simple_dai_props *dai_props = priv->dai_props; - struct device_node *np; - char *name; - unsigned int daifmt; int ret;
/* parsing the card name from DT */ snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
- /* get CPU/CODEC common format via simple-audio-card,format */ - daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,", NULL, - NULL) & - (SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK); - /* off-codec widgets */ if (of_property_read_bool(node, "simple-audio-card,widgets")) { ret = snd_soc_of_parse_audio_simple_widgets(&priv->snd_card, @@ -221,71 +302,31 @@ static int asoc_simple_card_parse_of(struct device_node *node, return ret; }
- /* loop on the DAI links */ - np = NULL; - for (;;) { - if (multi) { - np = of_get_next_child(node, np); - if (!np) - break; + dev_dbg(dev, "New simple-card: %s\n", priv->snd_card.name ? + priv->snd_card.name : ""); + + if (multi) { + struct device_node *np = NULL; + int i; + for (i = 0; (np = of_get_next_child(node, np)); i++) { + dev_dbg(dev, "\tlink %d:\n", i); + ret = simple_card_dai_link_of(np, dev, dai_link + i, + dai_props + i); + if (ret < 0) { + of_node_put(np); + return ret; + } } - - ret = simple_card_cpu_codec_of(multi ? np : node, - daifmt, dai_link, dai_props); + } else { + ret = simple_card_dai_link_of(node, dev, dai_link, dai_props); if (ret < 0) - goto err; - - /* - * overwrite cpu_dai->fmt as its DAIFMT_MASTER bit is based on CODEC - * while the other bits should be identical unless buggy SW/HW design. - */ - dai_props->cpu_dai.fmt = dai_props->codec_dai.fmt; - - if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name) { - ret = -EINVAL; - goto err; - } - - /* 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; - - if (!multi) - break; - - dai_link++; - dai_props++; + return ret; }
- /* card name is created from CPU/CODEC dai name */ - dai_link = priv->snd_card.dai_link; if (!priv->snd_card.name) - priv->snd_card.name = dai_link->name; - - dev_dbg(dev, "card-name : %s\n", priv->snd_card.name); - dev_dbg(dev, "platform : %04x\n", daifmt); - dai_props = priv->dai_props; - dev_dbg(dev, "cpu : %s / %04x / %d\n", - dai_link->cpu_dai_name, - dai_props->cpu_dai.fmt, - dai_props->cpu_dai.sysclk); - dev_dbg(dev, "codec : %s / %04x / %d\n", - dai_link->codec_dai_name, - dai_props->codec_dai.fmt, - dai_props->codec_dai.sysclk); + priv->snd_card.name = priv->snd_card.dai_link->name;
return 0; - -err: - of_node_put(np); - return ret; }
/* update the reference count of the devices nodes at end of probe */
On Mon, 24 Mar 2014 12:15:23 +0200 Jyri Sarha jsarha@ti.com wrote:
This patch is implemented on top of late patches from Jean-Francois Moine [1].
These patches implement the main part of the simple-card changes discussed in alsa-devel mailing list [2].
Acked-by: Jean-Francois Moine moinejf@free.fr
participants (3)
-
Jean-Francois Moine
-
Jyri Sarha
-
Mark Brown