From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bjorn Andersson <bjorn.andersson@linaro.org>
Subject: Re: [RESEND PATCH v4 4/7] remoteproc: qcom: Modify clock enable and
 disable routine
Date: Thu, 8 Dec 2016 18:57:26 -0800
Message-ID: <20161209025726.GR30492@tuxbot>
References: <1479981638-32069-1-git-send-email-akdwived@codeaurora.org>
 <1479981638-32069-5-git-send-email-akdwived@codeaurora.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Return-path: <linux-arm-msm-owner@vger.kernel.org>
Received: from mail-pf0-f180.google.com ([209.85.192.180]:35894 "EHLO
        mail-pf0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1753198AbcLIC5b (ORCPT
        <rfc822;linux-arm-msm@vger.kernel.org>);
        Thu, 8 Dec 2016 21:57:31 -0500
Received: by mail-pf0-f180.google.com with SMTP id 189so1166282pfz.3
        for <linux-arm-msm@vger.kernel.org>; Thu, 08 Dec 2016 18:57:30 -0800 (PST)
Content-Disposition: inline
In-Reply-To: <1479981638-32069-5-git-send-email-akdwived@codeaurora.org>
Sender: linux-arm-msm-owner@vger.kernel.org
List-Id: linux-arm-msm@vger.kernel.org
To: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org>
Cc: sboyd@codeaurora.org, agross@codeaurora.org, linux-arm-msm@vger.kernel.org

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.

> +	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.

> +{
> +	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.

Regards,
Bjorn