From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S934405AbbEOIq1 (ORCPT <rfc822;w@1wt.eu>);
	Fri, 15 May 2015 04:46:27 -0400
Received: from mail-wg0-f41.google.com ([74.125.82.41]:36067 "EHLO
	mail-wg0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S934130AbbEOIqW (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 15 May 2015 04:46:22 -0400
Message-ID: <5555B259.10508@linaro.org>
Date: Fri, 15 May 2015 09:46:17 +0100
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0
MIME-Version: 1.0
To: Patrick Lai <plai@codeaurora.org>, Mark Brown <broonie@kernel.org>,
        Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
        Ian Campbell <ijc+devicetree@hellion.org.uk>,
        Kumar Gala <galak@codeaurora.org>,
        Banajit Goswami <bgoswami@codeaurora.org>,
        Liam Girdwood <lgirdwood@gmail.com>, Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.de>, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org,
        linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v1 10/13] ASoC: qcom: Add apq8016 lpass driver support
References: <1431518302-7139-1-git-send-email-srinivas.kandagatla@linaro.org> <1431518586-7709-1-git-send-email-srinivas.kandagatla@linaro.org> <20150515052340.GB31687@kwestfie-linux.qualcomm.com>
In-Reply-To: <20150515052340.GB31687@kwestfie-linux.qualcomm.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Thanks for review,

On 15/05/15 06:23, Kenneth Westfield wrote:
> On Wed, May 13, 2015 at 05:03:06AM -0700, Srinivas Kandagatla wrote:
>> This patch adds apq8016 lpass driver support. APQ8016 has 4 MI2S which
>> can be routed to one internal codec and 2 external codec interfaces.
>>
>> Primary, Secondary, Quaternary I2S can do Rx(playback) and Tertiary and
>> Quaternary can do Tx(capture).
>
>> diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
>> index 865205e..9cc5ed7 100644
>> --- a/sound/soc/qcom/Kconfig
>> +++ b/sound/soc/qcom/Kconfig
>> @@ -20,6 +20,12 @@ config SND_SOC_LPASS_IPQ806X
>>   	select SND_SOC_LPASS_CPU
>>   	select SND_SOC_LPASS_PLATFORM
>>
>> +config SND_SOC_LPASS_APQ8016
>> +	tristate
>> +	depends on SND_SOC_QCOM
>> +	select SND_SOC_LPASS_CPU
>> +	select SND_SOC_LPASS_PLATFORM
>
> Continuing from my comments on patch 2/13, should an OF dependency be added
> here as well?
LPASS_CPU and LPASS_PLATFORM already has this dependency, so there is no 
chance that APQ8016 can be selected without OF.

>
>> +
>>   config SND_SOC_STORM
>>   	tristate "ASoC I2S support for Storm boards"
>>   	depends on (ARCH_QCOM && SND_SOC_QCOM) || COMPILE_TEST
>
>> diff --git a/sound/soc/qcom/lpass-apq8016.c
>> b/sound/soc/qcom/lpass-apq8016.c
>> new file mode 100644
>> index 0000000..5cbf17f0
>> --- /dev/null
>> +++ b/sound/soc/qcom/lpass-apq8016.c
>
>> +static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int
>> chan)
>> +{
>> +	clear_bit(chan, &drvdata->rdma_ch_bit_map);
>> +
>> +	return 0;
>> +}
>> +
>> +static int apq8016_lpass_init(struct platform_device *pdev)
>> +{
>> +	struct lpass_data *drvdata = platform_get_drvdata(pdev);
>> +	struct device *dev = &pdev->dev;
>> +	int ret;
>> +
>> +	drvdata->pcnoc_mport_clk = devm_clk_get(dev, "pcnoc-mport-clk");
>> +	if (IS_ERR(drvdata->pcnoc_mport_clk)) {
>> +		dev_err(&pdev->dev, "%s() error getting pcnoc-mport-clk:
>> %ld\n",
>> +				__func__,
>> PTR_ERR(drvdata->pcnoc_mport_clk));
>> +		return PTR_ERR(drvdata->pcnoc_mport_clk);
>> +	}
>> +
>> +	ret = clk_prepare_enable(drvdata->pcnoc_mport_clk);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "%s() Error enabling ahbix_clk: %d\n",
>
> Please correct the clock name in the log message ...
>
Yep, will fix it.
>> +				__func__, ret);
>> +		return ret;
>> +	}
>> +
>> +	drvdata->pcnoc_sway_clk = devm_clk_get(dev, "pcnoc-sway-clk");
>> +	if (IS_ERR(drvdata->pcnoc_sway_clk)) {
>> +		dev_err(&pdev->dev, "%s() error getting pcnoc-sway-clk:
>> %ld\n",
>> +				__func__,
>> PTR_ERR(drvdata->pcnoc_sway_clk));
>> +		return PTR_ERR(drvdata->pcnoc_sway_clk);
>> +	}
>> +
>> +	ret = clk_prepare_enable(drvdata->pcnoc_sway_clk);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "%s() Error enabling ahbix_clk: %d\n",
>
> ... here too.
>
Sure, I will fix it.

--srini
>> +				__func__, ret);
>> +		return ret;
>> +	}
>