Re: [alsa-devel] [PATCH RFC 2/2] ASoC: simple-card: Move dai-link level properties away from dai subnodes
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
---
(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 = "
[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
--
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
Re: [PATCH RFC 2/2] ASoC: simple-card: Move dai-link level properties away from dai subnodes
On 03/23/2014 11:54 AM, Jean-Francois Moine wrote:
On Fri, 21 Mar 2014 18:47:23 +0200
Jyri Sarha 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
---
[...]
-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
Best regards,
Jyri
--
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
Re: [alsa-devel] [PATCH RFC 2/2] ASoC: simple-card: Move dai-link level properties away from dai subnodes
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
> ---
(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 = "
[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
--
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
Re: [PATCH RFC 2/2] ASoC: simple-card: Move dai-link level properties away from dai subnodes
On Fri, 21 Mar 2014 18:47:23 +0200 Jyri Sarha 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 > --- > .../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 > ---
[PATCH RFC 2/2] ASoC: simple-card: Move dai-link level properties away from dai subnodes
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 --- .../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: -- 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 m
