Hi Loic, On 12/10/20 2:02 AM, Loic Poulain wrote:
Add support for system wide suspend/resume. During suspend, MHI device controller must be put in M3 state and PCI bus in D3 state. Add a recovery procedure allowing to reinitialize the device in case of error during resume steps, which can happen if device loses power (and so its context) while system suspend. Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx> --- [..] +int __maybe_unused mhi_pci_suspend(struct device *dev)
suspend API is not static but resume is. Any reason for that ?
+{ + struct pci_dev *pdev = to_pci_dev(dev); + struct mhi_pci_device *mhi_pdev = dev_get_drvdata(dev); + struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl; + + cancel_work_sync(&mhi_pdev->recovery_work); + + /* Transition to M3 state */ + mhi_pm_suspend(mhi_cntrl); + + pci_save_state(pdev); + pci_disable_device(pdev); + pci_wake_from_d3(pdev, true); + pci_set_power_state(pdev, PCI_D3hot); + + return 0; +} + +static int __maybe_unused mhi_pci_resume(struct device *dev) +{ + struct pci_dev *pdev = to_pci_dev(dev); + struct mhi_pci_device *mhi_pdev = dev_get_drvdata(dev); + struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl; + int err; + + pci_set_power_state(pdev, PCI_D0); + pci_restore_state(pdev); + pci_set_master(pdev); + + err = pci_enable_device(pdev); + if (err) + goto err_recovery; + + /* Exit M3, transition to M0 state */ + err = mhi_pm_resume(mhi_cntrl); + if (err) { + dev_err(&pdev->dev, "failed to resume device: %d\n", err); + goto err_recovery; + } + + return 0; + +err_recovery: + /* The device may have loose power or crashed, try recovering it */ + queue_work(system_long_wq, &mhi_pdev->recovery_work); + return 0; +} + +static const struct dev_pm_ops mhi_pci_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(mhi_pci_suspend, mhi_pci_resume) +}; + static struct pci_driver mhi_pci_driver = { .name = "mhi-pci-generic", .id_table = mhi_pci_id_table, .probe = mhi_pci_probe, .remove = mhi_pci_remove, .err_handler = &mhi_pci_err_handler, + .driver.pm = &mhi_pci_pm_ops }; module_pci_driver(mhi_pci_driver);
Overall change looks good from my side. Once you address or answer my query you can add my
Reviewed-by: Hemant Kumar <hemantk@xxxxxxxxxxxxxx> Thanks, Hemant -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project