* Re: [PATCH v2] accel/qaic: Add support for PM callbacks
@ 2025-10-28 0:09 kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2025-10-28 0:09 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20251024162916.801196-1-youssef.abdulrahman@oss.qualcomm.com>
References: <20251024162916.801196-1-youssef.abdulrahman@oss.qualcomm.com>
TO: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
TO: jeff.hugo@oss.qualcomm.com
TO: carl.vanderlip@oss.qualcomm.com
TO: troy.hanson@oss.qualcomm.com
TO: zachary.mckevitt@oss.qualcomm.com
CC: ogabbay@kernel.org
CC: lizhi.hou@amd.com
CC: karol.wachowski@linux.intel.com
CC: linux-arm-msm@vger.kernel.org
CC: dri-devel@lists.freedesktop.org
Hi Youssef,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-tip/drm-tip linus/master v6.18-rc3 next-20251027]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Youssef-Samir/accel-qaic-Add-support-for-PM-callbacks/20251025-003501
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20251024162916.801196-1-youssef.abdulrahman%40oss.qualcomm.com
patch subject: [PATCH v2] accel/qaic: Add support for PM callbacks
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: x86_64-randconfig-161-20251026 (https://download.01.org/0day-ci/archive/20251028/202510280738.gGmJs0WE-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202510280738.gGmJs0WE-lkp@intel.com/
smatch warnings:
drivers/accel/qaic/qaic_drv.c:700 qaic_data_path_busy() error: uninitialized symbol 'ret'.
vim +/ret +700 drivers/accel/qaic/qaic_drv.c
41074b262e7e90 Youssef Samir 2025-10-24 670
41074b262e7e90 Youssef Samir 2025-10-24 671 static bool __maybe_unused qaic_data_path_busy(struct qaic_device *qdev)
41074b262e7e90 Youssef Samir 2025-10-24 672 {
41074b262e7e90 Youssef Samir 2025-10-24 673 int dev_rcu_id;
41074b262e7e90 Youssef Samir 2025-10-24 674 bool ret;
41074b262e7e90 Youssef Samir 2025-10-24 675 int i;
41074b262e7e90 Youssef Samir 2025-10-24 676
41074b262e7e90 Youssef Samir 2025-10-24 677 dev_rcu_id = srcu_read_lock(&qdev->dev_lock);
41074b262e7e90 Youssef Samir 2025-10-24 678 if (qdev->dev_state != QAIC_ONLINE) {
41074b262e7e90 Youssef Samir 2025-10-24 679 srcu_read_unlock(&qdev->dev_lock, dev_rcu_id);
41074b262e7e90 Youssef Samir 2025-10-24 680 return false;
41074b262e7e90 Youssef Samir 2025-10-24 681 }
41074b262e7e90 Youssef Samir 2025-10-24 682 for (i = 0; i < qdev->num_dbc; i++) {
41074b262e7e90 Youssef Samir 2025-10-24 683 struct dma_bridge_chan *dbc = &qdev->dbc[i];
41074b262e7e90 Youssef Samir 2025-10-24 684 unsigned long flags;
41074b262e7e90 Youssef Samir 2025-10-24 685 int ch_rcu_id;
41074b262e7e90 Youssef Samir 2025-10-24 686
41074b262e7e90 Youssef Samir 2025-10-24 687 ch_rcu_id = srcu_read_lock(&dbc->ch_lock);
41074b262e7e90 Youssef Samir 2025-10-24 688 if (!dbc->usr || !dbc->in_use) {
41074b262e7e90 Youssef Samir 2025-10-24 689 srcu_read_unlock(&dbc->ch_lock, ch_rcu_id);
41074b262e7e90 Youssef Samir 2025-10-24 690 continue;
41074b262e7e90 Youssef Samir 2025-10-24 691 }
41074b262e7e90 Youssef Samir 2025-10-24 692 spin_lock_irqsave(&dbc->xfer_lock, flags);
41074b262e7e90 Youssef Samir 2025-10-24 693 ret = !list_empty(&dbc->xfer_list);
41074b262e7e90 Youssef Samir 2025-10-24 694 spin_unlock_irqrestore(&dbc->xfer_lock, flags);
41074b262e7e90 Youssef Samir 2025-10-24 695 srcu_read_unlock(&dbc->ch_lock, ch_rcu_id);
41074b262e7e90 Youssef Samir 2025-10-24 696 if (ret)
41074b262e7e90 Youssef Samir 2025-10-24 697 break;
41074b262e7e90 Youssef Samir 2025-10-24 698 }
41074b262e7e90 Youssef Samir 2025-10-24 699 srcu_read_unlock(&qdev->dev_lock, dev_rcu_id);
41074b262e7e90 Youssef Samir 2025-10-24 @700 return ret;
41074b262e7e90 Youssef Samir 2025-10-24 701 }
41074b262e7e90 Youssef Samir 2025-10-24 702
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2] accel/qaic: Add support for PM callbacks
@ 2025-10-24 16:29 Youssef Samir
2025-10-24 23:21 ` Carl Vanderlip
2025-10-26 2:15 ` Bjorn Andersson
0 siblings, 2 replies; 4+ messages in thread
From: Youssef Samir @ 2025-10-24 16:29 UTC (permalink / raw)
To: jeff.hugo, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel
Add initial support for suspend and hibernation PM callbacks to QAIC.
Signed-off-by: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
---
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/20251022204005.3888195-1-youssef.abdulrahman@oss.qualcomm.com
---
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;
+ }
+ 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
+ .pm = &qaic_pm_ops,
+ #endif
+ },
};
static int __init qaic_init(void)
diff --git a/drivers/accel/qaic/qaic_timesync.c b/drivers/accel/qaic/qaic_timesync.c
index 3fac540f8e03..8af2475f4f36 100644
--- a/drivers/accel/qaic/qaic_timesync.c
+++ b/drivers/accel/qaic/qaic_timesync.c
@@ -171,6 +171,13 @@ static void qaic_timesync_timer(struct timer_list *t)
dev_err(mqtsdev->dev, "%s mod_timer error:%d\n", __func__, ret);
}
+void qaic_mqts_ch_stop_timer(struct mhi_device *mhi_dev)
+{
+ struct mqts_dev *mqtsdev = dev_get_drvdata(&mhi_dev->dev);
+
+ timer_delete_sync(&mqtsdev->timer);
+}
+
static int qaic_timesync_probe(struct mhi_device *mhi_dev, const struct mhi_device_id *id)
{
struct qaic_device *qdev = pci_get_drvdata(to_pci_dev(mhi_dev->mhi_cntrl->cntrl_dev));
@@ -206,6 +213,7 @@ static int qaic_timesync_probe(struct mhi_device *mhi_dev, const struct mhi_devi
timer->expires = jiffies + msecs_to_jiffies(timesync_delay_ms);
add_timer(timer);
dev_set_drvdata(&mhi_dev->dev, mqtsdev);
+ qdev->mqts_ch = mhi_dev;
return 0;
@@ -221,6 +229,7 @@ static void qaic_timesync_remove(struct mhi_device *mhi_dev)
{
struct mqts_dev *mqtsdev = dev_get_drvdata(&mhi_dev->dev);
+ mqtsdev->qdev->mqts_ch = NULL;
timer_delete_sync(&mqtsdev->timer);
mhi_unprepare_from_transfer(mqtsdev->mhi_dev);
kfree(mqtsdev->sync_msg);
diff --git a/drivers/accel/qaic/qaic_timesync.h b/drivers/accel/qaic/qaic_timesync.h
index 851b7acd43bb..77b9c2b55057 100644
--- a/drivers/accel/qaic/qaic_timesync.h
+++ b/drivers/accel/qaic/qaic_timesync.h
@@ -6,6 +6,9 @@
#ifndef __QAIC_TIMESYNC_H__
#define __QAIC_TIMESYNC_H__
+#include <linux/mhi.h>
+
int qaic_timesync_init(void);
void qaic_timesync_deinit(void);
+void qaic_mqts_ch_stop_timer(struct mhi_device *mhi_dev);
#endif /* __QAIC_TIMESYNC_H__ */
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] accel/qaic: Add support for PM callbacks
2025-10-24 16:29 Youssef Samir
@ 2025-10-24 23:21 ` Carl Vanderlip
2025-10-26 2:15 ` Bjorn Andersson
1 sibling, 0 replies; 4+ messages in thread
From: Carl Vanderlip @ 2025-10-24 23:21 UTC (permalink / raw)
To: Youssef Samir, jeff.hugo, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel
On 10/24/2025 9:29 AM, Youssef Samir wrote:
> Add initial support for suspend and hibernation PM callbacks to QAIC.
>
> Signed-off-by: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
> ---
> 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/20251022204005.3888195-1-youssef.abdulrahman@oss.qualcomm.com
Reviewed-by: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] accel/qaic: Add support for PM callbacks
2025-10-24 16:29 Youssef Samir
2025-10-24 23:21 ` Carl Vanderlip
@ 2025-10-26 2:15 ` Bjorn Andersson
1 sibling, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2025-10-26 2:15 UTC (permalink / raw)
To: Youssef Samir
Cc: jeff.hugo, carl.vanderlip, troy.hanson, zachary.mckevitt, ogabbay,
lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel
On Fri, Oct 24, 2025 at 11:29 AM Youssef Samir
<youssef.abdulrahman@oss.qualcomm.com> 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 <youssef.abdulrahman@oss.qualcomm.com>
> ---
> 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/20251022204005.3888195-1-youssef.abdulrahman@oss.qualcomm.com
> ---
> 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/20250708-arm-scmi-modern-pm-ops-v1-1-14bc9c352888@kernel.org/
Regards,
Bjorn
> + .pm = &qaic_pm_ops,
> + #endif
> + },
> };
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-28 0:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-28 0:09 [PATCH v2] accel/qaic: Add support for PM callbacks kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2025-10-24 16:29 Youssef Samir
2025-10-24 23:21 ` Carl Vanderlip
2025-10-26 2:15 ` Bjorn Andersson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.