Re: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback for i.MX7ULP suspend

[Iuliana Prodan <iuliana.prodan@nxp.com>]

On 12/15/2025 9:26 AM, Jacky Bai wrote:
> On i.MX7ULP platform, certain drivers that depend on rpmsg may need
> to send an rpmsg request and receive an acknowledgment from the
> remote core during the late_suspend stage. In the current imx_rproc
> implementation, rx_callback defers message handling to a workqueue.
> However, this deferred work may not execute before the system enters
> the noirq_suspend stage. When that happens, pending mailbox IRQs
> will cause the suspend sequence to abort.
> 
> To address this, handle incoming messages synchronously within
> rx_callback when running on i.MX7ULP. This ensures the message is
> processed immediately, allows the mailbox IRQ to be cleared in time,
> and prevents suspend failures.
> 
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> ---
>   drivers/remoteproc/imx_rproc.c | 49 ++++++++++++++++++++++++++++++++++++++++--
>   drivers/remoteproc/imx_rproc.h |  1 +
>   2 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 3be8790c14a2ccc789dd40508ec274000ec09978..1127c56388fd94f296c80db5c01a5d431d669c4d 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -22,6 +22,7 @@
>   #include <linux/reboot.h>
>   #include <linux/regmap.h>
>   #include <linux/remoteproc.h>
> +#include <linux/suspend.h>
>   #include <linux/workqueue.h>
>   
>   #include "imx_rproc.h"
> @@ -111,11 +112,13 @@ struct imx_rproc {
>   	void __iomem			*rsc_table;
>   	struct imx_sc_ipc		*ipc_handle;
>   	struct notifier_block		rproc_nb;
> +	struct notifier_block		pm_nb;
>   	u32				rproc_pt;	/* partition id */
>   	u32				rsrc_id;	/* resource id */
>   	u32				entry;		/* cpu start address */
>   	u32				core_index;
>   	struct dev_pm_domain_list	*pd_list;
> +	bool				use_sync_rx;
>   };
>   
>   static const struct imx_rproc_att imx_rproc_att_imx93[] = {
> @@ -725,7 +728,10 @@ 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);
> +	if (priv->use_sync_rx)
> +		idr_for_each(&rproc->notifyids, imx_rproc_notified_idr_cb, rproc);
> +	else
> +		queue_work(priv->workqueue, &priv->rproc_work);
>   }
>   
>   static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)
> @@ -1009,6 +1015,38 @@ static int imx_rproc_sys_off_handler(struct sys_off_data *data)
>   	return NOTIFY_DONE;
>   }
>   
> +static int imx_rproc_pm_notify(struct notifier_block *nb,
> +	unsigned long action, void *data)
> +{
> +	int ret;
> +	struct imx_rproc *priv = container_of(nb, struct imx_rproc, pm_nb);
> +
> +	imx_rproc_free_mbox(priv->rproc);

Here you free mailbox unconditinally.
> +
> +	switch (action) {
> +	case PM_SUSPEND_PREPARE:

When switching to sync mode, any pending work in the workqueue should be 
flushed to avoid processing messages twice.
> +		ret = imx_rproc_xtr_mbox_init(priv->rproc, false);
> +		if (ret) {
> +			dev_err(&priv->rproc->dev, "Failed to request non-blocking mbox\n");
> +			return NOTIFY_BAD;
> +		}
> +		priv->use_sync_rx = true;
> +		break;
> +	case PM_POST_SUSPEND:
> +		ret = imx_rproc_xtr_mbox_init(priv->rproc, true);
> +		if (ret) {
> +			dev_err(&priv->rproc->dev, "Failed to request blocking mbox\n");
> +			return NOTIFY_BAD;
> +		}

If imx_rproc_xtr_mbox_init() fails in either case, the mailbox channels 
remain freed but the function returns NOTIFY_BAD. The system may 
continue with freed mailbox channels.

Only free the mailbox if re-initialization succeeds, or ensure proper 
cleanup on failure.

> +		priv->use_sync_rx = false;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
>   static void imx_rproc_destroy_workqueue(void *data)
>   {
>   	struct workqueue_struct *workqueue = data;
> @@ -1103,6 +1141,13 @@ static int imx_rproc_probe(struct platform_device *pdev)
>   			return dev_err_probe(dev, ret, "register restart handler failure\n");
>   	}
>   
> +	if (dcfg->flags & IMX_RPROC_NEED_PM_SYNC) {
> +		priv->pm_nb.notifier_call = imx_rproc_pm_notify;
> +		ret = register_pm_notifier(&priv->pm_nb);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "register pm notifier failure\n");
> +	}
> +

The PM notifier is registered in probe but never unregistered in remove 
or error paths.

Thanks,
Iulia

>   	pm_runtime_enable(dev);
>   	ret = pm_runtime_resume_and_get(dev);
>   	if (ret)
> @@ -1202,7 +1247,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = {
>   static const struct imx_rproc_dcfg imx_rproc_cfg_imx7ulp = {
>   	.att		= imx_rproc_att_imx7ulp,
>   	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7ulp),
> -	.flags		= IMX_RPROC_NEED_SYSTEM_OFF,
> +	.flags		= IMX_RPROC_NEED_SYSTEM_OFF | IMX_RPROC_NEED_PM_SYNC,
>   };
>   
>   static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
> diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
> index 1b2d9f4d6d19dcdc215be97f7e2ab3488281916a..0e3e460cef4ed9340fb4977183e03143c84764af 100644
> --- a/drivers/remoteproc/imx_rproc.h
> +++ b/drivers/remoteproc/imx_rproc.h
> @@ -18,6 +18,7 @@ struct imx_rproc_att {
>   /* dcfg flags */
>   #define IMX_RPROC_NEED_SYSTEM_OFF	BIT(0)
>   #define IMX_RPROC_NEED_CLKS		BIT(1)
> +#define IMX_RPROC_NEED_PM_SYNC		BIT(2)
>   
>   struct imx_rproc_plat_ops {
>   	int (*start)(struct rproc *rproc);
> 
> ---
> base-commit: 5ce74bc1b7cb2732b22f9c93082545bc655d6547
> change-id: 20251211-imx7ulp_rproc-57999329239b
> 
> Best regards,