[PATCH v4 0/9] PCI: EPC: Add support to wake up host from D3 states

[PATCH v4 0/9] PCI: EPC: Add support to wake up host from D3 states

[Krishna chaitanya chundru]

Here we propose this patch series to add support in PCI endpoint
driver to wake up host from D3 states.

As endpoint cannot send any data/MSI when the D-state is in
D3cold or D3hot. Endpoint needs to bring the device back to D0
to send any kind of data.

For this endpoint needs to send inband PME the device is in D3 state or
toggle wake when the device is D3 cold and vaux is not supplied.

As EPF doestn't know the D-state of the PCI, added a notify op whenever
device state changes.

Based on the D-state the EPF driver decides to wake host either by
toggling wake or by sending PME.

When the MHI state is in M3 MHI driver will wakeup the host using the
wakeup op.

---
Changes from v3:
	- changed the bool return type to int for waking the host in mhi ep driver
	 as suggested by dan and bjorn.
	- Changed commit logs as suggested by bjorn.
Changes from v2:
        - Addressed review comments made by mani.
Changes from v1:
        - Moved from RFC patch to regular patch
        - Inclueded EPF patch and added a new op patch to notify D-state change.
---

Krishna chaitanya chundru (9):
  PCI: endpoint: Add D-state change notifier support
  PCI: qcom-ep: Add support for D-state change notification
  PCI: epf-mhi: Add support for handling D-state notify from EPC
  PCI: qcom-ep: Update the D-state log
  PCI: endpoint: Add wakeup host API to EPC core
  PCI: dwc: Add wakeup host op to pci_epc_ops
  PCI: qcom-ep: Add wake up host op to dw_pcie_ep_ops
  PCI: epf-mhi: Add wakeup host op
  bus: mhi: ep: wake up host if the MHI state is in M3

 Documentation/PCI/endpoint/pci-endpoint.rst     | 11 +++++
 drivers/bus/mhi/ep/main.c                       | 27 ++++++++++++
 drivers/pci/controller/dwc/pcie-designware-ep.c | 12 +++++
 drivers/pci/controller/dwc/pcie-designware.h    |  3 ++
 drivers/pci/controller/dwc/pcie-qcom-ep.c       | 36 ++++++++++++++-
 drivers/pci/endpoint/functions/pci-epf-mhi.c    | 27 ++++++++++++
 drivers/pci/endpoint/pci-epc-core.c             | 58 +++++++++++++++++++++++++
 include/linux/mhi_ep.h                          |  4 ++
 include/linux/pci-epc.h                         | 12 +++++
 include/linux/pci-epf.h                         |  1 +
 10 files changed, 190 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v4 1/9] 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 |  5 +++++
 drivers/pci/endpoint/pci-epc-core.c         | 27 +++++++++++++++++++++++++++
 include/linux/pci-epc.h                     |  1 +
 include/linux/pci-epf.h                     |  1 +
 4 files changed, 34 insertions(+)

diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
index 4f5622a..3a54713 100644
--- a/Documentation/PCI/endpoint/pci-endpoint.rst
+++ b/Documentation/PCI/endpoint/pci-endpoint.rst
@@ -78,6 +78,11 @@ by the PCI controller driver.
    Cleanup the pci_epc_mem structure allocated during pci_epc_mem_init().
 
 
