* [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback for i.MX7ULP suspend
@ 2025-12-15 7:26 Jacky Bai
2025-12-15 16:12 ` Frank Li
2025-12-15 18:32 ` Iuliana Prodan
0 siblings, 2 replies; 5+ messages in thread
From: Jacky Bai @ 2025-12-15 7:26 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: linux-remoteproc, imx, linux-arm-kernel, linux-kernel, peng.fan,
Jacky Bai
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);
+
+ switch (action) {
+ case PM_SUSPEND_PREPARE:
+ 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;
+ }
+ 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");
+ }
+
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,
--
Jacky Bai <ping.bai@nxp.com>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback for i.MX7ULP suspend
2025-12-15 7:26 [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback for i.MX7ULP suspend Jacky Bai
@ 2025-12-15 16:12 ` Frank Li
2025-12-16 2:26 ` Jacky Bai
2025-12-15 18:32 ` Iuliana Prodan
1 sibling, 1 reply; 5+ messages in thread
From: Frank Li @ 2025-12-15 16:12 UTC (permalink / raw)
To: Jacky Bai
Cc: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-remoteproc, imx,
linux-arm-kernel, linux-kernel, peng.fan
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@nxp.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()?
> + 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..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,
> --
> Jacky Bai <ping.bai@nxp.com>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback for i.MX7ULP suspend
2025-12-15 7:26 [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback for i.MX7ULP suspend Jacky Bai
2025-12-15 16:12 ` Frank Li
@ 2025-12-15 18:32 ` Iuliana Prodan
2025-12-16 2:29 ` Jacky Bai
1 sibling, 1 reply; 5+ messages in thread
From: Iuliana Prodan @ 2025-12-15 18:32 UTC (permalink / raw)
To: Jacky Bai, Bjorn Andersson, Mathieu Poirier, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: linux-remoteproc, imx, linux-arm-kernel, linux-kernel, peng.fan
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,
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback for i.MX7ULP suspend
2025-12-15 16:12 ` Frank Li
@ 2025-12-16 2:26 ` Jacky Bai
0 siblings, 0 replies; 5+ messages in thread
From: Jacky Bai @ 2025-12-16 2:26 UTC (permalink / raw)
To: Frank Li
Cc: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam,
linux-remoteproc@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Peng Fan
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@nxp.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@nxp.com>
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback for i.MX7ULP suspend
2025-12-15 18:32 ` Iuliana Prodan
@ 2025-12-16 2:29 ` Jacky Bai
0 siblings, 0 replies; 5+ messages in thread
From: Jacky Bai @ 2025-12-16 2:29 UTC (permalink / raw)
To: Iuliana Prodan, Bjorn Andersson, Mathieu Poirier, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: linux-remoteproc@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Peng Fan
HI Luliana,
> Subject: Re: [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback
> for i.MX7ULP suspend
>
> 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..1127c56388fd94f296c80db
> 5c01a
> > 5d431d669c4d 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.
Please ignore this patch, changes is not perfect and too complex. Will try to
Fix the issue from mailbox.
Thx for your comments.
BR
Jacky Bai
> > +
> > + 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..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,
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-12-16 2:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-15 7:26 [PATCH] remoteproc: imx_rproc: Ues sync handling in rx_callback for i.MX7ULP suspend Jacky Bai
2025-12-15 16:12 ` Frank Li
2025-12-16 2:26 ` Jacky Bai
2025-12-15 18:32 ` Iuliana Prodan
2025-12-16 2:29 ` Jacky Bai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox