[PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[Jyri Sarha]

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(-)

-- 
1.7.9.5

[PATCH RFC v2 1/2] ASoC: core: Add is_cpu_dai_node-parameter to snd_soc_of_parse_daifmt()

[Jyri Sarha]

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;
-- 
1.7.9.5

[PATCH RFC v2 2/2] ASoC: simple-card: Take snd_soc_of_parse_daifmt() change in to account

[Jyri Sarha]

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;
 
-- 
1.7.9.5

Re: [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [alsa-devel] [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[Kuninori Morimoto]

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

Re: [alsa-devel] [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[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;
};

Re: [alsa-devel] [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[Richard Lee]

 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.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
> --
> 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

Re: [alsa-devel] [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[Kuninori Morimoto]

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"...

Re: [alsa-devel] [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[Jyri Sarha]

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

Re: [alsa-devel] [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[Kuninori Morimoto]

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 ?

Re: [alsa-devel] [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[Jyri Sarha]

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

Re: [alsa-devel] [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[Kuninori Morimoto]

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

Re: [alsa-devel] [PATCH RFC v2 0/2] Fix simple-card *-master DT parameter handling

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

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).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]