+* pci_epc_dstate_notity()
+
+   In order to notify all the function devices 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 6c54fa5..ea76baf 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -785,6 +785,33 @@ void pci_epc_bme_notify(struct pci_epc *epc)
 EXPORT_SYMBOL_GPL(pci_epc_bme_notify);
 
 /**
+ * pci_epc_dstate_notity() - Notify the EPF device 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 has D-state has
+ * changed.
+ */
+void pci_epc_dstate_notity(struct pci_epc *epc, pci_power_t state)
+{
+	struct pci_epf *epf;
+
+	if (!epc || IS_ERR(epc))
+		return;
+
+	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_notity);
+
+/**
  * 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 5cb6940..26a1108 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -251,4 +251,5 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
 				     phys_addr_t *phys_addr, size_t size);
 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_change(struct pci_epc *epc, pci_power_t state);
 #endif /* __LINUX_PCI_EPC_H */
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 3f44b6a..529075b 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -79,6 +79,7 @@ struct pci_epc_event_ops {
 	int (*link_up)(struct pci_epf *epf);
 	int (*link_down)(struct pci_epf *epf);
 	int (*bme)(struct pci_epf *epf);
+	int (*dstate_notify)(struct pci_epf *epf, pci_power_t state);
 };
 
 /**
-- 
2.7.4

Re: [PATCH v4 1/9] PCI: endpoint: Add D-state change notifier support

[Manivannan Sadhasivam]

On Thu, Jul 13, 2023 at 12:40:10PM +0530, Krishna chaitanya chundru wrote:
> 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 |  5 +++++
>  drivers/pci/endpoint/pci-epc-core.c         | 27 +++++++++++++++++++++++++++
>  include/linux/pci-epc.h                     |  1 +
>  include/linux/pci-epf.h                     |  1 +
>  4 files changed, 34 insertions(+)
> 
> diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
> index 4f5622a..3a54713 100644
> --- a/Documentation/PCI/endpoint/pci-endpoint.rst
> +++ b/Documentation/PCI/endpoint/pci-endpoint.rst
> @@ -78,6 +78,11 @@ by the PCI controller driver.
>     Cleanup the pci_epc_mem structure allocated during pci_epc_mem_init().
>  
>  
> +* pci_epc_dstate_notity()
> +
> +   In order to notify all the function devices that the EPC device has
> +   changed its D-state.

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 6c54fa5..ea76baf 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -785,6 +785,33 @@ void pci_epc_bme_notify(struct pci_epc *epc)
>  EXPORT_SYMBOL_GPL(pci_epc_bme_notify);
>  
>  /**
> + * pci_epc_dstate_notity() - Notify the EPF device that EPC device D-state

EPF driver

- Mani

> + *			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 has D-state has
> + * changed.
> + */
> +void pci_epc_dstate_notity(struct pci_epc *epc, pci_power_t state)
> +{
> +	struct pci_epf *epf;
> +
> +	if (!epc || IS_ERR(epc))
> +		return;
> +
> +	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_notity);
> +
> +/**
>   * 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 5cb6940..26a1108 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -251,4 +251,5 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
>  				     phys_addr_t *phys_addr, size_t size);
>  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_change(struct pci_epc *epc, pci_power_t state);
>  #endif /* __LINUX_PCI_EPC_H */
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index 3f44b6a..529075b 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -79,6 +79,7 @@ struct pci_epc_event_ops {
>  	int (*link_up)(struct pci_epf *epf);
>  	int (*link_down)(struct pci_epf *epf);
>  	int (*bme)(struct pci_epf *epf);
> +	int (*dstate_notify)(struct pci_epf *epf, pci_power_t state);
>  };
>  
>  /**
> -- 
> 2.7.4
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH v4 2/9] 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.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 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 0fe7f06..66fd421 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);
+		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_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.7.4

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

[Manivannan Sadhasivam]

On Thu, Jul 13, 2023 at 12:40:11PM +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>

One nit below:

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

> ---
>  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 0fe7f06..66fd421 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);
> +		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);

Newline here.

- Mani

> +			state = PCI_D3hot;
> +			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.7.4
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH v4 3/9] 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 9c1f5a1..ee91bfc 100644
--- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
+++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
@@ -339,6 +339,16 @@ static int pci_epf_mhi_bme(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);
@@ -394,6 +404,7 @@ static struct pci_epc_event_ops pci_epf_mhi_event_ops = {
 	.link_up = pci_epf_mhi_link_up,
 	.link_down = pci_epf_mhi_link_down,
 	.bme = pci_epf_mhi_bme,
+	.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 f198a8a..c3a0685 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
 
@@ -139,6 +140,8 @@ struct mhi_ep_cntrl {
 
 	enum mhi_state mhi_state;
 
+	pci_power_t dstate;
+
 	u32 max_chan;
 	u32 mru;
 	u32 event_rings;
-- 
2.7.4

Re: [PATCH v4 3/9] PCI: epf-mhi: Add support for handling D-state notify from EPC

[Manivannan Sadhasivam]

On Thu, Jul 13, 2023 at 12:40:12PM +0530, Krishna chaitanya chundru wrote:
> 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 9c1f5a1..ee91bfc 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> @@ -339,6 +339,16 @@ static int pci_epf_mhi_bme(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;

Where is this variable being used? Also, don't we need any locking?

- Mani

> +
> +	return 0;
> +}
> +
>  static int pci_epf_mhi_bind(struct pci_epf *epf)
>  {
>  	struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
> @@ -394,6 +404,7 @@ static struct pci_epc_event_ops pci_epf_mhi_event_ops = {
>  	.link_up = pci_epf_mhi_link_up,
>  	.link_down = pci_epf_mhi_link_down,
>  	.bme = pci_epf_mhi_bme,
> +	.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 f198a8a..c3a0685 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
>  
> @@ -139,6 +140,8 @@ struct mhi_ep_cntrl {
>  
>  	enum mhi_state mhi_state;
>  
> +	pci_power_t dstate;
> +
>  	u32 max_chan;
>  	u32 mru;
>  	u32 event_rings;
> -- 
> 2.7.4
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 3/9] PCI: epf-mhi: Add support for handling D-state notify from EPC

[Krishna Chaitanya Chundru]

On 7/28/2023 9:39 AM, Manivannan Sadhasivam wrote:
> On Thu, Jul 13, 2023 at 12:40:12PM +0530, Krishna chaitanya chundru wrote:
>> 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 9c1f5a1..ee91bfc 100644
>> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
>> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
>> @@ -339,6 +339,16 @@ static int pci_epf_mhi_bme(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;
> Where is this variable being used? Also, don't we need any locking?
>
> - Mani

we are using this variable in wakeup host op which is introduced on 
patch [PATCH v4 8/9] PCI: epf-mhi: Add wakeup host op

I will add lock in my next series.

- KC

>
>> +
>> +	return 0;
>> +}
>> +
>>   static int pci_epf_mhi_bind(struct pci_epf *epf)
>>   {
>>   	struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
>> @@ -394,6 +404,7 @@ static struct pci_epc_event_ops pci_epf_mhi_event_ops = {
>>   	.link_up = pci_epf_mhi_link_up,
>>   	.link_down = pci_epf_mhi_link_down,
>>   	.bme = pci_epf_mhi_bme,
>> +	.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 f198a8a..c3a0685 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
>>   
>> @@ -139,6 +140,8 @@ struct mhi_ep_cntrl {
>>   
>>   	enum mhi_state mhi_state;
>>   
>> +	pci_power_t dstate;
>> +
>>   	u32 max_chan;
>>   	u32 mru;
>>   	u32 event_rings;
>> -- 
>> 2.7.4
>>

Re: [PATCH v4 3/9] PCI: epf-mhi: Add support for handling D-state notify from EPC

[Krishna Chaitanya Chundru]

On 7/31/2023 11:05 AM, Krishna Chaitanya Chundru wrote:
>
> On 7/28/2023 9:39 AM, Manivannan Sadhasivam wrote:
>> On Thu, Jul 13, 2023 at 12:40:12PM +0530, Krishna chaitanya chundru 
>> wrote:
>>> 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 9c1f5a1..ee91bfc 100644
>>> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
>>> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
>>> @@ -339,6 +339,16 @@ static int pci_epf_mhi_bme(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;
>> Where is this variable being used? Also, don't we need any locking?
>>
>> - Mani
>
> we are using this variable in wakeup host op which is introduced on 
> patch [PATCH v4 8/9] PCI: epf-mhi: Add wakeup host op
>
> I will add lock in my next series.
>
> - KC

Mani, as this is being called from IRQ context do we need to add any 
lock here.

- KC

>
>>
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   static int pci_epf_mhi_bind(struct pci_epf *epf)
>>>   {
>>>       struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
>>> @@ -394,6 +404,7 @@ static struct pci_epc_event_ops 
>>> pci_epf_mhi_event_ops = {
>>>       .link_up = pci_epf_mhi_link_up,
>>>       .link_down = pci_epf_mhi_link_down,
>>>       .bme = pci_epf_mhi_bme,
>>> +    .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 f198a8a..c3a0685 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
>>>   @@ -139,6 +140,8 @@ struct mhi_ep_cntrl {
>>>         enum mhi_state mhi_state;
>>>   +    pci_power_t dstate;
>>> +
>>>       u32 max_chan;
>>>       u32 mru;
>>>       u32 event_rings;
>>> -- 
>>> 2.7.4
>>>
>

Re: [PATCH v4 3/9] PCI: epf-mhi: Add support for handling D-state notify from EPC

[Manivannan Sadhasivam]

On Tue, Aug 01, 2023 at 10:31:42AM +0530, Krishna Chaitanya Chundru wrote:
> 
> On 7/31/2023 11:05 AM, Krishna Chaitanya Chundru wrote:
> > 
> > On 7/28/2023 9:39 AM, Manivannan Sadhasivam wrote:
> > > On Thu, Jul 13, 2023 at 12:40:12PM +0530, Krishna chaitanya chundru
> > > wrote:
> > > > 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 9c1f5a1..ee91bfc 100644
> > > > --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
> > > > +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> > > > @@ -339,6 +339,16 @@ static int pci_epf_mhi_bme(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;
> > > Where is this variable being used? Also, don't we need any locking?
> > > 
> > > - Mani
> > 
> > we are using this variable in wakeup host op which is introduced on
> > patch [PATCH v4 8/9] PCI: epf-mhi: Add wakeup host op
> > 
> > I will add lock in my next series.
> > 
> > - KC
> 
> Mani, as this is being called from IRQ context do we need to add any lock
> here.
> 

Notifiers are invoked in process context. And here, the context doesn't matter
as either way you need locking to prevent concurrent access to dstate variable.

But I think it is safe to ignore lock for now provided that wakeup_host callback
is only called while MHI is in M3 state. Even if dstate changes while processing
wakeup_host, it won't affect the behavior.

- Mani

> - KC
> 
> > 
> > > 
> > > > +
> > > > +    return 0;
> > > > +}
> > > > +
> > > >   static int pci_epf_mhi_bind(struct pci_epf *epf)
> > > >   {
> > > >       struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
> > > > @@ -394,6 +404,7 @@ static struct pci_epc_event_ops
> > > > pci_epf_mhi_event_ops = {
> > > >       .link_up = pci_epf_mhi_link_up,
> > > >       .link_down = pci_epf_mhi_link_down,
> > > >       .bme = pci_epf_mhi_bme,
> > > > +    .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 f198a8a..c3a0685 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
> > > >   @@ -139,6 +140,8 @@ struct mhi_ep_cntrl {
> > > >         enum mhi_state mhi_state;
> > > >   +    pci_power_t dstate;
> > > > +
> > > >       u32 max_chan;
> > > >       u32 mru;
> > > >       u32 event_rings;
> > > > -- 
> > > > 2.7.4
> > > > 
> > 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH v4 4/9] PCI: qcom-ep: Update the D-state log

[Krishna chaitanya chundru]

Updated the D-state log which prints in D-state in string format.

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 66fd421..75ab6d6 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -583,7 +583,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 == 3) {
 			val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
@@ -593,6 +592,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 D-state:%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.7.4

Re: [PATCH v4 4/9] PCI: qcom-ep: Update the D-state log

[Manivannan Sadhasivam]

On Thu, Jul 13, 2023 at 12:40:13PM +0530, Krishna chaitanya chundru wrote:
> Updated the D-state log which prints in D-state in string format.
> 

How about:

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

Also, this patch should be sent ahead of previous patch.

> 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 66fd421..75ab6d6 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -583,7 +583,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 == 3) {
>  			val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
> @@ -593,6 +592,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 D-state:%s event\n", pci_power_name(state));

dev_dbg(dev, "Received %s event\n", pci_power_name(state));

- Mani

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

-- 
மணிவண்ணன் சதாசிவம்

[PATCH v4 5/9] PCI: endpoint: Add wakeup host API to EPC core

[Krishna chaitanya chundru]

Endpoint cannot send any data/MSI when the D-state is in
D3cold or D3hot. Endpoint needs to wake up the host to
bring the D-state to D0.

Endpoint can toggle wake signal when the D-state is in D3cold and vaux is
not supplied or can send inband PME.

To support this add wakeup_host() callback to the EPC core.

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

diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
index 3a54713..eb79b77 100644
--- a/Documentation/PCI/endpoint/pci-endpoint.rst
+++ b/Documentation/PCI/endpoint/pci-endpoint.rst
@@ -53,6 +53,7 @@ by the PCI controller driver.
 	 * raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
 	 * start: ops to start the PCI link
 	 * stop: ops to stop the PCI link
+	 * wakeup_host: ops to wakeup host
 
    The PCI controller driver can then create a new EPC device by invoking
    devm_pci_epc_create()/pci_epc_create().
@@ -122,6 +123,11 @@ by the PCI endpoint function driver.
    The PCI endpoint function driver should use pci_epc_mem_free_addr() to
    free the memory space allocated using pci_epc_mem_alloc_addr().
 
+* pci_epc_wakeup_host()
+
+   The PCI endpoint function driver should use pci_epc_wakeup_host() to wakeup
+   host.
+
 Other EPC APIs
 ~~~~~~~~~~~~~~
 
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index ea76baf..b419eff 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -167,6 +167,37 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
 EXPORT_SYMBOL_GPL(pci_epc_get_features);
 
 /**
+ * pci_epc_wakeup_host() - Wakeup the host
+ * @epc: the EPC device which has to wakeup the host
+ * @func_no: the physical endpoint function number in the EPC device
+ * @vfunc_no: the virtual endpoint function number in the physical function
+ * @type: specify the type of wakeup: WAKEUP_FROM_D3COLD, WAKEUP_FROM_D3HOT
+ *
+ * Invoke to wakeup host
+ */
+bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
+			enum pci_epc_wakeup_host_type type)
+{
+	int ret;
+
+	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
+		return false;
+
+	if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
+		return false;
+
+	if (!epc->ops->wakeup_host)
+		return true;
+
+	mutex_lock(&epc->lock);
+	ret = epc->ops->wakeup_host(epc, func_no, vfunc_no, type);
+	mutex_unlock(&epc->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_epc_wakeup_host);
+
+/**
  * pci_epc_stop() - stop the PCI link
  * @epc: the link of the EPC device that has to be stopped
  *
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 26a1108..d262179 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -26,6 +26,12 @@ enum pci_epc_irq_type {
 	PCI_EPC_IRQ_MSIX,
 };
 
+enum pci_epc_wakeup_host_type {
+	PCI_WAKEUP_UNKNOWN,
+	PCI_WAKEUP_SEND_PME,
+	PCI_WAKEUP_TOGGLE_WAKE,
+};
+
 static inline const char *
 pci_epc_interface_string(enum pci_epc_interface_type type)
 {
@@ -59,6 +65,7 @@ pci_epc_interface_string(enum pci_epc_interface_type type)
  * @start: ops to start the PCI link
  * @stop: ops to stop the PCI link
  * @get_features: ops to get the features supported by the EPC
+ * @wakeup_host: ops to wakeup the host
  * @owner: the module owner containing the ops
  */
 struct pci_epc_ops {
@@ -88,6 +95,8 @@ struct pci_epc_ops {
 	void	(*stop)(struct pci_epc *epc);
 	const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
 						       u8 func_no, u8 vfunc_no);
+	bool	(*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
+				enum pci_epc_wakeup_host_type type);
 	struct module *owner;
 };
 
@@ -234,6 +243,8 @@ int pci_epc_start(struct pci_epc *epc);
 void pci_epc_stop(struct pci_epc *epc);
 const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
 						    u8 func_no, u8 vfunc_no);
+bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
+					enum pci_epc_wakeup_host_type type);
 enum pci_barno
 pci_epc_get_first_free_bar(const struct pci_epc_features *epc_features);
 enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
-- 
2.7.4

Re: [PATCH v4 5/9] PCI: endpoint: Add wakeup host API to EPC core

[Manivannan Sadhasivam]

On Thu, Jul 13, 2023 at 12:40:14PM +0530, Krishna chaitanya chundru wrote:
> Endpoint cannot send any data/MSI when the D-state is in
> D3cold or D3hot. Endpoint needs to wake up the host to
> bring the D-state to D0.
> 
> Endpoint can toggle wake signal when the D-state is in D3cold and vaux is
> not supplied or can send inband PME.
> 
> To support this add wakeup_host() callback to the EPC core.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  Documentation/PCI/endpoint/pci-endpoint.rst |  6 ++++++
>  drivers/pci/endpoint/pci-epc-core.c         | 31 +++++++++++++++++++++++++++++
>  include/linux/pci-epc.h                     | 11 ++++++++++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
> index 3a54713..eb79b77 100644
> --- a/Documentation/PCI/endpoint/pci-endpoint.rst
> +++ b/Documentation/PCI/endpoint/pci-endpoint.rst
> @@ -53,6 +53,7 @@ by the PCI controller driver.
>  	 * raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
>  	 * start: ops to start the PCI link
>  	 * stop: ops to stop the PCI link
> +	 * wakeup_host: ops to wakeup host
>  
>     The PCI controller driver can then create a new EPC device by invoking
>     devm_pci_epc_create()/pci_epc_create().
> @@ -122,6 +123,11 @@ by the PCI endpoint function driver.
>     The PCI endpoint function driver should use pci_epc_mem_free_addr() to
>     free the memory space allocated using pci_epc_mem_alloc_addr().
>  
> +* pci_epc_wakeup_host()
> +
> +   The PCI endpoint function driver should use pci_epc_wakeup_host() to wakeup
> +   host.
> +
>  Other EPC APIs
>  ~~~~~~~~~~~~~~
>  
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index ea76baf..b419eff 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -167,6 +167,37 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
>  EXPORT_SYMBOL_GPL(pci_epc_get_features);
>  
>  /**
> + * pci_epc_wakeup_host() - Wakeup the host
> + * @epc: the EPC device which has to wakeup the host
> + * @func_no: the physical endpoint function number in the EPC device
> + * @vfunc_no: the virtual endpoint function number in the physical function
> + * @type: specify the type of wakeup: WAKEUP_FROM_D3COLD, WAKEUP_FROM_D3HOT
> + *
> + * Invoke to wakeup host
> + */
> +bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +			enum pci_epc_wakeup_host_type type)
> +{
> +	int ret;
> +
> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
> +		return false;
> +
> +	if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
> +		return false;
> +
> +	if (!epc->ops->wakeup_host)
> +		return true;
> +
> +	mutex_lock(&epc->lock);
> +	ret = epc->ops->wakeup_host(epc, func_no, vfunc_no, type);
> +	mutex_unlock(&epc->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_wakeup_host);
> +
> +/**
>   * pci_epc_stop() - stop the PCI link
>   * @epc: the link of the EPC device that has to be stopped
>   *
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 26a1108..d262179 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -26,6 +26,12 @@ enum pci_epc_irq_type {
>  	PCI_EPC_IRQ_MSIX,
>  };
>  
> +enum pci_epc_wakeup_host_type {
> +	PCI_WAKEUP_UNKNOWN,
> +	PCI_WAKEUP_SEND_PME,
> +	PCI_WAKEUP_TOGGLE_WAKE,

I think I asked you to get rid of these enums and use a bool. I don't see any
new wakeup mechanism going to be added to the spec. So using enums for just 2
options looks overkill to me.

If you defer, please discuss it here.

- Mani

> +};
> +
>  static inline const char *
>  pci_epc_interface_string(enum pci_epc_interface_type type)
>  {
> @@ -59,6 +65,7 @@ pci_epc_interface_string(enum pci_epc_interface_type type)
>   * @start: ops to start the PCI link
>   * @stop: ops to stop the PCI link
>   * @get_features: ops to get the features supported by the EPC
> + * @wakeup_host: ops to wakeup the host
>   * @owner: the module owner containing the ops
>   */
>  struct pci_epc_ops {
> @@ -88,6 +95,8 @@ struct pci_epc_ops {
>  	void	(*stop)(struct pci_epc *epc);
>  	const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
>  						       u8 func_no, u8 vfunc_no);
> +	bool	(*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +				enum pci_epc_wakeup_host_type type);
>  	struct module *owner;
>  };
>  
> @@ -234,6 +243,8 @@ int pci_epc_start(struct pci_epc *epc);
>  void pci_epc_stop(struct pci_epc *epc);
>  const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
>  						    u8 func_no, u8 vfunc_no);
> +bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +					enum pci_epc_wakeup_host_type type);
>  enum pci_barno
>  pci_epc_get_first_free_bar(const struct pci_epc_features *epc_features);
>  enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
> -- 
> 2.7.4
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 5/9] PCI: endpoint: Add wakeup host API to EPC core

