From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Dwivedi, Avaneesh Kumar (avani)" <akdwived@codeaurora.org>
Subject: Re: [RESEND PATCH v4 4/7] remoteproc: qcom: Modify clock enable and
 disable routine
Date: Mon, 12 Dec 2016 13:53:38 +0530
Message-ID: <3cf7f66b-c416-a5b0-6252-cd05a9d083cd@codeaurora.org>
References: <1479981638-32069-1-git-send-email-akdwived@codeaurora.org>
 <1479981638-32069-5-git-send-email-akdwived@codeaurora.org>
 <20161209025726.GR30492@tuxbot>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Return-path: <linux-arm-msm-owner@vger.kernel.org>
Received: from smtp.codeaurora.org ([198.145.29.96]:55014 "EHLO
        smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752600AbcLLIXn (ORCPT
        <rfc822;linux-arm-msm@vger.kernel.org>);
        Mon, 12 Dec 2016 03:23:43 -0500
In-Reply-To: <20161209025726.GR30492@tuxbot>
Sender: linux-arm-msm-owner@vger.kernel.org
List-Id: linux-arm-msm@vger.kernel.org
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: sboyd@codeaurora.org, agross@codeaurora.org, linux-arm-msm@vger.kernel.org



On 12/9/2016 8:27 AM, Bjorn Andersson wrote:
> On Thu 24 Nov 02:00 PST 2016, Avaneesh Kumar Dwivedi wrote:
>
>> Clock handling is made generic than earlier way to add new clock
>> every time when required to support new hexagon version. Also
>> clock disable interface is separated out into two separate routine
>> to unvote proxy clock alone when handover interrupt is arrived.
>>
>> Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org>
>> ---
>>   drivers/remoteproc/qcom_q6v5_pil.c | 93 ++++++++++++++++++++++++++------------
>>   1 file changed, 65 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
>> index 06d5bb2..c55dc9a 100644
>> --- a/drivers/remoteproc/qcom_q6v5_pil.c
>> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
>> @@ -126,10 +126,6 @@ struct q6v5 {
>>   	struct qcom_smem_state *state;
>>   	unsigned stop_bit;
>>   
>> -
>> -	struct clk *ahb_clk;
>> -	struct clk *axi_clk;
>> -	struct clk *rom_clk;
>>   	struct clk *active_clks[8];
>>   	struct clk *proxy_clks[4];
>>   	struct reg_info active_regs[1];
>> @@ -284,6 +280,51 @@ static void q6v5_regulator_disable(struct q6v5 *qproc)
>>   	q6v5_active_regulator_disable(qproc);
>>   }
>>   
>> +static int q6v5_clk_enable(struct device *dev, struct clk **clks,
>> +								int clk_count)
>> +{
>> +	int rc = 0;
> No need to initialize to 0.
OK.
>
>> +	int i;
>> +
>> +	for (i = 0; i < clk_count; i++) {
>> +		rc = clk_prepare_enable(clks[i]);
>> +		if (rc) {
>> +			dev_err(dev, "Clock enable failed\n");
>> +			goto err;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +err:
>> +	for (i--; i >= 0; i--)
>> +		clk_disable_unprepare(clks[i]);
>> +
>> +	return rc;
>> +}
>> +
>> +static void q6v5_proxy_clk_disable(struct q6v5 *qproc)
> Please make these follow the enable case, by taking the clock list and
> size as parameters.
OK.
>
>> +{
>> +	int i;
>> +	struct clk **clks = qproc->proxy_clks;
>> +
>> +	for (i = 0; i < qproc->proxy_clk_count; i++)
>> +		clk_disable_unprepare(clks[i]);
>> +}
>> +
>> +static void q6v5_active_clk_disable(struct q6v5 *qproc)
>> +{
>> +	int i;
>> +	struct clk **clks = qproc->proxy_clks;
>> +
>> +	for (i = 0; i < qproc->proxy_clk_count; i++)
>> +		clk_disable_unprepare(clks[i]);
>> +}
>> +
>> +static void q6v5_clk_disable(struct q6v5 *qproc)
>> +{
>> +	q6v5_proxy_clk_disable(qproc);
>> +	q6v5_active_clk_disable(qproc);
>> +}
> Just inline the two disable calls where you need them, rather than
> having this wrapper.
OK.
>
> Regards,
> Bjorn

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.