Received: by 2002:a25:8b91:0:0:0:0:0 with SMTP id j17csp11407571ybl;
        Fri, 27 Dec 2019 13:26:41 -0800 (PST)
X-Google-Smtp-Source: APXvYqxdi09TyCtvtuV0i9Y57GB9E9A8ifTjzzajn/Nl9dW9uOftBORa/ZUT4Ij33ZeuDRn17699
X-Received: by 2002:a9d:5545:: with SMTP id h5mr2232936oti.296.1577482001176;
        Fri, 27 Dec 2019 13:26:41 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1577482001; cv=none;
        d=google.com; s=arc-20160816;
        b=iXJgKokztpW5AivXbrkk3G4SK6hZx9c0Y37atJKEMICMCre55j4zyNsvFdC0TPIae+
         CsEGOUdBAP8h+kw1p0gzuPVSDwMIgpfHtyfXQyM+c9y6veMsgt5mF3ilGx/Sbid1UDQQ
         9phZ9rjrL59R6OKYzgvRnChm4A/zTC4bX0z2x+fv6z4P6r8Yajj4YMOqkKBfPPMFvRWk
         xm/xcDfJJsC4O7e9gC2subBovDJ3fQU7OXzUl61XQ7KD4l/PSLMOpkOUCZ8Zl9kUUw+X
         u25ypG4g8UvsgiQUDSI7iyKyFoCncUiAAcBShui7kA12SJJkGb11R1A2eeVYu/b7/WBy
         P4iw==
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=NhLysUMDR5A8pJ/qWwjX71WWrzvkEcKkywTqxVpwI/M=;
        b=kGNeVe8zuBTg5/h+ir92dr00QU8ldw3Vd2NpMaYWaZDA/a7fTE9scJLMhHUoC2NUCl
         iJKyPmj9PoAHfubV1zr7+Cf3gDHB0lag3A5VGOxZSlj2/LeTyGq46+EuURprkUKSH/K3
         2EBtxI9hHZj0RbisTqw4MBZZplqWFCZgOhwoSvG49wkFoRZriHGvTkeWZSFtZ/hf2Tiw
         WewZ9v2ba9Y4yxlIrnMktJ5RXDduSbqmBRGnQWeE0+dXH3ueGVLbmD2NnsSgDaLJuLtS
         uDMo6+knyW1GCnqu7X7b203YQ1qY9d3kPFStWmQOQWUsIAO7Jhp9famrgD8inqrnwktw
         lV3g==
ARC-Authentication-Results: i=1; mx.google.com;
       dkim=pass header.i=@nvidia.com header.s=n1 header.b=AzMye6NQ;
       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: <linux-kernel-owner@vger.kernel.org>
Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67])
        by mx.google.com with ESMTP id l3si15442554oih.0.2019.12.27.13.26.26;
        Fri, 27 Dec 2019 13:26:41 -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=AzMye6NQ;
       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 S1726562AbfL0VZ2 (ORCPT <rfc822;tanxianhu@gmail.com> + 99 others);
        Fri, 27 Dec 2019 16:25:28 -0500