[Krishna Chaitanya Chundru]

On 7/28/2023 9:51 AM, Manivannan Sadhasivam wrote:
> On Thu, Jul 13, 2023 at 12:40:14PM +0530, Krishna chaitanya chundru wrote:
>> Endpoint cannot send any data/MSI when the D-state is in
>> D3cold or D3hot. Endpoint needs to wake up the host to
>> bring the D-state to D0.
>>
>> Endpoint can toggle wake signal when the D-state is in D3cold and vaux is
>> not supplied or can send inband PME.
>>
>> To support this add wakeup_host() callback to the EPC core.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   Documentation/PCI/endpoint/pci-endpoint.rst |  6 ++++++
>>   drivers/pci/endpoint/pci-epc-core.c         | 31 +++++++++++++++++++++++++++++
>>   include/linux/pci-epc.h                     | 11 ++++++++++
>>   3 files changed, 48 insertions(+)
>>
>> diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
>> index 3a54713..eb79b77 100644
>> --- a/Documentation/PCI/endpoint/pci-endpoint.rst
>> +++ b/Documentation/PCI/endpoint/pci-endpoint.rst
>> @@ -53,6 +53,7 @@ by the PCI controller driver.
>>   	 * raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
>>   	 * start: ops to start the PCI link
>>   	 * stop: ops to stop the PCI link
>> +	 * wakeup_host: ops to wakeup host
>>   
>>      The PCI controller driver can then create a new EPC device by invoking
>>      devm_pci_epc_create()/pci_epc_create().
>> @@ -122,6 +123,11 @@ by the PCI endpoint function driver.
>>      The PCI endpoint function driver should use pci_epc_mem_free_addr() to
>>      free the memory space allocated using pci_epc_mem_alloc_addr().
>>   
>> +* pci_epc_wakeup_host()
>> +
>> +   The PCI endpoint function driver should use pci_epc_wakeup_host() to wakeup
>> +   host.
>> +
>>   Other EPC APIs
>>   ~~~~~~~~~~~~~~
>>   
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index ea76baf..b419eff 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -167,6 +167,37 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
>>   EXPORT_SYMBOL_GPL(pci_epc_get_features);
>>   
>>   /**
>> + * pci_epc_wakeup_host() - Wakeup the host
>> + * @epc: the EPC device which has to wakeup the host
>> + * @func_no: the physical endpoint function number in the EPC device
>> + * @vfunc_no: the virtual endpoint function number in the physical function
>> + * @type: specify the type of wakeup: WAKEUP_FROM_D3COLD, WAKEUP_FROM_D3HOT
>> + *
>> + * Invoke to wakeup host
>> + */
>> +bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> +			enum pci_epc_wakeup_host_type type)
>> +{
>> +	int ret;
>> +
>> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
>> +		return false;
>> +
>> +	if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
>> +		return false;
>> +
>> +	if (!epc->ops->wakeup_host)
>> +		return true;
>> +
>> +	mutex_lock(&epc->lock);
>> +	ret = epc->ops->wakeup_host(epc, func_no, vfunc_no, type);
>> +	mutex_unlock(&epc->lock);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_epc_wakeup_host);
>> +
>> +/**
>>    * pci_epc_stop() - stop the PCI link
>>    * @epc: the link of the EPC device that has to be stopped
>>    *
>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>> index 26a1108..d262179 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -26,6 +26,12 @@ enum pci_epc_irq_type {
>>   	PCI_EPC_IRQ_MSIX,
>>   };
>>   
>> +enum pci_epc_wakeup_host_type {
>> +	PCI_WAKEUP_UNKNOWN,
>> +	PCI_WAKEUP_SEND_PME,
>> +	PCI_WAKEUP_TOGGLE_WAKE,
> I think I asked you to get rid of these enums and use a bool. I don't see any
> new wakeup mechanism going to be added to the spec. So using enums for just 2
> options looks overkill to me.
>
> If you defer, please discuss it here.
>
> - Mani

I will remove the enum move in to bool in next series.

- KC

>> +};
>> +
>>   static inline const char *
>>   pci_epc_interface_string(enum pci_epc_interface_type type)
>>   {
>> @@ -59,6 +65,7 @@ pci_epc_interface_string(enum pci_epc_interface_type type)
>>    * @start: ops to start the PCI link
>>    * @stop: ops to stop the PCI link
>>    * @get_features: ops to get the features supported by the EPC
>> + * @wakeup_host: ops to wakeup the host
>>    * @owner: the module owner containing the ops
>>    */
>>   struct pci_epc_ops {
>> @@ -88,6 +95,8 @@ struct pci_epc_ops {
>>   	void	(*stop)(struct pci_epc *epc);
>>   	const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
>>   						       u8 func_no, u8 vfunc_no);
>> +	bool	(*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> +				enum pci_epc_wakeup_host_type type);
>>   	struct module *owner;
>>   };
>>   
>> @@ -234,6 +243,8 @@ int pci_epc_start(struct pci_epc *epc);
>>   void pci_epc_stop(struct pci_epc *epc);
>>   const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
>>   						    u8 func_no, u8 vfunc_no);
>> +bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> +					enum pci_epc_wakeup_host_type type);
>>   enum pci_barno
>>   pci_epc_get_first_free_bar(const struct pci_epc_features *epc_features);
>>   enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
>> -- 
>> 2.7.4
>>

