[PATCH RFC v1 1/3] PCI: endpoint: Add wakeup host API to EPC core

[PATCH RFC v1 1/3] PCI: endpoint: Add wakeup host API to EPC core

[Krishna chaitanya chundru]

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

For this endpoint can send inband PME the device is in D3hot or
toggle wake when the device is D3 cold.

To support this adding wake up host to epc core.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/endpoint/pci-epc-core.c | 29 +++++++++++++++++++++++++++++
 include/linux/pci-epc.h             |  3 +++
 2 files changed, 32 insertions(+)

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 46c9a5c..d203947 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -167,6 +167,35 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
 EXPORT_SYMBOL_GPL(pci_epc_get_features);
 
 /**
+ * pci_epc_wakeup_host() - interrupt the host system
+ * @epc: the EPC device which has to interrupt the host
+ * @func_no: the physical endpoint function number in the EPC device
+ * @vfunc_no: the virtual endpoint function number in the physical function
+ *
+ * Invoke to wakeup host
+ */
+int pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
+{
+	int ret;
+
+	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
+		return -EINVAL;
+
+	if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
+		return -EINVAL;
+
+	if (!epc->ops->wakeup_host)
+		return 0;
+
+	mutex_lock(&epc->lock);
+	ret = epc->ops->wakeup_host(epc, func_no, vfunc_no);
+	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 301bb0e..a8496be 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -59,6 +59,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 +89,7 @@ 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);
+	int	(*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
 	struct module *owner;
 };
 
@@ -232,6 +234,7 @@ 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);
+int pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
 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 RFC v1 2/3] 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 | 11 +++++++++++
 drivers/pci/controller/dwc/pcie-designware.h    |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index f9182f8..60cc323 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -463,6 +463,16 @@ dw_pcie_ep_get_features(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
 	return ep->ops->get_features(ep);
 }
 
