| Message ID | 1686754850-29817-2-git-send-email-quic_krichai@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | [RFC,v1,1/3] PCI: endpoint: Add wakeup host API to EPC core | expand |
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
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 >
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 >>
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 --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
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(+)