Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3 01/11] PCI: qcom: add missing ipq806x clocks in PCIe
 driver
To:     Ansuel Smith <ansuelsmth@gmail.com>,
        Bjorn Andersson <bjorn.andersson@linaro.org>
Cc:     Sham Muthayyan <smuthayy@codeaurora.org>, stable@vger.kernel.org,
        Andy Gross <agross@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Andrew Murray <amurray@thegoodpenguin.co.uk>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20200430220619.3169-1-ansuelsmth@gmail.com>
 <20200430220619.3169-2-ansuelsmth@gmail.com>
From:   Stanimir Varbanov <svarbanov@mm-sol.com>
Message-ID: <3228e401-f9cd-a550-0bd4-80f01b35971a@mm-sol.com>
Date:   Fri, 8 May 2020 14:51:39 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <20200430220619.3169-2-ansuelsmth@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Ansuel,

On 5/1/20 1:06 AM, Ansuel Smith wrote:
> Aux and Ref clk are missing in PCIe qcom driver.
> Add support in the driver to fix PCIe initialization in ipq806x.
> 
> Fixes: 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver
> Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> Cc: stable@vger.kernel.org # v4.5+
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 44 ++++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 5ea527a6bd9f..2a39dfdccfc8 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -88,6 +88,8 @@ struct qcom_pcie_resources_2_1_0 {
>  	struct clk *iface_clk;
>  	struct clk *core_clk;
>  	struct clk *phy_clk;
> +	struct clk *aux_clk;
> +	struct clk *ref_clk;
>  	struct reset_control *pci_reset;
>  	struct reset_control *axi_reset;
>  	struct reset_control *ahb_reset;
> @@ -246,6 +248,14 @@ static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
>  	if (IS_ERR(res->phy_clk))
>  		return PTR_ERR(res->phy_clk);
>  
> +	res->aux_clk = devm_clk_get_optional(dev, "aux");
> +	if (IS_ERR(res->aux_clk))
> +		return PTR_ERR(res->aux_clk);
> +
> +	res->ref_clk = devm_clk_get_optional(dev, "ref");
> +	if (IS_ERR(res->ref_clk))
> +		return PTR_ERR(res->ref_clk);
> +
>  	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
>  	if (IS_ERR(res->pci_reset))
>  		return PTR_ERR(res->pci_reset);
> @@ -278,6 +288,8 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie)
>  	clk_disable_unprepare(res->iface_clk);
>  	clk_disable_unprepare(res->core_clk);
>  	clk_disable_unprepare(res->phy_clk);
> +	clk_disable_unprepare(res->aux_clk);
> +	clk_disable_unprepare(res->ref_clk);
>  	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
>  }
>  
> @@ -307,16 +319,32 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
>  		goto err_assert_ahb;
>  	}
>  
> +	ret = clk_prepare_enable(res->core_clk);
> +	if (ret) {
> +		dev_err(dev, "cannot prepare/enable core clock\n");
> +		goto err_clk_core;
> +	}
> +
>  	ret = clk_prepare_enable(res->phy_clk);
>  	if (ret) {
>  		dev_err(dev, "cannot prepare/enable phy clock\n");
>  		goto err_clk_phy;
>  	}
>  
> -	ret = clk_prepare_enable(res->core_clk);
> -	if (ret) {
> -		dev_err(dev, "cannot prepare/enable core clock\n");
> -		goto err_clk_core;
> +	if (res->aux_clk) {

you don't need this check, clk_prepare_enable handles NULL

> +		ret = clk_prepare_enable(res->aux_clk);
> +		if (ret) {
> +			dev_err(dev, "cannot prepare/enable aux clock\n");
> +			goto err_clk_aux;
> +		}
> +	}
> +
> +	if (res->ref_clk) {

here too

> +		ret = clk_prepare_enable(res->ref_clk);
> +		if (ret) {
> +			dev_err(dev, "cannot prepare/enable ref clock\n");
> +			goto err_clk_ref;
> +		}
>  	}
>  
>  	ret = reset_control_deassert(res->ahb_reset);
> @@ -372,10 +400,14 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
>  	return 0;
>  
>  err_deassert_ahb:
> -	clk_disable_unprepare(res->core_clk);
> -err_clk_core:
> +	clk_disable_unprepare(res->ref_clk);
> +err_clk_ref:
> +	clk_disable_unprepare(res->aux_clk);
> +err_clk_aux:
>  	clk_disable_unprepare(res->phy_clk);
>  err_clk_phy:
> +	clk_disable_unprepare(res->core_clk);
> +err_clk_core:
>  	clk_disable_unprepare(res->iface_clk);
>  err_assert_ahb:
>  	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
> 

-- 
regards,
Stan