[PATCH v4 6/9] PCI: dwc: Add wakeup host op to pci_epc_ops

[Krishna chaitanya chundru]

Add wakeup host op to wake up host from D3cold or D3hot.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c | 12 ++++++++++++
 drivers/pci/controller/dwc/pcie-designware.h    |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index f9182f8..f61b172 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -463,6 +463,17 @@ dw_pcie_ep_get_features(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
 	return ep->ops->get_features(ep);
 }
 
+static bool dw_pcie_ep_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
+				enum pci_epc_wakeup_host_type type)
+{
+	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+
+	if (!ep->ops->wakeup_host)
+		return false;
+
+	return ep->ops->wakeup_host(ep, func_no, type);
+}
+
 static const struct pci_epc_ops epc_ops = {
 	.write_header		= dw_pcie_ep_write_header,
 	.set_bar		= dw_pcie_ep_set_bar,
@@ -477,6 +488,7 @@ static const struct pci_epc_ops epc_ops = {
 	.start			= dw_pcie_ep_start,
 	.stop			= dw_pcie_ep_stop,
 	.get_features		= dw_pcie_ep_get_features,
+	.wakeup_host		= dw_pcie_ep_wakeup_host,
 };
 
 int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 6156606..9417651 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -330,6 +330,9 @@ struct dw_pcie_ep_ops {
 	 * driver.
 	 */
 	unsigned int (*func_conf_select)(struct dw_pcie_ep *ep, u8 func_no);
+
+	bool	(*wakeup_host)(struct dw_pcie_ep *ep, u8 func_no,
+				enum pci_epc_wakeup_host_type type);
 };
 
 struct dw_pcie_ep_func {
-- 
2.7.4

[PATCH v4 7/9] PCI: qcom-ep: Add wake up host op to dw_pcie_ep_ops

[Krishna chaitanya chundru]

Add wakeup host op to dw_pcie_ep_ops to wake up host.
If the wakeup type is PME, then trigger inband PME by writing to the PARF
PARF_PM_CTRL register, otherwise toggle #WAKE.

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

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 75ab6d6..6472554 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -89,6 +89,7 @@
 /* PARF_PM_CTRL register fields */
 #define PARF_PM_CTRL_REQ_EXIT_L1		BIT(1)
 #define PARF_PM_CTRL_READY_ENTR_L23		BIT(2)
+#define PARF_PM_CTRL_XMT_PME			BIT(4)
 #define PARF_PM_CTRL_REQ_NOT_ENTR_L1		BIT(5)
 
 /* PARF_MHI_CLOCK_RESET_CTRL fields */
@@ -729,10 +730,37 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
 		dw_pcie_ep_reset_bar(pci, bar);
 }
 
+static bool qcom_pcie_ep_wakeup_host(struct dw_pcie_ep *ep, u8 func_no,
+					enum pci_epc_wakeup_host_type type)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	struct qcom_pcie_ep *pcie_ep = to_pcie_ep(pci);
+	struct device *dev = pci->dev;
+	u32 val;
+
+	if (type == PCI_WAKEUP_TOGGLE_WAKE) {
+		dev_dbg(dev, "Waking up the host by toggling WAKE#\n");
+		gpiod_set_value_cansleep(pcie_ep->wake, 1);
+		usleep_range(WAKE_DELAY_US, WAKE_DELAY_US + 500);
+		gpiod_set_value_cansleep(pcie_ep->wake, 0);
+
+	} else if (type == PCI_WAKEUP_SEND_PME) {
+		dev_dbg(dev, "Waking up the host using PME\n");
+		val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
+		writel_relaxed(val | PARF_PM_CTRL_XMT_PME, pcie_ep->parf + PARF_PM_CTRL);
+		writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
+
+	} else
+		return false;
+
+	return true;
+}
+
 static const struct dw_pcie_ep_ops pci_ep_ops = {
 	.ep_init = qcom_pcie_ep_init,
 	.raise_irq = qcom_pcie_ep_raise_irq,
 	.get_features = qcom_pcie_epc_get_features,
+	.wakeup_host = qcom_pcie_ep_wakeup_host,
 };
 
 static int qcom_pcie_ep_probe(struct platform_device *pdev)
