From patchwork Mon Sep 1 09:39:10 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jyri Sarha X-Patchwork-Id: 4818121 Return-Path: X-Original-To: patchwork-alsa-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 959E6C0338 for ; Mon, 1 Sep 2014 09:39:40 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B199520158 for ; Mon, 1 Sep 2014 09:39:39 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.kernel.org (Postfix) with ESMTP id 60498200E3 for ; Mon, 1 Sep 2014 09:39:38 +0000 (UTC) Received: by alsa0.perex.cz (Postfix, from userid 1000) id 7CDC1264F34; Mon, 1 Sep 2014 11:39:36 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,NO_DNS_FOR_FROM, UNPARSEABLE_RELAY autolearn=no version=3.3.1 Received: from alsa0.perex.cz (localhost [IPv6:::1]) by alsa0.perex.cz (Postfix) with ESMTP id 66A7E261A74; Mon, 1 Sep 2014 11:39:25 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 280CE2616CB; Mon, 1 Sep 2014 11:39:24 +0200 (CEST) Received: from bear.ext.ti.com (bear.ext.ti.com [192.94.94.41]) by alsa0.perex.cz (Postfix) with ESMTP id E64B02616CB for ; Mon, 1 Sep 2014 11:39:15 +0200 (CEST) Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by bear.ext.ti.com (8.13.7/8.13.7) with ESMTP id s819dDVf015294; Mon, 1 Sep 2014 04:39:14 -0500 Received: from DLEE70.ent.ti.com (dlee70.ent.ti.com [157.170.170.113]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id s819dDE2015816; Mon, 1 Sep 2014 04:39:13 -0500 Received: from dlep32.itg.ti.com (157.170.170.100) by DLEE70.ent.ti.com (157.170.170.113) with Microsoft SMTP Server id 14.3.174.1; Mon, 1 Sep 2014 04:39:12 -0500 Received: from [10.1.3.6] (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep32.itg.ti.com (8.14.3/8.13.8) with ESMTP id s819dBai019900; Mon, 1 Sep 2014 04:39:11 -0500 Message-ID: <54043EBE.9020808@ti.com> Date: Mon, 1 Sep 2014 12:39:10 +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: Xiubo Li , , References: <1409294797-21948-1-git-send-email-Li.Xiubo@freescale.com> In-Reply-To: <1409294797-21948-1-git-send-email-Li.Xiubo@freescale.com> Cc: Nicolin Chen , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Kuninori Morimoto , Jean-Francois Moine Subject: Re: [alsa-devel] [RFC][PATCH] ASoC: simple-card: Merge single and muti DAI link code. X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP This patch would break the current syntax without introducing any improvements over it (actually the opposite, see bellow). So in its current form I don't like this patch at all. On 08/29/2014 09:46 AM, Xiubo Li wrote: > This patch merge single DAI link and muti-DAI links code together, > and simply the simple-card driver code. > > And also do some other improvement: > > 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. > But there is no such problem with the current code any more. The new preferred way is to indicate bitclock and frame master with phandles that refer to the master DAI on the link (see the examples bellow). > The old code will cause confusion, and we should be clear that the > letter 'C' here mean to Codec. > > Signed-off-by: Xiubo Li > Cc: Kuninori Morimoto > Cc: Jean-Francois Moine > Cc: Jyri Sarha > Cc: Nicolin Chen > --- > > Hi, > > This patch will break the old DT, so i just send one RFC version, and > will add the old DT patches in next version if this patch can work > well. I do not object in getting rid of this simplified description for a single dailink card: (example 1) sound { compatible = "simple-audio-card"; ... simple-audio-card,format = "left_j"; simple-audio-card,bitclock-master = <&dailink0_master>; simple-audio-card,frame-master = <&dailink0_master>; simple-audio-card,cpu { sound-dai = <&sh_fsi2 0>; }; dailink0_master: simple-audio-card,codec { sound-dai = <&ak4648>; clocks = <&osc>; }; }; and describing the DAI-links always like this: (example 2) sound { compatible = "simple-audio-card"; ... simple-audio-card,dai-link@0 { format = "i2s"; bitclock-master = <&dailink0_master>; frame-master = <&dailink0_master>; dailink0_master: cpu { sound-dai = <&audio1 0>; }; codec { sound-dai = <&tda998x 0>; }; }; }; But I do object going back to the old syntax exclusively and describing the bitclock and frame master always with simple boolean properties, like this: (example 3) sound { compatible = "simple-audio-card"; ... simple-audio-card,dai-link@0 { format = "i2s"; dailink0_master: cpu { sound-dai = <&audio1 0>; }; codec { sound-dai = <&tda998x 0>; bitclock-master; frame-master; }; }; }; In my opinion we should rather get rid of this syntax than move exclusively to it. The syntax of example 2 is simply superior to the syntax of example 3. With the syntax on example 2 it is possible to describe TDM setups where there is multiple codecs on a single i2s bus. The syntax of example 3 simply can not describe that kind of setup. If you need to have the backwards compatibility syntax describing the clock masters to work also in the multilink mode (example 3), then do just that (it is simple enough [1]). But please do not break the new phandle based syntax!!! Best regards, Jyri [1] To enable legacy syntax in multilink mode just apply this patch: dai_props->cpu_dai.fmt = dai_props->codec_dai.fmt = diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 8dd7957..aed3423 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -242,10 +242,10 @@ static int simple_card_dai_link_of(struct device_node *node, if (ret < 0) goto dai_link_of_err; - if (strlen(prefix) && !bitclkmaster && !framemaster) { - /* No dai-link level and master setting was not found from - sound node level, revert back to legacy DT parsing and - take the settings from codec node. */ + if (!bitclkmaster && !framemaster) { + /* No dai-link level master setting was found, revert + back to legacy DT parsing and take the settings + from the codec node boolean properties. */ dev_dbg(dev, "%s: Revert to legacy daifmt parsing\n", __func__);