lkml.org 
[lkml]   [2013]   [Dec]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH] ASoC: simple-card: cleanup asoc_simple_card_parse_of() code
Date
Hi Mark and all,

Merry Christmas.


> Subject: Re: [PATCH] ASoC: simple-card: cleanup asoc_simple_card_parse_of()
> code
>
> On Fri, Dec 27, 2013 at 11:14:38AM +0800, Xiubo Li wrote:
> > Since there is only one common format via "simple-audio-card,format"
> > for simple card driver(CPU & CODEC DAI), there is no need to do the
> > INV mask.
>
> I don't understand this change, how is the fact that the CPU and CODEC
> DAI share a common format related to using the inversion? The inversion
> specifies the format relative to the spec rather than relative to the
> other end of the link and for any DAI link the two devices should be
> using the same format.

Sorry, sorry for confusion.

From the "Documentation/devicetree/bindings/sound/simple-card.txt":
------
Optional properties:

- simple-audio-card,format : CPU/CODEC common audio format.
"i2s", "right_j", "left_j" , "dsp_a"
"dsp_b", "ac97", "pdm", "msb", "lsb"
------
There is only one common daifmt properties in simple-card parent dt node, and
its bits mask is SND_SOC_DAIFMT_FORMAT_MASK.

And also in the same binding documents:
------
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
- 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)
------

And in "sound/soc/generic/simple-card.c", function : asoc_simple_card_parse_of()
-----
......

/* get CPU/CODEC common format via simple-audio-card,format */
info->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,") &
(SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);

......

ret = asoc_simple_card_sub_parse_of(cpu, ...);

...

ret = asoc_simple_card_sub_parse_of(codec, ...);

......
-----
The SND_SOC_DAIFMT_INV_MASK bits mask should be for "bitclock-master" and
"frame-inversion" in subnodes' daifmt parsing if need, or why shouldn't
SND_SOC_DAIFMT_CLOCK_MASK be here like SND_SOC_DAIFMT_INV_MASK ?

IMO, the SND_SOC_DAIFMT_INV_MASK bits mask here maybe a little confused.

Certainly, these two and other subnode properties can also be common properties
like "[prefix]format" for other drivers.

It's the limitation of snd_soc_of_parse_daifmt() that the simple-card driver must
do the mask(except SND_SOC_DAIFMT_MASTER_MASK, because it default value is not zero)
operation.

Or should we change "bitclock-master" and "frame-master" as one string property
like "[prefix]master = XXX", and set the default value as zero like others. And then
we can remove all the bits mask operations in simple-card and other drivers.
If need, I will send this patch of this.


Thanks,

--
Best Regards,
Xiubo



\
 
 \ /
  Last update: 2013-12-31 03:21    [W:0.062 / U:0.716 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site