Re: [PATCH v2 2/8] PCI: qcom-ep: Add support for D-state change notification

[Krishna Chaitanya Chundru <quic_krichai@quicinc.com>]

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