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: <afd48178-fa6d-a2ac-7a50-cf558be6aa01@quicinc.com>
Date:   Fri, 7 Jul 2023 16:24:08 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH v2 2/8] PCI: qcom-ep: Add support for D-state change
 notification
Content-Language: en-US
To:     Manivannan Sadhasivam <mani@kernel.org>
CC:     <manivannan.sadhasivam@linaro.org>, <helgaas@kernel.org>,
        <linux-pci@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <quic_vbadigan@quicinc.com>,
        <quic_nitegupt@quicinc.com>, <quic_skananth@quicinc.com>,
        <quic_ramkri@quicinc.com>, <krzysztof.kozlowski@linaro.org>,
        "Lorenzo Pieralisi" <lpieralisi@kernel.org>,
        =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?= <kw@linux.com>,
        Rob Herring <robh@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>
References: <1688122331-25478-1-git-send-email-quic_krichai@quicinc.com>
 <1688122331-25478-3-git-send-email-quic_krichai@quicinc.com>
 <20230707055121.GB6001@thinkpad>
From:   Krishna Chaitanya Chundru <quic_krichai@quicinc.com>
In-Reply-To: <20230707055121.GB6001@thinkpad>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 7/7/2023 11:21 AM, Manivannan Sadhasivam wrote:
> On Fri, Jun 30, 2023 at 04:22:05PM +0530, Krishna chaitanya chundru wrote:
>> Add support to pass D-state change notification to Endpoint
>> function driver.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> Few comments below.
>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom-ep.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> index 1435f51..e75aec4 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> @@ -561,6 +561,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
>>   	struct device *dev = pci->dev;
>>   	u32 status = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_STATUS);
>>   	u32 mask = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_MASK);
>> +	pci_power_t state;
>>   	u32 dstate, val;
>>   
>>   	writel_relaxed(status, pcie_ep->parf + PARF_INT_ALL_CLEAR);
>> @@ -583,11 +584,16 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
>>   		dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
>>   					   DBI_CON_STATUS_POWER_STATE_MASK;
>>   		dev_dbg(dev, "Received D%d state event\n", dstate);
> We should also change this debug output to make use of pci_power_name() based on
> the "state" variable and move it just above pci_epc_dstate_change(). But this
> should be a separate patch.
I will add a seperate patch in the same series
>
>> +		state = dstate;
>>   		if (dstate == 3) {
>>   			val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
>>   			val |= PARF_PM_CTRL_REQ_EXIT_L1;
>>   			writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
>> +			state = PCI_D3hot;
>> +			if (gpiod_get_value(pcie_ep->reset))
>> +				state = PCI_D3cold;
>>   		}
>> +		pci_epc_dstate_change(pci->ep.epc, state);
> Need to change this API as per my comment on patch 1.
>
> - Mani

sure will change it as you suggested.

- KC

>
>>   	} else if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
>>   		dev_dbg(dev, "Received Linkup event. Enumeration complete!\n");
>>   		dw_pcie_ep_linkup(&pci->ep);
>> -- 
>> 2.7.4
>>