[PATCH v7 0/4] PCI: endpoint: add D-state change notifier support

[PATCH v7 0/4] PCI: endpoint: add D-state change notifier support

[Krishna chaitanya chundru]

In this series we added the support to nofity the EPF driver
whenever there is change in the D-state if the EPF driver
registered for it.

This series needed by the following series for epf driver to know
whether link is in D3Cold or D3hot to wake the host because EPF driver
needs to send PME when the link is D3hot and toggle wake when the link
is in D3Cold('PCI: EPC: Add support to wake up host from D3 states').

The following link is for older series a newer series will be sent after
rebasing on this series.
https://lore.kernel.org/linux-pci/1690952359-8625-4-git-send-email-quic_krichai@quicinc.com/T/

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
Changes from v6:
	- Rebased on linux next.
	- Link to v6: https://lore.kernel.org/all/20230908-dstate_change-v6-0-b414a6edd765@quicinc.com/T/
Changes from v5:
	- Fixed compilation errors & removed checks in the dstate_notify()
	  function as suggested by bjorn.

---
Krishna chaitanya chundru (4):
      PCI: endpoint: Add D-state change notifier support
      PCI: qcom-ep: Add support for D-state change notification
      PCI: qcom-ep: Print D-state name to distinguish D3hot/D3cold
      PCI: epf-mhi: Add support for handling D-state notify from EPC

 Documentation/PCI/endpoint/pci-endpoint.rst  |  3 +++
 drivers/pci/controller/dwc/pcie-qcom-ep.c    | 10 ++++++++--
 drivers/pci/endpoint/functions/pci-epf-mhi.c | 11 +++++++++++
 drivers/pci/endpoint/pci-epc-core.c          | 24 ++++++++++++++++++++++++
 include/linux/mhi_ep.h                       |  3 +++
 include/linux/pci-epc.h                      |  2 ++
 include/linux/pci-epf.h                      |  2 ++
 7 files changed, 53 insertions(+), 2 deletions(-)
---
base-commit: 82d01fe6ee52086035b201cfa1410a3b04384257
change-id: 20240710-dstate_notifier-2c2f4e2b4eed

Best regards,
-- 
Krishna chaitanya chundru <quic_krichai@quicinc.com>

[PATCH v7 1/4] PCI: endpoint: Add D-state change notifier support

[Krishna chaitanya chundru]

Add support to notify the EPF device about the D-state change event
from the EPC device.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 Documentation/PCI/endpoint/pci-endpoint.rst |  3 +++
 drivers/pci/endpoint/pci-epc-core.c         | 24 ++++++++++++++++++++++++
 include/linux/pci-epc.h                     |  2 ++
 include/linux/pci-epf.h                     |  2 ++
 4 files changed, 31 insertions(+)

diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
index 21507e3cc238..3eb5648ca7ec 100644
--- a/Documentation/PCI/endpoint/pci-endpoint.rst
+++ b/Documentation/PCI/endpoint/pci-endpoint.rst
@@ -77,6 +77,9 @@ by the PCI controller driver.
 
    Cleanup the pci_epc_mem structure allocated during pci_epc_mem_init().
 
+* pci_epc_dstate_notify()
+
+   Notify all the function drivers that the EPC device has changed its D-state.
 
 EPC APIs for the PCI Endpoint Function Driver
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 84309dfe0c68..e2b9c458f2c4 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -828,6 +828,30 @@ void pci_epc_bus_master_enable_notify(struct pci_epc *epc)
 }
 EXPORT_SYMBOL_GPL(pci_epc_bus_master_enable_notify);
 