+static int dw_pcie_ep_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
+{
+	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+
+	if (!ep->ops->wakeup_host)
+		return -EINVAL;
+
+	return ep->ops->wakeup_host(ep, func_no);
+}
+
 static const struct pci_epc_ops epc_ops = {
 	.write_header		= dw_pcie_ep_write_header,
 	.set_bar		= dw_pcie_ep_set_bar,
@@ -477,6 +487,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 79713ce..e338591 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -330,6 +330,8 @@ struct dw_pcie_ep_ops {
 	 * driver.
 	 */
 	unsigned int (*func_conf_select)(struct dw_pcie_ep *ep, u8 func_no);
+
+	int	(*wakeup_host)(struct dw_pcie_ep *ep, u8 func_no);
 };
 
 struct dw_pcie_ep_func {
-- 
2.7.4

[PATCH RFC v1 3/3] 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 from D3cold
or D3hot.

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

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 5d146ec..916a138 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -91,6 +91,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 */
@@ -794,10 +795,43 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
 		dw_pcie_ep_reset_bar(pci, bar);
 }
 
+static int qcom_pcie_ep_wakeup_host(struct dw_pcie_ep *ep, u8 func_no)
+{
+	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 perst, dstate, val;
+
+	perst = gpiod_get_value(pcie_ep->reset);
+	/* Toggle wake GPIO when device is in D3 cold */
+	if (perst) {
+		dev_info(dev, "Device is in D3 cold 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);
+		return 0;
+	}
+
+	dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
+				   DBI_CON_STATUS_POWER_STATE_MASK;
+	if (dstate == 3) {
+		dev_info(dev, "Device is in D3 hot sending inband PME\n");
+		val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
+		val |= PARF_PM_CTRL_XMT_PME;
+		writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
+	} else {
+		dev_err(dev, "Device is not in D3 state wakeup is not supported\n");
+		return -EPERM;
+	}
+
+	return 0;
+}
+
 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

Re: [PATCH RFC v1 1/3] PCI: endpoint: Add wakeup host API to EPC core

[Kishon Vijay Abraham I]

Hi Krishna,

On 6/14/2023 8:30 PM, Krishna chaitanya chundru wrote:
> Endpoint cannot send any data/MSI when the device state is in
> D3cold or D3hot. Endpoint needs to bring the device back to D0
> to send any kind of data.
> 
> For this endpoint can send inband PME the device is in D3hot or
> toggle wake when the device is D3 cold.
> 
> To support this adding wake up host to epc core.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>   drivers/pci/endpoint/pci-epc-core.c | 29 +++++++++++++++++++++++++++++
>   include/linux/pci-epc.h             |  3 +++
>   2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 46c9a5c..d203947 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -167,6 +167,35 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
>   EXPORT_SYMBOL_GPL(pci_epc_get_features);
>   
>   /**
> + * pci_epc_wakeup_host() - interrupt the host system
> + * @epc: the EPC device which has to interrupt the host
> + * @func_no: the physical endpoint function number in the EPC device
> + * @vfunc_no: the virtual endpoint function number in the physical function
> + *
> + * Invoke to wakeup host
> + */
> +int pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> +{
> +	int ret;
> +
> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
> +		return -EINVAL;
> +
> +	if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
> +		return -EINVAL;
> +
> +	if (!epc->ops->wakeup_host)
> +		return 0;
> +
> +	mutex_lock(&epc->lock);
> +	ret = epc->ops->wakeup_host(epc, func_no, vfunc_no);
> +	mutex_unlock(&epc->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_wakeup_host);

Which endpoint function driver uses this? And when does it have to be used?

Update Documentation for any new API's here
Documentation/PCI/endpoint/pci-endpoint.rst

Thanks,
Kishon
> +
> +/**
>    * 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 301bb0e..a8496be 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -59,6 +59,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 +89,7 @@ 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);
> +	int	(*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
>   	struct module *owner;
>   };
>   
> @@ -232,6 +234,7 @@ 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);
> +int pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
>   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

Re: [PATCH RFC v1 1/3] PCI: endpoint: Add wakeup host API to EPC core

[Manivannan Sadhasivam]

On Wed, Jun 14, 2023 at 08:30:47PM +0530, Krishna chaitanya chundru wrote:
> Endpoint cannot send any data/MSI when the device state is in
> D3cold or D3hot. Endpoint needs to bring the device back to D0
> to send any kind of data.
> 
> For this endpoint can send inband PME the device is in D3hot or
> toggle wake when the device is D3 cold.
> 

Are you referring to "host" as the "device"? If so, then it is a wrong
terminology.

> To support this adding wake up host to epc core.
> 

Commit message should be imperative.

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/endpoint/pci-epc-core.c | 29 +++++++++++++++++++++++++++++
>  include/linux/pci-epc.h             |  3 +++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 46c9a5c..d203947 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -167,6 +167,35 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
>  EXPORT_SYMBOL_GPL(pci_epc_get_features);
>  
>  /**
> + * pci_epc_wakeup_host() - interrupt the host system

s/interrupt the host system/Wakeup the host

> + * @epc: the EPC device which has to interrupt the host

s/interrupt/wake

> + * @func_no: the physical endpoint function number in the EPC device
> + * @vfunc_no: the virtual endpoint function number in the physical function
> + *
> + * Invoke to wakeup host
> + */
> +int pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> +{
> +	int ret;
> +
> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
> +		return -EINVAL;
> +
> +	if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
> +		return -EINVAL;
> +

Use proper errno for both of the above.

- Mani

> +	if (!epc->ops->wakeup_host)
> +		return 0;
> +
> +	mutex_lock(&epc->lock);
> +	ret = epc->ops->wakeup_host(epc, func_no, vfunc_no);
> +	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 301bb0e..a8496be 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -59,6 +59,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 +89,7 @@ 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);
> +	int	(*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
>  	struct module *owner;
>  };
>  
> @@ -232,6 +234,7 @@ 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);
> +int pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
>  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 RFC v1 3/3] PCI: qcom: ep: Add wake up host op to dw_pcie_ep_ops

[Manivannan Sadhasivam]

On Wed, Jun 14, 2023 at 08:30:49PM +0530, Krishna chaitanya chundru wrote:
> Add wakeup host op to dw_pcie_ep_ops to wake up host from D3cold
> or D3hot.
> 

Commit message should describe how the wakeup is implemented in the driver.

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom-ep.c | 34 +++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 5d146ec..916a138 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -91,6 +91,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 */
> @@ -794,10 +795,43 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
>  		dw_pcie_ep_reset_bar(pci, bar);
>  }
>  
> +static int qcom_pcie_ep_wakeup_host(struct dw_pcie_ep *ep, u8 func_no)
> +{
> +	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 perst, dstate, val;
> +
> +	perst = gpiod_get_value(pcie_ep->reset);
> +	/* Toggle wake GPIO when device is in D3 cold */
> +	if (perst) {
> +		dev_info(dev, "Device is in D3 cold toggling wake\n");

dev_dbg(). "Waking up the host by toggling WAKE#"

> +		gpiod_set_value_cansleep(pcie_ep->wake, 1);

Waking a device from D3cold requires power-on sequence by the host and in the
presence of Vaux, the EPF should be prepared for that. In that case, the mode of
wakeup should be decided by the EPF driver. So the wakeup API should have an
argument to decide whether the wakeup is through PME or sideband WAKE#.

Also note that as per PCIe Spec 3.0, the devices can support PME generation from
D3cold provided that the Vaux is supplied to the device. I do not know if that
is supported by Qcom devices but API should honor the spec. So the wakeup
control should come from EPF driver as I suggested above.

> +		usleep_range(WAKE_DELAY_US, WAKE_DELAY_US + 500);
> +		gpiod_set_value_cansleep(pcie_ep->wake, 0);
> +		return 0;
> +	}
> +
> +	dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
> +				   DBI_CON_STATUS_POWER_STATE_MASK;
> +	if (dstate == 3) {
> +		dev_info(dev, "Device is in D3 hot sending inband PME\n");

dev_dbg(). As I said, the device can sent PME from D3cold also. So the log could
be "Waking up the host using PME".

> +		val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
> +		val |= PARF_PM_CTRL_XMT_PME;
> +		writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
> +	} else {
> +		dev_err(dev, "Device is not in D3 state wakeup is not supported\n");
> +		return -EPERM;

-ENOTSUPP

- Mani

> +	}
> +
> +	return 0;
> +}
> +
>  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
> 

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

Re: [PATCH RFC v1 1/3] PCI: endpoint: Add wakeup host API to EPC core

[Krishna Chaitanya Chundru]