Received: from hqnvemgate26.nvidia.com ([216.228.121.65]:7134 "EHLO
        hqnvemgate26.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1726165AbfL0VZ2 (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 27 Dec 2019 16:25:28 -0500
Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate26.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA)
        id <B5e0676b90001>; Fri, 27 Dec 2019 13:25:13 -0800
Received: from hqmail.nvidia.com ([172.20.161.6])
  by hqpgpgate101.nvidia.com (PGP Universal service);
  Fri, 27 Dec 2019 13:25:26 -0800
X-PGP-Universal: processed;
        by hqpgpgate101.nvidia.com on Fri, 27 Dec 2019 13:25:26 -0800
Received: from [10.2.173.37] (10.124.1.5) by HQMAIL107.nvidia.com
 (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 27 Dec
 2019 21:25:25 +0000
Subject: Re: [PATCH v5 12/19] ASoC: tegra: Add initial parent configuration
 for audio mclk
To:     Dmitry Osipenko <digetx@gmail.com>, <thierry.reding@gmail.com>,
        <jonathanh@nvidia.com>, <broonie@kernel.org>,
        <lgirdwood@gmail.com>, <perex@perex.cz>, <tiwai@suse.com>,
        <mperttunen@nvidia.com>, <gregkh@linuxfoundation.org>,
        <sboyd@kernel.org>, <robh+dt@kernel.org>, <mark.rutland@arm.com>
CC:     <pdeschrijver@nvidia.com>, <pgaikwad@nvidia.com>,
        <spujar@nvidia.com>, <josephl@nvidia.com>,
        <daniel.lezcano@linaro.org>, <mmaddireddy@nvidia.com>,
        <markz@nvidia.com>, <devicetree@vger.kernel.org>,
        <linux-clk@vger.kernel.org>, <linux-tegra@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <1576880825-15010-1-git-send-email-skomatineni@nvidia.com>
 <1576880825-15010-13-git-send-email-skomatineni@nvidia.com>
 <a6567ff1-7bc2-3ca5-1200-92a63eb44ddb@gmail.com>
 <1b79ab92-c163-3857-dd38-df35c509b823@gmail.com>
From:   Sowjanya Komatineni <skomatineni@nvidia.com>
Message-ID: <4fd01e04-8240-8562-467d-324f7adbdbdf@nvidia.com>
Date:   Fri, 27 Dec 2019 13:25:25 -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: <1b79ab92-c163-3857-dd38-df35c509b823@gmail.com>
X-Originating-IP: [10.124.1.5]
X-ClientProxiedBy: HQMAIL101.nvidia.com (172.20.187.10) 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=1577481914; bh=NhLysUMDR5A8pJ/qWwjX71WWrzvkEcKkywTqxVpwI/M=;
        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=AzMye6NQ2mWSVw3lnFblMI2o1tRqdDsGzIQXAbl+KbAbqFfj4spzqLF3+ETqh1SYI
         2in6f1xBxlvgk1GKSu6t4Cpe5Eha6HRHcXU1lYvg+x2xr++OpprB883qu//xaWhxZa
         9ziERtL6eXznaJhgA+sd8jZDwbY+39G2/oc1pPSQgbJLMzX7k0TH3p8VxHOml5/kfI
         KMaoTtd7XpGSHXNpIOwg/fGaQNmfDhVVzu2hhdCp20WSz2zaZYMdXRSjVD/pFt3560
         kxl/5mniINbFNPO1U8uG8wDfyyiAYAuw6Y/6qmRP2gYHrwVVVSoAm2EtLuKahkRF5i
         YH/5JxDyQ110A==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 12/22/19 1:18 PM, Dmitry Osipenko wrote:
> 23.12.2019 00:14, Dmitry Osipenko =D0=BF=D0=B8=D1=88=D0=B5=D1=82:
>> 21.12.2019 01:26, Sowjanya Komatineni =D0=BF=D0=B8=D1=88=D0=B5=D1=82:
>>> Tegra PMC clock clk_out_1 is dedicated for audio mclk from Tegra30
>>> through Tegra210 and currently Tegra clock driver does initial parent
>>> configuration for audio mclk "clk_out_1" and enables them by default.
>>>
>>> With the move of Tera PMC clocks from clock driver to Tegra PMC
>>> driver, initial parent configuration for audio clocks are through
>>> the device tree using assigned-clock-parents property.
>>>
>>> Default clock parents can be specified in device tree using
>>> assigned-clocks and assigned-clock-parents and there is no need
>>> to have clock driver do parent configuration and enable audio related
>>> clocks.
>>>
>>> This patch has implementation for initial parent configuration in
>>> audio driver when default parent configuration is not specified in the
>>> device tree using assigned-clock properties and enables audio clocks
>>> during the clock rate change.
>>>
>>> This patch configures PLLA_OUT0 as parent to extern1 and extern1
>>> as parent to clk_out_1 and uses clk_out_1 as cdev1 clock to allow
>>> mclk control from this driver.
>>>
>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>> ---
>>>   sound/soc/tegra/tegra_asoc_utils.c | 71 ++++++++++++++++++++++-------=
---------
>>>   1 file changed, 41 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/sound/soc/tegra/tegra_asoc_utils.c b/sound/soc/tegra/tegra=
_asoc_utils.c
>>> index 38535962029c..fc3135c08f43 100644
>>> --- a/sound/soc/tegra/tegra_asoc_utils.c
>>> +++ b/sound/soc/tegra/tegra_asoc_utils.c
>>> @@ -7,6 +7,7 @@
>>>    */
>>>  =20
>>>   #include <linux/clk.h>
>>> +#include <linux/clk-provider.h>
>> This is illegal, it is not a clock provider.
>>
__clk_is_enabled API is used in this patch to disable clock only when=20
its enabled.

__clk_is_enabled API is from clk-provider.h

>>>   #include <linux/device.h>
>>>   #include <linux/err.h>
>>>   #include <linux/kernel.h>
>>> @@ -59,9 +60,8 @@ int tegra_asoc_utils_set_rate(struct tegra_asoc_utils=
_data *data, int srate,
>>>   	data->set_baseclock =3D 0;
>>>   	data->set_mclk =3D 0;
>>>  =20
>>> -	clk_disable_unprepare(data->clk_cdev1);
>>> -	clk_disable_unprepare(data->clk_pll_a_out0);
>>> -	clk_disable_unprepare(data->clk_pll_a);
>>> +	if (__clk_is_enabled(data->clk_cdev1))
>>> +		clk_disable_unprepare(data->clk_cdev1);
>> The root of the problem is that you removed clocks enabling from
>> tegra_asoc_utils_init().
>>
>> I'm not sure why clocks should be disabled during the rate-changing,
>> probably this action is not really needed.
>>
>> diff --git a/sound/soc/tegra/tegra_asoc_utils.c
>> b/sound/soc/tegra/tegra_asoc_utils.c
>> index 46ff70c16b74..789fd03e51a7 100644
>> --- a/sound/soc/tegra/tegra_asoc_utils.c
>> +++ b/sound/soc/tegra/tegra_asoc_utils.c
>> @@ -7,7 +7,6 @@
>>    */
>>
>>   #include <linux/clk.h>
>> -#include <linux/clk-provider.h>
>>   #include <linux/device.h>
>>   #include <linux/err.h>
>>   #include <linux/kernel.h>
>> @@ -60,9 +59,6 @@ int tegra_asoc_utils_set_rate(struct
>> tegra_asoc_utils_data *data, int srate,
>>   	data->set_baseclock =3D 0;
>>   	data->set_mclk =3D 0;
>>
>> -	if (__clk_is_enabled(data->clk_cdev1))
>> -		clk_disable_unprepare(data->clk_cdev1);
>> -
>>   	err =3D clk_set_rate(data->clk_pll_a, new_baseclock);
>>   	if (err) {
>>   		dev_err(data->dev, "Can't set pll_a rate: %d\n", err);
>> @@ -77,12 +73,6 @@ int tegra_asoc_utils_set_rate(struct
>> tegra_asoc_utils_data *data, int srate,
>>
>>   	/* Don't set cdev1/extern1 rate; it's locked to pll_a_out0 */
>>
>> -	err =3D clk_prepare_enable(data->clk_cdev1);
>> -	if (err) {
>> -		dev_err(data->dev, "Can't enable cdev1: %d\n", err);
>> -		return err;
>> -	}
>> -
>>   	data->set_baseclock =3D new_baseclock;
>>   	data->set_mclk =3D mclk;
>>
>> @@ -96,9 +86,6 @@ int tegra_asoc_utils_set_ac97_rate(struct
>> tegra_asoc_utils_data *data)
>>   	const int ac97_rate =3D 24576000;
>>   	int err;
>>
>> -	if (__clk_is_enabled(data->clk_cdev1))
>> -		clk_disable_unprepare(data->clk_cdev1);
>> -
>>   	/*
>>   	 * AC97 rate is fixed at 24.576MHz and is used for both the host
>>   	 * controller and the external codec
>> @@ -117,12 +104,6 @@ int tegra_asoc_utils_set_ac97_rate(struct
>> tegra_asoc_utils_data *data)
>>
>>   	/* Don't set cdev1/extern1 rate; it's locked to pll_a_out0 */
>>
>> -	err =3D clk_prepare_enable(data->clk_cdev1);
>> -	if (err) {
>> -		dev_err(data->dev, "Can't enable cdev1: %d\n", err);
>> -		return err;
>> -	}
>> -
>>   	data->set_baseclock =3D pll_rate;
>>   	data->set_mclk =3D ac97_rate;
>>
>> @@ -213,6 +194,12 @@ int tegra_asoc_utils_init(struct
>> tegra_asoc_utils_data *data,
>>   		data->clk_cdev1 =3D clk_out_1;
>>   	}
>>
>> +	ret =3D clk_prepare_enable(data->clk_cdev1);
>> +	if (ret) {
>> +		dev_err(data->dev, "Can't enable cdev1: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>>   	ret =3D tegra_asoc_utils_set_rate(data, 44100, 256 * 44100);
>>
>>   	return ret;
>>
>>
>>>   	err =3D clk_set_rate(data->clk_pll_a, new_baseclock);
>>>   	if (err) {
>>> @@ -77,18 +77,6 @@ int tegra_asoc_utils_set_rate(struct tegra_asoc_util=
s_data *data, int srate,
>>>  =20
>>>   	/* Don't set cdev1/extern1 rate; it's locked to pll_a_out0 */
>>>  =20
>>> -	err =3D clk_prepare_enable(data->clk_pll_a);
>>> -	if (err) {
>>> -		dev_err(data->dev, "Can't enable pll_a: %d\n", err);
>>> -		return err;
>>> -	}
>>> -
>>> -	err =3D clk_prepare_enable(data->clk_pll_a_out0);
>>> -	if (err) {
>>> -		dev_err(data->dev, "Can't enable pll_a_out0: %d\n", err);
>>> -		return err;
>>> -	}
>>> -
>>>   	err =3D clk_prepare_enable(data->clk_cdev1);
>>>   	if (err) {
>>>   		dev_err(data->dev, "Can't enable cdev1: %d\n", err);
>>> @@ -108,9 +96,8 @@ int tegra_asoc_utils_set_ac97_rate(struct tegra_asoc=
_utils_data *data)
>>>   	const int ac97_rate =3D 24576000;
>>>   	int err;
>>>  =20
>>> -	clk_disable_unprepare(data->clk_cdev1);
>>> -	clk_disable_unprepare(data->clk_pll_a_out0);
>>> -	clk_disable_unprepare(data->clk_pll_a);
>>> +	if (__clk_is_enabled(data->clk_cdev1))
>>> +		clk_disable_unprepare(data->clk_cdev1);
>>>  =20
>>>   	/*
>>>   	 * AC97 rate is fixed at 24.576MHz and is used for both the host
>>> @@ -130,18 +117,6 @@ int tegra_asoc_utils_set_ac97_rate(struct tegra_as=
oc_utils_data *data)
>>>  =20
>>>   	/* Don't set cdev1/extern1 rate; it's locked to pll_a_out0 */
>>>  =20
>>> -	err =3D clk_prepare_enable(data->clk_pll_a);
>>> -	if (err) {
>>> -		dev_err(data->dev, "Can't enable pll_a: %d\n", err);
>>> -		return err;
>>> -	}
>>> -
>>> -	err =3D clk_prepare_enable(data->clk_pll_a_out0);
>>> -	if (err) {
>>> -		dev_err(data->dev, "Can't enable pll_a_out0: %d\n", err);
>>> -		return err;
>>> -	}
>>> -
>>>   	err =3D clk_prepare_enable(data->clk_cdev1);
>>>   	if (err) {
>>>   		dev_err(data->dev, "Can't enable cdev1: %d\n", err);
>>> @@ -158,6 +133,7 @@ EXPORT_SYMBOL_GPL(tegra_asoc_utils_set_ac97_rate);
>>>   int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data,
>>>   			  struct device *dev)
>>>   {
>>> +	struct clk *clk_out_1, *clk_extern1;
>>>   	int ret;
>>>  =20
>>>   	data->dev =3D dev;
>>> @@ -193,6 +169,41 @@ int tegra_asoc_utils_init(struct tegra_asoc_utils_=
data *data,
>>>   		return PTR_ERR(data->clk_cdev1);
>>>   	}
>>>  =20
>>> +	/*
>>> +	 * If clock parents are not set in DT, configure here to use clk_out_=
1
>>> +	 * as mclk and extern1 as parent for Tegra30 and higher.
>>> +	 */
>>> +	if (!of_find_property(dev->of_node, "assigned-clock-parents", NULL) &=
&
>>> +	    data->soc > TEGRA_ASOC_UTILS_SOC_TEGRA20) {
>> Please add a message here about falling back to configuring clocks for a
>> legacy device-tree, telling that device-tree needs to be updated.
>>
Will add in v6
>>> +		clk_extern1 =3D devm_clk_get(dev, "extern1");
>>> +		if (IS_ERR(clk_extern1)) {
>>> +			dev_err(data->dev, "Can't retrieve clk extern1\n");
>>> +			return PTR_ERR(clk_extern1);
>>> +		}
>>> +
>>> +		ret =3D clk_set_parent(clk_extern1, data->clk_pll_a_out0);
>>> +		if (ret < 0) {
>>> +			dev_err(data->dev,
>>> +				"Set parent failed for clk extern1\n");
>>> +			return ret;
>>> +		}
>>> +
>>> +		clk_out_1 =3D devm_clk_get(dev, "clk_out_1");
>>> +		if (IS_ERR(clk_out_1)) {
>>> +			dev_err(data->dev, "Can't retrieve clk clk_out_1\n");
>>> +			return PTR_ERR(clk_out_1);
>>> +		}
>>> +
>>> +		ret =3D clk_set_parent(clk_out_1, clk_extern1);
>>> +		if (ret < 0) {
>>> +			dev_err(data->dev,
>>> +				"Set parent failed for clk_out_1\n");
>>> +			return ret;
>>> +		}
>>> +
>>> +		data->clk_cdev1 =3D clk_out_1;
>>> +	}
>>> +
>>>   	ret =3D tegra_asoc_utils_set_rate(data, 44100, 256 * 44100);
> Actually, this tegra_asoc_utils_set_rate() should be removed because it
> doesn't do anything useful since drivers configure the clock rate when
> necessary.
>
>>>   	return ret;
>>>
>> I'd also add tegra_asoc_utils_deinit() to disable clock on drivers remov=
al.
Will add this in v6