Received: by 2002:a25:8b91:0:0:0:0:0 with SMTP id j17csp1266816ybl; Fri, 10 Jan 2020 15:16:01 -0800 (PST) X-Google-Smtp-Source: APXvYqyizcxN3XtwIicA/8RdMZP8/j0yUPF1yBqUlqsuwALQHe4a4QG4GOhbXrWI4v/jNdofV+Qt X-Received: by 2002:a9d:3d0a:: with SMTP id a10mr4720445otc.327.1578698161121; Fri, 10 Jan 2020 15:16:01 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1578698161; cv=none; d=google.com; s=arc-20160816; b=RXpI6foDef/8N4SmVQyfU0i8afo3XqYNvapgVQsESKoYLY1q6xXqnqJrBM5D1vNzZi ITdum7kb4oe8aj8gQyPFFWeOV6kD+1cCSjoUn857rHj0XaBQhORowxeDbJkg2uCGNr1U vSQpbVO+8OSC3PYp3q7CKwEitVsFjLxvD47SYyxIqzrZUt8FoAXVlEvl7y3in2I05/k5 FM3gUN+kDPzDu9ZIVgAykrseExJJmrvDU9mQQkLXV1UPzzsCZ/awJ1rMd0zewmIKz3fy PZ38jF1w/n2lXhwiQl+X+xLFRgDpsK/e0ZxAYS7FvgJkmvzHHtN2BUvm/jM2Y3sMuXmx C+Iw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:dkim-signature:content-language :content-transfer-encoding:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=u0UQrhfGeSXOXRblMCVznbZgOmAsjauC4YT/s6lv+3I=; b=KkzxhXhHba1XMubWFTSUTi2dzvDKnynJ7lBgmZ2/UdsBd7NxG/a9ON3BL8Alo8zOze 2Vi3VYnjp7Hyb1hhn70aSY7e0wbxJuu0e3fYxzr6J2T1NBeagUIkOflUTUfTVzBHAfki 0YWCVrLeZKEkaT1rPRn6XMtPVY7hy4k1gS/qlJY5vQBBGR1//u+X2wW/ZsLP8FFMmiAr Eq89OM6ibUtP5UcAfjAxZU0KaGoTqPW9jg4dxKbE9kP5mv7vqpaQiUECqrBrIW5xVulP BYuQznPFvpQCB0IxaadfaaVWLhqp1Dd19Y5706SNlMSri4qHVzX0unB7wTKu9e5KdqZD +bAw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@nvidia.com header.s=n1 header.b=FhlWPsFo; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id w15si2168756otm.263.2020.01.10.15.15.47; Fri, 10 Jan 2020 15:16:01 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@nvidia.com header.s=n1 header.b=FhlWPsFo; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727470AbgAJXOu (ORCPT + 99 others); Fri, 10 Jan 2020 18:14:50 -0500 Received: from hqnvemgate24.nvidia.com ([216.228.121.143]:1800 "EHLO hqnvemgate24.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727324AbgAJXOu (ORCPT ); Fri, 10 Jan 2020 18:14:50 -0500 Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate24.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Fri, 10 Jan 2020 15:13:58 -0800 Received: from hqmail.nvidia.com ([172.20.161.6]) by hqpgpgate101.nvidia.com (PGP Universal service); Fri, 10 Jan 2020 15:14:49 -0800 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Fri, 10 Jan 2020 15:14:49 -0800 Received: from [10.2.160.1] (10.124.1.5) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 10 Jan 2020 23:14:48 +0000 Subject: Re: [PATCH v7 15/21] ASoC: tegra: Add fallback implementation for audio mclk To: Dmitry Osipenko , Sameer Pujar , , , , , , , , , , , CC: , , , , , , , , , References: <1578457515-3477-1-git-send-email-skomatineni@nvidia.com> <1578457515-3477-16-git-send-email-skomatineni@nvidia.com> <745b8c7b-4fe3-c9ea-284e-b89546e8ad87@nvidia.com> <705edf9b-d1bc-8090-017e-fa4ad445f9fb@nvidia.com> <135f0c0b-86d1-9b1a-af02-c14c4b5308c4@gmail.com> <575aa30e-1b5a-2a2d-5893-3f6832f416f1@nvidia.com> <9bca6c3e-bfe0-7130-b233-3f25c436f76e@gmail.com> From: Sowjanya Komatineni Message-ID: Date: Fri, 10 Jan 2020 15:14:47 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: <9bca6c3e-bfe0-7130-b233-3f25c436f76e@gmail.com> X-Originating-IP: [10.124.1.5] X-ClientProxiedBy: HQMAIL111.nvidia.com (172.20.187.18) To HQMAIL107.nvidia.com (172.20.187.13) Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: quoted-printable Content-Language: en-US DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1578698038; bh=u0UQrhfGeSXOXRblMCVznbZgOmAsjauC4YT/s6lv+3I=; h=X-PGP-Universal:Subject:To:CC:References:From:Message-ID:Date: User-Agent:MIME-Version:In-Reply-To:X-Originating-IP: X-ClientProxiedBy:Content-Type:Content-Transfer-Encoding: Content-Language; b=FhlWPsFobB3l+M+5f6REuvNqb/Zt8Sys2EVmyPFbVa3H0Q8QoQUivYd8nbHJIFcvn C4CsxNXXBkrOmR7uLrDTYBf9j9+zzNcI1IK8tOVYsvPFr2bbosfBiy2ZFx1vDrrqSA DeE5ENz7x+6CQpZLp4pntzsrOXpb6kyWNu4SBJrRaKc+WvztS5jZyZTtx9vjaZpE3S tZbHwKq+/NxWLVvoGBQPFUCfgpU458fEKgxekOMnCzs5T8FtDyZgpM/lew9Sqe7T+J GseV6t5YT60kZ0p8ZD5DsK75HuYD5dWn3JRdpjlM5GRELniOlhm2KVuKpV9A6OLI+t De7DQkEWvCidw== Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 1/10/20 3:02 PM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 11.01.2020 01:13, Sowjanya Komatineni =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> On 1/10/20 2:05 PM, Dmitry Osipenko wrote: >>> External email: Use caution opening links or attachments >>> >>> >>> 10.01.2020 20:04, Sowjanya Komatineni =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>>> On 1/9/20 10:52 AM, Sowjanya Komatineni wrote: >>>>> On 1/7/20 10:28 PM, Sameer Pujar wrote: >>>>>> On 1/8/2020 11:18 AM, Sowjanya Komatineni wrote: >>>>>>> On 1/7/20 9:34 PM, Sameer Pujar wrote: >>>>>>>> On 1/8/2020 9:55 AM, Sowjanya Komatineni wrote: >>>>>>>>> mclk is from clk_out_1 which is part of Tegra PMC block and pmc >>>>>>>>> clocks >>>>>>>>> are moved to Tegra PMC driver with pmc as clock provider and usin= g >>>>>>>>> pmc >>>>>>>>> clock ids. >>>>>>>>> >>>>>>>>> New device tree uses clk_out_1 from pmc clock provider. >>>>>>>>> >>>>>>>>> So, this patch adds implementation for mclk fallback to extern1 >>>>>>>>> when >>>>>>>>> retrieving mclk returns -ENOENT to be backward compatible of new >>>>>>>>> device >>>>>>>>> tree with older kernels. >>>>>>>>> >>>>>>>>> Tested-by: Dmitry Osipenko >>>>>>>>> Reviewed-by: Dmitry Osipenko >>>>>>>>> Signed-off-by: Sowjanya Komatineni >>>>>>>>> --- >>>>>>>>> sound/soc/tegra/tegra_asoc_utils.c | 11 ++++++++++- >>>>>>>>> 1 file changed, 10 insertions(+), 1 deletion(-) >>>>>>>>> >>>>>>>>> diff --git a/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>>> b/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>>> index 9cfebef74870..9a5f81039491 100644 >>>>>>>>> --- a/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>>> +++ b/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>>> @@ -183,7 +183,16 @@ int tegra_asoc_utils_init(struct >>>>>>>>> tegra_asoc_utils_data *data, >>>>>>>>> data->clk_cdev1 =3D devm_clk_get(dev, "mclk"); >>>>>>>>> if (IS_ERR(data->clk_cdev1)) { >>>>>>>>> dev_err(data->dev, "Can't retrieve clk cdev1\n"); >>>>>>>> This error print can be moved inside below if, when this actually >>>>>>>> meant to be an error condition. >>>>>>>> >>>>>>> Want to show error even if mclk retrieval returns ENOENT to clearly >>>>>>> indicate mclk does not exist along with message of falling back to >>>>>>> extern1. >>>>>> Yes, but falling back essentially means 'mclk' is not available and >>>>>> fallback print is not an error. >>>>>> Not a major issue though, you can consider updating. Otherwise LGTM. >>>>>> >>>>> Will update >>>>>>>>> - return PTR_ERR(data->clk_cdev1); >>>>>>>>> + if (PTR_ERR(data->clk_cdev1) !=3D -ENOENT) >>>>>>>>> + return PTR_ERR(data->clk_cdev1); >>>>>>>>> + /* Fall back to extern1 */ >>>>>>>>> + data->clk_cdev1 =3D devm_clk_get(dev, "extern1"); >>>>>>>>> + if (IS_ERR(data->clk_cdev1)) { >>>>>>>>> + dev_err(data->dev, "Can't retrieve clk extern1\n"); >>>>>>>>> + return PTR_ERR(data->clk_cdev1); >>>>>>>>> + } >>>>>>>>> + >>>>>>>>> + dev_err(data->dev, "Falling back to extern1\n"); >>>>>>>> This can be a info print? >>>>> Will use dev_info >>>>>>>>> } >>>>>>>>> /* >>>>> Dmitry/Rob, there was discussion in v3 regarding backporting mclk >>>>> fallback. >>>>> >>>>> Dmitry wanted Rob to confirm on this >>>>> >>>>> I think openSUSE Leap could be one of those distros that use LTS kern= el >>>>> with newer device-trees, but that's not 100%. Maybe Rob could help >>>>> clarifying that. >>>>> >>>>> Dmitry/Rob, Can you please confirm if mclk fallback patch need >>>>> backport to have new device tree work with old kernels? >>>>> >>>> Dmitry, >>>> >>>> Can you please confirm if we need to backport this mclk fallback patch= ? >>>> >>> Seems only T210 was making use of the CaR's TEGRA*_CLK_CLK_OUT_*, thus >>> the backporting isn't needed. >> Thanks Dmitry >>> Also, please use 'git rebase --exec make ...' to make sure that all >>> patches are compiling without problems. The removal of the legacy clock >>> IDs should be done after the device-trees changes, otherwise it looks >>> like DTBs compilation will fail. It's possible that the order of the >>> patches could be changed if Thierry will chose to split this series int= o >>> several pull requests, nevertheless all patches should compile and work >>> in the original order. >> OK, Will move patches of device tree updates to use new DT ID prior to >> removal of old ID. > Oh, wait. But then the newer device-trees won't work with the stable > kernels, so it actually won't hurt to backport this change. ok will add Fixes tag to have this mclk fallback patch backported. > > Secondly, please move the "Use device managed resource APIs to get the > clock" after the ASoC patches with the stable tags, such that the stable > patches could be applied cleanly. OK > > Lastly, please separate the assigned-clocks change from the the audio > mclk enable/disable into a standalone patch. These changes are not > interdependent, unless I'm missing something. But parent configuration when assigned-clock-parents are not in DT are=20 needed along with mclk enable as we are removing audio clocks parent configuration and enabling them=20 together from clock driver. So doesn't both parent configuration and enabling mclk together need to=20 be in same patch to match what we are removing from clock driver?