-- 
2.7.4

[PATCH v4 8/9] PCI: epf-mhi: Add wakeup host op

[Krishna chaitanya chundru]

Add wakeup host op for MHI EPF.
If the D-state is in D3cold toggle wake signal, otherwise send PME.

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

diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
index ee91bfc..b608505 100644
--- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
+++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
@@ -237,6 +237,21 @@ static int pci_epf_mhi_write_to_host(struct mhi_ep_cntrl *mhi_cntrl,
 	return 0;
 }
 
+static int pci_epf_mhi_wakeup_host(struct mhi_ep_cntrl *mhi_cntrl)
+{
+	struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl);
+	struct pci_epf *epf = epf_mhi->epf;
+	enum pci_epc_wakeup_host_type type;
+	struct pci_epc *epc = epf->epc;
+
+	type = PCI_WAKEUP_SEND_PME;
+	if (mhi_cntrl->dstate == PCI_D3cold)
+		type = PCI_WAKEUP_TOGGLE_WAKE;
+
+	return pci_epc_wakeup_host(epc, epf->func_no, epf->vfunc_no, type);
+
+}
+
 static int pci_epf_mhi_core_init(struct pci_epf *epf)
 {
 	struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
@@ -293,6 +308,7 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf)
 	mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free;
 	mhi_cntrl->read_from_host = pci_epf_mhi_read_from_host;
 	mhi_cntrl->write_to_host = pci_epf_mhi_write_to_host;
+	mhi_cntrl->wakeup_host = pci_epf_mhi_wakeup_host;
 
 	/* Register the MHI EP controller */
 	ret = mhi_ep_register_controller(mhi_cntrl, info->config);
diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
index c3a0685..e353c429 100644
--- a/include/linux/mhi_ep.h
+++ b/include/linux/mhi_ep.h
@@ -137,6 +137,7 @@ struct mhi_ep_cntrl {
 			   void __iomem *virt, size_t size);
 	int (*read_from_host)(struct mhi_ep_cntrl *mhi_cntrl, u64 from, void *to, size_t size);
 	int (*write_to_host)(struct mhi_ep_cntrl *mhi_cntrl, void *from, u64 to, size_t size);
+	int (*wakeup_host)(struct mhi_ep_cntrl *mhi_cntrl);
 
 	enum mhi_state mhi_state;
 
-- 
2.7.4

[PATCH v4 9/9] bus: mhi: ep: wake up host if the MHI state is in M3

[Krishna chaitanya chundru]

If the MHI state is in M3 then the most probably the host kept the
device in D3 hot or D3 cold, due to that endpoint transctions will not
be read by the host, so endpoint wakes up host to bring the host to D0
which eventually bring back the MHI state to M0.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/bus/mhi/ep/main.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index 6008818..46a888e 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -25,6 +25,26 @@ static DEFINE_IDA(mhi_ep_cntrl_ida);
 static int mhi_ep_create_device(struct mhi_ep_cntrl *mhi_cntrl, u32 ch_id);
 static int mhi_ep_destroy_device(struct device *dev, void *data);
 
