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

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

*** BLURB HERE ***

Krishna chaitanya chundru (9):
PCI: endpoint: Add dstate change notifier support
PCI: qcom-ep: Add support for D-state change notification
PCI: qcom-ep: Update the D-state log
PCI: epf-mhi: Add support for handling D-state notify from EPC
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 is the MHI state is in M3

Documentation/PCI/endpoint/pci-endpoint.rst | 11 +++++
drivers/bus/mhi/ep/main.c | 28 ++++++++++++
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 | 28 ++++++++++++
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, 192 insertions(+), 1 deletion(-)

--
2.7.4



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

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


Subject: [PATCH v3 4/9] PCI: epf-mhi: Add support for handling D-state notify from EPC

Add support for handling D-state notify for MHI EPF.

Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
drivers/pci/endpoint/functions/pci-epf-mhi.c | 11 +++++++++++
include/linux/mhi_ep.h | 3 +++
2 files changed, 14 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
index 9c1f5a1..631eb2f7 100644
--- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
+++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
@@ -339,6 +339,16 @@ static int pci_epf_mhi_bme(struct pci_epf *epf)
return 0;
}

+static int pci_epf_mhi_dstate_notify(struct pci_epf *epf, pci_power_t state)
+{
+ struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
+ struct mhi_ep_cntrl *mhi_cntrl = &epf_mhi->mhi_cntrl;
+
+ mhi_cntrl->dstate = state;
+
+ return 0;
+}
+
static int pci_epf_mhi_bind(struct pci_epf *epf)
{
struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
@@ -394,6 +404,7 @@ static struct pci_epc_event_ops pci_epf_mhi_event_ops = {
.link_up = pci_epf_mhi_link_up,
.link_down = pci_epf_mhi_link_down,
.bme = pci_epf_mhi_bme,
+ .dstate_notify = pci_epf_mhi_dstate_notify,
};

static int pci_epf_mhi_probe(struct pci_epf *epf,
diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
index f198a8a..c3a0685 100644
--- a/include/linux/mhi_ep.h
+++ b/include/linux/mhi_ep.h
@@ -8,6 +8,7 @@

#include <linux/dma-direction.h>
#include <linux/mhi.h>
+#include <linux/pci.h>

#define MHI_EP_DEFAULT_MTU 0x8000

@@ -139,6 +140,8 @@ struct mhi_ep_cntrl {

enum mhi_state mhi_state;

+ pci_power_t dstate;
+
u32 max_chan;
u32 mru;
u32 event_rings;
--
2.7.4


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

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 | 17 +++++++++++++++++
include/linux/mhi_ep.h | 1 +
2 files changed, 18 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
index 631eb2f7..d84f869 100644
--- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
+++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
@@ -237,6 +237,22 @@ 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;
+ int ret;
+
+ 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 +309,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


Subject: [PATCH v3 3/9] PCI: qcom-ep: Update the D-state log

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

Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 66fd421..75ab6d6 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -583,7 +583,6 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
} else if (FIELD_GET(PARF_INT_ALL_DSTATE_CHANGE, status)) {
dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
DBI_CON_STATUS_POWER_STATE_MASK;
- dev_dbg(dev, "Received D%d state event\n", dstate);
state = dstate;
if (dstate == 3) {
val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
@@ -593,6 +592,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
if (gpiod_get_value(pcie_ep->reset))
state = PCI_D3cold;
}
+ dev_dbg(dev, "Received D-state:%s event\n", pci_power_name(state));
pci_epc_dstate_notify(pci->ep.epc, state);
} else if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
dev_dbg(dev, "Received Linkup event. Enumeration complete!\n");
--
2.7.4


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

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

Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom-ep.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 75ab6d6..6472554 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -89,6 +89,7 @@
/* PARF_PM_CTRL register fields */
#define PARF_PM_CTRL_REQ_EXIT_L1 BIT(1)
#define PARF_PM_CTRL_READY_ENTR_L23 BIT(2)
+#define PARF_PM_CTRL_XMT_PME BIT(4)
#define PARF_PM_CTRL_REQ_NOT_ENTR_L1 BIT(5)

/* PARF_MHI_CLOCK_RESET_CTRL fields */
@@ -729,10 +730,37 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
dw_pcie_ep_reset_bar(pci, bar);
}

+static bool qcom_pcie_ep_wakeup_host(struct dw_pcie_ep *ep, u8 func_no,
+ enum pci_epc_wakeup_host_type type)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ struct qcom_pcie_ep *pcie_ep = to_pcie_ep(pci);
+ struct device *dev = pci->dev;
+ u32 val;
+
+ if (type == PCI_WAKEUP_TOGGLE_WAKE) {
+ dev_dbg(dev, "Waking up the host by toggling WAKE#\n");
+ gpiod_set_value_cansleep(pcie_ep->wake, 1);
+ usleep_range(WAKE_DELAY_US, WAKE_DELAY_US + 500);
+ gpiod_set_value_cansleep(pcie_ep->wake, 0);
+
+ } else if (type == PCI_WAKEUP_SEND_PME) {
+ dev_dbg(dev, "Waking up the host using PME\n");
+ val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
+ writel_relaxed(val | PARF_PM_CTRL_XMT_PME, pcie_ep->parf + PARF_PM_CTRL);
+ writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
+
+ } else
+ return false;
+
+ return true;
+}
+
static const struct dw_pcie_ep_ops pci_ep_ops = {
.ep_init = qcom_pcie_ep_init,
.raise_irq = qcom_pcie_ep_raise_irq,
.get_features = qcom_pcie_epc_get_features,
+ .wakeup_host = qcom_pcie_ep_wakeup_host,
};

