Re: [PATCH v2 7/9] mailbox: imx: Move the RXDB part of the mailbox into the threaded handler

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Error paths in `imx_mu_seco_rxdb` fail to clear the hardware interrupt status (`GIPn`), leading to an infinite IRQ storm because the threaded handler unconditionally unmasks the interrupt.
--

commit fc70fc114c374d42412c37d5886dc2cfe0a507bb
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: mailbox: imx: Move the RXDB part of the mailbox into the threaded handler

This commit moves the RXDB callback handling from the hardirq to the threaded
interrupt handler. It also changes the behavior to unconditionally unmask
the RXDB interrupt after the callback returns.

> 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

[ ... ]

> @@ -559,6 +559,12 @@ 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:
> +		cp->pending = false;
> +		priv->dcfg->rxdb(priv, cp);
> +		imx_mu_xcr_set_act(priv, cp, IMX_MU_GIER, IMX_MU_xCR_GIEn(priv->dcfg->type, cp->idx));

[Severity: High]
This is a pre-existing issue, but since this handler now unconditionally
unmasks the interrupt, could this lead to an infinite interrupt storm if an
error occurs in the callback?

Looking at imx_mu_seco_rxdb(), if the received message size is invalid or a
read timeout occurs, the function jumps to the error path:

drivers/mailbox/imx-mailbox.c:imx_mu_seco_rxdb() {
    ...
        goto error;
    ...
error:
    mbox_chan_received_data(cp->chan, ERR_PTR(err));

exit:
    return err;
}

This error path skips writing to the GSR register to clear the GIPn hardware
interrupt pending bit. Since imx_mu_isr_th() ignores the return value of
rxdb() and unconditionally unmasks the interrupt via imx_mu_xcr_set_act(),
won't the uncleared hardware status bit immediately re-trigger the hardirq
and cause a livelock?

> +		break;
> +
>  	default:
>  		dev_warn_ratelimited(priv->dev, "Unhandled channel type %d\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-imx_mbox_rproc-v2-0-a0059dc3b69a@linutronix.de?part=7