From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755908AbaICIha (ORCPT ); Wed, 3 Sep 2014 04:37:30 -0400 Received: from bear.ext.ti.com ([192.94.94.41]:46191 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755616AbaICIhV (ORCPT ); Wed, 3 Sep 2014 04:37:21 -0400 Message-ID: <5406D317.5000403@ti.com> Date: Wed, 3 Sep 2014 11:36:39 +0300 From: Jyri Sarha User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.0 MIME-Version: 1.0 To: "Li.Xiubo@freescale.com" , "broonie@kernel.org" , "perex@perex.cz" , "lgirdwood@gmail.com" , "tiwai@suse.de" , "moinejf@free.fr" , "andrew@lunn.ch" , "kuninori.morimoto.gx@renesas.com" , "devicetree@vger.kernel.org" , "alsa-devel@alsa-project.org" , "robh+dt@kernel.org" , "pawel.moll@arm.com" , "mark.rutland@arm.com" , "ijc+devicetree@hellion.org.uk" , "galak@codeaurora.org" CC: "linux-kernel@vger.kernel.org" Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code. References: <1409649969-15759-1-git-send-email-Li.Xiubo@freescale.com> <1409649969-15759-2-git-send-email-Li.Xiubo@freescale.com> <5405A598.4050208@ti.com> <707d1400a4514be9b599d4b7a6449ba7@BY2PR0301MB0613.namprd03.prod.outlook.com> In-Reply-To: <707d1400a4514be9b599d4b7a6449ba7@BY2PR0301MB0613.namprd03.prod.outlook.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/03/2014 05:37 AM, Li.Xiubo@freescale.com wrote: >> Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() ... >> >> This won't work. The logic for cpu node needs to be negated for codec node. >> > > Yes, actually it should be. > > As my previous patches about this: > ---- > Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx' > mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's > frame clock is as master/slave. > > So these same DAI formats should be informed to CPU and CODE DAIs at > the same time. For the Codec driver will set the bit clock and frame > clock as the DAI formats said, but for the CPU driver, if the the > bit clock or frame clock is as Codec master, so it should be set CPU > DAI device as bit clock or frame clock as slave, and vice versa. > > The old code will cause confusion, and we should be clear that the > letter 'C' here mean to Codec. > ---- > > For the master format, no matter for CPU or CODEC, it always means Codec > is master or slave for bit/frame clock, not means the local DAI device's > bit/frame clock as master or slave. > > So your CPU DAI device driver should negate this locally as the existed > Ones do. > Yes, but there is double negation in this patch. The switch-case assignments depend on whether the bitclkmaster and framemaster DT-node pointers are compared to a cpu-dai-node or codec-dai-node. When your patch compares the codec-node, it does the decisions like it was a cpu-node, which produces inverted CBM and CFM setting. However, Kurinori-san's patch fixes this problem because it just uses the daifmt generated by comparing to codec node for both cpu and codec nodes. The reason why I did the comparison per node basis, was to make the code more ready for tdm setups with multiple codecs on a same wire. But writing code for something that is not really needed yet is usually a bad idea, like it was this time too. Kurinori-san's version of the fix should be fine and it cleans up the code quite nicely. Best regards, Jyri