From patchwork Sun Aug 10 18:42:23 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jon Smirl X-Patchwork-Id: 4704961 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 5A870C0338 for ; Sun, 10 Aug 2014 18:42:49 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 423AE2018E for ; Sun, 10 Aug 2014 18:42:48 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.kernel.org (Postfix) with ESMTP id C35ED20166 for ; Sun, 10 Aug 2014 18:42:46 +0000 (UTC) Received: by alsa0.perex.cz (Postfix, from userid 1000) id 465D3264EFF; Sun, 10 Aug 2014 20:42:45 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Status: No, score=-1.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=no version=3.3.1 Received: from alsa0.perex.cz (localhost [IPv6:::1]) by alsa0.perex.cz (Postfix) with ESMTP id 56762261B32; Sun, 10 Aug 2014 20:42:34 +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 A99F92625EA; Sun, 10 Aug 2014 20:42:32 +0200 (CEST) Received: from mail-lb0-f170.google.com (mail-lb0-f170.google.com [209.85.217.170]) by alsa0.perex.cz (Postfix) with ESMTP id 7617E261B31 for ; Sun, 10 Aug 2014 20:42:24 +0200 (CEST) Received: by mail-lb0-f170.google.com with SMTP id l4so3329955lbv.15 for ; Sun, 10 Aug 2014 11:42:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=ozi7yr2PxpJsidbnbda5Ih4nxqK7AiQOlkZiZIJzMd4=; b=pnwj3YD5+EeQ7DhQmJIiAaHLofRKUH5yaj4kgDfpJ9Kl5TrFHpHX/9hsATGjRq1HNT 4atarLCNq3hrtE7lH+nu1/JrEGr20GnJ7BAyF8UvAol3Ft/tmJrc+6W6vPTvnc3br2qB sZRrHTM4eNM+NokgCjwS9CKEsSWFWifJ3/KlB1SGmOkLdQw0i43E6zglE/LPBEvW+GkD BCBLsMvcMyGNX5MlV8lvysM5v8epM6yyJjzLE2LeqHgkq+zZVorW84VYyl7dSI92K3Jf zRUG47cNW5DfJoLmd27Om2KYbxwrADL7b21RS5ZhmKMxKydrWmwm7PBTfJG1w+RAlOYQ dQCA== MIME-Version: 1.0 X-Received: by 10.152.36.73 with SMTP id o9mr3529444laj.88.1407696143891; Sun, 10 Aug 2014 11:42:23 -0700 (PDT) Received: by 10.112.40.167 with HTTP; Sun, 10 Aug 2014 11:42:23 -0700 (PDT) In-Reply-To: References: <20140810105405.GW17528@sirena.org.uk> Date: Sun, 10 Aug 2014 14:42:23 -0400 Message-ID: From: "jonsmirl@gmail.com" To: Mark Brown Cc: alsa-devel mailing list , Lars-Peter Clausen , Liam Girdwood , zengzm.kernel@gmail.com, Kuninori Morimoto Subject: Re: [alsa-devel] Choosing the sysclk in simple-card looks broken to me. 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 should fix it to use the output clocks, but does it break any of the other DTS using simple-audio? On Sun, Aug 10, 2014 at 8:34 AM, jonsmirl@gmail.com wrote: > On Sun, Aug 10, 2014 at 6:54 AM, Mark Brown wrote: >> On Sat, Aug 09, 2014 at 08:08:45PM -0400, jonsmirl@gmail.com wrote: >> >>> The problem is in asoc_simple_card_sub_parse_of() >>> in the else case.. >> >>> } else { >>> clk = of_clk_get(node, 0); >>> if (!IS_ERR(clk)) >>> dai->sysclk = clk_get_rate(clk); >>> } >> >>> This is picking up the input clocks to the devices. On the cpu-dai >>> you want to pick up the ouput mclk (my first input clock is an 80Mhz >>> bus clock). My IIS output clock is: clock-output-names = "mclk0"; >> >>> Setting clocks="" doesn't work either, it also picks up the input clock. >> >> I'm sorry but I'm having an awfully hard time working out what you are >> trying to say in this mail. When you say things like "you want to pick >> up the ouput mclk" it's really not clear what any of that means - why do >> you want to pick it and what is it being picked for? > > Inside simple audio card, __asoc_simple_card_dai_init() calls > snd_soc_dai_set_sysclk(). So where is it getting the clock values used > in __asoc_simple_card_dai_init()? > > Here is my DTS for my cpu-dai... > > iis0: iis@01c22400 { > #clock-cells = <0>; > #sound-dai-cells = <0>; > compatible = "allwinner,sun7i-a20-iis"; > reg = <0x01C22400 0x40>; > interrupts = <0 16 4>; > clocks = <&apb0_gates 3>, <&i2s0_clk>; > clock-names = "apb", "iis"; > clock-output-names = "mclk0"; > dmas = <&dma 0 3>, <&dma 0 3>; > dma-names = "rx", "tx"; > status = "disabled"; > }; > > This unit has three clocks. > apb - bus input 80Mhz to run the unit > iis - a programable input from a system PLL > mclk0 - a clock that is output to the MCLK pin > > This is my codec > > sgtl5000: sgtl5000@a { > compatible = "fsl,sgtl5000"; > reg = <0x0a>; > clocks = <&iis0>; > > #sound-dai-cells = <0>; > VDDA-supply = <®_vcc3v3>; > VDDIO-supply = <®_vcc3v3>; > }; > > It has single input clock > clocks (unnamed) - it is wired to the MCLK pin > > My simple audio card section... > > sound { > compatible = "simple-audio-card"; > simple-audio-card,format = "i2s"; > simple-audio-card,mclk-fs = <256>; > > simple-audio-card,cpu { > sound-dai = <&iis0>; > }; > > simple-audio-card,codec { > sound-dai = <&sgtl5000>; > }; > }; > > Now look at what simple_card_dai_link_of() does. It parse this section > and then calls asoc_simple_card_sub_parse_of() for each of the two > DAI's. > > Now look look at what asoc_simple_card_sub_parse_of() does when it > parses each of those subnodes. It looks up the first input clock off > from both of those nodes and saves it as dai->sysclk. Then in > __asoc_simple_card_dai_init() it pokes those values back into my > driver via snd_soc_dai_set_sysclk(). > > So on the first node. > simple-audio-card,cpu { > sound-dai = <&iis0>; > }; > It goes over to the iis driver and gets rate from the first input > clock for iis0 - apb (80Mhz) and sets this back into my driver via > snd_soc_dai_set_sysclk(). > > On the second node > simple-audio-card,codec { > sound-dai = <&sgtl5000>; > }; > Then it does a getrate() on the first input clock for sgtl5000. But > that clock has not been initialized yet. And then it sets the value > back in via snd_soc_dai_set_sysclk(). > > ------ > > The problem is that asoc_simple_card_sub_parse_of() is doing getrate() > on input clocks when it should be doing it on output clocks. > > So in my case, iis has a single output clock 'mclk0'. simple can > getrate() on it and then set the rate back in via > snd_soc_dai_set_sysclk(). Codec has no output clocks so it does > nothing with sysclk. > > > > > >> >>> But then simple goes and tries to set both system clocks - my 80Mhz >>> bus clock and the 22.5 true sysclk. And my cpu-dai barfs when it is >>> passed a 80Mhz sysclk. >> >> Naturally... this does suggest you're doing something wrong but like I >> say I really don't understand what's going on. What does your system >> look like, what are you trying to get it to do and how are you going >> about that? > > > > -- > Jon Smirl > jonsmirl@gmail.com diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 03a7fdc..8c4b267 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -116,6 +116,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np, { struct device_node *node; struct clk *clk; + const char *clk_name = NULL; int ret; /* @@ -156,11 +157,14 @@ asoc_simple_card_sub_parse_of(struct device_node *np, "system-clock-frequency", &dai->sysclk); } else { - clk = of_clk_get(node, 0); - if (!IS_ERR(clk)) - dai->sysclk = clk_get_rate(clk); + of_property_read_string(node, "clock-output-names", &clk_name); + if (clk_name) { + clk = of_clk_get_by_name(node, clk_name); + if (!IS_ERR(clk)) { + dai->sysclk = clk_get_rate(clk); + } + } } - return 0; }