[alsa-devel] [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling
Since RFC: - fixed commit msg typo - added include/sound/soc.h changes too
The sematics of bitclock-master and frame-master DT parameters should depend on whether they are found from a cpu-dai or codec sub-node.
- bitclock-master in cpu-dai node means Codec-Bitclock-Slave - frame-master in cpu-dai node means Codec-Frame-Slave - bitclock-master in codec node means Codec-Bitclock-Master - frame-master in codec node means Codec-Frame-Master
For example in a cpu-dai mode bitclock-master parameter should produce SND_SOC_DAIFMT_CBS_* daifmt flags and a codec node SND_SOC_DAIFMT_CBM_* flags.
Best regards, Jyri
Jyri Sarha (2): ASoC: core: Add is_cpu_dai_node-parameter to snd_soc_of_parse_daifmt() ASoC: simple-card: Take snd_soc_of_parse_daifmt() change in to account
include/sound/soc.h | 3 ++- sound/soc/generic/simple-card.c | 13 ++++++++----- sound/soc/soc-core.c | 8 +++++++- 3 files changed, 17 insertions(+), 7 deletions(-)
The sematics of bitclock-master and frame-master DT parameters should be inversed when parsing a cpu-dai node.
Signed-off-by: Jyri Sarha jsarha@ti.com --- include/sound/soc.h | 3 ++- sound/soc/soc-core.c | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index c0b6656..ecd0745 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1231,7 +1231,8 @@ 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, + bool is_cpu_dai_node); int snd_soc_of_get_dai_name(struct device_node *of_node, const char **dai_name);
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 8ddb15c..dfff75f 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -4613,7 +4613,8 @@ 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, + bool is_cpu_dai_node) { int ret, i; char prop[128]; @@ -4700,6 +4701,11 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np, snprintf(prop, sizeof(prop), "%sframe-master", prefix); frame = !!of_get_property(np, prop, NULL);
+ if (is_cpu_dai_node) { + bit = !bit; + frame = !frame; + } + switch ((bit << 4) + frame) { case 0x11: format |= SND_SOC_DAIFMT_CBM_CFM;
snd_soc_of_parse_daifmt() needs to know if it is parsing a cpu-dai node.
Signed-off-by: Jyri Sarha jsarha@ti.com --- sound/soc/generic/simple-card.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 7cabcc5..7cbbf44 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -87,7 +87,8 @@ 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) + const char **name, + bool is_cpu_dai_node) { struct device_node *node; struct clk *clk; @@ -117,7 +118,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, is_cpu_dai_node); dai->fmt |= daifmt;
/* @@ -165,7 +166,7 @@ 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 */ - priv->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,") & + priv->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,", 0) & (SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);
/* off-codec widgets */ @@ -191,7 +192,8 @@ static int asoc_simple_card_parse_of(struct device_node *node, ret = asoc_simple_card_sub_parse_of(np, priv->daifmt, &priv->cpu_dai, &dai_link->cpu_of_node, - &dai_link->cpu_dai_name); + &dai_link->cpu_dai_name, + true); if (ret < 0) return ret;
@@ -202,7 +204,8 @@ static int asoc_simple_card_parse_of(struct device_node *node, ret = asoc_simple_card_sub_parse_of(np, priv->daifmt, &priv->codec_dai, &dai_link->codec_of_node, - &dai_link->codec_dai_name); + &dai_link->codec_dai_name, + false); if (ret < 0) return ret;
On Mon, Mar 10, 2014 at 01:41:14PM +0200, Jyri Sarha wrote:
Since RFC:
- fixed commit msg typo
- added include/sound/soc.h changes too
The sematics of bitclock-master and frame-master DT parameters should depend on whether they are found from a cpu-dai or codec sub-node.
- bitclock-master in cpu-dai node means Codec-Bitclock-Slave
- frame-master in cpu-dai node means Codec-Frame-Slave
- bitclock-master in codec node means Codec-Bitclock-Master
- frame-master in codec node means Codec-Frame-Master
For example in a cpu-dai mode bitclock-master parameter should produce SND_SOC_DAIFMT_CBS_* daifmt flags and a codec node SND_SOC_DAIFMT_CBM_* flags.
I've added Morimoto-san and Xiubo to the CCs - can you please have a look at this? I can't see any in tree users but presumably there are some existing out of tree users of the binding that use it without current problems.
Hi Jyri
Since RFC:
- fixed commit msg typo
- added include/sound/soc.h changes too
The sematics of bitclock-master and frame-master DT parameters should depend on whether they are found from a cpu-dai or codec sub-node.
- bitclock-master in cpu-dai node means Codec-Bitclock-Slave
- frame-master in cpu-dai node means Codec-Frame-Slave
- bitclock-master in codec node means Codec-Bitclock-Master
- frame-master in codec node means Codec-Frame-Master
For example in a cpu-dai mode bitclock-master parameter should produce SND_SOC_DAIFMT_CBS_* daifmt flags and a codec node SND_SOC_DAIFMT_CBM_* flags.
SND_SOC_DAIFMT_xxx comment indicates "codec clk/FRM" indeed. but does this "codec" means "codec chip" ?? I'm not sure.
but anyway, if my understanding is correct,
simple-audio-card,cpu { ... bitclock-master; frame-master; };
simple-audio-card,codec { ... bitclock-master; frame-master; };
This will be cpu : SND_SOC_DAIFMT_CBS_CFS codec : SND_SOC_DAIFMT_CBM_CFM
but, it is un-understandable/confusable for me, and it breaks our sound card.
${LINUX}/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts ${LINUX}/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
I guess you want like this ?
codec-bitclock-master; codec-frame-master;
simple-audio-card,cpu { ... };
simple-audio-card,codec { ... };
# And I guess [1/2] and [2/2] should be 1 patch. # otherwise, it breaks git-bisect :P
Best regards --- Kuninori Morimoto
Hi Mark, Jyri
Since RFC:
- fixed commit msg typo
- added include/sound/soc.h changes too
The sematics of bitclock-master and frame-master DT parameters should depend on whether they are found from a cpu-dai or codec sub-node.
- bitclock-master in cpu-dai node means Codec-Bitclock-Slave
- frame-master in cpu-dai node means Codec-Frame-Slave
- bitclock-master in codec node means Codec-Bitclock-Master
- frame-master in codec node means Codec-Frame-Master
For example in a cpu-dai mode bitclock-master parameter should produce SND_SOC_DAIFMT_CBS_* daifmt flags and a codec node SND_SOC_DAIFMT_CBM_* flags.
I had misunderstood about SND_SOC_DAIFMT_xxx So, please ignore my previous comment.
But, I wounder, if cpu/codec use identical format flags, then, asoc_simple_dai don't need fmt ?
struct asoc_simple_dai { const char *name; unsigned int fmt; <= we can/should remove this ? unsigned int sysclk; };
On Wed, Mar 12, 2014 at 9:13 AM, Kuninori Morimoto kuninori.morimoto.gx@gmail.com wrote:
Hi Jyri
Since RFC:
- fixed commit msg typo
- added include/sound/soc.h changes too
The sematics of bitclock-master and frame-master DT parameters should depend on whether they are found from a cpu-dai or codec sub-node.
- bitclock-master in cpu-dai node means Codec-Bitclock-Slave
- frame-master in cpu-dai node means Codec-Frame-Slave
- bitclock-master in codec node means Codec-Bitclock-Master
- frame-master in codec node means Codec-Frame-Master
For example in a cpu-dai mode bitclock-master parameter should produce SND_SOC_DAIFMT_CBS_* daifmt flags and a codec node SND_SOC_DAIFMT_CBM_* flags.
SND_SOC_DAIFMT_xxx comment indicates "codec clk/FRM" indeed. but does this "codec" means "codec chip" ?? I'm not sure.
but anyway, if my understanding is correct,
simple-audio-card,cpu { ... bitclock-master; frame-master; }; simple-audio-card,codec { ... bitclock-master; frame-master; };This will be cpu : SND_SOC_DAIFMT_CBS_CFS codec : SND_SOC_DAIFMT_CBM_CFM
Yes, That's also what my understanding of this patches.
But, IMO, if you want the CPU DAI be CBS_CFS and CODEC be CBM_CFM, you could just do it like this: simple-audio-card,cpu { ... };
simple-audio-card,codec { ... bitclock-master; frame-master; };
and vice versa.
Thanks,
(I could find this mails in my Freescale acount, so I will reply it here.)
-- Best Regards, Xiubo
but, it is un-understandable/confusable for me, and it breaks our sound card.
${LINUX}/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts ${LINUX}/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dtsI guess you want like this ?
codec-bitclock-master; codec-frame-master; simple-audio-card,cpu { ... }; simple-audio-card,codec { ... };# And I guess [1/2] and [2/2] should be 1 patch. # otherwise, it breaks git-bisect :P
Best regards
Kuninori Morimoto
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Richard
but anyway, if my understanding is correct,
simple-audio-card,cpu { ... bitclock-master; frame-master; }; simple-audio-card,codec { ... bitclock-master; frame-master; };This will be cpu : SND_SOC_DAIFMT_CBS_CFS codec : SND_SOC_DAIFMT_CBM_CFM
Yes, That's also what my understanding of this patches.
Thank you. I understand. (and I need to send fixup patches :)
# "codec-is-bitclock-master" seems understandable than # current "bitclock-master"...
On 03/12/2014 08:11 AM, Kuninori Morimoto wrote:
Hi Richard
but anyway, if my understanding is correct,
simple-audio-card,cpu { ... bitclock-master; frame-master; }; simple-audio-card,codec { ... bitclock-master; frame-master; };This will be cpu : SND_SOC_DAIFMT_CBS_CFS codec : SND_SOC_DAIFMT_CBM_CFM
Yes, That's also what my understanding of this patches.
Thank you. I understand. (and I need to send fixup patches :)
# "codec-is-bitclock-master" seems understandable than # current "bitclock-master"...
Sounds great to me!
Best regards, Jyri
Hi Jyri, Mark
but anyway, if my understanding is correct,
simple-audio-card,cpu { ... bitclock-master; frame-master; }; simple-audio-card,codec { ... bitclock-master; frame-master; };This will be cpu : SND_SOC_DAIFMT_CBS_CFS codec : SND_SOC_DAIFMT_CBM_CFM
Yes, That's also what my understanding of this patches.
Thank you. I understand. (and I need to send fixup patches :)
# "codec-is-bitclock-master" seems understandable than # current "bitclock-master"...
Sounds great to me!
Could you please teach me current status of this patch ?
On 04/14/2014 08:07 AM, Kuninori Morimoto wrote:
Hi Jyri, Mark
...
Could you please teach me current status of this patch ?
Hi, This patch-set was never merged as such. Instead there is another patch-set that has been applied by Mark to cover this issue [1]. The patch maintains backward compatibility, with the Nicolin Chen's patch [2]. In other words, old style "bitclock-master" and "frame-master" boolean parameters are read from the codec node if they can not be found from the top level node. The backward compatibility does not apply to multi-link [3] configurations.
Best regards, Jyri
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074717.html
[2] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074219.html
[3] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074592.html
ps. The discussions that lead to the current solution can be found here: http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074388.html
Hi Jyri
This patch-set was never merged as such. Instead there is another patch-set that has been applied by Mark to cover this issue [1]. The patch maintains backward compatibility, with the Nicolin Chen's patch [2]. In other words, old style "bitclock-master" and "frame-master" boolean parameters are read from the codec node if they can not be found from the top level node. The backward compatibility does not apply to multi-link [3] configurations.
Thank you ! I could find them
On Wed, Mar 12, 2014 at 01:00:02PM +0800, Richard Lee wrote:
But, IMO, if you want the CPU DAI be CBS_CFS and CODEC be CBM_CFM, you could just do it like this: simple-audio-card,cpu { ... };
simple-audio-card,codec { ... bitclock-master; frame-master; };
and vice versa.
Thanks,
(I could find this mails in my Freescale acount, so I will reply it here.)
Yes, that'd been what I'd thought the binding did (and it's what Jyri's patch makes it do).
participants (4)
-
Jyri Sarha -
Kuninori Morimoto -
Mark Brown -
Richard Lee