On 2/20/2024 7:00 PM, Baochen Qiang wrote: > MHI devices may not be destroyed during suspend/hibernation, so need > to unprepare/prepare MHI channels throughout the transition. > > The RFC version adds new API to MHI stack with which an MHI controller > driver can do unprepare/prepare directly by itself, see > > https://patchwork.kernel.org/project/linux-wireless/patch/20231127162022.518834-3-kvalo@xxxxxxxxxx/ > > Although it works well Mani pointed out that the design is not good > because MHI channels are managed by MHI client driver thus should not > be touched by others. See the discussion > > https://lore.kernel.org/mhi/20231127162022.518834-1-kvalo@xxxxxxxxxx/ > > This version changes to add suspend/resume callbacks to achieve the > same purpose. 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> > --- > net/qrtr/mhi.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c > index 9ced13c0627a..b54a6c2113e9 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 qcom_mhi_qrtr_pm_suspend_late(struct device *dev) Don't your new functions also need to be annotated as __maybe_unused? > +{ > + struct mhi_device *mhi_dev = container_of(dev, struct mhi_device, dev); > + > + mhi_unprepare_from_transfer(mhi_dev); > + > + return 0; > +} > + > +static int 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 __maybe_unused qcom_mhi_qrtr_pm_ops = { this does not need to be __maybe_unused, see below > + 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,9 @@ static struct mhi_driver qcom_mhi_qrtr_driver = { > .id_table = qcom_mhi_qrtr_id_table, > .driver = { > .name = "qcom_mhi_qrtr", > +#ifdef CONFIG_PM conditional compilation isn't necessary here since the 'pm' member is always present > + .pm = &qcom_mhi_qrtr_pm_ops, > +#endif > }, > }; >
