RE: [PATCH] ASoC: simple-card: cleanup asoc_simple_card_parse_of()code

[Li.Xiubo@xxxxxxxxxxxxx]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/