[alsa-devel] [PATCH RFC 2/2] ASoC: simple-card: Move dai-link level properties away from dai subnodes
Jean-Francois Moine
moinejf at free.fr
Sun Mar 23 10:54:12 CET 2014
On Fri, 21 Mar 2014 18:47:23 +0200
Jyri Sarha <jsarha at ti.com> wrote:
> 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 at ti.com>
> ---
> .../devicetree/bindings/sound/simple-card.txt | 91 ++++----
> sound/soc/generic/simple-card.c | 237 ++++++++++++--------
> 2 files changed, 191 insertions(+), 137 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
> index 131aa2a..3cafa9e 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:
>
> @@ -8,28 +8,54 @@ Required properties:
>
> Optional properties:
>
> -- simple-audio-card,name : User specified audio sound card name, one string
> +- 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:
> +Requred dai-link subnodes:
Typo: 'Required'
>
> -- 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 the same
> +properties should not be present at dai-link level and the
> +bitclock-inversion and frame-inversion properties should also be placed
> +in the codec node if needed.
>
> Required CPU/CODEC subnodes properties:
>
[snip]
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 21f1ccb..f4663d0 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -8,6 +8,7 @@
> * it under the terms of the GNU General Public License version 2 as
> * published by the Free Software Foundation.
> */
> +#define DEBUG
Should be removed.
> #include <linux/clk.h>
> #include <linux/device.h>
> #include <linux/module.h>
> @@ -88,7 +89,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 +117,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);
> - dai->fmt |= daifmt;
> -
> - /*
> * dai->sysclk come from
> * "clocks = <&xxx>" (if system has common clock)
> * or "system-clock-frequency = <xxx>"
> @@ -151,37 +143,134 @@ 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 (!bitclkmaster && !framemaster) {
> + /* Master setting not found from dai_link level, revert back
> + to legacy DT parsing and take 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);
We are here each time there is no bitclock-master and no frame-master,
i.e. each time for me. It could be simpler to keep the first 'daifmt'
instead of parsing again.
> + } 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,18 +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,") &
> - (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,
> @@ -220,71 +302,30 @@ 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;
> - }
> -
> - ret = simple_card_cpu_codec_of(multi ? np : node,
> - daifmt, 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;
> + dev_dbg(dev, "New simple-card: %s\n", priv->snd_card.name ?
> + priv->snd_card.name : "");
>
> - if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name) {
> - ret = -EINVAL;
> - goto err;
> + 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);
I feel that my loop was quicker:
struct device_node *np = NULL;
for (;;) {
np = of_get_next_child(node, np);
if (!np)
break;
dev_dbg(dev, "\tlink %d:\n", dai_link - priv->snd_card.dai_link);
ret = simple_card_dai_link_of(np, dev, dai_link++,
dai_props++);
> + of_node_put(np);
> + if (ret < 0)
> + return ret;
The np reference count is updated in of_get_next_child(), so:
if (ret < 0) {
of_node_put(np);
return ret;
}
Otherwise, it works for me.
Tested-by: Jean-Francois Moine <moinejf at free.fr>
> }
> -
> - /* 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++;
> + } else {
> + ret = simple_card_dai_link_of(node, dev, dai_link, dai_props);
> + if (ret < 0)
> + 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 */
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
More information about the Alsa-devel
mailing list