On 2/27/2024 3:15 PM, Manivannan Sadhasivam wrote:
On Tue, Feb 27, 2024 at 02:36:12PM +0800, Baochen Qiang wrote:
MHI devices may not be destroyed during suspend/hibernation, so need
to unprepare/prepare MHI channels throughout the transition, this is
done by adding suspend/resume callbacks.
The suspend callback is called in the late suspend stage, this means
MHI channels are still alive at suspend stage, and that makes it
possible for an MHI controller driver to communicate with others over
those channels at suspend stage. While the resume callback is called
in the early resume stage, for a similar reason.
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30
Signed-off-by: Baochen Qiang <quic_bqiang@xxxxxxxxxxx>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
Could you please confirm if this patch can be applied separately and won't cause
regression?
Just searched the kernel and found another two drivers using IPCR
channels, one is pci_epf_mhi_driver in
drivers/pci/endpoint/functions/pci-epf-mhi.c and the other is
mhi_pci_driver in drivers/bus/mhi/host/pci_generic.c.
For pci_epf_mhi_driver, this is not impacted because the devices for
those channels are attached to mhi_ep_bus_type while QRTR MHI driver
attached to mhi_bus_type.
For mhi_pci_driver, I am afraid there would be regressions caused by
this patch. The regression is because when system suspends,
mhi_pci_suspend() is called and then qcom_mhi_qrtr_pm_suspend_late()
called, however the latter would fail because MHI is moved to M3 state
by call mhi_pm_suspend() in mhi_pci_suspend(). To address this, there
might be two options: the first is to move mhi_pci_suspend() to a late
suspend stage which would be called after
qcom_mhi_qrtr_pm_suspend_late(). and the second is to avoid whole QRTR
suspend operation in such cases. I prefer the second one, because if MHI
is going to suspend, instead of power down, it is pointless to unprepare
MHI channels and re-prepare them after resume back. We can achieve this
purpose by adding a status_cb() to QRTR MHI driver which would be called
when MHI goes to low power mode. And then QRTR MHI driver could decide
not to suspend/resume if low power mode is notified.
Your thoughts?
- Mani
---
net/qrtr/mhi.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index 9ced13c0627a..e96b82a6742c 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c
@@ -118,6 +118,32 @@ static const struct mhi_device_id qcom_mhi_qrtr_id_table[] = {
};
MODULE_DEVICE_TABLE(mhi, qcom_mhi_qrtr_id_table);
+static int __maybe_unused qcom_mhi_qrtr_pm_suspend_late(struct device *dev)
+{
+ struct mhi_device *mhi_dev = container_of(dev, struct mhi_device, dev);
+
+ mhi_unprepare_from_transfer(mhi_dev);
+
+ return 0;
+}
+
+static int __maybe_unused qcom_mhi_qrtr_pm_resume_early(struct device *dev)
+{
+ struct mhi_device *mhi_dev = container_of(dev, struct mhi_device, dev);
+ int rc;
+
+ rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
+ if (rc)
+ dev_err(dev, "failed to prepare for autoqueue transfer %d\n", rc);
+
+ return rc;
+}
+
+static const struct dev_pm_ops qcom_mhi_qrtr_pm_ops = {
+ SET_LATE_SYSTEM_SLEEP_PM_OPS(qcom_mhi_qrtr_pm_suspend_late,
+ qcom_mhi_qrtr_pm_resume_early)
+};
+
static struct mhi_driver qcom_mhi_qrtr_driver = {
.probe = qcom_mhi_qrtr_probe,
.remove = qcom_mhi_qrtr_remove,
@@ -126,6 +152,7 @@ static struct mhi_driver qcom_mhi_qrtr_driver = {
.id_table = qcom_mhi_qrtr_id_table,
.driver = {
.name = "qcom_mhi_qrtr",
+ .pm = &qcom_mhi_qrtr_pm_ops,
},
};
--
2.25.1
