[PATCH v3 00/10] mailbox: imx: Use threaded handler to avoid kworker in imx's remoteproc

[PATCH v3 00/10] mailbox: imx: Use threaded handler to avoid kworker in imx's remoteproc

[Sebastian Andrzej Siewior]

The imx's remoteproc driver uses a kworker from its mailbox callback to
complete the request. The reason is that the imx mailbox driver invokes
the callback from its interrupt handler and the remoteproc callback (at
least the rpmsg-tty) requires a preemptible context.

This works but is problematic in a PREEMPT_RT environment where the
latency of the invocation is important. By scheduling a kworker the
high task priority from the threaded handler is lost and the kworker
competes for CPU ressources with every SCHED_OTHER task in the system.
This can lead to long delays on a busy system with other RT threads
which are less important than the completion of this request.

Looking over other mailbox driver, like the arm_mhu for instance, they
use a threaded interrupt handler to invoke the callback. This avoids the
kworker detour.

The here suggested change utilises a threaded interrupt to invoke the
callback. The primary handler mask the interrupt source so that the
handler can run without getting interrupted by the interrupt again.
Doing so avoids marking the interrupt IRQF_ONESHOT so that in a
shared-interrupt environment the other interrupt can still fire while
the first is masked.

The first four patches are result of the sashiko review. Does not look
critical.

This change was tested on a im93 board with rpmsg-tty driver.

v2…v3: https://lore.kernel.org/r/20260603-imx_mbox_rproc-v2-0-a0059dc3b69a@linutronix.de
  - Forward the error in imx_mu_generic_tx() to the caller (new patch
    #1)
  - Extend the patch description a bit for for "Start splitting the IRQ
    handler" to briefly explain why callbacks are moved to the threaded
    handler.
  - Drop imx_mu_con_priv::pending. The primary handler wakes its
    threaded handler. Once the handler is woken, the pending flag must
    be set and there is no need to set/ clear it.
  - Avoid the double clk_disable_unprepare() if
    devm_mbox_controller_register() fails.

v1…v2: https://lore.kernel.org/r/20260529-imx_mbox_rproc-v1-0-b8ffc36e11e5@linutronix.de
  - Using correct register to enable RXDB event.
  - Update commit description for the "threaded interrupt", "unmasks the
    interrupt" => "masks the interrupt event".
  - Add a shutdown field so that the interrupt does not unmask the
    interrupt if it has been already disabled because the channel is
    about to be shutdown.  A possible race mentioned by sashiko.
  - Use devm_pm_runtime_enable(). This should avoid a possible race
    sashiko mentioned.
  - Use devm_of_platform_populate().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
Sebastian Andrzej Siewior (10):
      mailbox: imx: Forward the timeout/ error in imx_mu_generic_tx()
      mailbox: imx: Add a channel shutdown field
      mailbox: imx: Use devm_pm_runtime_enable()
      mailbox: imx: use devm_of_platform_populate()
      mailbox: imx: Use channel index instead of zero in imx_mu_specific_rx()
      mailbox: imx: Start splitting the IRQ handler in primary and threaded handler
      mailbox: imx: Move the RX part of the mailbox into the threaded handler
      mailbox: imx: Move the RXDB part of the mailbox into the threaded handler
      mailbox: imx: Don't force-thread the primary handler
      remoteproc: imx_rproc: Invoke the callback directly

 drivers/mailbox/imx-mailbox.c  | 117 ++++++++++++++++++++++++++++++-----------
 drivers/remoteproc/imx_rproc.c |  33 +-----------
 2 files changed, 88 insertions(+), 62 deletions(-)
---
base-commit: b3f94b2b3f3e51ab880a51fc6510e1dafba654ed
change-id: 20260529-imx_mbox_rproc-7d512f5a6f78

Best regards,
-- 
Sebastian Andrzej Siewior <bigeasy@linutronix.de>

[PATCH v3 01/10] mailbox: imx: Forward the timeout/ error in imx_mu_generic_tx()

[Sebastian Andrzej Siewior]

imx_mu_generic_tx() for the IMX_MU_TYPE_TXDB_V2 type polls on a register
which may timeout and is recognized as an error. This error is siltently
dropped and not dropped to the caller.

Forward the error to the caller.

Fixes: b5ef17917f3a7 ("mailbox: imx: fix TXDB_V2 channel race condition")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mailbox/imx-mailbox.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 246a9a9e39520..0028073be4a71 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -227,6 +227,7 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
 	u32 val;
 	int ret, count;
 
+	ret = 0;
 	switch (cp->type) {
 	case IMX_MU_TYPE_TX:
 		imx_mu_write(priv, *arg, priv->dcfg->xTR + cp->idx * 4);
@@ -259,7 +260,7 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
 		return -EINVAL;
 	}
 
-	return 0;
+	return ret;
 }
 
 static int imx_mu_generic_rx(struct imx_mu_priv *priv,

-- 
2.53.0

[PATCH v3 02/10] mailbox: imx: Add a channel shutdown field

[Sebastian Andrzej Siewior]

sashiko complained about possible teardown problem. The scenario

 CPU 0                              CPU 1
  imx_mu_isr()                   imx_mu_shutdown()
                                   imx_mu_xcr_rmw(priv, IMX_MU_RCR, 0, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
    imx_mu_specific_rx()
      imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, 0), 0);
                                   free_irq()

