Re: [PATCH 4/5] mailbox: imx: Don't force-thread the primary handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Mailbox controller is registered before initialization completes, leading to a race condition and potential system crash on probe error paths.
--

commit 1073878552c10da376c68e938a69f8f471bd3ed5
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

mailbox: imx: Don't force-thread the primary handler

The primary interrupt handler no longer invokes callbacks; it only masks
the interrupt source and returns. This patch converts the control register
lock to a raw_spinlock_t to allow the primary handler to run in hardirq
context even on PREEMPT_RT, skipping one thread.

> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> --- a/drivers/mailbox/imx-mailbox.c
> +++ b/drivers/mailbox/imx-mailbox.c
[ ... ]
> @@ -964,7 +963,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;

[Severity: High]
This is a pre-existing issue, but could the ordering in imx_mu_probe() lead
to a race condition on probe error paths?

Looking at the initialization sequence, devm_mbox_controller_register() is
called shortly after this lock initialization, but before pm_runtime_enable()
and before the probe function is guaranteed to succeed. 

If a consumer requests a channel immediately after registration and calls
startup(), it gains access to the hardware. If probe() subsequently fails
(for example, if pm_runtime_resume_and_get() fails), the error path calls
clk_disable_unprepare() to manually turn off the hardware clock.

The driver core will then trigger devres cleanup, which unregisters the
mailbox controller and calls imx_mu_shutdown() for the active channel.

Since imx_mu_shutdown() attempts to write to hardware registers to mask
interrupts, and the clock has already been disabled by the probe error path,
will this register access result in a fatal bus error or system hang?

Should the devm_mbox_controller_register() call be moved to the very end of
probe() to ensure consumers cannot interact with the hardware during an early
teardown?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529-imx_mbox_rproc-v1-0-b8ffc36e11e5@linutronix.de?part=4