diff mbox series

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

Message ID 1686754850-29817-2-git-send-email-quic_krichai@quicinc.com (mailing list archive)
State Superseded
Headers show
Series PCI: EPC: Add support to wake up host from D3 states | expand

Commit Message

Krishna Chaitanya Chundru June 14, 2023, 3 p.m. UTC
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(+)

Comments

Kishon Vijay Abraham I June 19, 2023, 9:22 a.m. UTC | #1
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
Manivannan Sadhasivam June 23, 2023, 5:43 a.m. UTC | #2
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
>
Krishna Chaitanya Chundru June 26, 2023, 1:40 p.m. UTC | #3
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
>>
Manivannan Sadhasivam June 27, 2023, 2:36 p.m. UTC | #4
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
> > >
diff mbox series

Patch

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