+static int mhi_ep_wake_host(struct mhi_ep_cntrl *mhi_cntrl)
+{
+	enum mhi_state state;
+	bool mhi_reset;
+	u32 count = 0;
+
+	mhi_cntrl->wakeup_host(mhi_cntrl);
+
+	/* Wait for Host to set the M0 state */
+	while (count++ < M0_WAIT_COUNT) {
+		msleep(M0_WAIT_DELAY_MS);
+
+		mhi_ep_mmio_get_mhi_state(mhi_cntrl, &state, &mhi_reset);
+		if (state == MHI_STATE_M0)
+			return 0;
+	}
+
+	return -ENODEV;
+}
+
 static int mhi_ep_send_event(struct mhi_ep_cntrl *mhi_cntrl, u32 ring_idx,
 			     struct mhi_ring_element *el, bool bei)
 {
@@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
 	buf_left = skb->len;
 	ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
 
+	if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
+		if (mhi_ep_wake_host(mhi_cntrl)) {
+			dev_err(dev, "Failed to wakeup host\n");
+			return -ENODEV;
+		}
+	}
+
 	mutex_lock(&mhi_chan->lock);
 
 	do {
-- 
2.7.4

Re: [PATCH v4 9/9] bus: mhi: ep: wake up host if the MHI state is in M3

[Manivannan Sadhasivam]

On Thu, Jul 13, 2023 at 12:40:18PM +0530, Krishna chaitanya chundru wrote:
> If the MHI state is in M3 then the most probably the host kept the

s/then the/then

> device in D3 hot or D3 cold, due to that endpoint transctions will not

s/transctions/transactions

> be read by the host, so endpoint wakes up host to bring the host to D0

endpoint needs to wake up the host to bring the device to D0 state...

> which eventually bring back the MHI state to M0.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/bus/mhi/ep/main.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index 6008818..46a888e 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -25,6 +25,26 @@ static DEFINE_IDA(mhi_ep_cntrl_ida);
>  static int mhi_ep_create_device(struct mhi_ep_cntrl *mhi_cntrl, u32 ch_id);
>  static int mhi_ep_destroy_device(struct device *dev, void *data);
>  
> +static int mhi_ep_wake_host(struct mhi_ep_cntrl *mhi_cntrl)
> +{
> +	enum mhi_state state;
> +	bool mhi_reset;
> +	u32 count = 0;
> +
> +	mhi_cntrl->wakeup_host(mhi_cntrl);
> +
> +	/* Wait for Host to set the M0 state */
> +	while (count++ < M0_WAIT_COUNT) {
> +		msleep(M0_WAIT_DELAY_MS);
> +
> +		mhi_ep_mmio_get_mhi_state(mhi_cntrl, &state, &mhi_reset);
> +		if (state == MHI_STATE_M0)
> +			return 0;
> +	}
> +
> +	return -ENODEV;

ENODEV or ETIMEDOUT?

> +}
> +
>  static int mhi_ep_send_event(struct mhi_ep_cntrl *mhi_cntrl, u32 ring_idx,
>  			     struct mhi_ring_element *el, bool bei)
>  {
> @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
>  	buf_left = skb->len;
>  	ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
>  
> +	if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
> +		if (mhi_ep_wake_host(mhi_cntrl)) {

Don't you need lock here in the case of multiple queue requests?

- Mani

> +			dev_err(dev, "Failed to wakeup host\n");
> +			return -ENODEV;
> +		}
> +	}
> +
>  	mutex_lock(&mhi_chan->lock);
>  
>  	do {
> -- 
> 2.7.4
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 9/9] bus: mhi: ep: wake up host if the MHI state is in M3

[Dan Carpenter]

On Fri, Jul 28, 2023 at 10:04:52AM +0530, Manivannan Sadhasivam wrote:
> > @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
> >  	buf_left = skb->len;
> >  	ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
> >  
> > +	if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
> > +		if (mhi_ep_wake_host(mhi_cntrl)) {
> 
> Don't you need lock here in the case of multiple queue requests?
> 
> - Mani
> 
> > +			dev_err(dev, "Failed to wakeup host\n");
> > +			return -ENODEV;
> > +		}
> > +	}
> > +
> >  	mutex_lock(&mhi_chan->lock);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^
This lock isn't enough?

regards,
dan carpenter

Re: [PATCH v4 9/9] bus: mhi: ep: wake up host if the MHI state is in M3

[Manivannan Sadhasivam]

On Fri, Jul 28, 2023 at 08:50:55AM +0300, Dan Carpenter wrote:
> On Fri, Jul 28, 2023 at 10:04:52AM +0530, Manivannan Sadhasivam wrote:
> > > @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
> > >  	buf_left = skb->len;
> > >  	ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
> > >  
> > > +	if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
> > > +		if (mhi_ep_wake_host(mhi_cntrl)) {
> > 
> > Don't you need lock here in the case of multiple queue requests?
> > 
> > - Mani
> > 
> > > +			dev_err(dev, "Failed to wakeup host\n");
> > > +			return -ENODEV;
> > > +		}
> > > +	}
> > > +
> > >  	mutex_lock(&mhi_chan->lock);
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^
> This lock isn't enough?
> 

The position of this lock won't prevent cocurrent access to mhi_ep_wake_host().

- Mani

> regards,
> dan carpenter

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 9/9] bus: mhi: ep: wake up host if the MHI state is in M3

[Krishna Chaitanya Chundru]

On 7/28/2023 9:05 PM, Manivannan Sadhasivam wrote:
> On Fri, Jul 28, 2023 at 08:50:55AM +0300, Dan Carpenter wrote:
>> On Fri, Jul 28, 2023 at 10:04:52AM +0530, Manivannan Sadhasivam wrote:
>>>> @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
>>>>   	buf_left = skb->len;
>>>>   	ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
>>>>   
>>>> +	if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
>>>> +		if (mhi_ep_wake_host(mhi_cntrl)) {
>>> Don't you need lock here in the case of multiple queue requests?
>>>
>>> - Mani
>>>
>>>> +			dev_err(dev, "Failed to wakeup host\n");
>>>> +			return -ENODEV;
>>>> +		}
>>>> +	}
>>>> +
>>>>   	mutex_lock(&mhi_chan->lock);
>>          ^^^^^^^^^^^^^^^^^^^^^^^^^^
>> This lock isn't enough?
>>
> The position of this lock won't prevent cocurrent access to mhi_ep_wake_host().
>
> - Mani
I will add the lock in the next series.
>> regards,
>> dan carpenter

Re: [PATCH v4 9/9] bus: mhi: ep: wake up host if the MHI state is in M3

[Dan Carpenter]

On Thu, Jul 13, 2023 at 12:40:18PM +0530, Krishna chaitanya chundru wrote:
> @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
>  	buf_left = skb->len;
>  	ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
>  
> +	if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
> +		if (mhi_ep_wake_host(mhi_cntrl)) {
> +			dev_err(dev, "Failed to wakeup host\n");
> +			return -ENODEV;
> +		}

Since you're going to be redoing this patch anyway could you please
propage the error code:

		ret = mhi_ep_wake_host(mhi_cntrl);
		if (ret) {
			dev_err(dev, "Failed to wakeup host\n");
			return ret;
		}

regards,
dan carpenter

Re: [PATCH v4 9/9] bus: mhi: ep: wake up host if the MHI state is in M3

[Krishna Chaitanya Chundru]

On 7/28/2023 11:21 AM, Dan Carpenter wrote:
> On Thu, Jul 13, 2023 at 12:40:18PM +0530, Krishna chaitanya chundru wrote:
>> @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
>>   	buf_left = skb->len;
>>   	ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
>>   
>> +	if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
>> +		if (mhi_ep_wake_host(mhi_cntrl)) {
>> +			dev_err(dev, "Failed to wakeup host\n");
>> +			return -ENODEV;
>> +		}
> Since you're going to be redoing this patch anyway could you please
> propage the error code:
>
> 		ret = mhi_ep_wake_host(mhi_cntrl);
> 		if (ret) {
> 			dev_err(dev, "Failed to wakeup host\n");
> 			return ret;
> 		}
>
> regards,
> dan carpenter

