On Fri, Oct 24, 2025 at 11:29 AM Youssef Samir <[email protected]> wrote: > > Add initial support for suspend and hibernation PM callbacks to QAIC.
Tell me more. Say something about what needs to happen in order to suspend etc... > > Signed-off-by: Youssef Samir <[email protected]> > --- > Changes in v2: > - Guard the pm callbacks with CONFIG_PM_SLEEP to fix openrisc build error > - Add __maybe_unused to helper functions used only in PM callbacks currently > - Link to v1: > https://lore.kernel.org/all/[email protected] > --- > drivers/accel/qaic/qaic.h | 2 + > drivers/accel/qaic/qaic_drv.c | 93 ++++++++++++++++++++++++++++++ > drivers/accel/qaic/qaic_timesync.c | 9 +++ > drivers/accel/qaic/qaic_timesync.h | 3 + > 4 files changed, 107 insertions(+) > > diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h > index 820d133236dd..2bfc4ce203c5 100644 > --- a/drivers/accel/qaic/qaic.h > +++ b/drivers/accel/qaic/qaic.h > @@ -161,6 +161,8 @@ struct qaic_device { > struct mhi_device *qts_ch; > /* Work queue for tasks related to MHI "QAIC_TIMESYNC" channel */ > struct workqueue_struct *qts_wq; > + /* MHI "QAIC_TIMESYNC_PERIODIC" channel device */ > + struct mhi_device *mqts_ch; > /* Head of list of page allocated by MHI bootlog device */ > struct list_head bootlog; > /* MHI bootlog channel device */ > diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c > index e162f4b8a262..8d42866d758e 100644 > --- a/drivers/accel/qaic/qaic_drv.c > +++ b/drivers/accel/qaic/qaic_drv.c > @@ -660,6 +660,94 @@ static const struct pci_error_handlers > qaic_pci_err_handler = { > .reset_done = qaic_pci_reset_done, > }; > > +static bool __maybe_unused qaic_is_under_reset(struct qaic_device *qdev) > +{ > + int rcu_id; > + bool ret; > + > + rcu_id = srcu_read_lock(&qdev->dev_lock); > + ret = qdev->dev_state != QAIC_ONLINE; > + srcu_read_unlock(&qdev->dev_lock, rcu_id); > + return ret; > +} > + > +static bool __maybe_unused qaic_data_path_busy(struct qaic_device *qdev) > +{ > + int dev_rcu_id; > + bool ret; > + int i; > + > + dev_rcu_id = srcu_read_lock(&qdev->dev_lock); > + if (qdev->dev_state != QAIC_ONLINE) { > + srcu_read_unlock(&qdev->dev_lock, dev_rcu_id); > + return false; > + } > + for (i = 0; i < qdev->num_dbc; i++) { > + struct dma_bridge_chan *dbc = &qdev->dbc[i]; > + unsigned long flags; > + int ch_rcu_id; > + > + ch_rcu_id = srcu_read_lock(&dbc->ch_lock); > + if (!dbc->usr || !dbc->in_use) { > + srcu_read_unlock(&dbc->ch_lock, ch_rcu_id); > + continue; Perhaps I'm missing something here, but what if you have num_dbc number of unused dbcs? Won't we continue until we exit the loop and then return ret, which hasn't yet been initialized? > + } > + spin_lock_irqsave(&dbc->xfer_lock, flags); > + ret = !list_empty(&dbc->xfer_list); > + spin_unlock_irqrestore(&dbc->xfer_lock, flags); > + srcu_read_unlock(&dbc->ch_lock, ch_rcu_id); > + if (ret) > + break; > + } > + srcu_read_unlock(&qdev->dev_lock, dev_rcu_id); > + return ret; > +} > + > +#ifdef CONFIG_PM_SLEEP > +static int qaic_pm_suspend(struct device *dev) > +{ > + struct qaic_device *qdev = pci_get_drvdata(to_pci_dev(dev)); > + > + dev_dbg(dev, "Suspending..\n"); > + if (qaic_data_path_busy(qdev)) { > + dev_dbg(dev, "Device's datapath is busy. Aborting > suspend..\n"); > + return -EBUSY; > + } > + if (qaic_is_under_reset(qdev)) { > + dev_dbg(dev, "Device is under reset. Aborting suspend..\n"); > + return -EBUSY; > + } > + qaic_mqts_ch_stop_timer(qdev->mqts_ch); > + qaic_pci_reset_prepare(qdev->pdev); > + pci_save_state(qdev->pdev); > + pci_disable_device(qdev->pdev); > + pci_set_power_state(qdev->pdev, PCI_D3hot); > + return 0; > +} > + > +static int qaic_pm_resume(struct device *dev) > +{ > + struct qaic_device *qdev = pci_get_drvdata(to_pci_dev(dev)); > + int ret; > + > + dev_dbg(dev, "Resuming..\n"); > + pci_set_power_state(qdev->pdev, PCI_D0); > + pci_restore_state(qdev->pdev); > + ret = pci_enable_device(qdev->pdev); > + if (ret) { > + dev_err(dev, "pci_enable_device failed on resume %d\n", ret); > + return ret; > + } > + pci_set_master(qdev->pdev); > + qaic_pci_reset_done(qdev->pdev); > + return 0; > +} > + > +static const struct dev_pm_ops qaic_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(qaic_pm_suspend, qaic_pm_resume) > +}; > +#endif > + > static struct pci_driver qaic_pci_driver = { > .name = QAIC_NAME, > .id_table = qaic_ids, > @@ -667,6 +755,11 @@ static struct pci_driver qaic_pci_driver = { > .remove = qaic_pci_remove, > .shutdown = qaic_pci_shutdown, > .err_handler = &qaic_pci_err_handler, > + .driver = { > + #ifdef CONFIG_PM_SLEEP No, that's not the right way, same with SET_SYSTEM_SLEEP_PM_OPS above. See e.g. https://lore.kernel.org/all/[email protected]/ Regards, Bjorn > + .pm = &qaic_pm_ops, > + #endif > + }, > };