static int qcom_pcie_ep_probe(struct platform_device *pdev)
--
2.7.4


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

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 | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index 6008818..46a8a3c 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -25,6 +25,27 @@ 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 bool 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 */
+ do {
+ msleep(M0_WAIT_DELAY_MS);
+ mhi_ep_mmio_get_mhi_state(mhi_cntrl, &state, &mhi_reset);
+ count++;
+ } while (state != MHI_STATE_M0 && count < M0_WAIT_COUNT);
+
+ if (state != MHI_STATE_M0)
+ return false;
+
+ return true;
+}
+
static int mhi_ep_send_event(struct mhi_ep_cntrl *mhi_cntrl, u32 ring_idx,
struct mhi_ring_element *el, bool bei)
{
@@ -464,6 +485,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


Subject: [PATCH v3 1/9] PCI: endpoint: Add dstate change notifier support

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


Subject: [PATCH v3 6/9] pci: dwc: Add wakeup host op to pci_epc_ops

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

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

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index f9182f8..f61b172 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -463,6 +463,17 @@ dw_pcie_ep_get_features(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
return ep->ops->get_features(ep);
}

+static bool dw_pcie_ep_wakeup_host(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
+ enum pci_epc_wakeup_host_type type)
+{
+ struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+
+ if (!ep->ops->wakeup_host)
+ return false;
+
+ return ep->ops->wakeup_host(ep, func_no, type);
+}
+
static const struct pci_epc_ops epc_ops = {
.write_header = dw_pcie_ep_write_header,
.set_bar = dw_pcie_ep_set_bar,
@@ -477,6 +488,7 @@ static const struct pci_epc_ops epc_ops = {
.start = dw_pcie_ep_start,
.stop = dw_pcie_ep_stop,
.get_features = dw_pcie_ep_get_features,
+ .wakeup_host = dw_pcie_ep_wakeup_host,
};

int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 6156606..9417651 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -330,6 +330,9 @@ struct dw_pcie_ep_ops {
* driver.
*/
unsigned int (*func_conf_select)(struct dw_pcie_ep *ep, u8 func_no);
+
+ bool (*wakeup_host)(struct dw_pcie_ep *ep, u8 func_no,
+ enum pci_epc_wakeup_host_type type);
};

struct dw_pcie_ep_func {
--
2.7.4


2023-07-07 12:10:15

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v3 9/9] bus: mhi: ep: wake up host is the MHI state is in M3

On Fri, Jul 07, 2023 at 04:33:56PM +0530, Krishna chaitanya chundru wrote:
> 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 | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index 6008818..46a8a3c 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -25,6 +25,27 @@ 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 bool 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 */
> + do {
> + msleep(M0_WAIT_DELAY_MS);
> + mhi_ep_mmio_get_mhi_state(mhi_cntrl, &state, &mhi_reset);
> + count++;
> + } while (state != MHI_STATE_M0 && count < M0_WAIT_COUNT);
> +
>+ if (state != MHI_STATE_M0)
>+ return false;

Functions which return false on success are an abomination. Also
boolean functions should be named for the question they answer such as
access_ok() or has_feature() etc.

Actually, I think it's the caller which is wrong. This returns true on
success and false on failure. But the caller assumes true is failure.
It suggests that this has not been tested.

> +
> + return true;
> +}

Write it like this:

static int mhi_ep_wake_host(struct mhi_ep_cntrl *mhi_cntrl)
{
enum mhi_state state;
bool mhi_reset;
int count = 0;

mhi_cntrl->wakeup_host(mhi_cntrl);

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 +485,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;
> + }

Then this becomes:

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

regards,
dan carpenter


2023-07-07 15:12:01

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] PCI: EPC: Add support to wake up host from D3 states

