Re: [PATCH v2 2/3] net: qrtr: support suspend/hibernation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


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

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux