Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <050108d3-20fc-fb10-dd79-093974bcc401@quicinc.com>
Date:   Tue, 28 Jun 2022 12:09:33 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH v1 1/3] PCI: qcom: Add missing sc7280 clocks in PCIe
 driver
Content-Language: en-US
To:     Bjorn Helgaas <helgaas@kernel.org>
CC:     <linux-pci@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <quic_vbadigan@quicinc.com>,
        <quic_hemantk@quicinc.com>, <quic_nitegupt@quicinc.com>,
        <quic_skananth@quicinc.com>, <quic_ramkri@quicinc.com>,
        <manivannan.sadhasivam@linaro.org>, <swboyd@chromium.org>,
        <dmitry.baryshkov@linaro.org>, Andy Gross <agross@kernel.org>,
        "Bjorn Andersson" <bjorn.andersson@linaro.org>,
        Stanimir Varbanov <svarbanov@mm-sol.com>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        "Rob Herring" <robh@kernel.org>,
        =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?= <kw@linux.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Prasad Malisetty <pmaliset@codeaurora.org>
References: <20220624170501.GA1541553@bhelgaas>
From:   Krishna Chaitanya Chundru <quic_krichai@quicinc.com>
In-Reply-To: <20220624170501.GA1541553@bhelgaas>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 6/24/2022 10:35 PM, Bjorn Helgaas wrote:
> [+cc Prasad, who added sc7280]
> On Fri, Jun 24, 2022 at 02:49:49PM +0530, Krishna chaitanya chundru wrote:
>> Add missing clocks in the pcie driver.
> s/pcie/PCIe/ as in subject.  Also, please mention the names of the
> clocks here, too.
Will correct in next patch.
> Does this add new functionality, or did sc7280 just never work in the

When ever system goes to suspend the pcie link is also going down even 
though

we didn't unvote any clocks/regulators.

When we added these two missing clocks, the link is stable now after 
suspend and resume also.

We didn't test suspend/resume previously that is reason we didn't see 
any issue.

> first place?
>
> If this fixes a problem, it's nice to mention the user-visible symptom
> in the commit log to make it easier to connect this fix with the
> problem.
Will update in the next patch.
> Looks like sc7280 was added by Prasad with b89ff410253d ("PCI: qcom:
> Replace ops with struct pcie_cfg in pcie match data"), and I assume it
> was tested at that time.
>
> If I had noticed, I would have asked that b89ff410253d be a strict
> conversion to struct qcom_pcie_cfg with no functional changes, with
> sc7280 support being added in a separate patch by itself.
FYI. Prasad has left our org and no longer associated with our org now.
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 2ea1375..a7202f0 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -1548,7 +1548,10 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
>>   static const struct qcom_pcie_cfg sc7280_cfg = {
>>   	.ops = &ops_1_9_0,
>>   	.has_tbu_clk = true,
>> +	.has_ddrss_sf_tbu_clk = true,
>>   	.pipe_clk_need_muxing = true,
>> +	.has_aggre0_clk = true,
>> +	.has_aggre1_clk = true,
>>   };
>>   
>>   static const struct qcom_pcie_cfg sc8180x_cfg = {
>> -- 
>> 2.7.4
>>