On 6/23/2023 11:13 AM, Manivannan Sadhasivam wrote:
> On Wed, Jun 14, 2023 at 08:30:47PM +0530, Krishna chaitanya chundru wrote:
>> Endpoint cannot send any data/MSI when the device state is in
>> D3cold or D3hot. Endpoint needs to bring the device back to D0
>> to send any kind of data.
>>
>> For this endpoint can send inband PME the device is in D3hot or
>> toggle wake when the device is D3 cold.
>>
> Are you referring to "host" as the "device"? If so, then it is a wrong
> terminology.
I will correct this in next series.
>
>> To support this adding wake up host to epc core.
>>
> Commit message should be imperative.
>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/pci/endpoint/pci-epc-core.c | 29 +++++++++++++++++++++++++++++
>>   include/linux/pci-epc.h             |  3 +++
>>   2 files changed, 32 insertions(+)
>>
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index 46c9a5c..d203947 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -167,6 +167,35 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
>>   EXPORT_SYMBOL_GPL(pci_epc_get_features);
>>   
>>   /**
>> + * pci_epc_wakeup_host() - interrupt the host system
> s/interrupt the host system/Wakeup the host
>
>> + * @epc: the EPC device which has to interrupt the host
> s/interrupt/wake
>
>> + * @func_no: the physical endpoint function number in the EPC device
>> + * @vfunc_no: the virtual endpoint function number in the physical function
>> + *
>> + * Invoke to wakeup host
>> + */
>> +int pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
>> +{
>> +	int ret;
>> +
>> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
>> +		return -EINVAL;
>> +
>> +	if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
>> +		return -EINVAL;
>> +
> Use proper errno for both of the above.
>
> - Mani

raise_irq functions also using errno can you please suggest correct value.

- KC

>> +	if (!epc->ops->wakeup_host)
>> +		return 0;
>> +
>> +	mutex_lock(&epc->lock);
>> +	ret = epc->ops->wakeup_host(epc, func_no, vfunc_no);
>> +	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 301bb0e..a8496be 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -59,6 +59,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 +89,7 @@ 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);
>> +	int	(*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
>>   	struct module *owner;
>>   };
>>   
>> @@ -232,6 +234,7 @@ 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);
>> +int pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
>>   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 RFC v1 3/3] PCI: qcom: ep: Add wake up host op to dw_pcie_ep_ops

[Krishna Chaitanya Chundru]