On Fri, Jul 07, 2023 at 04:33:47PM +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.
>
> 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
>
> *** BLURB HERE ***
>
> Krishna chaitanya chundru (9):
> PCI: endpoint: Add dstate change notifier support

"D-state" to match the other patches.

> PCI: qcom-ep: Add support for D-state change notification
> PCI: qcom-ep: Update the D-state log
> PCI: epf-mhi: Add support for handling D-state notify from EPC
> PCI: endpoint: Add wakeup host API to EPC core
> pci: dwc: Add wakeup host op to pci_epc_ops

"PCI:" to match the rest.

> 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 is the MHI state is in M3

"Wake up host ..." to match previous history of the file.

"*if* MHI state is M3"? (Not "is the ...")

> Documentation/PCI/endpoint/pci-endpoint.rst | 11 +++++
> drivers/bus/mhi/ep/main.c | 28 ++++++++++++
> 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 | 28 ++++++++++++
> 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, 192 insertions(+), 1 deletion(-)
>
> --
> 2.7.4
>

2023-07-07 15:23:59

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 9/9] bus: mhi: ep: wake up host is the MHI state is in M3

On Fri, Jul 07, 2023 at 02:41:57PM +0300, Dan Carpenter wrote:
> On Fri, Jul 07, 2023 at 04:33:56PM +0530, Krishna chaitanya chundru wrote:
> > 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 | 28 ++++++++++++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> > index 6008818..46a8a3c 100644
> > --- a/drivers/bus/mhi/ep/main.c
> > +++ b/drivers/bus/mhi/ep/main.c
> > @@ -25,6 +25,27 @@ 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 bool 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 */
> > + do {
> > + msleep(M0_WAIT_DELAY_MS);
> > + mhi_ep_mmio_get_mhi_state(mhi_cntrl, &state, &mhi_reset);
> > + count++;
> > + } while (state != MHI_STATE_M0 && count < M0_WAIT_COUNT);
> > +
> >+ if (state != MHI_STATE_M0)
> >+ return false;
>
> Functions which return false on success are an abomination. Also
> boolean functions should be named for the question they answer such
> as access_ok() or has_feature() etc.

+1. Also nice if boolean functions do not have side effects, so in
this case, where mhi_ep_wake_host() *does* something that might fail,
I think "return 0 for success or negative error value" is easier to
read.

> Write it like this:
>
> static int mhi_ep_wake_host(struct mhi_ep_cntrl *mhi_cntrl)
> {
> enum mhi_state state;
> bool mhi_reset;
> int count = 0;
>
> mhi_cntrl->wakeup_host(mhi_cntrl);
>
> 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;
> }

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


On 7/7/2023 8:42 PM, Bjorn Helgaas wrote:
> On Fri, Jul 07, 2023 at 02:41:57PM +0300, Dan Carpenter wrote:
>> On Fri, Jul 07, 2023 at 04:33:56PM +0530, Krishna chaitanya chundru wrote:
>>> 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 | 28 ++++++++++++++++++++++++++++
>>> 1 file changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
>>> index 6008818..46a8a3c 100644
>>> --- a/drivers/bus/mhi/ep/main.c
>>> +++ b/drivers/bus/mhi/ep/main.c
>>> @@ -25,6 +25,27 @@ 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 bool 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 */
>>> + do {
>>> + msleep(M0_WAIT_DELAY_MS);
>>> + mhi_ep_mmio_get_mhi_state(mhi_cntrl, &state, &mhi_reset);
>>> + count++;
>>> + } while (state != MHI_STATE_M0 && count < M0_WAIT_COUNT);
>>> +
>>> + if (state != MHI_STATE_M0)
>>> + return false;
>> Functions which return false on success are an abomination. Also
>> boolean functions should be named for the question they answer such
>> as access_ok() or has_feature() etc.
> +1. Also nice if boolean functions do not have side effects, so in
> this case, where mhi_ep_wake_host() *does* something that might fail,
> I think "return 0 for success or negative error value" is easier to
> read.

sure Dan and Bjorn I will replace bool with int return type as suggested
in the next patch series.

- KC

>> Write it like this:
>>
>> static int mhi_ep_wake_host(struct mhi_ep_cntrl *mhi_cntrl)
>> {
>> enum mhi_state state;
>> bool mhi_reset;
>> int count = 0;
>>
>> mhi_cntrl->wakeup_host(mhi_cntrl);
>>
>> 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;
>> }