Hi Frank,

> Subject: Re: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback
> for i.MX7ULP suspend
> 
> On Mon, Dec 15, 2025 at 03:26:05PM +0800, 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 <[email protected]>
> > ---
> >  drivers/remoteproc/imx_rproc.c | 49
> > ++++++++++++++++++++++++++++++++++++++++--
> >  drivers/remoteproc/imx_rproc.h |  1 +
> >  2 files changed, 48 insertions(+), 2 deletions(-)
> >
> ...
> >
> > -   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);
> 
>       struct imx_rproc *priv = container_of(nb, struct imx_rproc, pm_nb);
>       int ret;
> 
> > +
> > +   imx_rproc_free_mbox(priv->rproc);
> > +
> > +   switch (action) {
> > +   case PM_SUSPEND_PREPARE:
> > +           ret = imx_rproc_xtr_mbox_init(priv->rproc, false);
> 
> Any risk condition if on going message during free() and _init()?

After some further investigation, changes in remoteproc is too complex and many 
corner case
to handle. Plan to solve the issue directly from mailbox driver. Please ignore 
this patch.

Thx for your time.

BR
Jacky Bai
> 
> > +           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;
> > +           }
> > +           priv->use_sync_rx = false;
> > +           break;
> > +   default:
> > +           break;
> 
> imx_rproc_free_mbox() here? if other notify comming,  mbox is free() as
> expected?
> 
> Frank
> > +   }
> > +
> > +   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");
> > +   }
> > +
> >     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..0e3e460cef4ed9340fb4977
> 183e0
> > 3143c84764af 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,
> > --
> > Jacky Bai <[email protected]>
> >

Reply via email to