The RX event remains enabled because in this short window the RX event
was disabled in ->shutdown() while the interrupt was active and then
enabled again by the ISR while ->shutdown waited in free_irq().

This race requires timing and if happens can be problematic on shared
handlers if the "removed" channel triggers an interrupt. In this case
the irq-core will shutdown the interrupt with the "nobody cared"
message.

Introduce imx_mu_con_priv::shutdown to signal that the channel is
shutting down. This flag is set with the lock held (by
imx_mu_xcr_clr_shut()). The unmask side uses imx_mu_xcr_set_act() which
only enables the event if the channel has not been shutdown and
serialises on the same lock.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mailbox/imx-mailbox.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 0028073be4a71..e261b382d5c90 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -80,6 +80,7 @@ struct imx_mu_con_priv {
 	enum imx_mu_chan_type	type;
 	struct mbox_chan	*chan;
 	struct work_struct 	txdb_work;
+	bool			shutdown;
 };
 
 struct imx_mu_priv {
@@ -219,6 +220,36 @@ static u32 imx_mu_xcr_rmw(struct imx_mu_priv *priv, enum imx_mu_xcr type, u32 se
 	return val;
 }
 
+static void imx_mu_xcr_clr_shut(struct imx_mu_priv *priv, struct imx_mu_con_priv *cp,
+				enum imx_mu_xcr type, u32 clr)
+{
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&priv->xcr_lock, flags);
+	cp->shutdown = true;
+
+	val = imx_mu_read(priv, priv->dcfg->xCR[type]);
+	val &= ~clr;
+	imx_mu_write(priv, val, priv->dcfg->xCR[type]);
+	spin_unlock_irqrestore(&priv->xcr_lock, flags);
+}
+
+static void imx_mu_xcr_set_act(struct imx_mu_priv *priv, struct imx_mu_con_priv *cp,
+			       enum imx_mu_xcr type, u32 set)
+{
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&priv->xcr_lock, flags);
+	if (!cp->shutdown) {
+		val = imx_mu_read(priv, priv->dcfg->xCR[type]);
+		val |= set;
+		imx_mu_write(priv, val, priv->dcfg->xCR[type]);
+	}
+	spin_unlock_irqrestore(&priv->xcr_lock, flags);
+}
+
 static int imx_mu_generic_tx(struct imx_mu_priv *priv,
 			     struct imx_mu_con_priv *cp,
 			     void *data)
@@ -377,7 +408,7 @@ static int imx_mu_specific_rx(struct imx_mu_priv *priv, struct imx_mu_con_priv *
 		*data++ = imx_mu_read(priv, priv->dcfg->xRR + (i % num_rr) * 4);
 	}
 
-	imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, 0), 0);
+	imx_mu_xcr_set_act(priv, cp, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, 0));
 	mbox_chan_received_data(cp->chan, (void *)priv->msg);
 
 	return 0;
@@ -605,6 +636,7 @@ static int imx_mu_startup(struct mbox_chan *chan)
 		return ret;
 	}
 
+	cp->shutdown = false;
 	switch (cp->type) {
 	case IMX_MU_TYPE_RX:
 		imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx), 0);
