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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/