| Message ID | 0b1c0e2e6766c3a63f598c3b9435d4744b0e2835.1395419906.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 21 Mar 2014 18:47:23 +0200 Jyri Sarha <jsarha@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@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@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 */
Hi Jyri > 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> > --- (snip) > /* > - * 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>" [1/2] patch exchanged snd_soc_of_parse_daifmt() parameter, but, user code exchanged in [2/2]. It breaks git-bisect. > + 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; > + } The user of snd_soc_of_parse_daifmt() is only simple-card (?) I think SND_SOC_DAIFMT_CBxx_CFx cared by snd_soc_of_parse_daifmt() somehow is very reasonable. Best regards --- Kuninori Morimoto
On 03/23/2014 11:54 AM, Jean-Francois Moine wrote: > On Fri, 21 Mar 2014 18:47:23 +0200 > Jyri Sarha <jsarha@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@ti.com> >> --- [...] >> -Required subnodes: >> +Requred dai-link subnodes: > > Typo: 'Required' > Fixed. Thanks! [...] >> --- 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. > Removed. Thanks! [...] >> + 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. > Sorry, I missed this and comments bellow first time around. I'll make another patch set shortly. Adding another check to test if we are parsing top level sound node would save you from the legacy mode parsing. I'll do that and update the DT binding document accordingly. [...] >> + 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: > I was targeting for readability rather than speed. I doubt the difference is significant since most string processing is anyway taking most of the time anyway. > 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; > } > Oops, need to fix that. Thanks ! > Otherwise, it works for me. > > Tested-by: Jean-Francois Moine <moinejf@free.fr> > Best regards, Jyri
On 03/24/2014 02:05 AM, Kuninori Morimoto wrote: > > Hi Jyri > >> 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> >> --- > (snip) >> /* >> - * 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>" > > [1/2] patch exchanged snd_soc_of_parse_daifmt() parameter, > but, user code exchanged in [2/2]. > It breaks git-bisect. > Fixed that. Thanks! >> + 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; >> + } > > The user of snd_soc_of_parse_daifmt() is only simple-card (?) > I think SND_SOC_DAIFMT_CBxx_CFx cared by snd_soc_of_parse_daifmt() somehow > is very reasonable. > Yes, that is what git grep tells me. snd_soc_of_parse_daifmt() can still take care of SND_SOC_DAIFMT_CB?_CF? parsing in simple cases. I just felt that adding the full phandle parsing to the function would break its genericity and make the function parameters hard to understand. Best regards, Jyri
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: -- 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: @@ -37,29 +63,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 +87,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 +118,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 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 #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); + } 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); + of_node_put(np); + if (ret < 0) + return ret; } - - /* 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 */
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 | 91 ++++---- sound/soc/generic/simple-card.c | 237 ++++++++++++-------- 2 files changed, 191 insertions(+), 137 deletions(-)