On 6/23/2023 11:48 AM, Manivannan Sadhasivam wrote:
> On Wed, Jun 14, 2023 at 08:30:49PM +0530, Krishna chaitanya chundru wrote:
>> Add wakeup host op to dw_pcie_ep_ops to wake up host from D3cold
>> or D3hot.
>>
> Commit message should describe how the wakeup is implemented in the driver.
I will correct this in next series.
>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom-ep.c | 34 +++++++++++++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> index 5d146ec..916a138 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> @@ -91,6 +91,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 */
>> @@ -794,10 +795,43 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
>>   		dw_pcie_ep_reset_bar(pci, bar);
>>   }
>>   
>> +static int qcom_pcie_ep_wakeup_host(struct dw_pcie_ep *ep, u8 func_no)
>> +{
>> +	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 perst, dstate, val;
>> +
>> +	perst = gpiod_get_value(pcie_ep->reset);
>> +	/* Toggle wake GPIO when device is in D3 cold */
>> +	if (perst) {
>> +		dev_info(dev, "Device is in D3 cold toggling wake\n");
> dev_dbg(). "Waking up the host by toggling WAKE#"
>
>> +		gpiod_set_value_cansleep(pcie_ep->wake, 1);
> Waking a device from D3cold requires power-on sequence by the host and in the
> presence of Vaux, the EPF should be prepared for that. In that case, the mode of
> wakeup should be decided by the EPF driver. So the wakeup API should have an
> argument to decide whether the wakeup is through PME or sideband WAKE#.
>
> Also note that as per PCIe Spec 3.0, the devices can support PME generation from
> D3cold provided that the Vaux is supplied to the device. I do not know if that
> is supported by Qcom devices but API should honor the spec. So the wakeup
> control should come from EPF driver as I suggested above.

I aggre with you, but how will EPF know the PCI device state is in 
D3cold or D3hot.

And how the EPF knows whether Vaux is supported or not in D3cold?

If there is any existing mechanism can you please point me that.

FYI Qcom does not support vaux power in D3 cold.

>> +		usleep_range(WAKE_DELAY_US, WAKE_DELAY_US + 500);
>> +		gpiod_set_value_cansleep(pcie_ep->wake, 0);
>> +		return 0;
>> +	}
>> +
>> +	dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
>> +				   DBI_CON_STATUS_POWER_STATE_MASK;
>> +	if (dstate == 3) {
>> +		dev_info(dev, "Device is in D3 hot sending inband PME\n");
> dev_dbg(). As I said, the device can sent PME from D3cold also. So the log could
> be "Waking up the host using PME".
>
>> +		val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
>> +		val |= PARF_PM_CTRL_XMT_PME;
>> +		writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
>> +	} else {
>> +		dev_err(dev, "Device is not in D3 state wakeup is not supported\n");
>> +		return -EPERM;
> -ENOTSUPP
>
> - Mani
>
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   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
>>

Re: [PATCH RFC v1 3/3] PCI: qcom: ep: Add wake up host op to dw_pcie_ep_ops

[Manivannan Sadhasivam]

On Mon, Jun 26, 2023 at 07:18:49PM +0530, Krishna Chaitanya Chundru wrote:
> 
> On 6/23/2023 11:48 AM, Manivannan Sadhasivam wrote:
> > On Wed, Jun 14, 2023 at 08:30:49PM +0530, Krishna chaitanya chundru wrote:
> > > Add wakeup host op to dw_pcie_ep_ops to wake up host from D3cold
> > > or D3hot.
> > > 
> > Commit message should describe how the wakeup is implemented in the driver.
> I will correct this in next series.
> > 
> > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > > ---
> > >   drivers/pci/controller/dwc/pcie-qcom-ep.c | 34 +++++++++++++++++++++++++++++++
> > >   1 file changed, 34 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > index 5d146ec..916a138 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > @@ -91,6 +91,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 */
> > > @@ -794,10 +795,43 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
> > >   		dw_pcie_ep_reset_bar(pci, bar);
> > >   }
> > > +static int qcom_pcie_ep_wakeup_host(struct dw_pcie_ep *ep, u8 func_no)
> > > +{
> > > +	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 perst, dstate, val;
> > > +
> > > +	perst = gpiod_get_value(pcie_ep->reset);
> > > +	/* Toggle wake GPIO when device is in D3 cold */
> > > +	if (perst) {
> > > +		dev_info(dev, "Device is in D3 cold toggling wake\n");
> > dev_dbg(). "Waking up the host by toggling WAKE#"
> > 
> > > +		gpiod_set_value_cansleep(pcie_ep->wake, 1);
> > Waking a device from D3cold requires power-on sequence by the host and in the
> > presence of Vaux, the EPF should be prepared for that. In that case, the mode of
> > wakeup should be decided by the EPF driver. So the wakeup API should have an
> > argument to decide whether the wakeup is through PME or sideband WAKE#.
> > 
> > Also note that as per PCIe Spec 3.0, the devices can support PME generation from
> > D3cold provided that the Vaux is supplied to the device. I do not know if that
> > is supported by Qcom devices but API should honor the spec. So the wakeup
> > control should come from EPF driver as I suggested above.
> 
> I aggre with you, but how will EPF know the PCI device state is in D3cold or
> D3hot.
> 

We should add a notifier in the controller driver which signals EPF when it
receives the DState events.. Take a look at pci_epc_linkdown().

> And how the EPF knows whether Vaux is supported or not in D3cold?
> 
> If there is any existing mechanism can you please point me that.
> 
> FYI Qcom does not support vaux power in D3 cold.
> 

If the EPF can trigger wakeup event during D3Cold then it means it is powered
by Vaux, isn't it?

- Mani

> > > +		usleep_range(WAKE_DELAY_US, WAKE_DELAY_US + 500);
> > > +		gpiod_set_value_cansleep(pcie_ep->wake, 0);
> > > +		return 0;
> > > +	}
> > > +
> > > +	dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
> > > +				   DBI_CON_STATUS_POWER_STATE_MASK;
> > > +	if (dstate == 3) {
> > > +		dev_info(dev, "Device is in D3 hot sending inband PME\n");
> > dev_dbg(). As I said, the device can sent PME from D3cold also. So the log could
> > be "Waking up the host using PME".
> > 
> > > +		val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
> > > +		val |= PARF_PM_CTRL_XMT_PME;
> > > +		writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
> > > +	} else {
> > > +		dev_err(dev, "Device is not in D3 state wakeup is not supported\n");
> > > +		return -EPERM;
> > -ENOTSUPP
> > 
> > - Mani
> > 
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   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
> > > 

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

Re: [PATCH RFC v1 1/3] PCI: endpoint: Add wakeup host API to EPC core

[Manivannan Sadhasivam]

On Mon, Jun 26, 2023 at 07:10:53PM +0530, Krishna Chaitanya Chundru wrote:
> 
> On 6/23/2023 11:13 AM, Manivannan Sadhasivam wrote:
> > On Wed, Jun 14, 2023 at 08:30:47PM +0530, Krishna chaitanya chundru wrote:
> > > Endpoint cannot send any data/MSI when the device state is in
> > > D3cold or D3hot. Endpoint needs to bring the device back to D0
> > > to send any kind of data.
> > > 
> > > For this endpoint can send inband PME the device is in D3hot or
> > > toggle wake when the device is D3 cold.
> > > 
> > Are you referring to "host" as the "device"? If so, then it is a wrong
> > terminology.
> I will correct this in next series.
> > 
> > > To support this adding wake up host to epc core.
> > > 
> > Commit message should be imperative.
> > 
> > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > > ---
> > >   drivers/pci/endpoint/pci-epc-core.c | 29 +++++++++++++++++++++++++++++
> > >   include/linux/pci-epc.h             |  3 +++
> > >   2 files changed, 32 insertions(+)
> > > 
> > > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> > > index 46c9a5c..d203947 100644
> > > --- a/drivers/pci/endpoint/pci-epc-core.c
> > > +++ b/drivers/pci/endpoint/pci-epc-core.c
> > > @@ -167,6 +167,35 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
> > >   EXPORT_SYMBOL_GPL(pci_epc_get_features);
> > >   /**
> > > + * pci_epc_wakeup_host() - interrupt the host system
> > s/interrupt the host system/Wakeup the host
> > 
> > > + * @epc: the EPC device which has to interrupt the host
> > s/interrupt/wake
> > 
> > > + * @func_no: the physical endpoint function number in the EPC device
> > > + * @vfunc_no: the virtual endpoint function number in the physical function
> > > + *
> > > + * Invoke to wakeup host
> > > + */
> > > +int pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> > > +{
> > > +	int ret;
> > > +
> > > +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
> > > +		return -EINVAL;
> > > +
> > > +	if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
> > > +		return -EINVAL;
> > > +
> > Use proper errno for both of the above.
> > 
> > - Mani
> 
> raise_irq functions also using errno can you please suggest correct value.
> 

Let's keep it as it is, we can change all later.

- Mani

> - KC
> 
> > > +	if (!epc->ops->wakeup_host)
> > > +		return 0;
> > > +
> > > +	mutex_lock(&epc->lock);
> > > +	ret = epc->ops->wakeup_host(epc, func_no, vfunc_no);
> > > +	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 301bb0e..a8496be 100644
> > > --- a/include/linux/pci-epc.h
> > > +++ b/include/linux/pci-epc.h
> > > @@ -59,6 +59,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 +89,7 @@ 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);
> > > +	int	(*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
> > >   	struct module *owner;
> > >   };
> > > @@ -232,6 +234,7 @@ 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);
> > > +int pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
> > >   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 RFC v1 3/3] PCI: qcom: ep: Add wake up host op to dw_pcie_ep_ops

[Krishna Chaitanya Chundru]

On 6/27/2023 7:23 PM, Manivannan Sadhasivam wrote:
> On Mon, Jun 26, 2023 at 07:18:49PM +0530, Krishna Chaitanya Chundru wrote:
>> On 6/23/2023 11:48 AM, Manivannan Sadhasivam wrote:
>>> On Wed, Jun 14, 2023 at 08:30:49PM +0530, Krishna chaitanya chundru wrote:
>>>> Add wakeup host op to dw_pcie_ep_ops to wake up host from D3cold
>>>> or D3hot.
>>>>
>>> Commit message should describe how the wakeup is implemented in the driver.
>> I will correct this in next series.
>>>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>>>> ---
>>>>    drivers/pci/controller/dwc/pcie-qcom-ep.c | 34 +++++++++++++++++++++++++++++++
>>>>    1 file changed, 34 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>>>> index 5d146ec..916a138 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>>>> @@ -91,6 +91,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 */
>>>> @@ -794,10 +795,43 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
>>>>    		dw_pcie_ep_reset_bar(pci, bar);
>>>>    }
>>>> +static int qcom_pcie_ep_wakeup_host(struct dw_pcie_ep *ep, u8 func_no)
>>>> +{
>>>> +	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 perst, dstate, val;
>>>> +
>>>> +	perst = gpiod_get_value(pcie_ep->reset);
>>>> +	/* Toggle wake GPIO when device is in D3 cold */
>>>> +	if (perst) {
>>>> +		dev_info(dev, "Device is in D3 cold toggling wake\n");
>>> dev_dbg(). "Waking up the host by toggling WAKE#"
>>>
>>>> +		gpiod_set_value_cansleep(pcie_ep->wake, 1);
>>> Waking a device from D3cold requires power-on sequence by the host and in the
>>> presence of Vaux, the EPF should be prepared for that. In that case, the mode of
>>> wakeup should be decided by the EPF driver. So the wakeup API should have an
>>> argument to decide whether the wakeup is through PME or sideband WAKE#.
>>>
>>> Also note that as per PCIe Spec 3.0, the devices can support PME generation from
>>> D3cold provided that the Vaux is supplied to the device. I do not know if that
>>> is supported by Qcom devices but API should honor the spec. So the wakeup
>>> control should come from EPF driver as I suggested above.
>> I aggre with you, but how will EPF know the PCI device state is in D3cold or
>> D3hot.
>>
> We should add a notifier in the controller driver which signals EPF when it
> receives the DState events.. Take a look at pci_epc_linkdown().
Ok I will add this kind of Dstate change event
>
>> And how the EPF knows whether Vaux is supported or not in D3cold?
>>
>> If there is any existing mechanism can you please point me that.
>>
>> FYI Qcom does not support vaux power in D3 cold.
>>
> If the EPF can trigger wakeup event during D3Cold then it means it is powered
> by Vaux, isn't it?
>
> - Mani

EPF needs to know that if the endpoint is getting vaux in D3cold or not 
without this info

EPF doesn't know wheather to send toggle wake or send Inband PME right.

I mean EPF should have some  prior information wheather vaux is supplied 
or not to decide

wheather to toggle wake or send in band PME.

-KC

>
>>>> +		usleep_range(WAKE_DELAY_US, WAKE_DELAY_US + 500);
>>>> +		gpiod_set_value_cansleep(pcie_ep->wake, 0);
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
>>>> +				   DBI_CON_STATUS_POWER_STATE_MASK;
>>>> +	if (dstate == 3) {
>>>> +		dev_info(dev, "Device is in D3 hot sending inband PME\n");
>>> dev_dbg(). As I said, the device can sent PME from D3cold also. So the log could
>>> be "Waking up the host using PME".
>>>
>>>> +		val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
>>>> +		val |= PARF_PM_CTRL_XMT_PME;
>>>> +		writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
>>>> +	} else {
>>>> +		dev_err(dev, "Device is not in D3 state wakeup is not supported\n");
>>>> +		return -EPERM;
>>> -ENOTSUPP
>>>
>>> - Mani
>>>
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>>    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
>>>>

Re: [PATCH RFC v1 3/3] PCI: qcom: ep: Add wake up host op to dw_pcie_ep_ops

[Manivannan Sadhasivam]

On Wed, Jun 28, 2023 at 08:01:45AM +0530, Krishna Chaitanya Chundru wrote:
> 
> On 6/27/2023 7:23 PM, Manivannan Sadhasivam wrote:
> > On Mon, Jun 26, 2023 at 07:18:49PM +0530, Krishna Chaitanya Chundru wrote:
> > > On 6/23/2023 11:48 AM, Manivannan Sadhasivam wrote:
> > > > On Wed, Jun 14, 2023 at 08:30:49PM +0530, Krishna chaitanya chundru wrote:
> > > > > Add wakeup host op to dw_pcie_ep_ops to wake up host from D3cold
> > > > > or D3hot.
> > > > > 
> > > > Commit message should describe how the wakeup is implemented in the driver.
> > > I will correct this in next series.
> > > > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > > > > ---
> > > > >    drivers/pci/controller/dwc/pcie-qcom-ep.c | 34 +++++++++++++++++++++++++++++++
> > > > >    1 file changed, 34 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > > > index 5d146ec..916a138 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > > > @@ -91,6 +91,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 */
> > > > > @@ -794,10 +795,43 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
> > > > >    		dw_pcie_ep_reset_bar(pci, bar);
> > > > >    }
> > > > > +static int qcom_pcie_ep_wakeup_host(struct dw_pcie_ep *ep, u8 func_no)
> > > > > +{
> > > > > +	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 perst, dstate, val;
> > > > > +
> > > > > +	perst = gpiod_get_value(pcie_ep->reset);
> > > > > +	/* Toggle wake GPIO when device is in D3 cold */
> > > > > +	if (perst) {
> > > > > +		dev_info(dev, "Device is in D3 cold toggling wake\n");
> > > > dev_dbg(). "Waking up the host by toggling WAKE#"
> > > > 
> > > > > +		gpiod_set_value_cansleep(pcie_ep->wake, 1);
> > > > Waking a device from D3cold requires power-on sequence by the host and in the
> > > > presence of Vaux, the EPF should be prepared for that. In that case, the mode of
> > > > wakeup should be decided by the EPF driver. So the wakeup API should have an
> > > > argument to decide whether the wakeup is through PME or sideband WAKE#.
> > > > 
> > > > Also note that as per PCIe Spec 3.0, the devices can support PME generation from
> > > > D3cold provided that the Vaux is supplied to the device. I do not know if that
> > > > is supported by Qcom devices but API should honor the spec. So the wakeup
> > > > control should come from EPF driver as I suggested above.
> > > I aggre with you, but how will EPF know the PCI device state is in D3cold or
> > > D3hot.
> > > 
> > We should add a notifier in the controller driver which signals EPF when it
> > receives the DState events.. Take a look at pci_epc_linkdown().
> Ok I will add this kind of Dstate change event
> > 
> > > And how the EPF knows whether Vaux is supported or not in D3cold?
> > > 
> > > If there is any existing mechanism can you please point me that.
> > > 
> > > FYI Qcom does not support vaux power in D3 cold.
> > > 
> > If the EPF can trigger wakeup event during D3Cold then it means it is powered
> > by Vaux, isn't it?
> > 
> > - Mani
> 
> EPF needs to know that if the endpoint is getting vaux in D3cold or not
> without this info
> 
> EPF doesn't know wheather to send toggle wake or send Inband PME right.
> 
> I mean EPF should have some  prior information wheather vaux is supplied or
> not to decide
> 
> wheather to toggle wake or send in band PME.
> 

Controller driver can use the #PERST level to detect D3Cold. Then it can pass
that info to EPF over notifiers. EPF may decide whether to toggle #WAKE or
send PME based on its configuration. For MHI EPF, it can toggle #WAKE as PME
during D3Cold is not supported.

- Mani

> -KC
> 
> > 
> > > > > +		usleep_range(WAKE_DELAY_US, WAKE_DELAY_US + 500);
> > > > > +		gpiod_set_value_cansleep(pcie_ep->wake, 0);
> > > > > +		return 0;
> > > > > +	}
> > > > > +
> > > > > +	dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
> > > > > +				   DBI_CON_STATUS_POWER_STATE_MASK;
> > > > > +	if (dstate == 3) {
> > > > > +		dev_info(dev, "Device is in D3 hot sending inband PME\n");
> > > > dev_dbg(). As I said, the device can sent PME from D3cold also. So the log could
> > > > be "Waking up the host using PME".
> > > > 
> > > > > +		val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
> > > > > +		val |= PARF_PM_CTRL_XMT_PME;
> > > > > +		writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
> > > > > +	} else {
> > > > > +		dev_err(dev, "Device is not in D3 state wakeup is not supported\n");
> > > > > +		return -EPERM;
> > > > -ENOTSUPP
> > > > 
> > > > - Mani
> > > > 
> > > > > +	}
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > >    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
> > > > > 

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

Re: [PATCH RFC v1 3/3] PCI: qcom: ep: Add wake up host op to dw_pcie_ep_ops

[Krishna Chaitanya Chundru]

On 6/28/2023 10:27 AM, Manivannan Sadhasivam wrote:
> On Wed, Jun 28, 2023 at 08:01:45AM +0530, Krishna Chaitanya Chundru wrote:
>> On 6/27/2023 7:23 PM, Manivannan Sadhasivam wrote:
>>> On Mon, Jun 26, 2023 at 07:18:49PM +0530, Krishna Chaitanya Chundru wrote:
>>>> On 6/23/2023 11:48 AM, Manivannan Sadhasivam wrote:
>>>>> On Wed, Jun 14, 2023 at 08:30:49PM +0530, Krishna chaitanya chundru wrote:
>>>>>> Add wakeup host op to dw_pcie_ep_ops to wake up host from D3cold
>>>>>> or D3hot.
>>>>>>
>>>>> Commit message should describe how the wakeup is implemented in the driver.
>>>> I will correct this in next series.
>>>>>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>>>>>> ---
>>>>>>     drivers/pci/controller/dwc/pcie-qcom-ep.c | 34 +++++++++++++++++++++++++++++++
>>>>>>     1 file changed, 34 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>>>>>> index 5d146ec..916a138 100644
>>>>>> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
>>>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>>>>>> @@ -91,6 +91,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 */
>>>>>> @@ -794,10 +795,43 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
>>>>>>     		dw_pcie_ep_reset_bar(pci, bar);
>>>>>>     }
>>>>>> +static int qcom_pcie_ep_wakeup_host(struct dw_pcie_ep *ep, u8 func_no)
>>>>>> +{
>>>>>> +	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 perst, dstate, val;
>>>>>> +
>>>>>> +	perst = gpiod_get_value(pcie_ep->reset);
>>>>>> +	/* Toggle wake GPIO when device is in D3 cold */
>>>>>> +	if (perst) {
>>>>>> +		dev_info(dev, "Device is in D3 cold toggling wake\n");
>>>>> dev_dbg(). "Waking up the host by toggling WAKE#"
>>>>>
>>>>>> +		gpiod_set_value_cansleep(pcie_ep->wake, 1);
>>>>> Waking a device from D3cold requires power-on sequence by the host and in the
>>>>> presence of Vaux, the EPF should be prepared for that. In that case, the mode of
>>>>> wakeup should be decided by the EPF driver. So the wakeup API should have an
>>>>> argument to decide whether the wakeup is through PME or sideband WAKE#.
>>>>>
>>>>> Also note that as per PCIe Spec 3.0, the devices can support PME generation from
>>>>> D3cold provided that the Vaux is supplied to the device. I do not know if that
>>>>> is supported by Qcom devices but API should honor the spec. So the wakeup
>>>>> control should come from EPF driver as I suggested above.
>>>> I aggre with you, but how will EPF know the PCI device state is in D3cold or
>>>> D3hot.
>>>>
>>> We should add a notifier in the controller driver which signals EPF when it
>>> receives the DState events.. Take a look at pci_epc_linkdown().
>> Ok I will add this kind of Dstate change event
>>>> And how the EPF knows whether Vaux is supported or not in D3cold?
>>>>
>>>> If there is any existing mechanism can you please point me that.
>>>>
>>>> FYI Qcom does not support vaux power in D3 cold.
>>>>
>>> If the EPF can trigger wakeup event during D3Cold then it means it is powered
>>> by Vaux, isn't it?
>>>
>>> - Mani
>> EPF needs to know that if the endpoint is getting vaux in D3cold or not
>> without this info
>>
>> EPF doesn't know wheather to send toggle wake or send Inband PME right.
>>
>> I mean EPF should have some  prior information wheather vaux is supplied or
>> not to decide
>>
>> wheather to toggle wake or send in band PME.
>>
> Controller driver can use the #PERST level to detect D3Cold. Then it can pass
> that info to EPF over notifiers. EPF may decide whether to toggle #WAKE or
> send PME based on its configuration. For MHI EPF, it can toggle #WAKE as PME
> during D3Cold is not supported.
>
> - Mani

Can we add a new variable in ep features to know whether vaux is 
supported in D3 cold

or is there any register in the config space of EP which indicates that 
vaux is supported or not.

>
>> -KC
>>
>>>>>> +		usleep_range(WAKE_DELAY_US, WAKE_DELAY_US + 500);
>>>>>> +		gpiod_set_value_cansleep(pcie_ep->wake, 0);
>>>>>> +		return 0;
>>>>>> +	}
>>>>>> +
>>>>>> +	dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
>>>>>> +				   DBI_CON_STATUS_POWER_STATE_MASK;
>>>>>> +	if (dstate == 3) {
>>>>>> +		dev_info(dev, "Device is in D3 hot sending inband PME\n");
>>>>> dev_dbg(). As I said, the device can sent PME from D3cold also. So the log could
>>>>> be "Waking up the host using PME".
>>>>>
>>>>>> +		val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
>>>>>> +		val |= PARF_PM_CTRL_XMT_PME;
>>>>>> +		writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
>>>>>> +	} else {
>>>>>> +		dev_err(dev, "Device is not in D3 state wakeup is not supported\n");
>>>>>> +		return -EPERM;
>>>>> -ENOTSUPP
>>>>>
>>>>> - Mani
>>>>>
>>>>>> +	}
>>>>>> +
>>>>>> +	return 0;
>>>>>> +}
>>>>>> +
>>>>>>     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
>>>>>>

Re: [PATCH RFC v1 3/3] PCI: qcom: ep: Add wake up host op to dw_pcie_ep_ops

[Manivannan Sadhasivam]

On Wed, Jun 28, 2023 at 11:49:04AM +0530, Krishna Chaitanya Chundru wrote:
> 
> On 6/28/2023 10:27 AM, Manivannan Sadhasivam wrote:
> > On Wed, Jun 28, 2023 at 08:01:45AM +0530, Krishna Chaitanya Chundru wrote:
> > > On 6/27/2023 7:23 PM, Manivannan Sadhasivam wrote:
> > > > On Mon, Jun 26, 2023 at 07:18:49PM +0530, Krishna Chaitanya Chundru wrote:
> > > > > On 6/23/2023 11:48 AM, Manivannan Sadhasivam wrote:
> > > > > > On Wed, Jun 14, 2023 at 08:30:49PM +0530, Krishna chaitanya chundru wrote:
> > > > > > > Add wakeup host op to dw_pcie_ep_ops to wake up host from D3cold
> > > > > > > or D3hot.
> > > > > > > 
> > > > > > Commit message should describe how the wakeup is implemented in the driver.
> > > > > I will correct this in next series.
> > > > > > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > > > > > > ---
> > > > > > >     drivers/pci/controller/dwc/pcie-qcom-ep.c | 34 +++++++++++++++++++++++++++++++
> > > > > > >     1 file changed, 34 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > > > > > index 5d146ec..916a138 100644
> > > > > > > --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > > > > > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > > > > > @@ -91,6 +91,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 */
> > > > > > > @@ -794,10 +795,43 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
> > > > > > >     		dw_pcie_ep_reset_bar(pci, bar);
> > > > > > >     }
> > > > > > > +static int qcom_pcie_ep_wakeup_host(struct dw_pcie_ep *ep, u8 func_no)
> > > > > > > +{
> > > > > > > +	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 perst, dstate, val;
> > > > > > > +
> > > > > > > +	perst = gpiod_get_value(pcie_ep->reset);
> > > > > > > +	/* Toggle wake GPIO when device is in D3 cold */
> > > > > > > +	if (perst) {
> > > > > > > +		dev_info(dev, "Device is in D3 cold toggling wake\n");
> > > > > > dev_dbg(). "Waking up the host by toggling WAKE#"
> > > > > > 
> > > > > > > +		gpiod_set_value_cansleep(pcie_ep->wake, 1);
> > > > > > Waking a device from D3cold requires power-on sequence by the host and in the
> > > > > > presence of Vaux, the EPF should be prepared for that. In that case, the mode of
> > > > > > wakeup should be decided by the EPF driver. So the wakeup API should have an
> > > > > > argument to decide whether the wakeup is through PME or sideband WAKE#.
> > > > > > 
> > > > > > Also note that as per PCIe Spec 3.0, the devices can support PME generation from
> > > > > > D3cold provided that the Vaux is supplied to the device. I do not know if that
> > > > > > is supported by Qcom devices but API should honor the spec. So the wakeup
> > > > > > control should come from EPF driver as I suggested above.
> > > > > I aggre with you, but how will EPF know the PCI device state is in D3cold or
> > > > > D3hot.
> > > > > 
> > > > We should add a notifier in the controller driver which signals EPF when it
> > > > receives the DState events.. Take a look at pci_epc_linkdown().
> > > Ok I will add this kind of Dstate change event
> > > > > And how the EPF knows whether Vaux is supported or not in D3cold?
> > > > > 
> > > > > If there is any existing mechanism can you please point me that.
> > > > > 
> > > > > FYI Qcom does not support vaux power in D3 cold.
> > > > > 
> > > > If the EPF can trigger wakeup event during D3Cold then it means it is powered
> > > > by Vaux, isn't it?
> > > > 
> > > > - Mani
> > > EPF needs to know that if the endpoint is getting vaux in D3cold or not
> > > without this info
> > > 
> > > EPF doesn't know wheather to send toggle wake or send Inband PME right.
> > > 
> > > I mean EPF should have some  prior information wheather vaux is supplied or
> > > not to decide
> > > 
> > > wheather to toggle wake or send in band PME.
> > > 
> > Controller driver can use the #PERST level to detect D3Cold. Then it can pass
> > that info to EPF over notifiers. EPF may decide whether to toggle #WAKE or
> > send PME based on its configuration. For MHI EPF, it can toggle #WAKE as PME
> > during D3Cold is not supported.
> > 
> > - Mani
> 
> Can we add a new variable in ep features to know whether vaux is supported
> in D3 cold
> 
> or is there any register in the config space of EP which indicates that vaux
> is supported or not.
> 

I'm not aware of any method as of now but you do not need to worry. I was
just pointing it out as a possibility as per the spec. For this series, just
expose the wakeup logic to EPF as a flag.

- Mani

> > 
> > > -KC
> > > 
> > > > > > > +		usleep_range(WAKE_DELAY_US, WAKE_DELAY_US + 500);
> > > > > > > +		gpiod_set_value_cansleep(pcie_ep->wake, 0);
> > > > > > > +		return 0;
> > > > > > > +	}
> > > > > > > +
> > > > > > > +	dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
> > > > > > > +				   DBI_CON_STATUS_POWER_STATE_MASK;
> > > > > > > +	if (dstate == 3) {
> > > > > > > +		dev_info(dev, "Device is in D3 hot sending inband PME\n");
> > > > > > dev_dbg(). As I said, the device can sent PME from D3cold also. So the log could
> > > > > > be "Waking up the host using PME".
> > > > > > 
> > > > > > > +		val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
> > > > > > > +		val |= PARF_PM_CTRL_XMT_PME;
> > > > > > > +		writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
> > > > > > > +	} else {
> > > > > > > +		dev_err(dev, "Device is not in D3 state wakeup is not supported\n");
> > > > > > > +		return -EPERM;
> > > > > > -ENOTSUPP
> > > > > > 
> > > > > > - Mani
> > > > > > 
> > > > > > > +	}
> > > > > > > +
> > > > > > > +	return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > >     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
> > > > > > > 

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