Here we propose this patch series to add support in PCI endpoint
driver to wake up host from D3 states.
As endpoint cannot send any data/MSI when the D-state is in
D3cold or D3hot. Endpoint needs to bring the device back to D0
to send any kind of data.
For this endpoint needs to send inband PME the device is in D3 state or
toggle wake when the device is D3 cold and vaux is not supplied.
As EPF doestn't know the D-state of the PCI, added a notify op whenever
device state changes.
Based on the D-state the EPF driver decides to wake host either by
toggling wake or by sending PME.
When the MHI state is in M3 MHI driver will wakeup the host using the
wakeup op.
---
Changes from v3:
- changed the bool return type to int for waking the host in mhi ep driver
as suggested by dan and bjorn.
- Changed commit logs as suggested by bjorn.
Changes from v2:
- Addressed review comments made by mani.
Changes from v1:
- Moved from RFC patch to regular patch
- Inclueded EPF patch and added a new op patch to notify D-state change.
---
Krishna chaitanya chundru (9):
PCI: endpoint: Add D-state change notifier support
PCI: qcom-ep: Add support for D-state change notification
PCI: epf-mhi: Add support for handling D-state notify from EPC
PCI: qcom-ep: Update the D-state log
PCI: endpoint: Add wakeup host API to EPC core
PCI: dwc: Add wakeup host op to pci_epc_ops
PCI: qcom-ep: Add wake up host op to dw_pcie_ep_ops
PCI: epf-mhi: Add wakeup host op
bus: mhi: ep: wake up host if the MHI state is in M3
Documentation/PCI/endpoint/pci-endpoint.rst | 11 +++++
drivers/bus/mhi/ep/main.c | 27 ++++++++++++
drivers/pci/controller/dwc/pcie-designware-ep.c | 12 +++++
drivers/pci/controller/dwc/pcie-designware.h | 3 ++
drivers/pci/controller/dwc/pcie-qcom-ep.c | 36 ++++++++++++++-
drivers/pci/endpoint/functions/pci-epf-mhi.c | 27 ++++++++++++
drivers/pci/endpoint/pci-epc-core.c | 58 +++++++++++++++++++++++++
include/linux/mhi_ep.h | 4 ++
include/linux/pci-epc.h | 12 +++++
include/linux/pci-epf.h | 1 +
10 files changed, 190 insertions(+), 1 deletion(-)
--
2.7.4
Add support to notify the EPF device about the D-state change event
from the EPC device.
Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
Documentation/PCI/endpoint/pci-endpoint.rst | 5 +++++
drivers/pci/endpoint/pci-epc-core.c | 27 +++++++++++++++++++++++++++
include/linux/pci-epc.h | 1 +
include/linux/pci-epf.h | 1 +
4 files changed, 34 insertions(+)
diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
index 4f5622a..3a54713 100644
--- a/Documentation/PCI/endpoint/pci-endpoint.rst
+++ b/Documentation/PCI/endpoint/pci-endpoint.rst
@@ -78,6 +78,11 @@ by the PCI controller driver.
Cleanup the pci_epc_mem structure allocated during pci_epc_mem_init().
+* pci_epc_dstate_notity()
+
+ In order to notify all the function devices that the EPC device has
+ changed its D-state.
+
EPC APIs for the PCI Endpoint Function Driver
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 6c54fa5..ea76baf 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -785,6 +785,33 @@ void pci_epc_bme_notify(struct pci_epc *epc)
EXPORT_SYMBOL_GPL(pci_epc_bme_notify);
/**
+ * pci_epc_dstate_notity() - Notify the EPF device that EPC device D-state
+ * has changed
+ * @epc: the EPC device which has change in D-state
+ * @state: the changed D-state
+ *
+ * Invoke to Notify the EPF device that the EPC device has D-state has
+ * changed.
+ */
+void pci_epc_dstate_notity(struct pci_epc *epc, pci_power_t state)
+{
+ struct pci_epf *epf;
+
+ if (!epc || IS_ERR(epc))
+ return;
+
+ mutex_lock(&epc->list_lock);
+ list_for_each_entry(epf, &epc->pci_epf, list) {
+ mutex_lock(&epf->lock);
+ if (epf->event_ops && epf->event_ops->dstate_notify)
+ epf->event_ops->dstate_notify(epf, state);
+ mutex_unlock(&epf->lock);
+ }
+ mutex_unlock(&epc->list_lock);
+}
+EXPORT_SYMBOL_GPL(pci_epc_dstate_notity);
+
+/**
* pci_epc_destroy() - destroy the EPC device
* @epc: the EPC device that has to be destroyed
*
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 5cb6940..26a1108 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -251,4 +251,5 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
phys_addr_t *phys_addr, size_t size);
void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
void __iomem *virt_addr, size_t size);
+void pci_epc_dstate_change(struct pci_epc *epc, pci_power_t state);
#endif /* __LINUX_PCI_EPC_H */
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 3f44b6a..529075b 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -79,6 +79,7 @@ struct pci_epc_event_ops {
int (*link_up)(struct pci_epf *epf);
int (*link_down)(struct pci_epf *epf);
int (*bme)(struct pci_epf *epf);
+ int (*dstate_notify)(struct pci_epf *epf, pci_power_t state);
};
/**
--
2.7.4
Add wakeup host op for MHI EPF.
If the D-state is in D3cold toggle wake signal, otherwise send PME.
Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
drivers/pci/endpoint/functions/pci-epf-mhi.c | 16 ++++++++++++++++
include/linux/mhi_ep.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
index ee91bfc..b608505 100644
--- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
+++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
@@ -237,6 +237,21 @@ static int pci_epf_mhi_write_to_host(struct mhi_ep_cntrl *mhi_cntrl,
return 0;
}
+static int pci_epf_mhi_wakeup_host(struct mhi_ep_cntrl *mhi_cntrl)
+{
+ struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl);
+ struct pci_epf *epf = epf_mhi->epf;
+ enum pci_epc_wakeup_host_type type;
+ struct pci_epc *epc = epf->epc;
+
+ type = PCI_WAKEUP_SEND_PME;
+ if (mhi_cntrl->dstate == PCI_D3cold)
+ type = PCI_WAKEUP_TOGGLE_WAKE;
+
+ return pci_epc_wakeup_host(epc, epf->func_no, epf->vfunc_no, type);
+
+}
+
static int pci_epf_mhi_core_init(struct pci_epf *epf)
{
struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
@@ -293,6 +308,7 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf)
mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free;
mhi_cntrl->read_from_host = pci_epf_mhi_read_from_host;
mhi_cntrl->write_to_host = pci_epf_mhi_write_to_host;
+ mhi_cntrl->wakeup_host = pci_epf_mhi_wakeup_host;
/* Register the MHI EP controller */
ret = mhi_ep_register_controller(mhi_cntrl, info->config);
diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
index c3a0685..e353c429 100644
--- a/include/linux/mhi_ep.h
+++ b/include/linux/mhi_ep.h
@@ -137,6 +137,7 @@ struct mhi_ep_cntrl {
void __iomem *virt, size_t size);
int (*read_from_host)(struct mhi_ep_cntrl *mhi_cntrl, u64 from, void *to, size_t size);
int (*write_to_host)(struct mhi_ep_cntrl *mhi_cntrl, void *from, u64 to, size_t size);
+ int (*wakeup_host)(struct mhi_ep_cntrl *mhi_cntrl);
enum mhi_state mhi_state;
--
2.7.4
If the MHI state is in M3 then the most probably the host kept the
device in D3 hot or D3 cold, due to that endpoint transctions will not
be read by the host, so endpoint wakes up host to bring the host to D0
which eventually bring back the MHI state to M0.
Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
drivers/bus/mhi/ep/main.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index 6008818..46a888e 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -25,6 +25,26 @@ static DEFINE_IDA(mhi_ep_cntrl_ida);
static int mhi_ep_create_device(struct mhi_ep_cntrl *mhi_cntrl, u32 ch_id);
static int mhi_ep_destroy_device(struct device *dev, void *data);
+static int mhi_ep_wake_host(struct mhi_ep_cntrl *mhi_cntrl)
+{
+ enum mhi_state state;
+ bool mhi_reset;
+ u32 count = 0;
+
+ mhi_cntrl->wakeup_host(mhi_cntrl);
+
+ /* Wait for Host to set the M0 state */
+ while (count++ < M0_WAIT_COUNT) {
+ msleep(M0_WAIT_DELAY_MS);
+
+ mhi_ep_mmio_get_mhi_state(mhi_cntrl, &state, &mhi_reset);
+ if (state == MHI_STATE_M0)
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
static int mhi_ep_send_event(struct mhi_ep_cntrl *mhi_cntrl, u32 ring_idx,
struct mhi_ring_element *el, bool bei)
{
@@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
buf_left = skb->len;
ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
+ if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
+ if (mhi_ep_wake_host(mhi_cntrl)) {
+ dev_err(dev, "Failed to wakeup host\n");
+ return -ENODEV;
+ }
+ }
+
mutex_lock(&mhi_chan->lock);
do {
--
2.7.4
Endpoint cannot send any data/MSI when the D-state is in
D3cold or D3hot. Endpoint needs to wake up the host to
bring the D-state to D0.
Endpoint can toggle wake signal when the D-state is in D3cold and vaux is
not supplied or can send inband PME.
To support this add wakeup_host() callback to the EPC core.
Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
Documentation/PCI/endpoint/pci-endpoint.rst | 6 ++++++
drivers/pci/endpoint/pci-epc-core.c | 31 +++++++++++++++++++++++++++++
include/linux/pci-epc.h | 11 ++++++++++
3 files changed, 48 insertions(+)
diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
index 3a54713..eb79b77 100644
--- a/Documentation/PCI/endpoint/pci-endpoint.rst
+++ b/Documentation/PCI/endpoint/pci-endpoint.rst
@@ -53,6 +53,7 @@ by the PCI controller driver.
* raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
* start: ops to start the PCI link
* stop: ops to stop the PCI link
+ * wakeup_host: ops to wakeup host
The PCI controller driver can then create a new EPC device by invoking
devm_pci_epc_create()/pci_epc_create().
@@ -122,6 +123,11 @@ by the PCI endpoint function driver.
The PCI endpoint function driver should use pci_epc_mem_free_addr() to
free the memory space allocated using pci_epc_mem_alloc_addr().
+* pci_epc_wakeup_host()
+
+ The PCI endpoint function driver should use pci_epc_wakeup_host() to wakeup
+ host.
+
Other EPC APIs
~~~~~~~~~~~~~~
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index ea76baf..b419eff 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -167,6 +167,37 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
EXPORT_SYMBOL_GPL(pci_epc_get_features);
/**
+ * pci_epc_wakeup_host() - Wakeup the host
+ * @epc: the EPC device which has to wakeup the host
+ * @func_no: the physical endpoint function number in the EPC device
+ * @vfunc_no: the virtual endpoint function number in the physical function
+ * @type: specify the type of wakeup: WAKEUP_FROM_D3COLD, WAKEUP_FROM_D3HOT
+ *
+ * Invoke to wakeup host
+ */
+bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
+ enum pci_epc_wakeup_host_type type)
+{
+ int ret;
+
+ if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
+ return false;
+
+ if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
+ return false;
+
+ if (!epc->ops->wakeup_host)
+ return true;
+
+ mutex_lock(&epc->lock);
+ ret = epc->ops->wakeup_host(epc, func_no, vfunc_no, type);
+ mutex_unlock(&epc->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(pci_epc_wakeup_host);
+
+/**
* pci_epc_stop() - stop the PCI link
* @epc: the link of the EPC device that has to be stopped
*
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 26a1108..d262179 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -26,6 +26,12 @@ enum pci_epc_irq_type {
PCI_EPC_IRQ_MSIX,
};
+enum pci_epc_wakeup_host_type {
+ PCI_WAKEUP_UNKNOWN,
+ PCI_WAKEUP_SEND_PME,
+ PCI_WAKEUP_TOGGLE_WAKE,
+};
+
static inline const char *
pci_epc_interface_string(enum pci_epc_interface_type type)
{
@@ -59,6 +65,7 @@ pci_epc_interface_string(enum pci_epc_interface_type type)
* @start: ops to start the PCI link
* @stop: ops to stop the PCI link
* @get_features: ops to get the features supported by the EPC
+ * @wakeup_host: ops to wakeup the host
* @owner: the module owner containing the ops
*/
struct pci_epc_ops {
@@ -88,6 +95,8 @@ struct pci_epc_ops {
void (*stop)(struct pci_epc *epc);
const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
u8 func_no, u8 vfunc_no);
+ bool (*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
+ enum pci_epc_wakeup_host_type type);
struct module *owner;
};
@@ -234,6 +243,8 @@ int pci_epc_start(struct pci_epc *epc);
void pci_epc_stop(struct pci_epc *epc);
const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
u8 func_no, u8 vfunc_no);
+bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
+ enum pci_epc_wakeup_host_type type);
enum pci_barno
pci_epc_get_first_free_bar(const struct pci_epc_features *epc_features);
enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
--
2.7.4
Add support to pass D-state change notification to Endpoint
function driver.
Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom-ep.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 0fe7f06..66fd421 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -561,6 +561,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
struct device *dev = pci->dev;
u32 status = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_STATUS);
u32 mask = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_MASK);
+ pci_power_t state;
u32 dstate, val;
writel_relaxed(status, pcie_ep->parf + PARF_INT_ALL_CLEAR);
@@ -583,11 +584,16 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
DBI_CON_STATUS_POWER_STATE_MASK;
dev_dbg(dev, "Received D%d state event\n", dstate);
+ state = dstate;
if (dstate == 3) {
val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
val |= PARF_PM_CTRL_REQ_EXIT_L1;
writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
+ state = PCI_D3hot;
+ if (gpiod_get_value(pcie_ep->reset))
+ state = PCI_D3cold;
}
+ pci_epc_dstate_notify(pci->ep.epc, state);
} else if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
dev_dbg(dev, "Received Linkup event. Enumeration complete!\n");
dw_pcie_ep_linkup(&pci->ep);
--
2.7.4
On Thu, Jul 13, 2023 at 12:40:10PM +0530, Krishna chaitanya chundru wrote:
> Add support to notify the EPF device about the D-state change event
> from the EPC device.
>
> Signed-off-by: Krishna chaitanya chundru <[email protected]>
> ---
> Documentation/PCI/endpoint/pci-endpoint.rst | 5 +++++
> drivers/pci/endpoint/pci-epc-core.c | 27 +++++++++++++++++++++++++++
> include/linux/pci-epc.h | 1 +
> include/linux/pci-epf.h | 1 +
> 4 files changed, 34 insertions(+)
>
> diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
> index 4f5622a..3a54713 100644
> --- a/Documentation/PCI/endpoint/pci-endpoint.rst
> +++ b/Documentation/PCI/endpoint/pci-endpoint.rst
> @@ -78,6 +78,11 @@ by the PCI controller driver.
> Cleanup the pci_epc_mem structure allocated during pci_epc_mem_init().
>
>
> +* pci_epc_dstate_notity()
> +
> + In order to notify all the function devices that the EPC device has
> + changed its D-state.
Notify all the function drivers that the EPC device has changed its D-state.
> +
> EPC APIs for the PCI Endpoint Function Driver
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 6c54fa5..ea76baf 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -785,6 +785,33 @@ void pci_epc_bme_notify(struct pci_epc *epc)
> EXPORT_SYMBOL_GPL(pci_epc_bme_notify);
>
> /**
> + * pci_epc_dstate_notity() - Notify the EPF device that EPC device D-state
EPF driver
- Mani
> + * has changed
> + * @epc: the EPC device which has change in D-state
> + * @state: the changed D-state
> + *
> + * Invoke to Notify the EPF device that the EPC device has D-state has
> + * changed.
> + */
> +void pci_epc_dstate_notity(struct pci_epc *epc, pci_power_t state)
> +{
> + struct pci_epf *epf;
> +
> + if (!epc || IS_ERR(epc))
> + return;
> +
> + mutex_lock(&epc->list_lock);
> + list_for_each_entry(epf, &epc->pci_epf, list) {
> + mutex_lock(&epf->lock);
> + if (epf->event_ops && epf->event_ops->dstate_notify)
> + epf->event_ops->dstate_notify(epf, state);
> + mutex_unlock(&epf->lock);
> + }
> + mutex_unlock(&epc->list_lock);
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_dstate_notity);
> +
> +/**
> * pci_epc_destroy() - destroy the EPC device
> * @epc: the EPC device that has to be destroyed
> *
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 5cb6940..26a1108 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -251,4 +251,5 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
> phys_addr_t *phys_addr, size_t size);
> void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
> void __iomem *virt_addr, size_t size);
> +void pci_epc_dstate_change(struct pci_epc *epc, pci_power_t state);
> #endif /* __LINUX_PCI_EPC_H */
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index 3f44b6a..529075b 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -79,6 +79,7 @@ struct pci_epc_event_ops {
> int (*link_up)(struct pci_epf *epf);
> int (*link_down)(struct pci_epf *epf);
> int (*bme)(struct pci_epf *epf);
> + int (*dstate_notify)(struct pci_epf *epf, pci_power_t state);
> };
>
> /**
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
On Thu, Jul 13, 2023 at 12:40:14PM +0530, Krishna chaitanya chundru wrote:
> Endpoint cannot send any data/MSI when the D-state is in
> D3cold or D3hot. Endpoint needs to wake up the host to
> bring the D-state to D0.
>
> Endpoint can toggle wake signal when the D-state is in D3cold and vaux is
> not supplied or can send inband PME.
>
> To support this add wakeup_host() callback to the EPC core.
>
> Signed-off-by: Krishna chaitanya chundru <[email protected]>
> ---
> Documentation/PCI/endpoint/pci-endpoint.rst | 6 ++++++
> drivers/pci/endpoint/pci-epc-core.c | 31 +++++++++++++++++++++++++++++
> include/linux/pci-epc.h | 11 ++++++++++
> 3 files changed, 48 insertions(+)
>
> diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
> index 3a54713..eb79b77 100644
> --- a/Documentation/PCI/endpoint/pci-endpoint.rst
> +++ b/Documentation/PCI/endpoint/pci-endpoint.rst
> @@ -53,6 +53,7 @@ by the PCI controller driver.
> * raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
> * start: ops to start the PCI link
> * stop: ops to stop the PCI link
> + * wakeup_host: ops to wakeup host
>
> The PCI controller driver can then create a new EPC device by invoking
> devm_pci_epc_create()/pci_epc_create().
> @@ -122,6 +123,11 @@ by the PCI endpoint function driver.
> The PCI endpoint function driver should use pci_epc_mem_free_addr() to
> free the memory space allocated using pci_epc_mem_alloc_addr().
>
> +* pci_epc_wakeup_host()
> +
> + The PCI endpoint function driver should use pci_epc_wakeup_host() to wakeup
> + host.
> +
> Other EPC APIs
> ~~~~~~~~~~~~~~
>
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index ea76baf..b419eff 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -167,6 +167,37 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
> EXPORT_SYMBOL_GPL(pci_epc_get_features);
>
> /**
> + * pci_epc_wakeup_host() - Wakeup the host
> + * @epc: the EPC device which has to wakeup the host
> + * @func_no: the physical endpoint function number in the EPC device
> + * @vfunc_no: the virtual endpoint function number in the physical function
> + * @type: specify the type of wakeup: WAKEUP_FROM_D3COLD, WAKEUP_FROM_D3HOT
> + *
> + * Invoke to wakeup host
> + */
> +bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> + enum pci_epc_wakeup_host_type type)
> +{
> + int ret;
> +
> + if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
> + return false;
> +
> + if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
> + return false;
> +
> + if (!epc->ops->wakeup_host)
> + return true;
> +
> + mutex_lock(&epc->lock);
> + ret = epc->ops->wakeup_host(epc, func_no, vfunc_no, type);
> + mutex_unlock(&epc->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_wakeup_host);
> +
> +/**
> * pci_epc_stop() - stop the PCI link
> * @epc: the link of the EPC device that has to be stopped
> *
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 26a1108..d262179 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -26,6 +26,12 @@ enum pci_epc_irq_type {
> PCI_EPC_IRQ_MSIX,
> };
>
> +enum pci_epc_wakeup_host_type {
> + PCI_WAKEUP_UNKNOWN,
> + PCI_WAKEUP_SEND_PME,
> + PCI_WAKEUP_TOGGLE_WAKE,
I think I asked you to get rid of these enums and use a bool. I don't see any
new wakeup mechanism going to be added to the spec. So using enums for just 2
options looks overkill to me.
If you defer, please discuss it here.
- Mani
> +};
> +
> static inline const char *
> pci_epc_interface_string(enum pci_epc_interface_type type)
> {
> @@ -59,6 +65,7 @@ pci_epc_interface_string(enum pci_epc_interface_type type)
> * @start: ops to start the PCI link
> * @stop: ops to stop the PCI link
> * @get_features: ops to get the features supported by the EPC
> + * @wakeup_host: ops to wakeup the host
> * @owner: the module owner containing the ops
> */
> struct pci_epc_ops {
> @@ -88,6 +95,8 @@ struct pci_epc_ops {
> void (*stop)(struct pci_epc *epc);
> const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
> u8 func_no, u8 vfunc_no);
> + bool (*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> + enum pci_epc_wakeup_host_type type);
> struct module *owner;
> };
>
> @@ -234,6 +243,8 @@ int pci_epc_start(struct pci_epc *epc);
> void pci_epc_stop(struct pci_epc *epc);
> const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
> u8 func_no, u8 vfunc_no);
> +bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> + enum pci_epc_wakeup_host_type type);
> enum pci_barno
> pci_epc_get_first_free_bar(const struct pci_epc_features *epc_features);
> enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
On Thu, Jul 13, 2023 at 12:40:11PM +0530, Krishna chaitanya chundru wrote:
> Add support to pass D-state change notification to Endpoint
> function driver.
>
> Signed-off-by: Krishna chaitanya chundru <[email protected]>
One nit below:
Reviewed-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 0fe7f06..66fd421 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -561,6 +561,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
> struct device *dev = pci->dev;
> u32 status = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_STATUS);
> u32 mask = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_MASK);
> + pci_power_t state;
> u32 dstate, val;
>
> writel_relaxed(status, pcie_ep->parf + PARF_INT_ALL_CLEAR);
> @@ -583,11 +584,16 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
> dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
> DBI_CON_STATUS_POWER_STATE_MASK;
> dev_dbg(dev, "Received D%d state event\n", dstate);
> + state = dstate;
> if (dstate == 3) {
> val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
> val |= PARF_PM_CTRL_REQ_EXIT_L1;
> writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
Newline here.
- Mani
> + state = PCI_D3hot;
> + if (gpiod_get_value(pcie_ep->reset))
> + state = PCI_D3cold;
> }
> + pci_epc_dstate_notify(pci->ep.epc, state);
> } else if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
> dev_dbg(dev, "Received Linkup event. Enumeration complete!\n");
> dw_pcie_ep_linkup(&pci->ep);
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
On Thu, Jul 13, 2023 at 12:40:09PM +0530, Krishna chaitanya chundru wrote:
> Here we propose this patch series to add support in PCI endpoint
> driver to wake up host from D3 states.
>
> As endpoint cannot send any data/MSI when the D-state is in
> D3cold or D3hot. Endpoint needs to bring the device back to D0
> to send any kind of data.
>
> For this endpoint needs to send inband PME the device is in D3 state or
> toggle wake when the device is D3 cold and vaux is not supplied.
>
> As EPF doestn't know the D-state of the PCI, added a notify op whenever
> device state changes.
>
> Based on the D-state the EPF driver decides to wake host either by
> toggling wake or by sending PME.
>
> When the MHI state is in M3 MHI driver will wakeup the host using the
> wakeup op.
>
Please split this series into two. One adding D-state support and another
(dependant) adding wakeup support. We can try to merge atleast first one for
6.6.
- Mani
> ---
> Changes from v3:
> - changed the bool return type to int for waking the host in mhi ep driver
> as suggested by dan and bjorn.
> - Changed commit logs as suggested by bjorn.
> Changes from v2:
> - Addressed review comments made by mani.
> Changes from v1:
> - Moved from RFC patch to regular patch
> - Inclueded EPF patch and added a new op patch to notify D-state change.
> ---
>
> Krishna chaitanya chundru (9):
> PCI: endpoint: Add D-state change notifier support
> PCI: qcom-ep: Add support for D-state change notification
> PCI: epf-mhi: Add support for handling D-state notify from EPC
> PCI: qcom-ep: Update the D-state log
> PCI: endpoint: Add wakeup host API to EPC core
> PCI: dwc: Add wakeup host op to pci_epc_ops
> PCI: qcom-ep: Add wake up host op to dw_pcie_ep_ops
> PCI: epf-mhi: Add wakeup host op
> bus: mhi: ep: wake up host if the MHI state is in M3
>
> Documentation/PCI/endpoint/pci-endpoint.rst | 11 +++++
> drivers/bus/mhi/ep/main.c | 27 ++++++++++++
> drivers/pci/controller/dwc/pcie-designware-ep.c | 12 +++++
> drivers/pci/controller/dwc/pcie-designware.h | 3 ++
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 36 ++++++++++++++-
> drivers/pci/endpoint/functions/pci-epf-mhi.c | 27 ++++++++++++
> drivers/pci/endpoint/pci-epc-core.c | 58 +++++++++++++++++++++++++
> include/linux/mhi_ep.h | 4 ++
> include/linux/pci-epc.h | 12 +++++
> include/linux/pci-epf.h | 1 +
> 10 files changed, 190 insertions(+), 1 deletion(-)
>
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
On Thu, Jul 13, 2023 at 12:40:18PM +0530, Krishna chaitanya chundru wrote:
> If the MHI state is in M3 then the most probably the host kept the
s/then the/then
> device in D3 hot or D3 cold, due to that endpoint transctions will not
s/transctions/transactions
> be read by the host, so endpoint wakes up host to bring the host to D0
endpoint needs to wake up the host to bring the device to D0 state...
> which eventually bring back the MHI state to M0.
>
> Signed-off-by: Krishna chaitanya chundru <[email protected]>
> ---
> drivers/bus/mhi/ep/main.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index 6008818..46a888e 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -25,6 +25,26 @@ static DEFINE_IDA(mhi_ep_cntrl_ida);
> static int mhi_ep_create_device(struct mhi_ep_cntrl *mhi_cntrl, u32 ch_id);
> static int mhi_ep_destroy_device(struct device *dev, void *data);
>
> +static int mhi_ep_wake_host(struct mhi_ep_cntrl *mhi_cntrl)
> +{
> + enum mhi_state state;
> + bool mhi_reset;
> + u32 count = 0;
> +
> + mhi_cntrl->wakeup_host(mhi_cntrl);
> +
> + /* Wait for Host to set the M0 state */
> + while (count++ < M0_WAIT_COUNT) {
> + msleep(M0_WAIT_DELAY_MS);
> +
> + mhi_ep_mmio_get_mhi_state(mhi_cntrl, &state, &mhi_reset);
> + if (state == MHI_STATE_M0)
> + return 0;
> + }
> +
> + return -ENODEV;
ENODEV or ETIMEDOUT?
> +}
> +
> static int mhi_ep_send_event(struct mhi_ep_cntrl *mhi_cntrl, u32 ring_idx,
> struct mhi_ring_element *el, bool bei)
> {
> @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
> buf_left = skb->len;
> ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
>
> + if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
> + if (mhi_ep_wake_host(mhi_cntrl)) {
Don't you need lock here in the case of multiple queue requests?
- Mani
> + dev_err(dev, "Failed to wakeup host\n");
> + return -ENODEV;
> + }
> + }
> +
> mutex_lock(&mhi_chan->lock);
>
> do {
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
On Fri, Jul 28, 2023 at 10:04:52AM +0530, Manivannan Sadhasivam wrote:
> > @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
> > buf_left = skb->len;
> > ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
> >
> > + if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
> > + if (mhi_ep_wake_host(mhi_cntrl)) {
>
> Don't you need lock here in the case of multiple queue requests?
>
> - Mani
>
> > + dev_err(dev, "Failed to wakeup host\n");
> > + return -ENODEV;
> > + }
> > + }
> > +
> > mutex_lock(&mhi_chan->lock);
^^^^^^^^^^^^^^^^^^^^^^^^^^
This lock isn't enough?
regards,
dan carpenter
On Thu, Jul 13, 2023 at 12:40:18PM +0530, Krishna chaitanya chundru wrote:
> @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
> buf_left = skb->len;
> ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
>
> + if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
> + if (mhi_ep_wake_host(mhi_cntrl)) {
> + dev_err(dev, "Failed to wakeup host\n");
> + return -ENODEV;
> + }
Since you're going to be redoing this patch anyway could you please
propage the error code:
ret = mhi_ep_wake_host(mhi_cntrl);
if (ret) {
dev_err(dev, "Failed to wakeup host\n");
return ret;
}
regards,
dan carpenter
On Fri, Jul 28, 2023 at 08:50:55AM +0300, Dan Carpenter wrote:
> On Fri, Jul 28, 2023 at 10:04:52AM +0530, Manivannan Sadhasivam wrote:
> > > @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
> > > buf_left = skb->len;
> > > ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
> > >
> > > + if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
> > > + if (mhi_ep_wake_host(mhi_cntrl)) {
> >
> > Don't you need lock here in the case of multiple queue requests?
> >
> > - Mani
> >
> > > + dev_err(dev, "Failed to wakeup host\n");
> > > + return -ENODEV;
> > > + }
> > > + }
> > > +
> > > mutex_lock(&mhi_chan->lock);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> This lock isn't enough?
>
The position of this lock won't prevent cocurrent access to mhi_ep_wake_host().
- Mani
> regards,
> dan carpenter
--
மணிவண்ணன் சதாசிவம்
On 7/28/2023 10:07 AM, Manivannan Sadhasivam wrote:
> On Thu, Jul 13, 2023 at 12:40:09PM +0530, Krishna chaitanya chundru wrote:
>> Here we propose this patch series to add support in PCI endpoint
>> driver to wake up host from D3 states.
>>
>> As endpoint cannot send any data/MSI when the D-state is in
>> D3cold or D3hot. Endpoint needs to bring the device back to D0
>> to send any kind of data.
>>
>> For this endpoint needs to send inband PME the device is in D3 state or
>> toggle wake when the device is D3 cold and vaux is not supplied.
>>
>> As EPF doestn't know the D-state of the PCI, added a notify op whenever
>> device state changes.
>>
>> Based on the D-state the EPF driver decides to wake host either by
>> toggling wake or by sending PME.
>>
>> When the MHI state is in M3 MHI driver will wakeup the host using the
>> wakeup op.
>>
> Please split this series into two. One adding D-state support and another
> (dependant) adding wakeup support. We can try to merge atleast first one for
> 6.6.
>
> - Mani
Sure, can you point me any patch series that is added dependant on
another series.
>> ---
>> Changes from v3:
>> - changed the bool return type to int for waking the host in mhi ep driver
>> as suggested by dan and bjorn.
>> - Changed commit logs as suggested by bjorn.
>> Changes from v2:
>> - Addressed review comments made by mani.
>> Changes from v1:
>> - Moved from RFC patch to regular patch
>> - Inclueded EPF patch and added a new op patch to notify D-state change.
>> ---
>>
>> Krishna chaitanya chundru (9):
>> PCI: endpoint: Add D-state change notifier support
>> PCI: qcom-ep: Add support for D-state change notification
>> PCI: epf-mhi: Add support for handling D-state notify from EPC
>> PCI: qcom-ep: Update the D-state log
>> PCI: endpoint: Add wakeup host API to EPC core
>> PCI: dwc: Add wakeup host op to pci_epc_ops
>> PCI: qcom-ep: Add wake up host op to dw_pcie_ep_ops
>> PCI: epf-mhi: Add wakeup host op
>> bus: mhi: ep: wake up host if the MHI state is in M3
>>
>> Documentation/PCI/endpoint/pci-endpoint.rst | 11 +++++
>> drivers/bus/mhi/ep/main.c | 27 ++++++++++++
>> drivers/pci/controller/dwc/pcie-designware-ep.c | 12 +++++
>> drivers/pci/controller/dwc/pcie-designware.h | 3 ++
>> drivers/pci/controller/dwc/pcie-qcom-ep.c | 36 ++++++++++++++-
>> drivers/pci/endpoint/functions/pci-epf-mhi.c | 27 ++++++++++++
>> drivers/pci/endpoint/pci-epc-core.c | 58 +++++++++++++++++++++++++
>> include/linux/mhi_ep.h | 4 ++
>> include/linux/pci-epc.h | 12 +++++
>> include/linux/pci-epf.h | 1 +
>> 10 files changed, 190 insertions(+), 1 deletion(-)
>>
>> --
>> 2.7.4
>>
On 7/28/2023 11:21 AM, Dan Carpenter wrote:
> On Thu, Jul 13, 2023 at 12:40:18PM +0530, Krishna chaitanya chundru wrote:
>> @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
>> buf_left = skb->len;
>> ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
>>
>> + if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
>> + if (mhi_ep_wake_host(mhi_cntrl)) {
>> + dev_err(dev, "Failed to wakeup host\n");
>> + return -ENODEV;
>> + }
> Since you're going to be redoing this patch anyway could you please
> propage the error code:
>
> ret = mhi_ep_wake_host(mhi_cntrl);
> if (ret) {
> dev_err(dev, "Failed to wakeup host\n");
> return ret;
> }
>
> regards,
> dan carpenter
I will add this in next series.
- KC
>
On 7/28/2023 9:51 AM, Manivannan Sadhasivam wrote:
> On Thu, Jul 13, 2023 at 12:40:14PM +0530, Krishna chaitanya chundru wrote:
>> Endpoint cannot send any data/MSI when the D-state is in
>> D3cold or D3hot. Endpoint needs to wake up the host to
>> bring the D-state to D0.
>>
>> Endpoint can toggle wake signal when the D-state is in D3cold and vaux is
>> not supplied or can send inband PME.
>>
>> To support this add wakeup_host() callback to the EPC core.
>>
>> Signed-off-by: Krishna chaitanya chundru <[email protected]>
>> ---
>> Documentation/PCI/endpoint/pci-endpoint.rst | 6 ++++++
>> drivers/pci/endpoint/pci-epc-core.c | 31 +++++++++++++++++++++++++++++
>> include/linux/pci-epc.h | 11 ++++++++++
>> 3 files changed, 48 insertions(+)
>>
>> diff --git a/Documentation/PCI/endpoint/pci-endpoint.rst b/Documentation/PCI/endpoint/pci-endpoint.rst
>> index 3a54713..eb79b77 100644
>> --- a/Documentation/PCI/endpoint/pci-endpoint.rst
>> +++ b/Documentation/PCI/endpoint/pci-endpoint.rst
>> @@ -53,6 +53,7 @@ by the PCI controller driver.
>> * raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
>> * start: ops to start the PCI link
>> * stop: ops to stop the PCI link
>> + * wakeup_host: ops to wakeup host
>>
>> The PCI controller driver can then create a new EPC device by invoking
>> devm_pci_epc_create()/pci_epc_create().
>> @@ -122,6 +123,11 @@ by the PCI endpoint function driver.
>> The PCI endpoint function driver should use pci_epc_mem_free_addr() to
>> free the memory space allocated using pci_epc_mem_alloc_addr().
>>
>> +* pci_epc_wakeup_host()
>> +
>> + The PCI endpoint function driver should use pci_epc_wakeup_host() to wakeup
>> + host.
>> +
>> Other EPC APIs
>> ~~~~~~~~~~~~~~
>>
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index ea76baf..b419eff 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -167,6 +167,37 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
>> EXPORT_SYMBOL_GPL(pci_epc_get_features);
>>
>> /**
>> + * pci_epc_wakeup_host() - Wakeup the host
>> + * @epc: the EPC device which has to wakeup the host
>> + * @func_no: the physical endpoint function number in the EPC device
>> + * @vfunc_no: the virtual endpoint function number in the physical function
>> + * @type: specify the type of wakeup: WAKEUP_FROM_D3COLD, WAKEUP_FROM_D3HOT
>> + *
>> + * Invoke to wakeup host
>> + */
>> +bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> + enum pci_epc_wakeup_host_type type)
>> +{
>> + int ret;
>> +
>> + if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
>> + return false;
>> +
>> + if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no]))
>> + return false;
>> +
>> + if (!epc->ops->wakeup_host)
>> + return true;
>> +
>> + mutex_lock(&epc->lock);
>> + ret = epc->ops->wakeup_host(epc, func_no, vfunc_no, type);
>> + mutex_unlock(&epc->lock);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_epc_wakeup_host);
>> +
>> +/**
>> * pci_epc_stop() - stop the PCI link
>> * @epc: the link of the EPC device that has to be stopped
>> *
>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>> index 26a1108..d262179 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -26,6 +26,12 @@ enum pci_epc_irq_type {
>> PCI_EPC_IRQ_MSIX,
>> };
>>
>> +enum pci_epc_wakeup_host_type {
>> + PCI_WAKEUP_UNKNOWN,
>> + PCI_WAKEUP_SEND_PME,
>> + PCI_WAKEUP_TOGGLE_WAKE,
> I think I asked you to get rid of these enums and use a bool. I don't see any
> new wakeup mechanism going to be added to the spec. So using enums for just 2
> options looks overkill to me.
>
> If you defer, please discuss it here.
>
> - Mani
I will remove the enum move in to bool in next series.
- KC
>> +};
>> +
>> static inline const char *
>> pci_epc_interface_string(enum pci_epc_interface_type type)
>> {
>> @@ -59,6 +65,7 @@ pci_epc_interface_string(enum pci_epc_interface_type type)
>> * @start: ops to start the PCI link
>> * @stop: ops to stop the PCI link
>> * @get_features: ops to get the features supported by the EPC
>> + * @wakeup_host: ops to wakeup the host
>> * @owner: the module owner containing the ops
>> */
>> struct pci_epc_ops {
>> @@ -88,6 +95,8 @@ struct pci_epc_ops {
>> void (*stop)(struct pci_epc *epc);
>> const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
>> u8 func_no, u8 vfunc_no);
>> + bool (*wakeup_host)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> + enum pci_epc_wakeup_host_type type);
>> struct module *owner;
>> };
>>
>> @@ -234,6 +243,8 @@ int pci_epc_start(struct pci_epc *epc);
>> void pci_epc_stop(struct pci_epc *epc);
>> const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
>> u8 func_no, u8 vfunc_no);
>> +bool pci_epc_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> + enum pci_epc_wakeup_host_type type);
>> enum pci_barno
>> pci_epc_get_first_free_bar(const struct pci_epc_features *epc_features);
>> enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
>> --
>> 2.7.4
>>
On 7/28/2023 9:05 PM, Manivannan Sadhasivam wrote:
> On Fri, Jul 28, 2023 at 08:50:55AM +0300, Dan Carpenter wrote:
>> On Fri, Jul 28, 2023 at 10:04:52AM +0530, Manivannan Sadhasivam wrote:
>>>> @@ -464,6 +484,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
>>>> buf_left = skb->len;
>>>> ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
>>>>
>>>> + if (mhi_cntrl->mhi_state == MHI_STATE_M3) {
>>>> + if (mhi_ep_wake_host(mhi_cntrl)) {
>>> Don't you need lock here in the case of multiple queue requests?
>>>
>>> - Mani
>>>
>>>> + dev_err(dev, "Failed to wakeup host\n");
>>>> + return -ENODEV;
>>>> + }
>>>> + }
>>>> +
>>>> mutex_lock(&mhi_chan->lock);
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^
>> This lock isn't enough?
>>
> The position of this lock won't prevent cocurrent access to mhi_ep_wake_host().
>
> - Mani
I will add the lock in the next series.
>> regards,
>> dan carpenter