On Fri, 21 Mar 2014 18:47:23 +0200
Jyri Sarha <jsa...@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 <jsa...@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 <moin...@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/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to