+/**
+ * pci_epc_dstate_notify() - Notify the EPF driver that EPC device D-state
+ *			has changed
+ * @epc: the EPC device which has change in D-state
+ * @state: the changed D-state
+ *
+ * Invoke to Notify the EPF device that the EPC device D-state has
+ * changed.
+ */
+void pci_epc_dstate_notify(struct pci_epc *epc, pci_power_t state)
+{
+	struct pci_epf *epf;
+
+	mutex_lock(&epc->list_lock);
+	list_for_each_entry(epf, &epc->pci_epf, list) {
+		mutex_lock(&epf->lock);
+		if (epf->event_ops && epf->event_ops->dstate_notify)
+			epf->event_ops->dstate_notify(epf, state);
+		mutex_unlock(&epf->lock);
+	}
+	mutex_unlock(&epc->list_lock);
+}
+EXPORT_SYMBOL_GPL(pci_epc_dstate_notify);
+
 /**
  * pci_epc_destroy() - destroy the EPC device
  * @epc: the EPC device that has to be destroyed
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 85bdf2adb760..e473d1780928 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -276,6 +276,8 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
 void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
 			   void __iomem *virt_addr, size_t size);
 
+void pci_epc_dstate_notify(struct pci_epc *epc, pci_power_t state);
+
 #else
 static inline void pci_epc_init_notify(struct pci_epc *epc)
 {
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 18a3aeb62ae4..d88063cdb067 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -75,6 +75,7 @@ struct pci_epf_ops {
  * @link_up: Callback for the EPC link up event
  * @link_down: Callback for the EPC link down event
  * @bus_master_enable: Callback for the EPC Bus Master Enable event
+ * @dstate_notify: Callback for the EPC D-state change event
  */
 struct pci_epc_event_ops {
 	int (*epc_init)(struct pci_epf *epf);
@@ -82,6 +83,7 @@ struct pci_epc_event_ops {
 	int (*link_up)(struct pci_epf *epf);
 	int (*link_down)(struct pci_epf *epf);
 	int (*bus_master_enable)(struct pci_epf *epf);
+	int (*dstate_notify)(struct pci_epf *epf, pci_power_t state);
 };
 
 /**

-- 
2.42.0

[PATCH v7 2/4] PCI: qcom-ep: Add support for D-state change notification

[Krishna chaitanya chundru]

Add support to pass D-state change notification to Endpoint
function driver.
Read perst value to determine if the link is in D3Cold/D3hot.

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom-ep.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 236229f66c80..817fad805c51 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -648,6 +648,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);
@@ -671,11 +672,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);
-		if (dstate == 3) {
+		state = dstate;
+		if (dstate == PCI_D3hot) {
 			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);
+
+			if (gpiod_get_value(pcie_ep->reset))
+				state = PCI_D3cold;
 		}
+		pci_epc_dstate_notify(pci->ep.epc, state);
 	} 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.42.0

[PATCH v7 3/4] PCI: qcom-ep: Print D-state name to distinguish D3hot/D3cold

[Krishna chaitanya chundru]

Now that the state event is stored as pci_power_t, use the PCI helper
pci_power_name() to print the state event.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 817fad805c51..627a33a1c5ca 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -671,7 +671,6 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
 	} else if (FIELD_GET(PARF_INT_ALL_DSTATE_CHANGE, status)) {
 		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);
 		state = dstate;
 		if (dstate == PCI_D3hot) {
 			val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
@@ -681,6 +680,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
 			if (gpiod_get_value(pcie_ep->reset))
 				state = PCI_D3cold;
 		}
+		dev_dbg(dev, "Received %s event\n", pci_power_name(state));
 		pci_epc_dstate_notify(pci->ep.epc, state);
 	} else if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
 		dev_dbg(dev, "Received Linkup event. Enumeration complete!\n");

-- 
2.42.0

[PATCH v7 4/4] PCI: epf-mhi: Add support for handling D-state notify from EPC

[Krishna chaitanya chundru]

Add support for handling D-state notify for MHI EPF.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/endpoint/functions/pci-epf-mhi.c | 11 +++++++++++
 include/linux/mhi_ep.h                       |  3 +++
 2 files changed, 14 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
index 7d070b1def11..6de9014e6e53 100644
--- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
+++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
@@ -863,6 +863,16 @@ static int pci_epf_mhi_bus_master_enable(struct pci_epf *epf)
 	return 0;
 }
 
+static int pci_epf_mhi_dstate_notify(struct pci_epf *epf, pci_power_t state)
+{
+	struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
+	struct mhi_ep_cntrl *mhi_cntrl = &epf_mhi->mhi_cntrl;
+
+	mhi_cntrl->dstate = state;
+
+	return 0;
+}
+
 static int pci_epf_mhi_bind(struct pci_epf *epf)
 {
 	struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
@@ -921,6 +931,7 @@ static const struct pci_epc_event_ops pci_epf_mhi_event_ops = {
 	.link_up = pci_epf_mhi_link_up,
 	.link_down = pci_epf_mhi_link_down,
 	.bus_master_enable = pci_epf_mhi_bus_master_enable,
+	.dstate_notify = pci_epf_mhi_dstate_notify,
 };
 
 static int pci_epf_mhi_probe(struct pci_epf *epf,
diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
index 7b40fc8cbe77..7c9e5895ea2c 100644
--- a/include/linux/mhi_ep.h
+++ b/include/linux/mhi_ep.h
@@ -8,6 +8,7 @@
 
 #include <linux/dma-direction.h>
 #include <linux/mhi.h>
+#include <linux/pci.h>
 
 #define MHI_EP_DEFAULT_MTU 0x8000
 
@@ -167,6 +168,8 @@ struct mhi_ep_cntrl {
 
 	enum mhi_state mhi_state;
 
+	pci_power_t dstate;
+
 	u32 max_chan;
 	u32 mru;
 	u32 event_rings;

-- 
2.42.0

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

[Bjorn Helgaas]

On Wed, Jul 10, 2024 at 04:38:15PM +0530, Krishna chaitanya chundru wrote:
> Add support to pass D-state change notification to Endpoint
> function driver.

Blank line between paragraphs.

> Read perst value to determine if the link is in D3Cold/D3hot.

I assume this reads the state of the PERST# signal driven by the host.
Style it to match the spec usage ("PERST#") to make that connection
clearer.

D3cold/D3hot is a device state and doesn't apply to a link.  Link
states are L0, L1, L2, L3,etc.  Also in cover letter.

I don't understand the connection between PERST# state and the device
D state.  D3cold is defined to mean main power is absent.  Is the
endpoint firmware still running when main power is absent?

> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom-ep.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 236229f66c80..817fad805c51 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -648,6 +648,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);
> @@ -671,11 +672,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);
> -		if (dstate == 3) {
> +		state = dstate;
> +		if (dstate == PCI_D3hot) {
>  			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);
> +
> +			if (gpiod_get_value(pcie_ep->reset))
> +				state = PCI_D3cold;
>  		}
> +		pci_epc_dstate_notify(pci->ep.epc, state);
>  	} 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.42.0
> 

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

[Mayank Rana]

On 7/10/2024 4:08 AM, Krishna chaitanya chundru wrote:
> Add support to pass D-state change notification to Endpoint
> function driver.
> Read perst value to determine if the link is in D3Cold/D3hot.
> 
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>   drivers/pci/controller/dwc/pcie-qcom-ep.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 236229f66c80..817fad805c51 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -648,6 +648,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);
> @@ -671,11 +672,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);
> -		if (dstate == 3) {
> +		state = dstate;
> +		if (dstate == PCI_D3hot) {
>   			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);
Can you please also check that do we really need to bring link back out 
of L1/L1SS on receiving D3 hot ?
> +			if (gpiod_get_value(pcie_ep->reset))
> +				state = PCI_D3cold;
>   		}
> +		pci_epc_dstate_notify(pci->ep.epc, state);
>   	} 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);
> 

Regards,
Mayank

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

[Krishna Chaitanya Chundru]

On 7/10/2024 5:41 PM, Bjorn Helgaas wrote:
> On Wed, Jul 10, 2024 at 04:38:15PM +0530, Krishna chaitanya chundru wrote:
>> Add support to pass D-state change notification to Endpoint
>> function driver.
> 
> Blank line between paragraphs.
> 
Ack.
>> Read perst value to determine if the link is in D3Cold/D3hot.
> 
> I assume this reads the state of the PERST# signal driven by the host.
> Style it to match the spec usage ("PERST#") to make that connection
> clearer.
> 
Ack.
> D3cold/D3hot is a device state and doesn't apply to a link.  Link
> states are L0, L1, L2, L3,etc.  Also in cover letter.
> 
Ack.
> I don't understand the connection between PERST# state and the device
> D state.  D3cold is defined to mean main power is absent.  Is the
> endpoint firmware still running when main power is absent?
> 
Host as part of its d3cold sequence will assert the perst. so we are
reading perst to know the link the state.

Qcom devices are drawing power from the PCIe, so even when PCIe is in
D3cold endpoint firmware can still run.

- Krishna Chaitanya.
>> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom-ep.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> index 236229f66c80..817fad805c51 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> @@ -648,6 +648,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);
>> @@ -671,11 +672,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);
>> -		if (dstate == 3) {
>> +		state = dstate;
>> +		if (dstate == PCI_D3hot) {
>>   			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);
>> +
>> +			if (gpiod_get_value(pcie_ep->reset))
>> +				state = PCI_D3cold;
>>   		}
>> +		pci_epc_dstate_notify(pci->ep.epc, state);
>>   	} 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.42.0
>>

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

[Bjorn Helgaas]

On Thu, Jul 11, 2024 at 11:57:35AM +0530, Krishna Chaitanya Chundru wrote:
> On 7/10/2024 5:41 PM, Bjorn Helgaas wrote:
> > On Wed, Jul 10, 2024 at 04:38:15PM +0530, Krishna chaitanya chundru wrote:
> > > Add support to pass D-state change notification to Endpoint
> > > function driver.
> ...

> > I don't understand the connection between PERST# state and the device
> > D state.  D3cold is defined to mean main power is absent.  Is the
> > endpoint firmware still running when main power is absent?
>
> Host as part of its d3cold sequence will assert the perst. so we are
> reading perst to know the link the state.

I think it's true that when the device is in D3cold, PERST# will be
asserted (PCIe CEM r5.0, sec 2.2.3, fig 2-6).

But I don't think it's necessarily true that when PERST# is asserted,
the device is in D3cold.  For example, PCIe Mini CEM r2.1, sec
3.2.5.3, says "The system may also use PERST# to cause a warm reset of
the add-in card."  In a warm reset, the component remains powered up,
i.e., it is not in D3cold (PCIe r6.0, sec 6.6.1).

I would think the endpoint firmware would be able to directly read the
state of main power or the LTSSM state of the link, without having to
use PERST# to infer it.

I guess the ultimate point of figuring out D3hot vs D3cold is to
figure out whether to use PME or WAKE#?  I'm a little bit dubious
about that being racy, as I mentioned elsewhere.  If there were a way
to attempt PME and fall back to WAKE# if you can determine that PME
failed, maybe that would be safer and obviate the need for the D-state
tracking?

> Qcom devices are drawing power from the PCIe, so even when PCIe is in
> D3cold endpoint firmware can still run.
> 
> - Krishna Chaitanya.
> > > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > > ---
> > >   drivers/pci/controller/dwc/pcie-qcom-ep.c | 8 +++++++-
> > >   1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > index 236229f66c80..817fad805c51 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > @@ -648,6 +648,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);
> > > @@ -671,11 +672,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);
> > > -		if (dstate == 3) {
> > > +		state = dstate;
> > > +		if (dstate == PCI_D3hot) {
> > >   			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);
> > > +
> > > +			if (gpiod_get_value(pcie_ep->reset))
> > > +				state = PCI_D3cold;
> > >   		}
> > > +		pci_epc_dstate_notify(pci->ep.epc, state);
> > >   	} 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.42.0
> > > 

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

[Yogesh Jadav]

On 7/10/2024 4:38 PM, Krishna chaitanya chundru wrote:
> Add support to pass D-state change notification to Endpoint
> function driver.
> Read perst value to determine if the link is in D3Cold/D3hot.
> 
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>   drivers/pci/controller/dwc/pcie-qcom-ep.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 236229f66c80..817fad805c51 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -648,6 +648,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);
> @@ -671,11 +672,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);
> -		if (dstate == 3) {
> +		state = dstate;
Can we use some meaningful name for variable "state" ? There is 
different purpose of variable "state" and "dstate" which is not getting 
reflected by looking variable names.

- Yogesh
> +		if (dstate == PCI_D3hot) {
>   			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);
> +
> +			if (gpiod_get_value(pcie_ep->reset))
> +				state = PCI_D3cold;
>   		}
> +		pci_epc_dstate_notify(pci->ep.epc, state);
>   	} 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);
> 

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

[Krishna Chaitanya Chundru]

On 7/12/2024 12:18 AM, Bjorn Helgaas wrote:
> On Thu, Jul 11, 2024 at 11:57:35AM +0530, Krishna Chaitanya Chundru wrote:
>> On 7/10/2024 5:41 PM, Bjorn Helgaas wrote:
>>> On Wed, Jul 10, 2024 at 04:38:15PM +0530, Krishna chaitanya chundru wrote:
>>>> Add support to pass D-state change notification to Endpoint
>>>> function driver.
>> ...
> 
>>> I don't understand the connection between PERST# state and the device
>>> D state.  D3cold is defined to mean main power is absent.  Is the
>>> endpoint firmware still running when main power is absent?
>>
>> Host as part of its d3cold sequence will assert the perst. so we are
>> reading perst to know the link the state.
> 
> I think it's true that when the device is in D3cold, PERST# will be
> asserted (PCIe CEM r5.0, sec 2.2.3, fig 2-6).
> 
> But I don't think it's necessarily true that when PERST# is asserted,
> the device is in D3cold.  For example, PCIe Mini CEM r2.1, sec
> 3.2.5.3, says "The system may also use PERST# to cause a warm reset of
> the add-in card."  In a warm reset, the component remains powered up,
> i.e., it is not in D3cold (PCIe r6.0, sec 6.6.1).
> 
> I would think the endpoint firmware would be able to directly read the
> state of main power or the LTSSM state of the link, without having to
> use PERST# to infer it.
>
Ack, we will use LTSSM state to know the link state.

> I guess the ultimate point of figuring out D3hot vs D3cold is to
> figure out whether to use PME or WAKE#?  I'm a little bit dubious
> about that being racy, as I mentioned elsewhere.  If there were a way
> to attempt PME and fall back to WAKE# if you can determine that PME
> failed, maybe that would be safer and obviate the need for the D-state
> tracking?
> 
We don't have a way to know that PME is received by the host.

We can add logic to send pme in d3hot and wait for sometime for the
state to move to D0. if it doesn't move to D0 for that period, the we
can check the LTSSM state to see if it in D3cold(L2/L3) then we can use
WAKE# else return with failure.

- Krishna Chaitanya.
>> Qcom devices are drawing power from the PCIe, so even when PCIe is in
>> D3cold endpoint firmware can still run.
>>
>> - Krishna Chaitanya.
>>>> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
>>>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>>>> ---
>>>>    drivers/pci/controller/dwc/pcie-qcom-ep.c | 8 +++++++-
>>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>>>> index 236229f66c80..817fad805c51 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>>>> @@ -648,6 +648,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);
>>>> @@ -671,11 +672,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);
>>>> -		if (dstate == 3) {
>>>> +		state = dstate;
>>>> +		if (dstate == PCI_D3hot) {
>>>>    			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);
>>>> +
>>>> +			if (gpiod_get_value(pcie_ep->reset))
>>>> +				state = PCI_D3cold;
>>>>    		}
>>>> +		pci_epc_dstate_notify(pci->ep.epc, state);
>>>>    	} 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.42.0
>>>>