I will add this in next series.

- KC

>

Re: [PATCH v4 0/9] PCI: EPC: Add support to wake up host from D3 states

[Manivannan Sadhasivam]

On Thu, Jul 13, 2023 at 12:40:09PM +0530, Krishna chaitanya chundru wrote:
> Here we propose this patch series to add support in PCI endpoint
> driver to wake up host from D3 states.
> 
> As endpoint cannot send any data/MSI when the D-state is in
> D3cold or D3hot. Endpoint needs to bring the device back to D0
> to send any kind of data.
> 
> For this endpoint needs to send inband PME the device is in D3 state or
> toggle wake when the device is D3 cold and vaux is not supplied.
> 
> As EPF doestn't know the D-state of the PCI, added a notify op whenever
> device state changes.
> 
> Based on the D-state the EPF driver decides to wake host either by
> toggling wake or by sending PME.
> 
> When the MHI state is in M3 MHI driver will wakeup the host using the
> wakeup op.
> 

Please split this series into two. One adding D-state support and another
(dependant) adding wakeup support. We can try to merge atleast first one for
6.6.

- Mani

> ---
> Changes from v3:
> 	- changed the bool return type to int for waking the host in mhi ep driver
> 	 as suggested by dan and bjorn.
> 	- Changed commit logs as suggested by bjorn.
> Changes from v2:
>         - Addressed review comments made by mani.
> Changes from v1:
>         - Moved from RFC patch to regular patch
>         - Inclueded EPF patch and added a new op patch to notify D-state change.
> ---
> 
> Krishna chaitanya chundru (9):
>   PCI: endpoint: Add D-state change notifier support
>   PCI: qcom-ep: Add support for D-state change notification
>   PCI: epf-mhi: Add support for handling D-state notify from EPC
>   PCI: qcom-ep: Update the D-state log
>   PCI: endpoint: Add wakeup host API to EPC core
>   PCI: dwc: Add wakeup host op to pci_epc_ops
>   PCI: qcom-ep: Add wake up host op to dw_pcie_ep_ops
>   PCI: epf-mhi: Add wakeup host op
>   bus: mhi: ep: wake up host if the MHI state is in M3
> 
>  Documentation/PCI/endpoint/pci-endpoint.rst     | 11 +++++
>  drivers/bus/mhi/ep/main.c                       | 27 ++++++++++++
>  drivers/pci/controller/dwc/pcie-designware-ep.c | 12 +++++
>  drivers/pci/controller/dwc/pcie-designware.h    |  3 ++
>  drivers/pci/controller/dwc/pcie-qcom-ep.c       | 36 ++++++++++++++-
>  drivers/pci/endpoint/functions/pci-epf-mhi.c    | 27 ++++++++++++
>  drivers/pci/endpoint/pci-epc-core.c             | 58 +++++++++++++++++++++++++
>  include/linux/mhi_ep.h                          |  4 ++
>  include/linux/pci-epc.h                         | 12 +++++
>  include/linux/pci-epf.h                         |  1 +
>  10 files changed, 190 insertions(+), 1 deletion(-)
> 
> -- 
> 2.7.4
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 0/9] PCI: EPC: Add support to wake up host from D3 states

[Krishna Chaitanya Chundru]

On 7/28/2023 10:07 AM, Manivannan Sadhasivam wrote:
> On Thu, Jul 13, 2023 at 12:40:09PM +0530, Krishna chaitanya chundru wrote:
>> Here we propose this patch series to add support in PCI endpoint
>> driver to wake up host from D3 states.
>>
>> As endpoint cannot send any data/MSI when the D-state is in
>> D3cold or D3hot. Endpoint needs to bring the device back to D0
>> to send any kind of data.
>>
>> For this endpoint needs to send inband PME the device is in D3 state or
>> toggle wake when the device is D3 cold and vaux is not supplied.
>>
>> As EPF doestn't know the D-state of the PCI, added a notify op whenever
>> device state changes.
>>
>> Based on the D-state the EPF driver decides to wake host either by
>> toggling wake or by sending PME.
>>
>> When the MHI state is in M3 MHI driver will wakeup the host using the
>> wakeup op.
>>
> Please split this series into two. One adding D-state support and another
> (dependant) adding wakeup support. We can try to merge atleast first one for
> 6.6.
>
> - Mani
Sure, can you point me any patch series that is added dependant on 
another series.
>> ---
>> Changes from v3:
>> 	- changed the bool return type to int for waking the host in mhi ep driver
>> 	 as suggested by dan and bjorn.
>> 	- Changed commit logs as suggested by bjorn.
>> Changes from v2:
>>          - Addressed review comments made by mani.
>> Changes from v1:
>>          - Moved from RFC patch to regular patch
>>          - Inclueded EPF patch and added a new op patch to notify D-state change.
>> ---
>>
>> Krishna chaitanya chundru (9):
>>    PCI: endpoint: Add D-state change notifier support
>>    PCI: qcom-ep: Add support for D-state change notification
>>    PCI: epf-mhi: Add support for handling D-state notify from EPC
>>    PCI: qcom-ep: Update the D-state log
>>    PCI: endpoint: Add wakeup host API to EPC core
>>    PCI: dwc: Add wakeup host op to pci_epc_ops
>>    PCI: qcom-ep: Add wake up host op to dw_pcie_ep_ops
>>    PCI: epf-mhi: Add wakeup host op
>>    bus: mhi: ep: wake up host if the MHI state is in M3
>>
>>   Documentation/PCI/endpoint/pci-endpoint.rst     | 11 +++++
>>   drivers/bus/mhi/ep/main.c                       | 27 ++++++++++++
>>   drivers/pci/controller/dwc/pcie-designware-ep.c | 12 +++++
>>   drivers/pci/controller/dwc/pcie-designware.h    |  3 ++
>>   drivers/pci/controller/dwc/pcie-qcom-ep.c       | 36 ++++++++++++++-
>>   drivers/pci/endpoint/functions/pci-epf-mhi.c    | 27 ++++++++++++
>>   drivers/pci/endpoint/pci-epc-core.c             | 58 +++++++++++++++++++++++++
>>   include/linux/mhi_ep.h                          |  4 ++
>>   include/linux/pci-epc.h                         | 12 +++++
>>   include/linux/pci-epf.h                         |  1 +
>>   10 files changed, 190 insertions(+), 1 deletion(-)
>>
>> -- 
>> 2.7.4
>>