@@ -639,13 +671,13 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
 
 	switch (cp->type) {
 	case IMX_MU_TYPE_TX:
-		imx_mu_xcr_rmw(priv, IMX_MU_TCR, 0, IMX_MU_xCR_TIEn(priv->dcfg->type, cp->idx));
+		imx_mu_xcr_clr_shut(priv, cp, IMX_MU_TCR, IMX_MU_xCR_TIEn(priv->dcfg->type, cp->idx));
 		break;
 	case IMX_MU_TYPE_RX:
-		imx_mu_xcr_rmw(priv, IMX_MU_RCR, 0, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
+		imx_mu_xcr_clr_shut(priv, cp, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
 		break;
 	case IMX_MU_TYPE_RXDB:
-		imx_mu_xcr_rmw(priv, IMX_MU_GIER, 0, IMX_MU_xCR_GIEn(priv->dcfg->type, cp->idx));
+		imx_mu_xcr_clr_shut(priv, cp, IMX_MU_GIER, IMX_MU_xCR_GIEn(priv->dcfg->type, cp->idx));
 		break;
 	case IMX_MU_TYPE_RST:
 		imx_mu_xcr_rmw(priv, IMX_MU_CR, IMX_MU_xCR_RST(priv->dcfg->type), 0);

-- 
2.53.0

[PATCH v3 03/10] mailbox: imx: Use devm_pm_runtime_enable()

[Sebastian Andrzej Siewior]

sashiko complained about early usage of the device while probe isn't
completed. This can be mitigated by delaying the pm_runtime_enable()
into the removal path instead doing it early. This ensures that in an
error case the device is removed (and imx_mu_shutdown()) before
pm_runtime_disable() so we don't have to do this manually.

For the order to work, lets move devm_mbox_controller_register() until
after the pm-runtime part. So the reverse order will be mbox-controller
removal followed by disabling pm runtime.

Use devm_pm_runtime_enable(), remove manual pm_runtime_disable()
invocations and move the pm_runtime handling in probe before
devm_mbox_controller_register().

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mailbox/imx-mailbox.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index e261b382d5c90..516a05b64daa1 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -966,38 +966,36 @@ static int imx_mu_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, priv);
 
-	ret = devm_mbox_controller_register(dev, &priv->mbox);
-	if (ret)
+	ret = devm_pm_runtime_enable(dev);
+	if (ret < 0)
 		goto disable_clk;
 
-	of_platform_populate(dev->of_node, NULL, NULL, dev);
-
-	pm_runtime_enable(dev);
-
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
-		goto disable_runtime_pm;
+		goto disable_clk;
 
 	ret = pm_runtime_put_sync(dev);
 	if (ret < 0)
-		goto disable_runtime_pm;
+		goto disable_clk;
 
 	clk_disable_unprepare(priv->clk);
 
+	ret = devm_mbox_controller_register(dev, &priv->mbox);
+	if (ret)
+		goto err_out;
+
+	of_platform_populate(dev->of_node, NULL, NULL, dev);
+
 	return 0;
 
-disable_runtime_pm:
-	pm_runtime_disable(dev);
 disable_clk:
 	clk_disable_unprepare(priv->clk);
+err_out:
 	return ret;
 }
 
 static void imx_mu_remove(struct platform_device *pdev)
 {
-	struct imx_mu_priv *priv = platform_get_drvdata(pdev);
-
-	pm_runtime_disable(priv->dev);
 }
 
 static const struct imx_mu_dcfg imx_mu_cfg_imx6sx = {

-- 
2.53.0

[PATCH v3 04/10] mailbox: imx: use devm_of_platform_populate()

[Sebastian Andrzej Siewior]

The driver uses of_platform_populate() but does not remove the added
devices on removal. This can lead to "double devices" on module removal
followed by adding the module again.

Use devm_of_platform_populate() to remove the populated devices once the
parent device is removed.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mailbox/imx-mailbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 516a05b64daa1..a8d4e970fb786 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -984,7 +984,7 @@ static int imx_mu_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_out;
 
-	of_platform_populate(dev->of_node, NULL, NULL, dev);
+	devm_of_platform_populate(dev);
 
 	return 0;
 

-- 
2.53.0

[PATCH v3 05/10] mailbox: imx: Use channel index instead of zero in imx_mu_specific_rx()

[Sebastian Andrzej Siewior]

imx_mu_specific_rx() masks channel 0 and unmasks it again at the end of
the function. Given that at startup the channel index got unmasked it
should do the right job.

This here either unmasks the actual channel or another one but should
have no impact given that it reverses its doing at the end.

Peng Fan commented here:
| For specific rx channel, whether it is i.MX8 SCU or i.MX ELE, actually there is
| only 1 channel as of now, but it seems better to use cp->idx in case more
| channels in future.

Use the channel index instead of zero.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mailbox/imx-mailbox.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index a8d4e970fb786..408cd083c64ee 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -381,7 +381,7 @@ static int imx_mu_specific_rx(struct imx_mu_priv *priv, struct imx_mu_con_priv *
 
 	data = (u32 *)priv->msg;
 
-	imx_mu_xcr_rmw(priv, IMX_MU_RCR, 0, IMX_MU_xCR_RIEn(priv->dcfg->type, 0));
+	imx_mu_xcr_rmw(priv, IMX_MU_RCR, 0, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
 	*data++ = imx_mu_read(priv, priv->dcfg->xRR);
 
 	if (priv->dcfg->type & IMX_MU_V2_S4) {
@@ -408,7 +408,7 @@ static int imx_mu_specific_rx(struct imx_mu_priv *priv, struct imx_mu_con_priv *
 		*data++ = imx_mu_read(priv, priv->dcfg->xRR + (i % num_rr) * 4);
 	}
 
-	imx_mu_xcr_set_act(priv, cp, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, 0));
+	imx_mu_xcr_set_act(priv, cp, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
 	mbox_chan_received_data(cp->chan, (void *)priv->msg);
 
 	return 0;

-- 
2.53.0

[PATCH v3 06/10] mailbox: imx: Start splitting the IRQ handler in primary and threaded handler

[Sebastian Andrzej Siewior]

Split the mailbox irq handling into a primary handler (imx_mu_isr()) and
a threaded handler (imx_mu_isr_th()). The primary handler masks the
interrupt event so the threaded handler can run without raising the
interrupt again.

The goal here is to invoke the mailbox core functions (such as
mbox_chan_received_data(), mbox_chan_txdone()) in preemptible context which is
made possible by using an threaded interrupt handler. This in turn means that
mailbox's client callbacks are invoked in preemptible context, too. This then
allows the mailbox client callback to skip an indirection via a workqueue if
it requries preemptible callback.

As a first step, prepare the logic and move TX handling part.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mailbox/imx-mailbox.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 408cd083c64ee..87acc43cb99c4 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -540,11 +540,31 @@ static void imx_mu_txdb_work(struct work_struct *t)
 	mbox_chan_txdone(cp->chan, 0);
 }
 
+static irqreturn_t imx_mu_isr_th(int irq, void *p)
+{
+	struct mbox_chan *chan = p;
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+	struct imx_mu_con_priv *cp = chan->con_priv;
+
+	switch (cp->type) {
+	case IMX_MU_TYPE_TX:
+		mbox_chan_txdone(chan, 0);
+		break;
+
+	default:
+		dev_warn_ratelimited(priv->dev, "Unhandled channel type %d\n",
+				     cp->type);
+		return IRQ_NONE;
+	}
+	return IRQ_HANDLED;
+}
+
 static irqreturn_t imx_mu_isr(int irq, void *p)
 {
 	struct mbox_chan *chan = p;
 	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
 	struct imx_mu_con_priv *cp = chan->con_priv;
+	irqreturn_t ret = IRQ_HANDLED;
 	u32 val, ctrl;
 
 	switch (cp->type) {
@@ -580,7 +600,7 @@ static irqreturn_t imx_mu_isr(int irq, void *p)
 	if ((val == IMX_MU_xSR_TEn(priv->dcfg->type, cp->idx)) &&
 	    (cp->type == IMX_MU_TYPE_TX)) {
 		imx_mu_xcr_rmw(priv, IMX_MU_TCR, 0, IMX_MU_xCR_TIEn(priv->dcfg->type, cp->idx));
-		mbox_chan_txdone(chan, 0);
+		ret = IRQ_WAKE_THREAD;
 	} else if ((val == IMX_MU_xSR_RFn(priv->dcfg->type, cp->idx)) &&
 		   (cp->type == IMX_MU_TYPE_RX)) {
 		priv->dcfg->rx(priv, cp);
@@ -595,7 +615,7 @@ static irqreturn_t imx_mu_isr(int irq, void *p)
 	if (priv->suspend)
 		pm_system_wakeup();
 
-	return IRQ_HANDLED;
+	return ret;
 }
 
 static int imx_mu_send_data(struct mbox_chan *chan, void *data)
@@ -630,7 +650,8 @@ static int imx_mu_startup(struct mbox_chan *chan)
 	if (!(priv->dcfg->type & IMX_MU_V2_IRQ))
 		irq_flag |= IRQF_SHARED;
 
-	ret = request_irq(priv->irq[cp->type], imx_mu_isr, irq_flag, cp->irq_desc, chan);
+	ret = request_threaded_irq(priv->irq[cp->type], imx_mu_isr, imx_mu_isr_th,
+				   irq_flag, cp->irq_desc, chan);
 	if (ret) {
 		dev_err(priv->dev, "Unable to acquire IRQ %d\n", priv->irq[cp->type]);
 		return ret;

-- 
2.53.0

[PATCH v3 07/10] mailbox: imx: Move the RX part of the mailbox into the threaded handler

[Sebastian Andrzej Siewior]

Move RX callback handling into the threaded handler. This is similar to
the TX side except that we explicitly mask the source interrupt in the
primary handler and unmask it in the threaded handler again after
success. This was done automatically in the TX part.

The masking/ unmasking can be removed from imx_mu_specific_rx() since it
already happens in the primary/ threaded handler before invoking the
channel specific callback.

Move RX channel handling into threaded handler.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mailbox/imx-mailbox.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 87acc43cb99c4..1219c35b116e4 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -381,7 +381,6 @@ static int imx_mu_specific_rx(struct imx_mu_priv *priv, struct imx_mu_con_priv *
 
 	data = (u32 *)priv->msg;
 
-	imx_mu_xcr_rmw(priv, IMX_MU_RCR, 0, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
 	*data++ = imx_mu_read(priv, priv->dcfg->xRR);
 
 	if (priv->dcfg->type & IMX_MU_V2_S4) {
@@ -408,7 +407,6 @@ static int imx_mu_specific_rx(struct imx_mu_priv *priv, struct imx_mu_con_priv *
 		*data++ = imx_mu_read(priv, priv->dcfg->xRR + (i % num_rr) * 4);
 	}
 
-	imx_mu_xcr_set_act(priv, cp, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
 	mbox_chan_received_data(cp->chan, (void *)priv->msg);
 
 	return 0;
@@ -551,6 +549,11 @@ static irqreturn_t imx_mu_isr_th(int irq, void *p)
 		mbox_chan_txdone(chan, 0);
 		break;
 
+	case IMX_MU_TYPE_RX:
+		if (!priv->dcfg->rx(priv, cp))
+			imx_mu_xcr_set_act(priv, cp, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
+		break;
+
 	default:
 		dev_warn_ratelimited(priv->dev, "Unhandled channel type %d\n",
 				     cp->type);
@@ -603,7 +606,8 @@ static irqreturn_t imx_mu_isr(int irq, void *p)
 		ret = IRQ_WAKE_THREAD;
 	} else if ((val == IMX_MU_xSR_RFn(priv->dcfg->type, cp->idx)) &&
 		   (cp->type == IMX_MU_TYPE_RX)) {
-		priv->dcfg->rx(priv, cp);
+		imx_mu_xcr_rmw(priv, IMX_MU_RCR, 0, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
+		ret = IRQ_WAKE_THREAD;
 	} else if ((val == IMX_MU_xSR_GIPn(priv->dcfg->type, cp->idx)) &&
 		   (cp->type == IMX_MU_TYPE_RXDB)) {
 		priv->dcfg->rxdb(priv, cp);

-- 
2.53.0

[PATCH v3 08/10] mailbox: imx: Move the RXDB part of the mailbox into the threaded handler

[Sebastian Andrzej Siewior]

Move RXDB callback handling into the threaded handler. This similar to
the RX side and since the imx_mu_dcfg::rxdb callback can return an error, the
interrupt is only enabled on success.

Move RXDB callback handling into the threaded handler.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mailbox/imx-mailbox.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 1219c35b116e4..1bd434bdff87a 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -554,6 +554,11 @@ static irqreturn_t imx_mu_isr_th(int irq, void *p)
 			imx_mu_xcr_set_act(priv, cp, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
 		break;
 
+	case IMX_MU_TYPE_RXDB:
+		if (!priv->dcfg->rxdb(priv, cp))
+			imx_mu_xcr_set_act(priv, cp, IMX_MU_GIER, IMX_MU_xCR_GIEn(priv->dcfg->type, cp->idx));
+		break;
+
 	default:
 		dev_warn_ratelimited(priv->dev, "Unhandled channel type %d\n",
 				     cp->type);
@@ -610,7 +615,8 @@ static irqreturn_t imx_mu_isr(int irq, void *p)
 		ret = IRQ_WAKE_THREAD;
 	} else if ((val == IMX_MU_xSR_GIPn(priv->dcfg->type, cp->idx)) &&
 		   (cp->type == IMX_MU_TYPE_RXDB)) {
-		priv->dcfg->rxdb(priv, cp);
+		imx_mu_xcr_rmw(priv, IMX_MU_GIER, 0, IMX_MU_xCR_GIEn(priv->dcfg->type, cp->idx));
+		ret = IRQ_WAKE_THREAD;
 	} else {
 		dev_warn_ratelimited(priv->dev, "Not handled interrupt\n");
 		return IRQ_NONE;

-- 
2.53.0

[PATCH v3 09/10] mailbox: imx: Don't force-thread the primary handler

[Sebastian Andrzej Siewior]

The primary interrupt handler (imx_mu_isr()) no longer invokes any
callbacks it only masks the interrupt source and returns. In a
forced-threaded environment the IRQ-core will force-thread the primary
handler which can be avoided.

The primary handler uses a spinlock_t to protect the RMW operation in
imx_mu_xcr_rmw() - nothing that may introduce long latencies.

The lock can be turned into a raw_spinlock_t and then the primary
handler can run in hardirq context even on PREEMPT_RT skipping one
thread.

Make struct imx_mu_priv::xcr_lock a raw_spinlock_t and skip
force-threading the primrary handler by marking it IRQF_NO_THREAD.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mailbox/imx-mailbox.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 1bd434bdff87a..40bad2b3e4d19 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -87,7 +87,7 @@ struct imx_mu_priv {
 	struct device		*dev;
 	void __iomem		*base;
 	void			*msg;
-	spinlock_t		xcr_lock; /* control register lock */
+	raw_spinlock_t		xcr_lock; /* control register lock */
 
 	struct mbox_controller	mbox;
 	struct mbox_chan	mbox_chans[IMX_MU_CHANS];
@@ -207,15 +207,14 @@ static int imx_mu_rx_waiting_read(struct imx_mu_priv *priv, u32 *val, u32 idx)
 
 static u32 imx_mu_xcr_rmw(struct imx_mu_priv *priv, enum imx_mu_xcr type, u32 set, u32 clr)
 {
-	unsigned long flags;
 	u32 val;
 
-	spin_lock_irqsave(&priv->xcr_lock, flags);
+	guard(raw_spinlock_irqsave)(&priv->xcr_lock);
+
 	val = imx_mu_read(priv, priv->dcfg->xCR[type]);
 	val &= ~clr;
 	val |= set;
 	imx_mu_write(priv, val, priv->dcfg->xCR[type]);
-	spin_unlock_irqrestore(&priv->xcr_lock, flags);
 
 	return val;
 }
@@ -223,31 +222,27 @@ static u32 imx_mu_xcr_rmw(struct imx_mu_priv *priv, enum imx_mu_xcr type, u32 se
 static void imx_mu_xcr_clr_shut(struct imx_mu_priv *priv, struct imx_mu_con_priv *cp,
 				enum imx_mu_xcr type, u32 clr)
 {
-	unsigned long flags;
 	u32 val;
 
-	spin_lock_irqsave(&priv->xcr_lock, flags);
+	guard(raw_spinlock_irqsave)(&priv->xcr_lock);
 	cp->shutdown = true;
 
 	val = imx_mu_read(priv, priv->dcfg->xCR[type]);
 	val &= ~clr;
 	imx_mu_write(priv, val, priv->dcfg->xCR[type]);
-	spin_unlock_irqrestore(&priv->xcr_lock, flags);
 }
 
 static void imx_mu_xcr_set_act(struct imx_mu_priv *priv, struct imx_mu_con_priv *cp,
 			       enum imx_mu_xcr type, u32 set)
 {
-	unsigned long flags;
 	u32 val;
 
-	spin_lock_irqsave(&priv->xcr_lock, flags);
+	guard(raw_spinlock_irqsave)(&priv->xcr_lock);
 	if (!cp->shutdown) {
 		val = imx_mu_read(priv, priv->dcfg->xCR[type]);
 		val |= set;
 		imx_mu_write(priv, val, priv->dcfg->xCR[type]);
 	}
-	spin_unlock_irqrestore(&priv->xcr_lock, flags);
 }
 
 static int imx_mu_generic_tx(struct imx_mu_priv *priv,
@@ -640,7 +635,7 @@ static int imx_mu_startup(struct mbox_chan *chan)
 {
 	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
 	struct imx_mu_con_priv *cp = chan->con_priv;
-	unsigned long irq_flag = 0;
+	unsigned long irq_flag = IRQF_NO_THREAD;
 	int ret;
 
 	pm_runtime_get_sync(priv->dev);
@@ -988,7 +983,7 @@ static int imx_mu_probe(struct platform_device *pdev)
 		goto disable_clk;
 	}
 
-	spin_lock_init(&priv->xcr_lock);
+	raw_spin_lock_init(&priv->xcr_lock);
 
 	priv->mbox.dev = dev;
 	priv->mbox.ops = &imx_mu_ops;

-- 
2.53.0

[PATCH v3 10/10] remoteproc: imx_rproc: Invoke the callback directly

[Sebastian Andrzej Siewior]

The imx-mailbox driver moved the callback invocation into the threaded
IRQ handler. This means the callback is invoked in preemptible context
and there is no need to schedule the kworker for the
imx_rproc_notified_idr_cb() invocation.

This was tested with the rpmsg-tty driver on imx93.

Remove the workqueue handling and invoke the imx_rproc_notified_idr_cb()
callback directly.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/remoteproc/imx_rproc.c | 33 +--------------------------------
 1 file changed, 1 insertion(+), 32 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 0dd80e688b0ea..c97bc1c401655 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -24,7 +24,6 @@
 #include <linux/regmap.h>
 #include <linux/remoteproc.h>
 #include <linux/scmi_imx_protocol.h>
-#include <linux/workqueue.h>
 
 #include "imx_rproc.h"
 #include "remoteproc_internal.h"
@@ -115,8 +114,6 @@ struct imx_rproc {
 	struct mbox_client		cl;
 	struct mbox_chan		*tx_ch;
 	struct mbox_chan		*rx_ch;
-	struct work_struct		rproc_work;
-	struct workqueue_struct		*workqueue;
 	void __iomem			*rsc_table;
 	struct imx_sc_ipc		*ipc_handle;
 	struct notifier_block		rproc_nb;
@@ -835,21 +832,11 @@ static int imx_rproc_notified_idr_cb(int id, void *ptr, void *data)
 	return 0;
 }
 
-static void imx_rproc_vq_work(struct work_struct *work)
-{
-	struct imx_rproc *priv = container_of(work, struct imx_rproc,
-					      rproc_work);
-	struct rproc *rproc = priv->rproc;
-
-	idr_for_each(&rproc->notifyids, imx_rproc_notified_idr_cb, rproc);
-}
-
 static void imx_rproc_rx_callback(struct mbox_client *cl, void *msg)
 {
 	struct rproc *rproc = dev_get_drvdata(cl->dev);
-	struct imx_rproc *priv = rproc->priv;
 
-	queue_work(priv->workqueue, &priv->rproc_work);
+	idr_for_each(&rproc->notifyids, imx_rproc_notified_idr_cb, rproc);
 }
 
 static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)
@@ -1214,13 +1201,6 @@ static int imx_rproc_sys_off_handler(struct sys_off_data *data)
 	return NOTIFY_DONE;
 }
 
-static void imx_rproc_destroy_workqueue(void *data)
-{
-	struct workqueue_struct *workqueue = data;
-
-	destroy_workqueue(workqueue);
-}
-
 static int imx_rproc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1249,17 +1229,6 @@ static int imx_rproc_probe(struct platform_device *pdev)
 		priv->ops = dcfg->ops;
 
 	dev_set_drvdata(dev, rproc);
-	priv->workqueue = create_workqueue(dev_name(dev));
-	if (!priv->workqueue) {
-		dev_err(dev, "cannot create workqueue\n");
-		return -ENOMEM;
-	}
-
-	ret = devm_add_action_or_reset(dev, imx_rproc_destroy_workqueue, priv->workqueue);
-	if (ret)
-		return dev_err_probe(dev, ret, "Failed to add devm destroy workqueue action\n");
-
-	INIT_WORK(&priv->rproc_work, imx_rproc_vq_work);
 
 	ret = imx_rproc_xtr_mbox_init(rproc, true);
 	if (ret)

-- 
2.53.0

Re: [PATCH v3 01/10] mailbox: imx: Forward the timeout/ error in imx_mu_generic_tx()

[Peng Fan]

Hi Sebastian,

Thanks for your patch.

On Wed, Jun 17, 2026 at 08:55:26AM +0200, Sebastian Andrzej Siewior wrote:
>imx_mu_generic_tx() for the IMX_MU_TYPE_TXDB_V2 type polls on a register
>which may timeout and is recognized as an error. This error is siltently
>dropped and not dropped to the caller.
>
>Forward the error to the caller.
>
>Fixes: b5ef17917f3a7 ("mailbox: imx: fix TXDB_V2 channel race condition")
>Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>---
> drivers/mailbox/imx-mailbox.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
>index 246a9a9e39520..0028073be4a71 100644
>--- a/drivers/mailbox/imx-mailbox.c
>+++ b/drivers/mailbox/imx-mailbox.c
>@@ -227,6 +227,7 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
> 	u32 val;
> 	int ret, count;
> 
>+	ret = 0;
> 	switch (cp->type) {
> 	case IMX_MU_TYPE_TX:
> 		imx_mu_write(priv, *arg, priv->dcfg->xTR + cp->idx * 4);
>@@ -259,7 +260,7 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
> 		return -EINVAL;
> 	}
> 
>-	return 0;
>+	return ret;
> }

I just rethink about the logic here and rewrite the logic as below.
error code is propogated to caller and poll timeout are removed.
Please see whether it looks good for you or not.

[PATCH] mailbox: imx: make TXDB non-blocking and avoid polling in atomic context

The IMX_MU_TYPE_TXDB_V2 path currently writes to the GIR register and
then polls until the bit is cleared using readl_poll_timeout().

Because send_data() is invoked under spin_lock_irqsave() from the mailbox
core, meaning the polling loop can run in atomic context with interrupts
disabled. In the worst case, the current implementation may busy-wait for
up to 100ms, leading to excessive interrupt latency and potential soft
lockup warnings.

Moreover, the TXDB channel implements a doorbell mechanism, where the
sender only needs to trigger the event when the channel is idle. Waiting
for the GIR bit to clear after the write is no good with polling.

Fix this by:
  - Checking the GIR bit before issuing the write
  - Returning -EBUSY if the channel is still active
  - Removing the post-write polling loop

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mailbox/imx-mailbox.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 796ba983db29e..ed53bcffec673 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -241,7 +241,6 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
 {
 	u32 *arg = data;
 	u32 val;
-	int ret, count;
 
 	switch (cp->type) {
 	case IMX_MU_TYPE_TX:
@@ -253,22 +252,14 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
 		queue_work(system_bh_wq, &cp->txdb_work);
 		break;
 	case IMX_MU_TYPE_TXDB_V2:
+		val = readl(priv->base + priv->dcfg->xCR[IMX_MU_GCR]);
+		if (val & IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx)) {
+			dev_info(priv->dev, "channel [%d] type: %d busy\n", cp->idx, cp->type);
+			return -EBUSY;
+		}
+
 		imx_mu_write(priv, IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx),
 			     priv->dcfg->xCR[IMX_MU_GCR]);
-		ret = -ETIMEDOUT;
-		count = 0;
-		while (ret && (count < 10)) {
-			ret =
-			readl_poll_timeout(priv->base + priv->dcfg->xCR[IMX_MU_GCR], val,
-					   !(val & IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx)),
-					   0, 10000);
-
-			if (ret) {
-				dev_warn_ratelimited(priv->dev,
-						     "channel type: %d timeout, %d times, retry\n",
-						     cp->type, ++count);
-			}
-		}
 		break;
 	default:
 		dev_warn_ratelimited(priv->dev, "Send data on wrong channel type: %d\n", cp->type);
-- 
2.50.1

Thanks,
Peng

> 
> static int imx_mu_generic_rx(struct imx_mu_priv *priv,
>
>-- 
>2.53.0
>

Re: [PATCH v3 01/10] mailbox: imx: Forward the timeout/ error in imx_mu_generic_tx()

[Peng Fan]

On Thu, Jun 18, 2026 at 09:50:20AM +0800, Peng Fan wrote:
>Hi Sebastian,
>
>Thanks for your patch.
>
>On Wed, Jun 17, 2026 at 08:55:26AM +0200, Sebastian Andrzej Siewior wrote:
>>imx_mu_generic_tx() for the IMX_MU_TYPE_TXDB_V2 type polls on a register
>>which may timeout and is recognized as an error. This error is siltently
>>dropped and not dropped to the caller.
>>
>>Forward the error to the caller.
>>
>>Fixes: b5ef17917f3a7 ("mailbox: imx: fix TXDB_V2 channel race condition")
>>Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>---
>> drivers/mailbox/imx-mailbox.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>>diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
>>index 246a9a9e39520..0028073be4a71 100644
>>--- a/drivers/mailbox/imx-mailbox.c
>>+++ b/drivers/mailbox/imx-mailbox.c
>>@@ -227,6 +227,7 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
>> 	u32 val;
>> 	int ret, count;
>> 
>>+	ret = 0;
>> 	switch (cp->type) {
>> 	case IMX_MU_TYPE_TX:
>> 		imx_mu_write(priv, *arg, priv->dcfg->xTR + cp->idx * 4);
>>@@ -259,7 +260,7 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
>> 		return -EINVAL;
>> 	}
>> 
>>-	return 0;
>>+	return ret;
>> }
>
>I just rethink about the logic here and rewrite the logic as below.
>error code is propogated to caller and poll timeout are removed.
>Please see whether it looks good for you or not.

Ignore this. Pushed the button too early

>
>[PATCH] mailbox: imx: make TXDB non-blocking and avoid polling in atomic context
>
>+
> 		imx_mu_write(priv, IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx),
> 			     priv->dcfg->xCR[IMX_MU_GCR]);

Oh. there might be issues without waiting here. For example, Linux just writes
the doorbell to notify SCMI processor power up a power domain, but when linux
continues to write registers, the SCMI processor may not fetch the message, then
linux will crash.

Regards
Peng

>-		ret = -ETIMEDOUT;
>-		count = 0;
>-		while (ret && (count < 10)) {
>-			ret =
>-			readl_poll_timeout(priv->base + priv->dcfg->xCR[IMX_MU_GCR], val,
>-					   !(val & IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx)),
>-					   0, 10000);
>-
>-			if (ret) {
>-				dev_warn_ratelimited(priv->dev,
>-						     "channel type: %d timeout, %d times, retry\n",
>-						     cp->type, ++count);
>-			}
>-		}
> 		break;
> 	default:
> 		dev_warn_ratelimited(priv->dev, "Send data on wrong channel type: %d\n", cp->type);
>-- 
>2.50.1
>
>Thanks,
>Peng
>
>> 
>> static int imx_mu_generic_rx(struct imx_mu_priv *priv,
>>
>>-- 
>>2.53.0
>>
>
>