[PATCH 2/2] mailbox: mtk-cmdq: Move pm_runimte_get and put to mbox_chan_ops API

From: Jason-JH . Lin
Date: Fri Jun 14 2024 - 00:01:51 EST


When we run kernel with lockdebug option, we will get the BUG below:
BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1164
in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 3616, name: kworker/u17:3
preempt_count: 1, expected: 0
RCU nest depth: 0, expected: 0
INFO: lockdep is turned off.
irq event stamp: 0
CPU: 1 PID: 3616 Comm: kworker/u17:3 Not tainted 6.1.87-lockdep-14133-g26e933aca785 #1
Hardware name: Google Ciri sku0/unprovisioned board (DT)
Workqueue: imgsys_runner imgsys_runner_func
Call trace:
dump_backtrace+0x100/0x120
show_stack+0x20/0x2c
dump_stack_lvl+0x84/0xb4
dump_stack+0x18/0x48
__might_resched+0x354/0x4c0
__might_sleep+0x98/0xe4
__pm_runtime_resume+0x70/0x124
cmdq_mbox_send_data+0xe4/0xb1c
msg_submit+0x194/0x2dc
mbox_send_message+0x190/0x330
imgsys_cmdq_sendtask+0x1618/0x2224
imgsys_runner_func+0xac/0x11c
process_one_work+0x638/0xf84
worker_thread+0x808/0xcd0
kthread+0x24c/0x324
ret_from_fork+0x10/0x20

We found that there is a spin_lock_irqsave protection in msg_submit()
of mailbox.c and it is in the atomic context.
So when cmdq driver calls pm_runtime_get_sync() in cmdq_mbox_send_data(),
it will get this BUG report.

To avoid using sleep in atomic context, move pm_runtime_get_sync to
mbox_chan_ops->power_get and also move pm_runtime_put_autosuspend to
mbox_chan_ops->power_put.

Fixes: 8afe816b0c99 ("mailbox: mtk-cmdq-mailbox: Implement Runtime PM with autosuspend")
Signed-off-by: Jason-JH.Lin <jason-jh.lin@xxxxxxxxxxxx>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 37 +++++++++++++++---------------
1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 4aa394e91109..02cef3eee35a 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -387,20 +387,13 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
struct cmdq_task *task;
unsigned long curr_pa, end_pa;
- int ret;

/* Client should not flush new tasks if suspended. */
WARN_ON(cmdq->suspended);

- ret = pm_runtime_get_sync(cmdq->mbox.dev);
- if (ret < 0)
- return ret;
-
task = kzalloc(sizeof(*task), GFP_ATOMIC);
- if (!task) {
- pm_runtime_put_autosuspend(cmdq->mbox.dev);
+ if (!task)
return -ENOMEM;
- }

task->cmdq = cmdq;
INIT_LIST_HEAD(&task->list_entry);
@@ -448,7 +441,6 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
list_move_tail(&task->list_entry, &thread->task_busy_list);

pm_runtime_mark_last_busy(cmdq->mbox.dev);
- pm_runtime_put_autosuspend(cmdq->mbox.dev);

return 0;
}
@@ -465,8 +457,6 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan)
struct cmdq_task *task, *tmp;
unsigned long flags;

- WARN_ON(pm_runtime_get_sync(cmdq->mbox.dev) < 0);
-
spin_lock_irqsave(&thread->chan->lock, flags);
if (list_empty(&thread->task_busy_list))
goto done;
@@ -496,7 +486,6 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan)
spin_unlock_irqrestore(&thread->chan->lock, flags);

pm_runtime_mark_last_busy(cmdq->mbox.dev);
- pm_runtime_put_autosuspend(cmdq->mbox.dev);
}

static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
@@ -507,11 +496,6 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
struct cmdq_task *task, *tmp;
unsigned long flags;
u32 enable;
- int ret;
-
- ret = pm_runtime_get_sync(cmdq->mbox.dev);
- if (ret < 0)
- return ret;

spin_lock_irqsave(&thread->chan->lock, flags);
if (list_empty(&thread->task_busy_list))
@@ -536,7 +520,6 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
out:
spin_unlock_irqrestore(&thread->chan->lock, flags);
pm_runtime_mark_last_busy(cmdq->mbox.dev);
- pm_runtime_put_autosuspend(cmdq->mbox.dev);

return 0;

@@ -551,15 +534,31 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
return -EFAULT;
}
pm_runtime_mark_last_busy(cmdq->mbox.dev);
- pm_runtime_put_autosuspend(cmdq->mbox.dev);
+
return 0;
}

+static int cmdq_mbox_pm_resume(struct mbox_chan *chan)
+{
+ struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
+
+ return pm_runtime_get_sync(cmdq->mbox.dev);
+}
+
+static void cmdq_mbox_pm_susepnd(struct mbox_chan *chan)
+{
+ struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
+
+ pm_runtime_put_autosuspend(cmdq->mbox.dev);
+}
+
static const struct mbox_chan_ops cmdq_mbox_chan_ops = {
.send_data = cmdq_mbox_send_data,
.startup = cmdq_mbox_startup,
.shutdown = cmdq_mbox_shutdown,
.flush = cmdq_mbox_flush,
+ .power_get = cmdq_mbox_pm_resume,
+ .power_put = cmdq_mbox_pm_susepnd,
};

static struct mbox_chan *cmdq_xlate(struct mbox_controller *mbox,
--
2.18.0