[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<VE1PR04MB721317F24C6F99F7468DE02B87AAA@VE1PR04MB7213.eurprd04.prod.outlook.com>
Date: Tue, 16 Dec 2025 02:26:54 +0000
From: Jacky Bai <ping.bai@....com>
To: Frank Li <frank.li@....com>
CC: Bjorn Andersson <andersson@...nel.org>, Mathieu Poirier
<mathieu.poirier@...aro.org>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer
<s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, "linux-remoteproc@...r.kernel.org"
<linux-remoteproc@...r.kernel.org>, "imx@...ts.linux.dev"
<imx@...ts.linux.dev>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, Peng Fan <peng.fan@....com>
Subject: RE: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback
for i.MX7ULP suspend
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 <ping.bai@....com>
> > ---
> > 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 <ping.bai@....com>
> >
Powered by blists - more mailing lists