* [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores @ 2025-11-25 8:37 Beleswar Padhi 2025-11-26 20:41 ` Patrick Oppenlander 2025-12-16 22:23 ` Mathieu Poirier 0 siblings, 2 replies; 17+ messages in thread From: Beleswar Padhi @ 2025-11-25 8:37 UTC (permalink / raw) To: andersson, mathieu.poirier Cc: richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, b-padhi, linux-remoteproc, linux-kernel From: Richard Genoud <richard.genoud@bootlin.com> Introduce software IPC handshake between the host running Linux and the remote processors to gracefully stop/reset the remote core. Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox message to the remotecore. The remote core is expected to: - relinquish all the resources acquired through Device Manager (DM) - disable its interrupts - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK - enter WFI state. Meanwhile, the K3 remoteproc driver does: - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core - wait for the remoteproc to enter WFI state - reset the remote core through device manager Based on work from: Hari Nagalla <hnagalla@ti.com> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> [b-padhi@ti.com: Extend support to all rprocs] Signed-off-by: Beleswar Padhi <b-padhi@ti.com> --- v2: Changelog: 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has entered WFI state. 3. Convert return type of is_core_in_wfi() to bool. Works better with readx_poll_timeout() condition. 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings when void* is 64 bit. 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc function. 6. Updated commit message to fix minor typos and such. Link to v1: https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ Testing done: 1. Tested Boot across all TI K3 EVM/SK boards. 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). 4. Tested R5 rprocs can now be shutdown and powered back on from userspace. 3. Tested that each patch in the series generates no new warnings/errors. drivers/remoteproc/omap_remoteproc.h | 9 ++- drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ drivers/remoteproc/ti_k3_common.h | 4 ++ drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ 6 files changed, 93 insertions(+), 1 deletion(-) diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h index 828e13256c023..c008f11fa2a43 100644 --- a/drivers/remoteproc/omap_remoteproc.h +++ b/drivers/remoteproc/omap_remoteproc.h @@ -42,6 +42,11 @@ * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor * on a suspend request * + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor + * + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a + * shutdown request. The remote processor should be in WFI state short after. + * * Introduce new message definitions if any here. * * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, - RP_MBOX_END_MSG = 0xFFFFFF14, + RP_MBOX_SHUTDOWN = 0xFFFFFF14, + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, + RP_MBOX_END_MSG = 0xFFFFFF16, }; #endif /* _OMAP_RPMSG_H */ diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c index 56b71652e449f..5d469f65115c3 100644 --- a/drivers/remoteproc/ti_k3_common.c +++ b/drivers/remoteproc/ti_k3_common.c @@ -18,7 +18,9 @@ * Hari Nagalla <hnagalla@ti.com> */ +#include <linux/delay.h> #include <linux/io.h> +#include <linux/iopoll.h> #include <linux/mailbox_client.h> #include <linux/module.h> #include <linux/of_address.h> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) case RP_MBOX_ECHO_REPLY: dev_info(dev, "received echo reply from %s\n", rproc->name); break; + case RP_MBOX_SHUTDOWN_ACK: + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); + complete(&kproc->shutdown_complete); + break; default: /* silently handle all other valid messages */ if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) } EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); +/** + * is_core_in_wfi - Utility function to check core status + * @kproc: remote core pointer used for checking core status + * + * This utility function is invoked by the shutdown sequence to ensure + * the remote core is in wfi, before asserting a reset. + */ +bool is_core_in_wfi(struct k3_rproc *kproc) +{ + int ret; + u64 boot_vec; + u32 cfg, ctrl, stat; + + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); + if (ret) + return false; + + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); +} +EXPORT_SYMBOL_GPL(is_core_in_wfi); + +/** + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown + * @kproc: remote core pointer used for sending mbox msg + * + * This function sends the shutdown prepare message to remote processor and + * waits for an ACK. Further, it checks if the remote processor has entered + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc + * has relinquished its resources before asserting a reset, so the shutdown + * happens cleanly. + */ +int notify_shutdown_rproc(struct k3_rproc *kproc) +{ + bool wfi_status = false; + int ret; + + reinit_completion(&kproc->shutdown_complete); + + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); + if (ret < 0) { + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); + return ret; + } + + ret = wait_for_completion_timeout(&kproc->shutdown_complete, + msecs_to_jiffies(5000)); + if (ret == 0) { + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", + __func__); + return -EBUSY; + } + + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, + 200, 2000); + if (ret) + return ret; + + return 0; +} +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); + /* * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a * generic module reset that powers on the device and allows the internal @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); int k3_rproc_stop(struct rproc *rproc) { struct k3_rproc *kproc = rproc->priv; + int ret; + + ret = notify_shutdown_rproc(kproc); + if (ret) + return ret; return k3_rproc_reset(kproc); } diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h index aee3c28dbe510..2a025f4894b82 100644 --- a/drivers/remoteproc/ti_k3_common.h +++ b/drivers/remoteproc/ti_k3_common.h @@ -22,6 +22,7 @@ #define REMOTEPROC_TI_K3_COMMON_H #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 /** * struct k3_rproc_mem - internal memory structure @@ -92,6 +93,7 @@ struct k3_rproc { u32 ti_sci_id; struct mbox_chan *mbox; struct mbox_client client; + struct completion shutdown_complete; void *priv; }; @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, void k3_mem_release(void *data); int k3_reserved_mem_init(struct k3_rproc *kproc); void k3_release_tsp(void *data); +bool is_core_in_wfi(struct k3_rproc *kproc); +int notify_shutdown_rproc(struct k3_rproc *kproc); #endif /* REMOTEPROC_TI_K3_COMMON_H */ diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c index d6ceea6dc920e..156ae09d8ee25 100644 --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) if (ret) return ret; + init_completion(&kproc->shutdown_complete); + ret = k3_rproc_of_get_memories(pdev, kproc); if (ret) return ret; diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c index 3a11fd24eb52b..64d99071279b0 100644 --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) if (ret) return ret; + init_completion(&kproc->shutdown_complete); + ret = k3_rproc_of_get_memories(pdev, kproc); if (ret) return ret; diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c index 04f23295ffc10..8748dc6089cc2 100644 --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) struct k3_r5_cluster *cluster = core->cluster; int ret; + ret = notify_shutdown_rproc(kproc); + if (ret) + return ret; + /* halt all applicable cores */ if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { list_for_each_entry(core, &cluster->cores, elem) { @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) goto out; } + init_completion(&kproc->shutdown_complete); init_rmem: k3_r5_adjust_tcm_sizes(kproc); -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2025-11-25 8:37 [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores Beleswar Padhi @ 2025-11-26 20:41 ` Patrick Oppenlander 2025-11-27 10:33 ` Beleswar Prasad Padhi 2025-12-16 22:23 ` Mathieu Poirier 1 sibling, 1 reply; 17+ messages in thread From: Patrick Oppenlander @ 2025-11-26 20:41 UTC (permalink / raw) To: Beleswar Padhi Cc: andersson, mathieu.poirier, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On Tue, 25 Nov 2025 at 19:39, Beleswar Padhi <b-padhi@ti.com> wrote: > > From: Richard Genoud <richard.genoud@bootlin.com> > > Introduce software IPC handshake between the host running Linux and the > remote processors to gracefully stop/reset the remote core. > > Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox > message to the remotecore. > The remote core is expected to: > - relinquish all the resources acquired through Device Manager (DM) > - disable its interrupts > - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK > - enter WFI state. What happens if the remote core is unable to action the shutdown request (maybe it has crashed). Is there a way to cleanup resources which the remote core allocated without rebooting the whole system? Patrick > > Meanwhile, the K3 remoteproc driver does: > - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core > - wait for the remoteproc to enter WFI state > - reset the remote core through device manager > > Based on work from: Hari Nagalla <hnagalla@ti.com> > > Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> > [b-padhi@ti.com: Extend support to all rprocs] > Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > --- > v2: Changelog: > 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) > 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has > entered WFI state. > 3. Convert return type of is_core_in_wfi() to bool. Works better with > readx_poll_timeout() condition. > 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings > when void* is 64 bit. > 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc > function. > 6. Updated commit message to fix minor typos and such. > > Link to v1: > https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ > > Testing done: > 1. Tested Boot across all TI K3 EVM/SK boards. > 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). > 4. Tested R5 rprocs can now be shutdown and powered back on > from userspace. > 3. Tested that each patch in the series generates no new > warnings/errors. > > drivers/remoteproc/omap_remoteproc.h | 9 ++- > drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ > drivers/remoteproc/ti_k3_common.h | 4 ++ > drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + > drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + > drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ > 6 files changed, 93 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h > index 828e13256c023..c008f11fa2a43 100644 > --- a/drivers/remoteproc/omap_remoteproc.h > +++ b/drivers/remoteproc/omap_remoteproc.h > @@ -42,6 +42,11 @@ > * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor > * on a suspend request > * > + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor > + * > + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a > + * shutdown request. The remote processor should be in WFI state short after. > + * > * Introduce new message definitions if any here. > * > * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core > @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { > RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, > RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, > RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, > - RP_MBOX_END_MSG = 0xFFFFFF14, > + RP_MBOX_SHUTDOWN = 0xFFFFFF14, > + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, > + RP_MBOX_END_MSG = 0xFFFFFF16, > }; > > #endif /* _OMAP_RPMSG_H */ > diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > index 56b71652e449f..5d469f65115c3 100644 > --- a/drivers/remoteproc/ti_k3_common.c > +++ b/drivers/remoteproc/ti_k3_common.c > @@ -18,7 +18,9 @@ > * Hari Nagalla <hnagalla@ti.com> > */ > > +#include <linux/delay.h> > #include <linux/io.h> > +#include <linux/iopoll.h> > #include <linux/mailbox_client.h> > #include <linux/module.h> > #include <linux/of_address.h> > @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) > case RP_MBOX_ECHO_REPLY: > dev_info(dev, "received echo reply from %s\n", rproc->name); > break; > + case RP_MBOX_SHUTDOWN_ACK: > + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); > + complete(&kproc->shutdown_complete); > + break; > default: > /* silently handle all other valid messages */ > if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) > @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) > } > EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); > > +/** > + * is_core_in_wfi - Utility function to check core status > + * @kproc: remote core pointer used for checking core status > + * > + * This utility function is invoked by the shutdown sequence to ensure > + * the remote core is in wfi, before asserting a reset. > + */ > +bool is_core_in_wfi(struct k3_rproc *kproc) > +{ > + int ret; > + u64 boot_vec; > + u32 cfg, ctrl, stat; > + > + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); > + if (ret) > + return false; > + > + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); > +} > +EXPORT_SYMBOL_GPL(is_core_in_wfi); > + > +/** > + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown > + * @kproc: remote core pointer used for sending mbox msg > + * > + * This function sends the shutdown prepare message to remote processor and > + * waits for an ACK. Further, it checks if the remote processor has entered > + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc > + * has relinquished its resources before asserting a reset, so the shutdown > + * happens cleanly. > + */ > +int notify_shutdown_rproc(struct k3_rproc *kproc) > +{ > + bool wfi_status = false; > + int ret; > + > + reinit_completion(&kproc->shutdown_complete); > + > + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); > + if (ret < 0) { > + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); > + return ret; > + } > + > + ret = wait_for_completion_timeout(&kproc->shutdown_complete, > + msecs_to_jiffies(5000)); > + if (ret == 0) { > + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", > + __func__); > + return -EBUSY; > + } > + > + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, > + 200, 2000); > + if (ret) > + return ret; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); > + > /* > * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a > * generic module reset that powers on the device and allows the internal > @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); > int k3_rproc_stop(struct rproc *rproc) > { > struct k3_rproc *kproc = rproc->priv; > + int ret; > + > + ret = notify_shutdown_rproc(kproc); > + if (ret) > + return ret; > > return k3_rproc_reset(kproc); > } > diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h > index aee3c28dbe510..2a025f4894b82 100644 > --- a/drivers/remoteproc/ti_k3_common.h > +++ b/drivers/remoteproc/ti_k3_common.h > @@ -22,6 +22,7 @@ > #define REMOTEPROC_TI_K3_COMMON_H > > #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) > +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 > > /** > * struct k3_rproc_mem - internal memory structure > @@ -92,6 +93,7 @@ struct k3_rproc { > u32 ti_sci_id; > struct mbox_chan *mbox; > struct mbox_client client; > + struct completion shutdown_complete; > void *priv; > }; > > @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, > void k3_mem_release(void *data); > int k3_reserved_mem_init(struct k3_rproc *kproc); > void k3_release_tsp(void *data); > +bool is_core_in_wfi(struct k3_rproc *kproc); > +int notify_shutdown_rproc(struct k3_rproc *kproc); > #endif /* REMOTEPROC_TI_K3_COMMON_H */ > diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > index d6ceea6dc920e..156ae09d8ee25 100644 > --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) > if (ret) > return ret; > > + init_completion(&kproc->shutdown_complete); > + > ret = k3_rproc_of_get_memories(pdev, kproc); > if (ret) > return ret; > diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c > index 3a11fd24eb52b..64d99071279b0 100644 > --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c > @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) > if (ret) > return ret; > > + init_completion(&kproc->shutdown_complete); > + > ret = k3_rproc_of_get_memories(pdev, kproc); > if (ret) > return ret; > diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > index 04f23295ffc10..8748dc6089cc2 100644 > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > struct k3_r5_cluster *cluster = core->cluster; > int ret; > > + ret = notify_shutdown_rproc(kproc); > + if (ret) > + return ret; > + > /* halt all applicable cores */ > if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { > list_for_each_entry(core, &cluster->cores, elem) { > @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) > goto out; > } > > + init_completion(&kproc->shutdown_complete); > init_rmem: > k3_r5_adjust_tcm_sizes(kproc); > > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2025-11-26 20:41 ` Patrick Oppenlander @ 2025-11-27 10:33 ` Beleswar Prasad Padhi 2025-11-27 15:17 ` Mathieu Poirier 2026-01-09 16:53 ` Mathieu Poirier 0 siblings, 2 replies; 17+ messages in thread From: Beleswar Prasad Padhi @ 2025-11-27 10:33 UTC (permalink / raw) To: Patrick Oppenlander Cc: andersson, mathieu.poirier, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel Hi Patrick, Mathieu, On 27/11/25 02:11, Patrick Oppenlander wrote: > On Tue, 25 Nov 2025 at 19:39, Beleswar Padhi <b-padhi@ti.com> wrote: >> From: Richard Genoud <richard.genoud@bootlin.com> >> >> Introduce software IPC handshake between the host running Linux and the >> remote processors to gracefully stop/reset the remote core. >> >> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox >> message to the remotecore. >> The remote core is expected to: >> - relinquish all the resources acquired through Device Manager (DM) >> - disable its interrupts >> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK >> - enter WFI state. > What happens if the remote core is unable to action the shutdown > request We abort the shutdown sequence if the remoteproc does not respond with an ACK within the timeout assuming rproc is busy doing some work. > (maybe it has crashed). remoteproc core has the infra to handle rproc crash. It initiates a recovery mechanism by stopping and starting the rproc with the same firmware. Are you suggesting that we check if rproc_stop() is invoked from a recovery context, and forcefully reset the rproc without sending/waiting for SHUTDOWN msg as a crashed core can't respond to mbox irqs? > > Is there a way to cleanup resources which the remote core allocated > without rebooting the whole system? For SW resources (like mem, vdev): Yes However, I feel this is currently missing in rproc core. We should be making a call to rproc_resource_cleanup() in rproc_boot_recovery()'s error paths and in rproc_crash_handler_work() in case of subsequent crashes. ^^ Mathieu, thoughts about the above? For HW resources: No In TI Device Manager (DM) firmware, only the entity which requested a resource can relinquish it, no other host can do that cleanup on behalf of that entity. So, we can't do much here. Thanks, Beleswar > > Patrick > >> Meanwhile, the K3 remoteproc driver does: >> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core >> - wait for the remoteproc to enter WFI state >> - reset the remote core through device manager >> >> Based on work from: Hari Nagalla <hnagalla@ti.com> >> >> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> >> [b-padhi@ti.com: Extend support to all rprocs] >> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> >> --- >> v2: Changelog: >> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) >> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has >> entered WFI state. >> 3. Convert return type of is_core_in_wfi() to bool. Works better with >> readx_poll_timeout() condition. >> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings >> when void* is 64 bit. >> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc >> function. >> 6. Updated commit message to fix minor typos and such. >> >> Link to v1: >> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ >> >> Testing done: >> 1. Tested Boot across all TI K3 EVM/SK boards. >> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). >> 4. Tested R5 rprocs can now be shutdown and powered back on >> from userspace. >> 3. Tested that each patch in the series generates no new >> warnings/errors. >> >> drivers/remoteproc/omap_remoteproc.h | 9 ++- >> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ >> drivers/remoteproc/ti_k3_common.h | 4 ++ >> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + >> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + >> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ >> 6 files changed, 93 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h >> index 828e13256c023..c008f11fa2a43 100644 >> --- a/drivers/remoteproc/omap_remoteproc.h >> +++ b/drivers/remoteproc/omap_remoteproc.h >> @@ -42,6 +42,11 @@ >> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor >> * on a suspend request >> * >> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor >> + * >> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a >> + * shutdown request. The remote processor should be in WFI state short after. >> + * >> * Introduce new message definitions if any here. >> * >> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core >> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { >> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, >> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, >> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, >> - RP_MBOX_END_MSG = 0xFFFFFF14, >> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, >> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, >> + RP_MBOX_END_MSG = 0xFFFFFF16, >> }; >> >> #endif /* _OMAP_RPMSG_H */ >> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c >> index 56b71652e449f..5d469f65115c3 100644 >> --- a/drivers/remoteproc/ti_k3_common.c >> +++ b/drivers/remoteproc/ti_k3_common.c >> @@ -18,7 +18,9 @@ >> * Hari Nagalla <hnagalla@ti.com> >> */ >> >> +#include <linux/delay.h> >> #include <linux/io.h> >> +#include <linux/iopoll.h> >> #include <linux/mailbox_client.h> >> #include <linux/module.h> >> #include <linux/of_address.h> >> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) >> case RP_MBOX_ECHO_REPLY: >> dev_info(dev, "received echo reply from %s\n", rproc->name); >> break; >> + case RP_MBOX_SHUTDOWN_ACK: >> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); >> + complete(&kproc->shutdown_complete); >> + break; >> default: >> /* silently handle all other valid messages */ >> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) >> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) >> } >> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); >> >> +/** >> + * is_core_in_wfi - Utility function to check core status >> + * @kproc: remote core pointer used for checking core status >> + * >> + * This utility function is invoked by the shutdown sequence to ensure >> + * the remote core is in wfi, before asserting a reset. >> + */ >> +bool is_core_in_wfi(struct k3_rproc *kproc) >> +{ >> + int ret; >> + u64 boot_vec; >> + u32 cfg, ctrl, stat; >> + >> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); >> + if (ret) >> + return false; >> + >> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); >> +} >> +EXPORT_SYMBOL_GPL(is_core_in_wfi); >> + >> +/** >> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown >> + * @kproc: remote core pointer used for sending mbox msg >> + * >> + * This function sends the shutdown prepare message to remote processor and >> + * waits for an ACK. Further, it checks if the remote processor has entered >> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc >> + * has relinquished its resources before asserting a reset, so the shutdown >> + * happens cleanly. >> + */ >> +int notify_shutdown_rproc(struct k3_rproc *kproc) >> +{ >> + bool wfi_status = false; >> + int ret; >> + >> + reinit_completion(&kproc->shutdown_complete); >> + >> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); >> + if (ret < 0) { >> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); >> + return ret; >> + } >> + >> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, >> + msecs_to_jiffies(5000)); >> + if (ret == 0) { >> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", >> + __func__); >> + return -EBUSY; >> + } >> + >> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, >> + 200, 2000); >> + if (ret) >> + return ret; >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); >> + >> /* >> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a >> * generic module reset that powers on the device and allows the internal >> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); >> int k3_rproc_stop(struct rproc *rproc) >> { >> struct k3_rproc *kproc = rproc->priv; >> + int ret; >> + >> + ret = notify_shutdown_rproc(kproc); >> + if (ret) >> + return ret; >> >> return k3_rproc_reset(kproc); >> } >> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h >> index aee3c28dbe510..2a025f4894b82 100644 >> --- a/drivers/remoteproc/ti_k3_common.h >> +++ b/drivers/remoteproc/ti_k3_common.h >> @@ -22,6 +22,7 @@ >> #define REMOTEPROC_TI_K3_COMMON_H >> >> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) >> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 >> >> /** >> * struct k3_rproc_mem - internal memory structure >> @@ -92,6 +93,7 @@ struct k3_rproc { >> u32 ti_sci_id; >> struct mbox_chan *mbox; >> struct mbox_client client; >> + struct completion shutdown_complete; >> void *priv; >> }; >> >> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, >> void k3_mem_release(void *data); >> int k3_reserved_mem_init(struct k3_rproc *kproc); >> void k3_release_tsp(void *data); >> +bool is_core_in_wfi(struct k3_rproc *kproc); >> +int notify_shutdown_rproc(struct k3_rproc *kproc); >> #endif /* REMOTEPROC_TI_K3_COMMON_H */ >> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c >> index d6ceea6dc920e..156ae09d8ee25 100644 >> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c >> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c >> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) >> if (ret) >> return ret; >> >> + init_completion(&kproc->shutdown_complete); >> + >> ret = k3_rproc_of_get_memories(pdev, kproc); >> if (ret) >> return ret; >> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c >> index 3a11fd24eb52b..64d99071279b0 100644 >> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c >> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c >> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) >> if (ret) >> return ret; >> >> + init_completion(&kproc->shutdown_complete); >> + >> ret = k3_rproc_of_get_memories(pdev, kproc); >> if (ret) >> return ret; >> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c >> index 04f23295ffc10..8748dc6089cc2 100644 >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c >> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) >> struct k3_r5_cluster *cluster = core->cluster; >> int ret; >> >> + ret = notify_shutdown_rproc(kproc); >> + if (ret) >> + return ret; >> + >> /* halt all applicable cores */ >> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { >> list_for_each_entry(core, &cluster->cores, elem) { >> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) >> goto out; >> } >> >> + init_completion(&kproc->shutdown_complete); >> init_rmem: >> k3_r5_adjust_tcm_sizes(kproc); >> >> -- >> 2.34.1 >> >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2025-11-27 10:33 ` Beleswar Prasad Padhi @ 2025-11-27 15:17 ` Mathieu Poirier 2026-01-09 16:53 ` Mathieu Poirier 1 sibling, 0 replies; 17+ messages in thread From: Mathieu Poirier @ 2025-11-27 15:17 UTC (permalink / raw) To: Beleswar Prasad Padhi Cc: Patrick Oppenlander, andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On Thu, 27 Nov 2025 at 03:33, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: > > Hi Patrick, Mathieu, > > On 27/11/25 02:11, Patrick Oppenlander wrote: > > On Tue, 25 Nov 2025 at 19:39, Beleswar Padhi <b-padhi@ti.com> wrote: > >> From: Richard Genoud <richard.genoud@bootlin.com> > >> > >> Introduce software IPC handshake between the host running Linux and the > >> remote processors to gracefully stop/reset the remote core. > >> > >> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox > >> message to the remotecore. > >> The remote core is expected to: > >> - relinquish all the resources acquired through Device Manager (DM) > >> - disable its interrupts > >> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK > >> - enter WFI state. > > What happens if the remote core is unable to action the shutdown > > request > > > We abort the shutdown sequence if the remoteproc does not respond with > an ACK within the timeout assuming rproc is busy doing some work. > > > (maybe it has crashed). > > > remoteproc core has the infra to handle rproc crash. It initiates a > recovery mechanism by stopping and starting the rproc with the same > firmware. > > Are you suggesting that we check if rproc_stop() is invoked from a > recovery context, and forcefully reset the rproc without sending/waiting > for SHUTDOWN msg as a crashed core can't respond to mbox irqs? > > > > > Is there a way to cleanup resources which the remote core allocated > > without rebooting the whole system? > > For SW resources (like mem, vdev): Yes > However, I feel this is currently missing in rproc core. We should be > making a call to rproc_resource_cleanup() in rproc_boot_recovery()'s > error paths and in rproc_crash_handler_work() in case of subsequent > crashes. > > ^^ Mathieu, thoughts about the above? > Given the backlog of patchsets I have to review, Plumbers in two weeks and the December holidays, I won't be able to look at this issue before January. Thanks, Mathieu > For HW resources: No > In TI Device Manager (DM) firmware, only the entity which requested a > resource can relinquish it, no other host can do that cleanup on behalf > of that entity. So, we can't do much here. > > Thanks, > Beleswar > > > > > Patrick > > > >> Meanwhile, the K3 remoteproc driver does: > >> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core > >> - wait for the remoteproc to enter WFI state > >> - reset the remote core through device manager > >> > >> Based on work from: Hari Nagalla <hnagalla@ti.com> > >> > >> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> > >> [b-padhi@ti.com: Extend support to all rprocs] > >> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > >> --- > >> v2: Changelog: > >> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) > >> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has > >> entered WFI state. > >> 3. Convert return type of is_core_in_wfi() to bool. Works better with > >> readx_poll_timeout() condition. > >> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings > >> when void* is 64 bit. > >> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc > >> function. > >> 6. Updated commit message to fix minor typos and such. > >> > >> Link to v1: > >> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ > >> > >> Testing done: > >> 1. Tested Boot across all TI K3 EVM/SK boards. > >> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). > >> 4. Tested R5 rprocs can now be shutdown and powered back on > >> from userspace. > >> 3. Tested that each patch in the series generates no new > >> warnings/errors. > >> > >> drivers/remoteproc/omap_remoteproc.h | 9 ++- > >> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ > >> drivers/remoteproc/ti_k3_common.h | 4 ++ > >> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + > >> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + > >> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ > >> 6 files changed, 93 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h > >> index 828e13256c023..c008f11fa2a43 100644 > >> --- a/drivers/remoteproc/omap_remoteproc.h > >> +++ b/drivers/remoteproc/omap_remoteproc.h > >> @@ -42,6 +42,11 @@ > >> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor > >> * on a suspend request > >> * > >> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor > >> + * > >> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a > >> + * shutdown request. The remote processor should be in WFI state short after. > >> + * > >> * Introduce new message definitions if any here. > >> * > >> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core > >> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { > >> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, > >> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, > >> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, > >> - RP_MBOX_END_MSG = 0xFFFFFF14, > >> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, > >> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, > >> + RP_MBOX_END_MSG = 0xFFFFFF16, > >> }; > >> > >> #endif /* _OMAP_RPMSG_H */ > >> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > >> index 56b71652e449f..5d469f65115c3 100644 > >> --- a/drivers/remoteproc/ti_k3_common.c > >> +++ b/drivers/remoteproc/ti_k3_common.c > >> @@ -18,7 +18,9 @@ > >> * Hari Nagalla <hnagalla@ti.com> > >> */ > >> > >> +#include <linux/delay.h> > >> #include <linux/io.h> > >> +#include <linux/iopoll.h> > >> #include <linux/mailbox_client.h> > >> #include <linux/module.h> > >> #include <linux/of_address.h> > >> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) > >> case RP_MBOX_ECHO_REPLY: > >> dev_info(dev, "received echo reply from %s\n", rproc->name); > >> break; > >> + case RP_MBOX_SHUTDOWN_ACK: > >> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); > >> + complete(&kproc->shutdown_complete); > >> + break; > >> default: > >> /* silently handle all other valid messages */ > >> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) > >> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) > >> } > >> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); > >> > >> +/** > >> + * is_core_in_wfi - Utility function to check core status > >> + * @kproc: remote core pointer used for checking core status > >> + * > >> + * This utility function is invoked by the shutdown sequence to ensure > >> + * the remote core is in wfi, before asserting a reset. > >> + */ > >> +bool is_core_in_wfi(struct k3_rproc *kproc) > >> +{ > >> + int ret; > >> + u64 boot_vec; > >> + u32 cfg, ctrl, stat; > >> + > >> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); > >> + if (ret) > >> + return false; > >> + > >> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); > >> +} > >> +EXPORT_SYMBOL_GPL(is_core_in_wfi); > >> + > >> +/** > >> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown > >> + * @kproc: remote core pointer used for sending mbox msg > >> + * > >> + * This function sends the shutdown prepare message to remote processor and > >> + * waits for an ACK. Further, it checks if the remote processor has entered > >> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc > >> + * has relinquished its resources before asserting a reset, so the shutdown > >> + * happens cleanly. > >> + */ > >> +int notify_shutdown_rproc(struct k3_rproc *kproc) > >> +{ > >> + bool wfi_status = false; > >> + int ret; > >> + > >> + reinit_completion(&kproc->shutdown_complete); > >> + > >> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); > >> + if (ret < 0) { > >> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); > >> + return ret; > >> + } > >> + > >> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, > >> + msecs_to_jiffies(5000)); > >> + if (ret == 0) { > >> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", > >> + __func__); > >> + return -EBUSY; > >> + } > >> + > >> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, > >> + 200, 2000); > >> + if (ret) > >> + return ret; > >> + > >> + return 0; > >> +} > >> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); > >> + > >> /* > >> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a > >> * generic module reset that powers on the device and allows the internal > >> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); > >> int k3_rproc_stop(struct rproc *rproc) > >> { > >> struct k3_rproc *kproc = rproc->priv; > >> + int ret; > >> + > >> + ret = notify_shutdown_rproc(kproc); > >> + if (ret) > >> + return ret; > >> > >> return k3_rproc_reset(kproc); > >> } > >> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h > >> index aee3c28dbe510..2a025f4894b82 100644 > >> --- a/drivers/remoteproc/ti_k3_common.h > >> +++ b/drivers/remoteproc/ti_k3_common.h > >> @@ -22,6 +22,7 @@ > >> #define REMOTEPROC_TI_K3_COMMON_H > >> > >> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) > >> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 > >> > >> /** > >> * struct k3_rproc_mem - internal memory structure > >> @@ -92,6 +93,7 @@ struct k3_rproc { > >> u32 ti_sci_id; > >> struct mbox_chan *mbox; > >> struct mbox_client client; > >> + struct completion shutdown_complete; > >> void *priv; > >> }; > >> > >> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, > >> void k3_mem_release(void *data); > >> int k3_reserved_mem_init(struct k3_rproc *kproc); > >> void k3_release_tsp(void *data); > >> +bool is_core_in_wfi(struct k3_rproc *kproc); > >> +int notify_shutdown_rproc(struct k3_rproc *kproc); > >> #endif /* REMOTEPROC_TI_K3_COMMON_H */ > >> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >> index d6ceea6dc920e..156ae09d8ee25 100644 > >> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) > >> if (ret) > >> return ret; > >> > >> + init_completion(&kproc->shutdown_complete); > >> + > >> ret = k3_rproc_of_get_memories(pdev, kproc); > >> if (ret) > >> return ret; > >> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c > >> index 3a11fd24eb52b..64d99071279b0 100644 > >> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c > >> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c > >> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) > >> if (ret) > >> return ret; > >> > >> + init_completion(&kproc->shutdown_complete); > >> + > >> ret = k3_rproc_of_get_memories(pdev, kproc); > >> if (ret) > >> return ret; > >> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > >> index 04f23295ffc10..8748dc6089cc2 100644 > >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > >> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > >> struct k3_r5_cluster *cluster = core->cluster; > >> int ret; > >> > >> + ret = notify_shutdown_rproc(kproc); > >> + if (ret) > >> + return ret; > >> + > >> /* halt all applicable cores */ > >> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { > >> list_for_each_entry(core, &cluster->cores, elem) { > >> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) > >> goto out; > >> } > >> > >> + init_completion(&kproc->shutdown_complete); > >> init_rmem: > >> k3_r5_adjust_tcm_sizes(kproc); > >> > >> -- > >> 2.34.1 > >> > >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2025-11-27 10:33 ` Beleswar Prasad Padhi 2025-11-27 15:17 ` Mathieu Poirier @ 2026-01-09 16:53 ` Mathieu Poirier 2026-01-13 16:52 ` Beleswar Prasad Padhi 1 sibling, 1 reply; 17+ messages in thread From: Mathieu Poirier @ 2026-01-09 16:53 UTC (permalink / raw) To: Beleswar Prasad Padhi Cc: Patrick Oppenlander, andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On Thu, Nov 27, 2025 at 04:03:40PM +0530, Beleswar Prasad Padhi wrote: > Hi Patrick, Mathieu, > > On 27/11/25 02:11, Patrick Oppenlander wrote: > > On Tue, 25 Nov 2025 at 19:39, Beleswar Padhi <b-padhi@ti.com> wrote: > >> From: Richard Genoud <richard.genoud@bootlin.com> > >> > >> Introduce software IPC handshake between the host running Linux and the > >> remote processors to gracefully stop/reset the remote core. > >> > >> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox > >> message to the remotecore. > >> The remote core is expected to: > >> - relinquish all the resources acquired through Device Manager (DM) > >> - disable its interrupts > >> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK > >> - enter WFI state. > > What happens if the remote core is unable to action the shutdown > > request > > > We abort the shutdown sequence if the remoteproc does not respond with > an ACK within the timeout assuming rproc is busy doing some work. > > > (maybe it has crashed). > > > remoteproc core has the infra to handle rproc crash. It initiates a > recovery mechanism by stopping and starting the rproc with the same > firmware. > > Are you suggesting that we check if rproc_stop() is invoked from a > recovery context, and forcefully reset the rproc without sending/waiting > for SHUTDOWN msg as a crashed core can't respond to mbox irqs? I think that is a fair ask. > > > > > Is there a way to cleanup resources which the remote core allocated > > without rebooting the whole system? > > For SW resources (like mem, vdev): Yes > However, I feel this is currently missing in rproc core. We should be > making a call to rproc_resource_cleanup() in rproc_boot_recovery()'s > error paths and in rproc_crash_handler_work() in case of subsequent > crashes. > > ^^ Mathieu, thoughts about the above? So far a crash has been treated like a rproc_stop()/rproc_start() operation. Doing resource cleanup as part of a crash recovery would akin to a rproc_shutdown()/rproc_boot() operation, introducing a lot of churn and backward compatibility issues. > > For HW resources: No > In TI Device Manager (DM) firmware, only the entity which requested a > resource can relinquish it, no other host can do that cleanup on behalf > of that entity. So, we can't do much here. > > Thanks, > Beleswar > > > > > Patrick > > > >> Meanwhile, the K3 remoteproc driver does: > >> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core > >> - wait for the remoteproc to enter WFI state > >> - reset the remote core through device manager > >> > >> Based on work from: Hari Nagalla <hnagalla@ti.com> > >> > >> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> > >> [b-padhi@ti.com: Extend support to all rprocs] > >> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > >> --- > >> v2: Changelog: > >> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) > >> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has > >> entered WFI state. > >> 3. Convert return type of is_core_in_wfi() to bool. Works better with > >> readx_poll_timeout() condition. > >> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings > >> when void* is 64 bit. > >> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc > >> function. > >> 6. Updated commit message to fix minor typos and such. > >> > >> Link to v1: > >> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ > >> > >> Testing done: > >> 1. Tested Boot across all TI K3 EVM/SK boards. > >> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). > >> 4. Tested R5 rprocs can now be shutdown and powered back on > >> from userspace. > >> 3. Tested that each patch in the series generates no new > >> warnings/errors. > >> > >> drivers/remoteproc/omap_remoteproc.h | 9 ++- > >> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ > >> drivers/remoteproc/ti_k3_common.h | 4 ++ > >> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + > >> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + > >> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ > >> 6 files changed, 93 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h > >> index 828e13256c023..c008f11fa2a43 100644 > >> --- a/drivers/remoteproc/omap_remoteproc.h > >> +++ b/drivers/remoteproc/omap_remoteproc.h > >> @@ -42,6 +42,11 @@ > >> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor > >> * on a suspend request > >> * > >> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor > >> + * > >> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a > >> + * shutdown request. The remote processor should be in WFI state short after. > >> + * > >> * Introduce new message definitions if any here. > >> * > >> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core > >> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { > >> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, > >> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, > >> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, > >> - RP_MBOX_END_MSG = 0xFFFFFF14, > >> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, > >> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, > >> + RP_MBOX_END_MSG = 0xFFFFFF16, > >> }; > >> > >> #endif /* _OMAP_RPMSG_H */ > >> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > >> index 56b71652e449f..5d469f65115c3 100644 > >> --- a/drivers/remoteproc/ti_k3_common.c > >> +++ b/drivers/remoteproc/ti_k3_common.c > >> @@ -18,7 +18,9 @@ > >> * Hari Nagalla <hnagalla@ti.com> > >> */ > >> > >> +#include <linux/delay.h> > >> #include <linux/io.h> > >> +#include <linux/iopoll.h> > >> #include <linux/mailbox_client.h> > >> #include <linux/module.h> > >> #include <linux/of_address.h> > >> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) > >> case RP_MBOX_ECHO_REPLY: > >> dev_info(dev, "received echo reply from %s\n", rproc->name); > >> break; > >> + case RP_MBOX_SHUTDOWN_ACK: > >> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); > >> + complete(&kproc->shutdown_complete); > >> + break; > >> default: > >> /* silently handle all other valid messages */ > >> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) > >> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) > >> } > >> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); > >> > >> +/** > >> + * is_core_in_wfi - Utility function to check core status > >> + * @kproc: remote core pointer used for checking core status > >> + * > >> + * This utility function is invoked by the shutdown sequence to ensure > >> + * the remote core is in wfi, before asserting a reset. > >> + */ > >> +bool is_core_in_wfi(struct k3_rproc *kproc) > >> +{ > >> + int ret; > >> + u64 boot_vec; > >> + u32 cfg, ctrl, stat; > >> + > >> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); > >> + if (ret) > >> + return false; > >> + > >> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); > >> +} > >> +EXPORT_SYMBOL_GPL(is_core_in_wfi); > >> + > >> +/** > >> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown > >> + * @kproc: remote core pointer used for sending mbox msg > >> + * > >> + * This function sends the shutdown prepare message to remote processor and > >> + * waits for an ACK. Further, it checks if the remote processor has entered > >> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc > >> + * has relinquished its resources before asserting a reset, so the shutdown > >> + * happens cleanly. > >> + */ > >> +int notify_shutdown_rproc(struct k3_rproc *kproc) > >> +{ > >> + bool wfi_status = false; > >> + int ret; > >> + > >> + reinit_completion(&kproc->shutdown_complete); > >> + > >> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); > >> + if (ret < 0) { > >> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); > >> + return ret; > >> + } > >> + > >> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, > >> + msecs_to_jiffies(5000)); > >> + if (ret == 0) { > >> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", > >> + __func__); > >> + return -EBUSY; > >> + } > >> + > >> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, > >> + 200, 2000); > >> + if (ret) > >> + return ret; > >> + > >> + return 0; > >> +} > >> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); > >> + > >> /* > >> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a > >> * generic module reset that powers on the device and allows the internal > >> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); > >> int k3_rproc_stop(struct rproc *rproc) > >> { > >> struct k3_rproc *kproc = rproc->priv; > >> + int ret; > >> + > >> + ret = notify_shutdown_rproc(kproc); > >> + if (ret) > >> + return ret; > >> > >> return k3_rproc_reset(kproc); > >> } > >> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h > >> index aee3c28dbe510..2a025f4894b82 100644 > >> --- a/drivers/remoteproc/ti_k3_common.h > >> +++ b/drivers/remoteproc/ti_k3_common.h > >> @@ -22,6 +22,7 @@ > >> #define REMOTEPROC_TI_K3_COMMON_H > >> > >> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) > >> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 > >> > >> /** > >> * struct k3_rproc_mem - internal memory structure > >> @@ -92,6 +93,7 @@ struct k3_rproc { > >> u32 ti_sci_id; > >> struct mbox_chan *mbox; > >> struct mbox_client client; > >> + struct completion shutdown_complete; > >> void *priv; > >> }; > >> > >> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, > >> void k3_mem_release(void *data); > >> int k3_reserved_mem_init(struct k3_rproc *kproc); > >> void k3_release_tsp(void *data); > >> +bool is_core_in_wfi(struct k3_rproc *kproc); > >> +int notify_shutdown_rproc(struct k3_rproc *kproc); > >> #endif /* REMOTEPROC_TI_K3_COMMON_H */ > >> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >> index d6ceea6dc920e..156ae09d8ee25 100644 > >> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) > >> if (ret) > >> return ret; > >> > >> + init_completion(&kproc->shutdown_complete); > >> + > >> ret = k3_rproc_of_get_memories(pdev, kproc); > >> if (ret) > >> return ret; > >> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c > >> index 3a11fd24eb52b..64d99071279b0 100644 > >> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c > >> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c > >> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) > >> if (ret) > >> return ret; > >> > >> + init_completion(&kproc->shutdown_complete); > >> + > >> ret = k3_rproc_of_get_memories(pdev, kproc); > >> if (ret) > >> return ret; > >> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > >> index 04f23295ffc10..8748dc6089cc2 100644 > >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > >> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > >> struct k3_r5_cluster *cluster = core->cluster; > >> int ret; > >> > >> + ret = notify_shutdown_rproc(kproc); > >> + if (ret) > >> + return ret; > >> + > >> /* halt all applicable cores */ > >> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { > >> list_for_each_entry(core, &cluster->cores, elem) { > >> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) > >> goto out; > >> } > >> > >> + init_completion(&kproc->shutdown_complete); > >> init_rmem: > >> k3_r5_adjust_tcm_sizes(kproc); > >> > >> -- > >> 2.34.1 > >> > >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2026-01-09 16:53 ` Mathieu Poirier @ 2026-01-13 16:52 ` Beleswar Prasad Padhi 0 siblings, 0 replies; 17+ messages in thread From: Beleswar Prasad Padhi @ 2026-01-13 16:52 UTC (permalink / raw) To: Mathieu Poirier Cc: Patrick Oppenlander, andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On 09/01/26 22:23, Mathieu Poirier wrote: > On Thu, Nov 27, 2025 at 04:03:40PM +0530, Beleswar Prasad Padhi wrote: >> Hi Patrick, Mathieu, >> >> On 27/11/25 02:11, Patrick Oppenlander wrote: >>> On Tue, 25 Nov 2025 at 19:39, Beleswar Padhi <b-padhi@ti.com> wrote: >>>> From: Richard Genoud <richard.genoud@bootlin.com> >>>> >>>> Introduce software IPC handshake between the host running Linux and the >>>> remote processors to gracefully stop/reset the remote core. >>>> >>>> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox >>>> message to the remotecore. >>>> The remote core is expected to: >>>> - relinquish all the resources acquired through Device Manager (DM) >>>> - disable its interrupts >>>> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK >>>> - enter WFI state. >>> What happens if the remote core is unable to action the shutdown >>> request >> >> We abort the shutdown sequence if the remoteproc does not respond with >> an ACK within the timeout assuming rproc is busy doing some work. >> >>> (maybe it has crashed). >> >> remoteproc core has the infra to handle rproc crash. It initiates a >> recovery mechanism by stopping and starting the rproc with the same >> firmware. >> >> Are you suggesting that we check if rproc_stop() is invoked from a >> recovery context, and forcefully reset the rproc without sending/waiting >> for SHUTDOWN msg as a crashed core can't respond to mbox irqs? > I think that is a fair ask. Would need to pass on the `bool crashed` parameter to the stop() rproc_ops from rproc_stop(). Will address in v3... > >>> Is there a way to cleanup resources which the remote core allocated >>> without rebooting the whole system? >> For SW resources (like mem, vdev): Yes >> However, I feel this is currently missing in rproc core. We should be >> making a call to rproc_resource_cleanup() in rproc_boot_recovery()'s >> error paths and in rproc_crash_handler_work() in case of subsequent >> crashes. >> >> ^^ Mathieu, thoughts about the above? > So far a crash has been treated like a rproc_stop()/rproc_start() operation. > Doing resource cleanup as part of a crash recovery would akin to a > rproc_shutdown()/rproc_boot() operation, introducing a lot of churn and backward > compatibility issues. Makes sense to me. We're better off. Thanks, Beleswar > >> For HW resources: No >> In TI Device Manager (DM) firmware, only the entity which requested a >> resource can relinquish it, no other host can do that cleanup on behalf >> of that entity. So, we can't do much here. >> >> Thanks, >> Beleswar >> >>> Patrick >>> >>>> Meanwhile, the K3 remoteproc driver does: >>>> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core >>>> - wait for the remoteproc to enter WFI state >>>> - reset the remote core through device manager >>>> >>>> Based on work from: Hari Nagalla <hnagalla@ti.com> >>>> >>>> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> >>>> [b-padhi@ti.com: Extend support to all rprocs] >>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> >>>> --- >>>> v2: Changelog: >>>> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) >>>> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has >>>> entered WFI state. >>>> 3. Convert return type of is_core_in_wfi() to bool. Works better with >>>> readx_poll_timeout() condition. >>>> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings >>>> when void* is 64 bit. >>>> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc >>>> function. >>>> 6. Updated commit message to fix minor typos and such. >>>> >>>> Link to v1: >>>> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ >>>> >>>> Testing done: >>>> 1. Tested Boot across all TI K3 EVM/SK boards. >>>> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). >>>> 4. Tested R5 rprocs can now be shutdown and powered back on >>>> from userspace. >>>> 3. Tested that each patch in the series generates no new >>>> warnings/errors. >>>> >>>> drivers/remoteproc/omap_remoteproc.h | 9 ++- >>>> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ >>>> drivers/remoteproc/ti_k3_common.h | 4 ++ >>>> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + >>>> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + >>>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ >>>> 6 files changed, 93 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h >>>> index 828e13256c023..c008f11fa2a43 100644 >>>> --- a/drivers/remoteproc/omap_remoteproc.h >>>> +++ b/drivers/remoteproc/omap_remoteproc.h >>>> @@ -42,6 +42,11 @@ >>>> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor >>>> * on a suspend request >>>> * >>>> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor >>>> + * >>>> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a >>>> + * shutdown request. The remote processor should be in WFI state short after. >>>> + * >>>> * Introduce new message definitions if any here. >>>> * >>>> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core >>>> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { >>>> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, >>>> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, >>>> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, >>>> - RP_MBOX_END_MSG = 0xFFFFFF14, >>>> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, >>>> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, >>>> + RP_MBOX_END_MSG = 0xFFFFFF16, >>>> }; >>>> >>>> #endif /* _OMAP_RPMSG_H */ >>>> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c >>>> index 56b71652e449f..5d469f65115c3 100644 >>>> --- a/drivers/remoteproc/ti_k3_common.c >>>> +++ b/drivers/remoteproc/ti_k3_common.c >>>> @@ -18,7 +18,9 @@ >>>> * Hari Nagalla <hnagalla@ti.com> >>>> */ >>>> >>>> +#include <linux/delay.h> >>>> #include <linux/io.h> >>>> +#include <linux/iopoll.h> >>>> #include <linux/mailbox_client.h> >>>> #include <linux/module.h> >>>> #include <linux/of_address.h> >>>> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) >>>> case RP_MBOX_ECHO_REPLY: >>>> dev_info(dev, "received echo reply from %s\n", rproc->name); >>>> break; >>>> + case RP_MBOX_SHUTDOWN_ACK: >>>> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); >>>> + complete(&kproc->shutdown_complete); >>>> + break; >>>> default: >>>> /* silently handle all other valid messages */ >>>> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) >>>> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) >>>> } >>>> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); >>>> >>>> +/** >>>> + * is_core_in_wfi - Utility function to check core status >>>> + * @kproc: remote core pointer used for checking core status >>>> + * >>>> + * This utility function is invoked by the shutdown sequence to ensure >>>> + * the remote core is in wfi, before asserting a reset. >>>> + */ >>>> +bool is_core_in_wfi(struct k3_rproc *kproc) >>>> +{ >>>> + int ret; >>>> + u64 boot_vec; >>>> + u32 cfg, ctrl, stat; >>>> + >>>> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); >>>> + if (ret) >>>> + return false; >>>> + >>>> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); >>>> +} >>>> +EXPORT_SYMBOL_GPL(is_core_in_wfi); >>>> + >>>> +/** >>>> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown >>>> + * @kproc: remote core pointer used for sending mbox msg >>>> + * >>>> + * This function sends the shutdown prepare message to remote processor and >>>> + * waits for an ACK. Further, it checks if the remote processor has entered >>>> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc >>>> + * has relinquished its resources before asserting a reset, so the shutdown >>>> + * happens cleanly. >>>> + */ >>>> +int notify_shutdown_rproc(struct k3_rproc *kproc) >>>> +{ >>>> + bool wfi_status = false; >>>> + int ret; >>>> + >>>> + reinit_completion(&kproc->shutdown_complete); >>>> + >>>> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); >>>> + if (ret < 0) { >>>> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); >>>> + return ret; >>>> + } >>>> + >>>> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, >>>> + msecs_to_jiffies(5000)); >>>> + if (ret == 0) { >>>> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", >>>> + __func__); >>>> + return -EBUSY; >>>> + } >>>> + >>>> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, >>>> + 200, 2000); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + return 0; >>>> +} >>>> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); >>>> + >>>> /* >>>> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a >>>> * generic module reset that powers on the device and allows the internal >>>> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); >>>> int k3_rproc_stop(struct rproc *rproc) >>>> { >>>> struct k3_rproc *kproc = rproc->priv; >>>> + int ret; >>>> + >>>> + ret = notify_shutdown_rproc(kproc); >>>> + if (ret) >>>> + return ret; >>>> >>>> return k3_rproc_reset(kproc); >>>> } >>>> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h >>>> index aee3c28dbe510..2a025f4894b82 100644 >>>> --- a/drivers/remoteproc/ti_k3_common.h >>>> +++ b/drivers/remoteproc/ti_k3_common.h >>>> @@ -22,6 +22,7 @@ >>>> #define REMOTEPROC_TI_K3_COMMON_H >>>> >>>> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) >>>> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 >>>> >>>> /** >>>> * struct k3_rproc_mem - internal memory structure >>>> @@ -92,6 +93,7 @@ struct k3_rproc { >>>> u32 ti_sci_id; >>>> struct mbox_chan *mbox; >>>> struct mbox_client client; >>>> + struct completion shutdown_complete; >>>> void *priv; >>>> }; >>>> >>>> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, >>>> void k3_mem_release(void *data); >>>> int k3_reserved_mem_init(struct k3_rproc *kproc); >>>> void k3_release_tsp(void *data); >>>> +bool is_core_in_wfi(struct k3_rproc *kproc); >>>> +int notify_shutdown_rproc(struct k3_rproc *kproc); >>>> #endif /* REMOTEPROC_TI_K3_COMMON_H */ >>>> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c >>>> index d6ceea6dc920e..156ae09d8ee25 100644 >>>> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c >>>> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c >>>> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) >>>> if (ret) >>>> return ret; >>>> >>>> + init_completion(&kproc->shutdown_complete); >>>> + >>>> ret = k3_rproc_of_get_memories(pdev, kproc); >>>> if (ret) >>>> return ret; >>>> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c >>>> index 3a11fd24eb52b..64d99071279b0 100644 >>>> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c >>>> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c >>>> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) >>>> if (ret) >>>> return ret; >>>> >>>> + init_completion(&kproc->shutdown_complete); >>>> + >>>> ret = k3_rproc_of_get_memories(pdev, kproc); >>>> if (ret) >>>> return ret; >>>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c >>>> index 04f23295ffc10..8748dc6089cc2 100644 >>>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c >>>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c >>>> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) >>>> struct k3_r5_cluster *cluster = core->cluster; >>>> int ret; >>>> >>>> + ret = notify_shutdown_rproc(kproc); >>>> + if (ret) >>>> + return ret; >>>> + >>>> /* halt all applicable cores */ >>>> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { >>>> list_for_each_entry(core, &cluster->cores, elem) { >>>> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) >>>> goto out; >>>> } >>>> >>>> + init_completion(&kproc->shutdown_complete); >>>> init_rmem: >>>> k3_r5_adjust_tcm_sizes(kproc); >>>> >>>> -- >>>> 2.34.1 >>>> >>>> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2025-11-25 8:37 [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores Beleswar Padhi 2025-11-26 20:41 ` Patrick Oppenlander @ 2025-12-16 22:23 ` Mathieu Poirier 2026-01-13 16:37 ` Beleswar Prasad Padhi 1 sibling, 1 reply; 17+ messages in thread From: Mathieu Poirier @ 2025-12-16 22:23 UTC (permalink / raw) To: Beleswar Padhi Cc: andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel Hi Beleswar, On Tue, Nov 25, 2025 at 02:07:46PM +0530, Beleswar Padhi wrote: > From: Richard Genoud <richard.genoud@bootlin.com> > > Introduce software IPC handshake between the host running Linux and the > remote processors to gracefully stop/reset the remote core. > > Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox > message to the remotecore. > The remote core is expected to: > - relinquish all the resources acquired through Device Manager (DM) > - disable its interrupts > - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK > - enter WFI state. > > Meanwhile, the K3 remoteproc driver does: > - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core > - wait for the remoteproc to enter WFI state > - reset the remote core through device manager > > Based on work from: Hari Nagalla <hnagalla@ti.com> > > Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> > [b-padhi@ti.com: Extend support to all rprocs] > Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > --- > v2: Changelog: > 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) > 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has > entered WFI state. > 3. Convert return type of is_core_in_wfi() to bool. Works better with > readx_poll_timeout() condition. > 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings > when void* is 64 bit. > 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc > function. > 6. Updated commit message to fix minor typos and such. > > Link to v1: > https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ > > Testing done: > 1. Tested Boot across all TI K3 EVM/SK boards. > 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). > 4. Tested R5 rprocs can now be shutdown and powered back on > from userspace. > 3. Tested that each patch in the series generates no new > warnings/errors. > > drivers/remoteproc/omap_remoteproc.h | 9 ++- > drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ > drivers/remoteproc/ti_k3_common.h | 4 ++ > drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + > drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + > drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ > 6 files changed, 93 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h > index 828e13256c023..c008f11fa2a43 100644 > --- a/drivers/remoteproc/omap_remoteproc.h > +++ b/drivers/remoteproc/omap_remoteproc.h > @@ -42,6 +42,11 @@ > * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor > * on a suspend request > * > + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor > + * > + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a > + * shutdown request. The remote processor should be in WFI state short after. > + * > * Introduce new message definitions if any here. > * > * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core > @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { > RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, > RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, > RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, > - RP_MBOX_END_MSG = 0xFFFFFF14, > + RP_MBOX_SHUTDOWN = 0xFFFFFF14, > + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, > + RP_MBOX_END_MSG = 0xFFFFFF16, > }; > > #endif /* _OMAP_RPMSG_H */ > diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > index 56b71652e449f..5d469f65115c3 100644 > --- a/drivers/remoteproc/ti_k3_common.c > +++ b/drivers/remoteproc/ti_k3_common.c > @@ -18,7 +18,9 @@ > * Hari Nagalla <hnagalla@ti.com> > */ > > +#include <linux/delay.h> > #include <linux/io.h> > +#include <linux/iopoll.h> > #include <linux/mailbox_client.h> > #include <linux/module.h> > #include <linux/of_address.h> > @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) > case RP_MBOX_ECHO_REPLY: > dev_info(dev, "received echo reply from %s\n", rproc->name); > break; > + case RP_MBOX_SHUTDOWN_ACK: > + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); > + complete(&kproc->shutdown_complete); > + break; > default: > /* silently handle all other valid messages */ > if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) > @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) > } > EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); > > +/** > + * is_core_in_wfi - Utility function to check core status > + * @kproc: remote core pointer used for checking core status > + * > + * This utility function is invoked by the shutdown sequence to ensure > + * the remote core is in wfi, before asserting a reset. > + */ > +bool is_core_in_wfi(struct k3_rproc *kproc) > +{ > + int ret; > + u64 boot_vec; > + u32 cfg, ctrl, stat; > + > + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); > + if (ret) > + return false; > + > + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); > +} > +EXPORT_SYMBOL_GPL(is_core_in_wfi); > + > +/** > + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown > + * @kproc: remote core pointer used for sending mbox msg > + * > + * This function sends the shutdown prepare message to remote processor and > + * waits for an ACK. Further, it checks if the remote processor has entered > + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc > + * has relinquished its resources before asserting a reset, so the shutdown > + * happens cleanly. > + */ > +int notify_shutdown_rproc(struct k3_rproc *kproc) > +{ > + bool wfi_status = false; > + int ret; > + > + reinit_completion(&kproc->shutdown_complete); > + > + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); > + if (ret < 0) { > + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); > + return ret; > + } > + > + ret = wait_for_completion_timeout(&kproc->shutdown_complete, > + msecs_to_jiffies(5000)); > + if (ret == 0) { > + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", > + __func__); > + return -EBUSY; > + } > + Won't that create an issue on systems with an older FW that doesn't send a RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature needs to be backward compatible. Thanks, Mathieu > + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, > + 200, 2000); > + if (ret) > + return ret; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); > + > /* > * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a > * generic module reset that powers on the device and allows the internal > @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); > int k3_rproc_stop(struct rproc *rproc) > { > struct k3_rproc *kproc = rproc->priv; > + int ret; > + > + ret = notify_shutdown_rproc(kproc); > + if (ret) > + return ret; > > return k3_rproc_reset(kproc); > } > diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h > index aee3c28dbe510..2a025f4894b82 100644 > --- a/drivers/remoteproc/ti_k3_common.h > +++ b/drivers/remoteproc/ti_k3_common.h > @@ -22,6 +22,7 @@ > #define REMOTEPROC_TI_K3_COMMON_H > > #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) > +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 > > /** > * struct k3_rproc_mem - internal memory structure > @@ -92,6 +93,7 @@ struct k3_rproc { > u32 ti_sci_id; > struct mbox_chan *mbox; > struct mbox_client client; > + struct completion shutdown_complete; > void *priv; > }; > > @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, > void k3_mem_release(void *data); > int k3_reserved_mem_init(struct k3_rproc *kproc); > void k3_release_tsp(void *data); > +bool is_core_in_wfi(struct k3_rproc *kproc); > +int notify_shutdown_rproc(struct k3_rproc *kproc); > #endif /* REMOTEPROC_TI_K3_COMMON_H */ > diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > index d6ceea6dc920e..156ae09d8ee25 100644 > --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) > if (ret) > return ret; > > + init_completion(&kproc->shutdown_complete); > + > ret = k3_rproc_of_get_memories(pdev, kproc); > if (ret) > return ret; > diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c > index 3a11fd24eb52b..64d99071279b0 100644 > --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c > @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) > if (ret) > return ret; > > + init_completion(&kproc->shutdown_complete); > + > ret = k3_rproc_of_get_memories(pdev, kproc); > if (ret) > return ret; > diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > index 04f23295ffc10..8748dc6089cc2 100644 > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > struct k3_r5_cluster *cluster = core->cluster; > int ret; > > + ret = notify_shutdown_rproc(kproc); > + if (ret) > + return ret; > + > /* halt all applicable cores */ > if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { > list_for_each_entry(core, &cluster->cores, elem) { > @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) > goto out; > } > > + init_completion(&kproc->shutdown_complete); > init_rmem: > k3_r5_adjust_tcm_sizes(kproc); > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2025-12-16 22:23 ` Mathieu Poirier @ 2026-01-13 16:37 ` Beleswar Prasad Padhi 2026-01-14 16:36 ` Mathieu Poirier 0 siblings, 1 reply; 17+ messages in thread From: Beleswar Prasad Padhi @ 2026-01-13 16:37 UTC (permalink / raw) To: Mathieu Poirier Cc: andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel Hi Mathieu, Sorry for the delay in response here. Somehow all the messages in this thread ended up in spam. Didn't realize there were new msgs until I looked up on lore. On 17/12/25 03:53, Mathieu Poirier wrote: > Hi Beleswar, > > On Tue, Nov 25, 2025 at 02:07:46PM +0530, Beleswar Padhi wrote: >> From: Richard Genoud <richard.genoud@bootlin.com> >> >> Introduce software IPC handshake between the host running Linux and the >> remote processors to gracefully stop/reset the remote core. >> >> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox >> message to the remotecore. >> The remote core is expected to: >> - relinquish all the resources acquired through Device Manager (DM) >> - disable its interrupts >> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK >> - enter WFI state. >> >> Meanwhile, the K3 remoteproc driver does: >> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core >> - wait for the remoteproc to enter WFI state >> - reset the remote core through device manager >> >> Based on work from: Hari Nagalla <hnagalla@ti.com> >> >> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> >> [b-padhi@ti.com: Extend support to all rprocs] >> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> >> --- >> v2: Changelog: >> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) >> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has >> entered WFI state. >> 3. Convert return type of is_core_in_wfi() to bool. Works better with >> readx_poll_timeout() condition. >> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings >> when void* is 64 bit. >> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc >> function. >> 6. Updated commit message to fix minor typos and such. >> >> Link to v1: >> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ >> >> Testing done: >> 1. Tested Boot across all TI K3 EVM/SK boards. >> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). >> 4. Tested R5 rprocs can now be shutdown and powered back on >> from userspace. >> 3. Tested that each patch in the series generates no new >> warnings/errors. >> >> drivers/remoteproc/omap_remoteproc.h | 9 ++- >> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ >> drivers/remoteproc/ti_k3_common.h | 4 ++ >> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + >> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + >> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ >> 6 files changed, 93 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h >> index 828e13256c023..c008f11fa2a43 100644 >> --- a/drivers/remoteproc/omap_remoteproc.h >> +++ b/drivers/remoteproc/omap_remoteproc.h >> @@ -42,6 +42,11 @@ >> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor >> * on a suspend request >> * >> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor >> + * >> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a >> + * shutdown request. The remote processor should be in WFI state short after. >> + * >> * Introduce new message definitions if any here. >> * >> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core >> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { >> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, >> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, >> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, >> - RP_MBOX_END_MSG = 0xFFFFFF14, >> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, >> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, >> + RP_MBOX_END_MSG = 0xFFFFFF16, >> }; >> >> #endif /* _OMAP_RPMSG_H */ >> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c >> index 56b71652e449f..5d469f65115c3 100644 >> --- a/drivers/remoteproc/ti_k3_common.c >> +++ b/drivers/remoteproc/ti_k3_common.c >> @@ -18,7 +18,9 @@ >> * Hari Nagalla <hnagalla@ti.com> >> */ >> >> +#include <linux/delay.h> >> #include <linux/io.h> >> +#include <linux/iopoll.h> >> #include <linux/mailbox_client.h> >> #include <linux/module.h> >> #include <linux/of_address.h> >> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) >> case RP_MBOX_ECHO_REPLY: >> dev_info(dev, "received echo reply from %s\n", rproc->name); >> break; >> + case RP_MBOX_SHUTDOWN_ACK: >> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); >> + complete(&kproc->shutdown_complete); >> + break; >> default: >> /* silently handle all other valid messages */ >> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) >> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) >> } >> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); >> >> +/** >> + * is_core_in_wfi - Utility function to check core status >> + * @kproc: remote core pointer used for checking core status >> + * >> + * This utility function is invoked by the shutdown sequence to ensure >> + * the remote core is in wfi, before asserting a reset. >> + */ >> +bool is_core_in_wfi(struct k3_rproc *kproc) >> +{ >> + int ret; >> + u64 boot_vec; >> + u32 cfg, ctrl, stat; >> + >> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); >> + if (ret) >> + return false; >> + >> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); >> +} >> +EXPORT_SYMBOL_GPL(is_core_in_wfi); >> + >> +/** >> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown >> + * @kproc: remote core pointer used for sending mbox msg >> + * >> + * This function sends the shutdown prepare message to remote processor and >> + * waits for an ACK. Further, it checks if the remote processor has entered >> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc >> + * has relinquished its resources before asserting a reset, so the shutdown >> + * happens cleanly. >> + */ >> +int notify_shutdown_rproc(struct k3_rproc *kproc) >> +{ >> + bool wfi_status = false; >> + int ret; >> + >> + reinit_completion(&kproc->shutdown_complete); >> + >> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); >> + if (ret < 0) { >> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); >> + return ret; >> + } >> + >> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, >> + msecs_to_jiffies(5000)); >> + if (ret == 0) { >> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", >> + __func__); >> + return -EBUSY; >> + } >> + > > Won't that create an issue on systems with an older FW that doesn't send a > RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature > needs to be backward compatible. I feel it would be unsafe to just abruptly power off a core without some handshake.. The core could be executing something, there could be pending bus transactions leading to system hangs etc.. We start the IPC mechanism with a handshake, so we should end it with a handshake too.. And for firmwares that don't support this handshake, IMO its better to reject the shutdown request. What do you think? For older TI firmwares also, doing rproc_stop() resulted in those intermittent bugs as mentioned above. So we never really supported the stop() feature until now. Thanks, Beleswar > > Thanks, > Mathieu > >> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, >> + 200, 2000); >> + if (ret) >> + return ret; >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); >> + >> /* >> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a >> * generic module reset that powers on the device and allows the internal >> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); >> int k3_rproc_stop(struct rproc *rproc) >> { >> struct k3_rproc *kproc = rproc->priv; >> + int ret; >> + >> + ret = notify_shutdown_rproc(kproc); >> + if (ret) >> + return ret; >> >> return k3_rproc_reset(kproc); >> } >> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h >> index aee3c28dbe510..2a025f4894b82 100644 >> --- a/drivers/remoteproc/ti_k3_common.h >> +++ b/drivers/remoteproc/ti_k3_common.h >> @@ -22,6 +22,7 @@ >> #define REMOTEPROC_TI_K3_COMMON_H >> >> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) >> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 >> >> /** >> * struct k3_rproc_mem - internal memory structure >> @@ -92,6 +93,7 @@ struct k3_rproc { >> u32 ti_sci_id; >> struct mbox_chan *mbox; >> struct mbox_client client; >> + struct completion shutdown_complete; >> void *priv; >> }; >> >> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, >> void k3_mem_release(void *data); >> int k3_reserved_mem_init(struct k3_rproc *kproc); >> void k3_release_tsp(void *data); >> +bool is_core_in_wfi(struct k3_rproc *kproc); >> +int notify_shutdown_rproc(struct k3_rproc *kproc); >> #endif /* REMOTEPROC_TI_K3_COMMON_H */ >> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c >> index d6ceea6dc920e..156ae09d8ee25 100644 >> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c >> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c >> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) >> if (ret) >> return ret; >> >> + init_completion(&kproc->shutdown_complete); >> + >> ret = k3_rproc_of_get_memories(pdev, kproc); >> if (ret) >> return ret; >> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c >> index 3a11fd24eb52b..64d99071279b0 100644 >> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c >> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c >> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) >> if (ret) >> return ret; >> >> + init_completion(&kproc->shutdown_complete); >> + >> ret = k3_rproc_of_get_memories(pdev, kproc); >> if (ret) >> return ret; >> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c >> index 04f23295ffc10..8748dc6089cc2 100644 >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c >> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) >> struct k3_r5_cluster *cluster = core->cluster; >> int ret; >> >> + ret = notify_shutdown_rproc(kproc); >> + if (ret) >> + return ret; >> + >> /* halt all applicable cores */ >> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { >> list_for_each_entry(core, &cluster->cores, elem) { >> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) >> goto out; >> } >> >> + init_completion(&kproc->shutdown_complete); >> init_rmem: >> k3_r5_adjust_tcm_sizes(kproc); >> >> -- >> 2.34.1 >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2026-01-13 16:37 ` Beleswar Prasad Padhi @ 2026-01-14 16:36 ` Mathieu Poirier 2026-01-14 22:27 ` Patrick Oppenlander 2026-01-16 5:41 ` Beleswar Prasad Padhi 0 siblings, 2 replies; 17+ messages in thread From: Mathieu Poirier @ 2026-01-14 16:36 UTC (permalink / raw) To: Beleswar Prasad Padhi Cc: andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On Tue, 13 Jan 2026 at 09:37, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: > > Hi Mathieu, > > Sorry for the delay in response here. Somehow all the messages > in this thread ended up in spam. Didn't realize there were new > msgs until I looked up on lore. > I've been getting weird automated email replies from TI. > On 17/12/25 03:53, Mathieu Poirier wrote: > > Hi Beleswar, > > > > On Tue, Nov 25, 2025 at 02:07:46PM +0530, Beleswar Padhi wrote: > >> From: Richard Genoud <richard.genoud@bootlin.com> > >> > >> Introduce software IPC handshake between the host running Linux and the > >> remote processors to gracefully stop/reset the remote core. > >> > >> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox > >> message to the remotecore. > >> The remote core is expected to: > >> - relinquish all the resources acquired through Device Manager (DM) > >> - disable its interrupts > >> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK > >> - enter WFI state. > >> > >> Meanwhile, the K3 remoteproc driver does: > >> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core > >> - wait for the remoteproc to enter WFI state > >> - reset the remote core through device manager > >> > >> Based on work from: Hari Nagalla <hnagalla@ti.com> > >> > >> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> > >> [b-padhi@ti.com: Extend support to all rprocs] > >> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > >> --- > >> v2: Changelog: > >> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) > >> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has > >> entered WFI state. > >> 3. Convert return type of is_core_in_wfi() to bool. Works better with > >> readx_poll_timeout() condition. > >> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings > >> when void* is 64 bit. > >> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc > >> function. > >> 6. Updated commit message to fix minor typos and such. > >> > >> Link to v1: > >> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ > >> > >> Testing done: > >> 1. Tested Boot across all TI K3 EVM/SK boards. > >> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). > >> 4. Tested R5 rprocs can now be shutdown and powered back on > >> from userspace. > >> 3. Tested that each patch in the series generates no new > >> warnings/errors. > >> > >> drivers/remoteproc/omap_remoteproc.h | 9 ++- > >> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ > >> drivers/remoteproc/ti_k3_common.h | 4 ++ > >> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + > >> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + > >> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ > >> 6 files changed, 93 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h > >> index 828e13256c023..c008f11fa2a43 100644 > >> --- a/drivers/remoteproc/omap_remoteproc.h > >> +++ b/drivers/remoteproc/omap_remoteproc.h > >> @@ -42,6 +42,11 @@ > >> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor > >> * on a suspend request > >> * > >> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor > >> + * > >> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a > >> + * shutdown request. The remote processor should be in WFI state short after. > >> + * > >> * Introduce new message definitions if any here. > >> * > >> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core > >> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { > >> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, > >> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, > >> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, > >> - RP_MBOX_END_MSG = 0xFFFFFF14, > >> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, > >> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, > >> + RP_MBOX_END_MSG = 0xFFFFFF16, > >> }; > >> > >> #endif /* _OMAP_RPMSG_H */ > >> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > >> index 56b71652e449f..5d469f65115c3 100644 > >> --- a/drivers/remoteproc/ti_k3_common.c > >> +++ b/drivers/remoteproc/ti_k3_common.c > >> @@ -18,7 +18,9 @@ > >> * Hari Nagalla <hnagalla@ti.com> > >> */ > >> > >> +#include <linux/delay.h> > >> #include <linux/io.h> > >> +#include <linux/iopoll.h> > >> #include <linux/mailbox_client.h> > >> #include <linux/module.h> > >> #include <linux/of_address.h> > >> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) > >> case RP_MBOX_ECHO_REPLY: > >> dev_info(dev, "received echo reply from %s\n", rproc->name); > >> break; > >> + case RP_MBOX_SHUTDOWN_ACK: > >> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); > >> + complete(&kproc->shutdown_complete); > >> + break; > >> default: > >> /* silently handle all other valid messages */ > >> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) > >> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) > >> } > >> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); > >> > >> +/** > >> + * is_core_in_wfi - Utility function to check core status > >> + * @kproc: remote core pointer used for checking core status > >> + * > >> + * This utility function is invoked by the shutdown sequence to ensure > >> + * the remote core is in wfi, before asserting a reset. > >> + */ > >> +bool is_core_in_wfi(struct k3_rproc *kproc) > >> +{ > >> + int ret; > >> + u64 boot_vec; > >> + u32 cfg, ctrl, stat; > >> + > >> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); > >> + if (ret) > >> + return false; > >> + > >> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); > >> +} > >> +EXPORT_SYMBOL_GPL(is_core_in_wfi); > >> + > >> +/** > >> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown > >> + * @kproc: remote core pointer used for sending mbox msg > >> + * > >> + * This function sends the shutdown prepare message to remote processor and > >> + * waits for an ACK. Further, it checks if the remote processor has entered > >> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc > >> + * has relinquished its resources before asserting a reset, so the shutdown > >> + * happens cleanly. > >> + */ > >> +int notify_shutdown_rproc(struct k3_rproc *kproc) > >> +{ > >> + bool wfi_status = false; > >> + int ret; > >> + > >> + reinit_completion(&kproc->shutdown_complete); > >> + > >> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); > >> + if (ret < 0) { > >> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); > >> + return ret; > >> + } > >> + > >> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, > >> + msecs_to_jiffies(5000)); > >> + if (ret == 0) { > >> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", > >> + __func__); > >> + return -EBUSY; > >> + } > >> + > > > > Won't that create an issue on systems with an older FW that doesn't send a > > RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature > > needs to be backward compatible. > > > I feel it would be unsafe to just abruptly power off a core without some > handshake.. The core could be executing something, there could be > pending bus transactions leading to system hangs etc.. We start the > IPC mechanism with a handshake, so we should end it with a > handshake too.. And for firmwares that don't support this handshake, > IMO its better to reject the shutdown request. What do you think? > We can't affect the behavior of systems where old FW is coupled with a new kernel. If people want to address the bugs you referred to, they can update their FW as they see fit. As such things need to be backward compatible. NXP has recently implemented a handshake mechanism such as this, but to assert the readiness of a booting remote processor. They used the vendor specific resource table to store a flag that enables the handshake - I suggest using the same heuristic to implement this feature. > For older TI firmwares also, doing rproc_stop() resulted in those > intermittent bugs as mentioned above. So we never really supported > the stop() feature until now. > > Thanks, > Beleswar > > > > > Thanks, > > Mathieu > > > >> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, > >> + 200, 2000); > >> + if (ret) > >> + return ret; > >> + > >> + return 0; > >> +} > >> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); > >> + > >> /* > >> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a > >> * generic module reset that powers on the device and allows the internal > >> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); > >> int k3_rproc_stop(struct rproc *rproc) > >> { > >> struct k3_rproc *kproc = rproc->priv; > >> + int ret; > >> + > >> + ret = notify_shutdown_rproc(kproc); > >> + if (ret) > >> + return ret; > >> > >> return k3_rproc_reset(kproc); > >> } > >> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h > >> index aee3c28dbe510..2a025f4894b82 100644 > >> --- a/drivers/remoteproc/ti_k3_common.h > >> +++ b/drivers/remoteproc/ti_k3_common.h > >> @@ -22,6 +22,7 @@ > >> #define REMOTEPROC_TI_K3_COMMON_H > >> > >> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) > >> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 > >> > >> /** > >> * struct k3_rproc_mem - internal memory structure > >> @@ -92,6 +93,7 @@ struct k3_rproc { > >> u32 ti_sci_id; > >> struct mbox_chan *mbox; > >> struct mbox_client client; > >> + struct completion shutdown_complete; > >> void *priv; > >> }; > >> > >> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, > >> void k3_mem_release(void *data); > >> int k3_reserved_mem_init(struct k3_rproc *kproc); > >> void k3_release_tsp(void *data); > >> +bool is_core_in_wfi(struct k3_rproc *kproc); > >> +int notify_shutdown_rproc(struct k3_rproc *kproc); > >> #endif /* REMOTEPROC_TI_K3_COMMON_H */ > >> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >> index d6ceea6dc920e..156ae09d8ee25 100644 > >> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) > >> if (ret) > >> return ret; > >> > >> + init_completion(&kproc->shutdown_complete); > >> + > >> ret = k3_rproc_of_get_memories(pdev, kproc); > >> if (ret) > >> return ret; > >> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c > >> index 3a11fd24eb52b..64d99071279b0 100644 > >> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c > >> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c > >> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) > >> if (ret) > >> return ret; > >> > >> + init_completion(&kproc->shutdown_complete); > >> + > >> ret = k3_rproc_of_get_memories(pdev, kproc); > >> if (ret) > >> return ret; > >> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > >> index 04f23295ffc10..8748dc6089cc2 100644 > >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > >> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > >> struct k3_r5_cluster *cluster = core->cluster; > >> int ret; > >> > >> + ret = notify_shutdown_rproc(kproc); > >> + if (ret) > >> + return ret; > >> + > >> /* halt all applicable cores */ > >> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { > >> list_for_each_entry(core, &cluster->cores, elem) { > >> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) > >> goto out; > >> } > >> > >> + init_completion(&kproc->shutdown_complete); > >> init_rmem: > >> k3_r5_adjust_tcm_sizes(kproc); > >> > >> -- > >> 2.34.1 > >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2026-01-14 16:36 ` Mathieu Poirier @ 2026-01-14 22:27 ` Patrick Oppenlander 2026-01-16 5:58 ` Beleswar Prasad Padhi 2026-01-16 18:08 ` Mathieu Poirier 2026-01-16 5:41 ` Beleswar Prasad Padhi 1 sibling, 2 replies; 17+ messages in thread From: Patrick Oppenlander @ 2026-01-14 22:27 UTC (permalink / raw) To: Mathieu Poirier Cc: Beleswar Prasad Padhi, andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On Thu, 15 Jan 2026 at 03:36, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > On Tue, 13 Jan 2026 at 09:37, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: > > > > Hi Mathieu, > > > > Sorry for the delay in response here. Somehow all the messages > > in this thread ended up in spam. Didn't realize there were new > > msgs until I looked up on lore. > > > > I've been getting weird automated email replies from TI. > > > On 17/12/25 03:53, Mathieu Poirier wrote: > > > Hi Beleswar, > > > > > > On Tue, Nov 25, 2025 at 02:07:46PM +0530, Beleswar Padhi wrote: > > >> From: Richard Genoud <richard.genoud@bootlin.com> > > >> > > >> Introduce software IPC handshake between the host running Linux and the > > >> remote processors to gracefully stop/reset the remote core. > > >> > > >> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox > > >> message to the remotecore. > > >> The remote core is expected to: > > >> - relinquish all the resources acquired through Device Manager (DM) > > >> - disable its interrupts > > >> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK > > >> - enter WFI state. > > >> > > >> Meanwhile, the K3 remoteproc driver does: > > >> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core > > >> - wait for the remoteproc to enter WFI state > > >> - reset the remote core through device manager > > >> > > >> Based on work from: Hari Nagalla <hnagalla@ti.com> > > >> > > >> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> > > >> [b-padhi@ti.com: Extend support to all rprocs] > > >> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > > >> --- > > >> v2: Changelog: > > >> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) > > >> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has > > >> entered WFI state. > > >> 3. Convert return type of is_core_in_wfi() to bool. Works better with > > >> readx_poll_timeout() condition. > > >> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings > > >> when void* is 64 bit. > > >> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc > > >> function. > > >> 6. Updated commit message to fix minor typos and such. > > >> > > >> Link to v1: > > >> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ > > >> > > >> Testing done: > > >> 1. Tested Boot across all TI K3 EVM/SK boards. > > >> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). > > >> 4. Tested R5 rprocs can now be shutdown and powered back on > > >> from userspace. > > >> 3. Tested that each patch in the series generates no new > > >> warnings/errors. > > >> > > >> drivers/remoteproc/omap_remoteproc.h | 9 ++- > > >> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ > > >> drivers/remoteproc/ti_k3_common.h | 4 ++ > > >> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + > > >> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + > > >> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ > > >> 6 files changed, 93 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h > > >> index 828e13256c023..c008f11fa2a43 100644 > > >> --- a/drivers/remoteproc/omap_remoteproc.h > > >> +++ b/drivers/remoteproc/omap_remoteproc.h > > >> @@ -42,6 +42,11 @@ > > >> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor > > >> * on a suspend request > > >> * > > >> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor > > >> + * > > >> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a > > >> + * shutdown request. The remote processor should be in WFI state short after. > > >> + * > > >> * Introduce new message definitions if any here. > > >> * > > >> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core > > >> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { > > >> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, > > >> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, > > >> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, > > >> - RP_MBOX_END_MSG = 0xFFFFFF14, > > >> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, > > >> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, > > >> + RP_MBOX_END_MSG = 0xFFFFFF16, > > >> }; > > >> > > >> #endif /* _OMAP_RPMSG_H */ > > >> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > > >> index 56b71652e449f..5d469f65115c3 100644 > > >> --- a/drivers/remoteproc/ti_k3_common.c > > >> +++ b/drivers/remoteproc/ti_k3_common.c > > >> @@ -18,7 +18,9 @@ > > >> * Hari Nagalla <hnagalla@ti.com> > > >> */ > > >> > > >> +#include <linux/delay.h> > > >> #include <linux/io.h> > > >> +#include <linux/iopoll.h> > > >> #include <linux/mailbox_client.h> > > >> #include <linux/module.h> > > >> #include <linux/of_address.h> > > >> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) > > >> case RP_MBOX_ECHO_REPLY: > > >> dev_info(dev, "received echo reply from %s\n", rproc->name); > > >> break; > > >> + case RP_MBOX_SHUTDOWN_ACK: > > >> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); > > >> + complete(&kproc->shutdown_complete); > > >> + break; > > >> default: > > >> /* silently handle all other valid messages */ > > >> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) > > >> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) > > >> } > > >> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); > > >> > > >> +/** > > >> + * is_core_in_wfi - Utility function to check core status > > >> + * @kproc: remote core pointer used for checking core status > > >> + * > > >> + * This utility function is invoked by the shutdown sequence to ensure > > >> + * the remote core is in wfi, before asserting a reset. > > >> + */ > > >> +bool is_core_in_wfi(struct k3_rproc *kproc) > > >> +{ > > >> + int ret; > > >> + u64 boot_vec; > > >> + u32 cfg, ctrl, stat; > > >> + > > >> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); > > >> + if (ret) > > >> + return false; > > >> + > > >> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); > > >> +} > > >> +EXPORT_SYMBOL_GPL(is_core_in_wfi); > > >> + > > >> +/** > > >> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown > > >> + * @kproc: remote core pointer used for sending mbox msg > > >> + * > > >> + * This function sends the shutdown prepare message to remote processor and > > >> + * waits for an ACK. Further, it checks if the remote processor has entered > > >> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc > > >> + * has relinquished its resources before asserting a reset, so the shutdown > > >> + * happens cleanly. > > >> + */ > > >> +int notify_shutdown_rproc(struct k3_rproc *kproc) > > >> +{ > > >> + bool wfi_status = false; > > >> + int ret; > > >> + > > >> + reinit_completion(&kproc->shutdown_complete); > > >> + > > >> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); > > >> + if (ret < 0) { > > >> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); > > >> + return ret; > > >> + } > > >> + > > >> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, > > >> + msecs_to_jiffies(5000)); > > >> + if (ret == 0) { > > >> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", > > >> + __func__); > > >> + return -EBUSY; > > >> + } > > >> + > > > > > > Won't that create an issue on systems with an older FW that doesn't send a > > > RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature > > > needs to be backward compatible. > > > > > > I feel it would be unsafe to just abruptly power off a core without some > > handshake.. The core could be executing something, there could be > > pending bus transactions leading to system hangs etc.. We start the > > IPC mechanism with a handshake, so we should end it with a > > handshake too.. And for firmwares that don't support this handshake, > > IMO its better to reject the shutdown request. What do you think? > > > > We can't affect the behavior of systems where old FW is coupled with a > new kernel. If people want to address the bugs you referred to, they > can update their FW as they see fit. As such things need to be > backward compatible. NXP has recently implemented a handshake > mechanism such as this, but to assert the readiness of a booting > remote processor. They used the vendor specific resource table to > store a flag that enables the handshake - I suggest using the same > heuristic to implement this feature. A flag in a resource table enabling the new behaviour could work, but we would probably need another way to do the new thing, maybe with a devicetree flag. Why? Because people are running TI's kernel, which has had this feature since Feb 2025, and may want to migrate to a mainline kernel. Those firmwares already use the handshake. If we want to be nice to existing users, we should provide a way to be compatible with existing firmwares which don't support RP_MBOX_SHUTDOWN, and with existing firmwares which do. That said, restarting remote processors on k3 was quite broken without the shutdown handshake, often leading to hangs/crashes requiring a full system reboot to recover. This is why I previously asked about recovery if the remoteproc crashes or is unable to handle the shutdown request. I suspect that most real world users who are actually restarting remoteprocs on k3 are already using the handshake with TI's kernel. IMHO it's probably fine to just send RP_MBOX_SHUTDOWN to firmwares which don't support it, and handle old firmwares via the same recovery path as new firmwares which have crashed. This would mean that upgrading a system with an old firmware to a new kernel will have an additional delay when shutting down a remoteproc, but realistically, this shutdown path was broken anyway. Patrick > > For older TI firmwares also, doing rproc_stop() resulted in those > > intermittent bugs as mentioned above. So we never really supported > > the stop() feature until now. > > > > Thanks, > > Beleswar > > > > > > > > Thanks, > > > Mathieu > > > > > >> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, > > >> + 200, 2000); > > >> + if (ret) > > >> + return ret; > > >> + > > >> + return 0; > > >> +} > > >> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); > > >> + > > >> /* > > >> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a > > >> * generic module reset that powers on the device and allows the internal > > >> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); > > >> int k3_rproc_stop(struct rproc *rproc) > > >> { > > >> struct k3_rproc *kproc = rproc->priv; > > >> + int ret; > > >> + > > >> + ret = notify_shutdown_rproc(kproc); > > >> + if (ret) > > >> + return ret; > > >> > > >> return k3_rproc_reset(kproc); > > >> } > > >> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h > > >> index aee3c28dbe510..2a025f4894b82 100644 > > >> --- a/drivers/remoteproc/ti_k3_common.h > > >> +++ b/drivers/remoteproc/ti_k3_common.h > > >> @@ -22,6 +22,7 @@ > > >> #define REMOTEPROC_TI_K3_COMMON_H > > >> > > >> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) > > >> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 > > >> > > >> /** > > >> * struct k3_rproc_mem - internal memory structure > > >> @@ -92,6 +93,7 @@ struct k3_rproc { > > >> u32 ti_sci_id; > > >> struct mbox_chan *mbox; > > >> struct mbox_client client; > > >> + struct completion shutdown_complete; > > >> void *priv; > > >> }; > > >> > > >> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, > > >> void k3_mem_release(void *data); > > >> int k3_reserved_mem_init(struct k3_rproc *kproc); > > >> void k3_release_tsp(void *data); > > >> +bool is_core_in_wfi(struct k3_rproc *kproc); > > >> +int notify_shutdown_rproc(struct k3_rproc *kproc); > > >> #endif /* REMOTEPROC_TI_K3_COMMON_H */ > > >> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > > >> index d6ceea6dc920e..156ae09d8ee25 100644 > > >> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > > >> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > > >> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) > > >> if (ret) > > >> return ret; > > >> > > >> + init_completion(&kproc->shutdown_complete); > > >> + > > >> ret = k3_rproc_of_get_memories(pdev, kproc); > > >> if (ret) > > >> return ret; > > >> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c > > >> index 3a11fd24eb52b..64d99071279b0 100644 > > >> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c > > >> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c > > >> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) > > >> if (ret) > > >> return ret; > > >> > > >> + init_completion(&kproc->shutdown_complete); > > >> + > > >> ret = k3_rproc_of_get_memories(pdev, kproc); > > >> if (ret) > > >> return ret; > > >> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > > >> index 04f23295ffc10..8748dc6089cc2 100644 > > >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > > >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > > >> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > > >> struct k3_r5_cluster *cluster = core->cluster; > > >> int ret; > > >> > > >> + ret = notify_shutdown_rproc(kproc); > > >> + if (ret) > > >> + return ret; > > >> + > > >> /* halt all applicable cores */ > > >> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { > > >> list_for_each_entry(core, &cluster->cores, elem) { > > >> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) > > >> goto out; > > >> } > > >> > > >> + init_completion(&kproc->shutdown_complete); > > >> init_rmem: > > >> k3_r5_adjust_tcm_sizes(kproc); > > >> > > >> -- > > >> 2.34.1 > > >> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2026-01-14 22:27 ` Patrick Oppenlander @ 2026-01-16 5:58 ` Beleswar Prasad Padhi 2026-01-16 8:27 ` Patrick Oppenlander 2026-01-16 18:08 ` Mathieu Poirier 1 sibling, 1 reply; 17+ messages in thread From: Beleswar Prasad Padhi @ 2026-01-16 5:58 UTC (permalink / raw) To: Patrick Oppenlander, Mathieu Poirier Cc: andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On 15/01/26 03:57, Patrick Oppenlander wrote: > On Thu, 15 Jan 2026 at 03:36, Mathieu Poirier > <mathieu.poirier@linaro.org> wrote: >> On Tue, 13 Jan 2026 at 09:37, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: >>> Hi Mathieu, >>> >>> Sorry for the delay in response here. Somehow all the messages >>> in this thread ended up in spam. Didn't realize there were new >>> msgs until I looked up on lore. >>> >> I've been getting weird automated email replies from TI. >> >>> On 17/12/25 03:53, Mathieu Poirier wrote: >>>> Hi Beleswar, >>>> >>>> On Tue, Nov 25, 2025 at 02:07:46PM +0530, Beleswar Padhi wrote: >>>>> From: Richard Genoud <richard.genoud@bootlin.com> >>>>> >>>>> Introduce software IPC handshake between the host running Linux and the >>>>> remote processors to gracefully stop/reset the remote core. >>>>> >>>>> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox >>>>> message to the remotecore. >>>>> The remote core is expected to: >>>>> - relinquish all the resources acquired through Device Manager (DM) >>>>> - disable its interrupts >>>>> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK >>>>> - enter WFI state. >>>>> >>>>> Meanwhile, the K3 remoteproc driver does: >>>>> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core >>>>> - wait for the remoteproc to enter WFI state >>>>> - reset the remote core through device manager >>>>> >>>>> Based on work from: Hari Nagalla <hnagalla@ti.com> >>>>> >>>>> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> >>>>> [b-padhi@ti.com: Extend support to all rprocs] >>>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> >>>>> --- >>>>> v2: Changelog: >>>>> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) >>>>> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has >>>>> entered WFI state. >>>>> 3. Convert return type of is_core_in_wfi() to bool. Works better with >>>>> readx_poll_timeout() condition. >>>>> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings >>>>> when void* is 64 bit. >>>>> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc >>>>> function. >>>>> 6. Updated commit message to fix minor typos and such. >>>>> >>>>> Link to v1: >>>>> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ >>>>> >>>>> Testing done: >>>>> 1. Tested Boot across all TI K3 EVM/SK boards. >>>>> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). >>>>> 4. Tested R5 rprocs can now be shutdown and powered back on >>>>> from userspace. >>>>> 3. Tested that each patch in the series generates no new >>>>> warnings/errors. >>>>> >>>>> drivers/remoteproc/omap_remoteproc.h | 9 ++- >>>>> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ >>>>> drivers/remoteproc/ti_k3_common.h | 4 ++ >>>>> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + >>>>> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + >>>>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ >>>>> 6 files changed, 93 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h >>>>> index 828e13256c023..c008f11fa2a43 100644 >>>>> --- a/drivers/remoteproc/omap_remoteproc.h >>>>> +++ b/drivers/remoteproc/omap_remoteproc.h >>>>> @@ -42,6 +42,11 @@ >>>>> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor >>>>> * on a suspend request >>>>> * >>>>> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor >>>>> + * >>>>> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a >>>>> + * shutdown request. The remote processor should be in WFI state short after. >>>>> + * >>>>> * Introduce new message definitions if any here. >>>>> * >>>>> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core >>>>> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { >>>>> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, >>>>> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, >>>>> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, >>>>> - RP_MBOX_END_MSG = 0xFFFFFF14, >>>>> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, >>>>> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, >>>>> + RP_MBOX_END_MSG = 0xFFFFFF16, >>>>> }; >>>>> >>>>> #endif /* _OMAP_RPMSG_H */ >>>>> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c >>>>> index 56b71652e449f..5d469f65115c3 100644 >>>>> --- a/drivers/remoteproc/ti_k3_common.c >>>>> +++ b/drivers/remoteproc/ti_k3_common.c >>>>> @@ -18,7 +18,9 @@ >>>>> * Hari Nagalla <hnagalla@ti.com> >>>>> */ >>>>> >>>>> +#include <linux/delay.h> >>>>> #include <linux/io.h> >>>>> +#include <linux/iopoll.h> >>>>> #include <linux/mailbox_client.h> >>>>> #include <linux/module.h> >>>>> #include <linux/of_address.h> >>>>> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) >>>>> case RP_MBOX_ECHO_REPLY: >>>>> dev_info(dev, "received echo reply from %s\n", rproc->name); >>>>> break; >>>>> + case RP_MBOX_SHUTDOWN_ACK: >>>>> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); >>>>> + complete(&kproc->shutdown_complete); >>>>> + break; >>>>> default: >>>>> /* silently handle all other valid messages */ >>>>> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) >>>>> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) >>>>> } >>>>> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); >>>>> >>>>> +/** >>>>> + * is_core_in_wfi - Utility function to check core status >>>>> + * @kproc: remote core pointer used for checking core status >>>>> + * >>>>> + * This utility function is invoked by the shutdown sequence to ensure >>>>> + * the remote core is in wfi, before asserting a reset. >>>>> + */ >>>>> +bool is_core_in_wfi(struct k3_rproc *kproc) >>>>> +{ >>>>> + int ret; >>>>> + u64 boot_vec; >>>>> + u32 cfg, ctrl, stat; >>>>> + >>>>> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); >>>>> + if (ret) >>>>> + return false; >>>>> + >>>>> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); >>>>> +} >>>>> +EXPORT_SYMBOL_GPL(is_core_in_wfi); >>>>> + >>>>> +/** >>>>> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown >>>>> + * @kproc: remote core pointer used for sending mbox msg >>>>> + * >>>>> + * This function sends the shutdown prepare message to remote processor and >>>>> + * waits for an ACK. Further, it checks if the remote processor has entered >>>>> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc >>>>> + * has relinquished its resources before asserting a reset, so the shutdown >>>>> + * happens cleanly. >>>>> + */ >>>>> +int notify_shutdown_rproc(struct k3_rproc *kproc) >>>>> +{ >>>>> + bool wfi_status = false; >>>>> + int ret; >>>>> + >>>>> + reinit_completion(&kproc->shutdown_complete); >>>>> + >>>>> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); >>>>> + if (ret < 0) { >>>>> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, >>>>> + msecs_to_jiffies(5000)); >>>>> + if (ret == 0) { >>>>> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", >>>>> + __func__); >>>>> + return -EBUSY; >>>>> + } >>>>> + >>>> Won't that create an issue on systems with an older FW that doesn't send a >>>> RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature >>>> needs to be backward compatible. >>> >>> I feel it would be unsafe to just abruptly power off a core without some >>> handshake.. The core could be executing something, there could be >>> pending bus transactions leading to system hangs etc.. We start the >>> IPC mechanism with a handshake, so we should end it with a >>> handshake too.. And for firmwares that don't support this handshake, >>> IMO its better to reject the shutdown request. What do you think? >>> >> We can't affect the behavior of systems where old FW is coupled with a >> new kernel. If people want to address the bugs you referred to, they >> can update their FW as they see fit. As such things need to be >> backward compatible. NXP has recently implemented a handshake >> mechanism such as this, but to assert the readiness of a booting >> remote processor. They used the vendor specific resource table to >> store a flag that enables the handshake - I suggest using the same >> heuristic to implement this feature. > A flag in a resource table enabling the new behaviour could work, but > we would probably need another way to do the new thing, maybe with a > devicetree flag. That's hacky. Device tree is for hardware description only. We should not be putting SW required quirks in DT. It should be rightly placed in the vendor specific resource table. > Why? Because people are running TI's kernel, which > has had this feature since Feb 2025, and may want to migrate to a > mainline kernel. Those firmwares already use the handshake. Why should Upstream Linux care about the mess-ups in the vendor specific kernel? It should be the other way around, the feature should have been a part of the upstream kernel first, otherwise such a rework is expected. > > If we want to be nice to existing users, we should provide a way to be > compatible with existing firmwares which don't support > RP_MBOX_SHUTDOWN, and with existing firmwares which do. Existing users of *TI Kernel*, not *Upstream Kernel*. This is a miss from TI side not to have worked the feature upstream first, and for the same reason upstream need not care about maintaining backward compat with vendor specific features. Thanks, Beleswar > > That said, restarting remote processors on k3 was quite broken without > the shutdown handshake, often leading to hangs/crashes requiring a > full system reboot to recover. This is why I previously asked about > recovery if the remoteproc crashes or is unable to handle the shutdown > request. > > I suspect that most real world users who are actually restarting > remoteprocs on k3 are already using the handshake with TI's kernel. > > IMHO it's probably fine to just send RP_MBOX_SHUTDOWN to firmwares > which don't support it, and handle old firmwares via the same recovery > path as new firmwares which have crashed. This would mean that > upgrading a system with an old firmware to a new kernel will have an > additional delay when shutting down a remoteproc, but realistically, > this shutdown path was broken anyway. > > Patrick > >>> For older TI firmwares also, doing rproc_stop() resulted in those >>> intermittent bugs as mentioned above. So we never really supported >>> the stop() feature until now. >>> >>> Thanks, >>> Beleswar >>> >>>> Thanks, >>>> Mathieu >>>> >>>>> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, >>>>> + 200, 2000); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + return 0; >>>>> +} >>>>> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); >>>>> + >>>>> /* >>>>> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a >>>>> * generic module reset that powers on the device and allows the internal >>>>> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); >>>>> int k3_rproc_stop(struct rproc *rproc) >>>>> { >>>>> struct k3_rproc *kproc = rproc->priv; >>>>> + int ret; >>>>> + >>>>> + ret = notify_shutdown_rproc(kproc); >>>>> + if (ret) >>>>> + return ret; >>>>> >>>>> return k3_rproc_reset(kproc); >>>>> } >>>>> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h >>>>> index aee3c28dbe510..2a025f4894b82 100644 >>>>> --- a/drivers/remoteproc/ti_k3_common.h >>>>> +++ b/drivers/remoteproc/ti_k3_common.h >>>>> @@ -22,6 +22,7 @@ >>>>> #define REMOTEPROC_TI_K3_COMMON_H >>>>> >>>>> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) >>>>> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 >>>>> >>>>> /** >>>>> * struct k3_rproc_mem - internal memory structure >>>>> @@ -92,6 +93,7 @@ struct k3_rproc { >>>>> u32 ti_sci_id; >>>>> struct mbox_chan *mbox; >>>>> struct mbox_client client; >>>>> + struct completion shutdown_complete; >>>>> void *priv; >>>>> }; >>>>> >>>>> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, >>>>> void k3_mem_release(void *data); >>>>> int k3_reserved_mem_init(struct k3_rproc *kproc); >>>>> void k3_release_tsp(void *data); >>>>> +bool is_core_in_wfi(struct k3_rproc *kproc); >>>>> +int notify_shutdown_rproc(struct k3_rproc *kproc); >>>>> #endif /* REMOTEPROC_TI_K3_COMMON_H */ >>>>> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c >>>>> index d6ceea6dc920e..156ae09d8ee25 100644 >>>>> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c >>>>> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c >>>>> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) >>>>> if (ret) >>>>> return ret; >>>>> >>>>> + init_completion(&kproc->shutdown_complete); >>>>> + >>>>> ret = k3_rproc_of_get_memories(pdev, kproc); >>>>> if (ret) >>>>> return ret; >>>>> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c >>>>> index 3a11fd24eb52b..64d99071279b0 100644 >>>>> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c >>>>> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c >>>>> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) >>>>> if (ret) >>>>> return ret; >>>>> >>>>> + init_completion(&kproc->shutdown_complete); >>>>> + >>>>> ret = k3_rproc_of_get_memories(pdev, kproc); >>>>> if (ret) >>>>> return ret; >>>>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c >>>>> index 04f23295ffc10..8748dc6089cc2 100644 >>>>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c >>>>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c >>>>> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) >>>>> struct k3_r5_cluster *cluster = core->cluster; >>>>> int ret; >>>>> >>>>> + ret = notify_shutdown_rproc(kproc); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> /* halt all applicable cores */ >>>>> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { >>>>> list_for_each_entry(core, &cluster->cores, elem) { >>>>> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) >>>>> goto out; >>>>> } >>>>> >>>>> + init_completion(&kproc->shutdown_complete); >>>>> init_rmem: >>>>> k3_r5_adjust_tcm_sizes(kproc); >>>>> >>>>> -- >>>>> 2.34.1 >>>>> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2026-01-16 5:58 ` Beleswar Prasad Padhi @ 2026-01-16 8:27 ` Patrick Oppenlander 2026-01-19 4:51 ` Beleswar Prasad Padhi 0 siblings, 1 reply; 17+ messages in thread From: Patrick Oppenlander @ 2026-01-16 8:27 UTC (permalink / raw) To: Beleswar Prasad Padhi Cc: Mathieu Poirier, andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On Fri, 16 Jan 2026 at 16:58, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: > > > On 15/01/26 03:57, Patrick Oppenlander wrote: > > On Thu, 15 Jan 2026 at 03:36, Mathieu Poirier > > <mathieu.poirier@linaro.org> wrote: > >> On Tue, 13 Jan 2026 at 09:37, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: > >>> Hi Mathieu, > >>> > >>> Sorry for the delay in response here. Somehow all the messages > >>> in this thread ended up in spam. Didn't realize there were new > >>> msgs until I looked up on lore. > >>> > >> I've been getting weird automated email replies from TI. > >> > >>> On 17/12/25 03:53, Mathieu Poirier wrote: > >>>> Hi Beleswar, > >>>> > >>>> On Tue, Nov 25, 2025 at 02:07:46PM +0530, Beleswar Padhi wrote: > >>>>> From: Richard Genoud <richard.genoud@bootlin.com> > >>>>> > >>>>> Introduce software IPC handshake between the host running Linux and the > >>>>> remote processors to gracefully stop/reset the remote core. > >>>>> > >>>>> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox > >>>>> message to the remotecore. > >>>>> The remote core is expected to: > >>>>> - relinquish all the resources acquired through Device Manager (DM) > >>>>> - disable its interrupts > >>>>> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK > >>>>> - enter WFI state. > >>>>> > >>>>> Meanwhile, the K3 remoteproc driver does: > >>>>> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core > >>>>> - wait for the remoteproc to enter WFI state > >>>>> - reset the remote core through device manager > >>>>> > >>>>> Based on work from: Hari Nagalla <hnagalla@ti.com> > >>>>> > >>>>> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> > >>>>> [b-padhi@ti.com: Extend support to all rprocs] > >>>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > >>>>> --- > >>>>> v2: Changelog: > >>>>> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) > >>>>> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has > >>>>> entered WFI state. > >>>>> 3. Convert return type of is_core_in_wfi() to bool. Works better with > >>>>> readx_poll_timeout() condition. > >>>>> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings > >>>>> when void* is 64 bit. > >>>>> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc > >>>>> function. > >>>>> 6. Updated commit message to fix minor typos and such. > >>>>> > >>>>> Link to v1: > >>>>> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ > >>>>> > >>>>> Testing done: > >>>>> 1. Tested Boot across all TI K3 EVM/SK boards. > >>>>> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). > >>>>> 4. Tested R5 rprocs can now be shutdown and powered back on > >>>>> from userspace. > >>>>> 3. Tested that each patch in the series generates no new > >>>>> warnings/errors. > >>>>> > >>>>> drivers/remoteproc/omap_remoteproc.h | 9 ++- > >>>>> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ > >>>>> drivers/remoteproc/ti_k3_common.h | 4 ++ > >>>>> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + > >>>>> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + > >>>>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ > >>>>> 6 files changed, 93 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h > >>>>> index 828e13256c023..c008f11fa2a43 100644 > >>>>> --- a/drivers/remoteproc/omap_remoteproc.h > >>>>> +++ b/drivers/remoteproc/omap_remoteproc.h > >>>>> @@ -42,6 +42,11 @@ > >>>>> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor > >>>>> * on a suspend request > >>>>> * > >>>>> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor > >>>>> + * > >>>>> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a > >>>>> + * shutdown request. The remote processor should be in WFI state short after. > >>>>> + * > >>>>> * Introduce new message definitions if any here. > >>>>> * > >>>>> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core > >>>>> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { > >>>>> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, > >>>>> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, > >>>>> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, > >>>>> - RP_MBOX_END_MSG = 0xFFFFFF14, > >>>>> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, > >>>>> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, > >>>>> + RP_MBOX_END_MSG = 0xFFFFFF16, > >>>>> }; > >>>>> > >>>>> #endif /* _OMAP_RPMSG_H */ > >>>>> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > >>>>> index 56b71652e449f..5d469f65115c3 100644 > >>>>> --- a/drivers/remoteproc/ti_k3_common.c > >>>>> +++ b/drivers/remoteproc/ti_k3_common.c > >>>>> @@ -18,7 +18,9 @@ > >>>>> * Hari Nagalla <hnagalla@ti.com> > >>>>> */ > >>>>> > >>>>> +#include <linux/delay.h> > >>>>> #include <linux/io.h> > >>>>> +#include <linux/iopoll.h> > >>>>> #include <linux/mailbox_client.h> > >>>>> #include <linux/module.h> > >>>>> #include <linux/of_address.h> > >>>>> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) > >>>>> case RP_MBOX_ECHO_REPLY: > >>>>> dev_info(dev, "received echo reply from %s\n", rproc->name); > >>>>> break; > >>>>> + case RP_MBOX_SHUTDOWN_ACK: > >>>>> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); > >>>>> + complete(&kproc->shutdown_complete); > >>>>> + break; > >>>>> default: > >>>>> /* silently handle all other valid messages */ > >>>>> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) > >>>>> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) > >>>>> } > >>>>> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); > >>>>> > >>>>> +/** > >>>>> + * is_core_in_wfi - Utility function to check core status > >>>>> + * @kproc: remote core pointer used for checking core status > >>>>> + * > >>>>> + * This utility function is invoked by the shutdown sequence to ensure > >>>>> + * the remote core is in wfi, before asserting a reset. > >>>>> + */ > >>>>> +bool is_core_in_wfi(struct k3_rproc *kproc) > >>>>> +{ > >>>>> + int ret; > >>>>> + u64 boot_vec; > >>>>> + u32 cfg, ctrl, stat; > >>>>> + > >>>>> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); > >>>>> + if (ret) > >>>>> + return false; > >>>>> + > >>>>> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); > >>>>> +} > >>>>> +EXPORT_SYMBOL_GPL(is_core_in_wfi); > >>>>> + > >>>>> +/** > >>>>> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown > >>>>> + * @kproc: remote core pointer used for sending mbox msg > >>>>> + * > >>>>> + * This function sends the shutdown prepare message to remote processor and > >>>>> + * waits for an ACK. Further, it checks if the remote processor has entered > >>>>> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc > >>>>> + * has relinquished its resources before asserting a reset, so the shutdown > >>>>> + * happens cleanly. > >>>>> + */ > >>>>> +int notify_shutdown_rproc(struct k3_rproc *kproc) > >>>>> +{ > >>>>> + bool wfi_status = false; > >>>>> + int ret; > >>>>> + > >>>>> + reinit_completion(&kproc->shutdown_complete); > >>>>> + > >>>>> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); > >>>>> + if (ret < 0) { > >>>>> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); > >>>>> + return ret; > >>>>> + } > >>>>> + > >>>>> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, > >>>>> + msecs_to_jiffies(5000)); > >>>>> + if (ret == 0) { > >>>>> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", > >>>>> + __func__); > >>>>> + return -EBUSY; > >>>>> + } > >>>>> + > >>>> Won't that create an issue on systems with an older FW that doesn't send a > >>>> RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature > >>>> needs to be backward compatible. > >>> > >>> I feel it would be unsafe to just abruptly power off a core without some > >>> handshake.. The core could be executing something, there could be > >>> pending bus transactions leading to system hangs etc.. We start the > >>> IPC mechanism with a handshake, so we should end it with a > >>> handshake too.. And for firmwares that don't support this handshake, > >>> IMO its better to reject the shutdown request. What do you think? > >>> > >> We can't affect the behavior of systems where old FW is coupled with a > >> new kernel. If people want to address the bugs you referred to, they > >> can update their FW as they see fit. As such things need to be > >> backward compatible. NXP has recently implemented a handshake > >> mechanism such as this, but to assert the readiness of a booting > >> remote processor. They used the vendor specific resource table to > >> store a flag that enables the handshake - I suggest using the same > >> heuristic to implement this feature. > > A flag in a resource table enabling the new behaviour could work, but > > we would probably need another way to do the new thing, maybe with a > > devicetree flag. > > > That's hacky. Device tree is for hardware description only. We > should not be putting SW required quirks in DT. It should be > rightly placed in the vendor specific resource table. There's plenty of places in devicetree such things are already done (most stuff starting with "linux,", and plenty of others). > > Why? Because people are running TI's kernel, which > > has had this feature since Feb 2025, and may want to migrate to a > > mainline kernel. Those firmwares already use the handshake. > > > Why should Upstream Linux care about the mess-ups in the > vendor specific kernel? It should be the other way around, > the feature should have been a part of the upstream kernel > first, otherwise such a rework is expected. > > > > > If we want to be nice to existing users, we should provide a way to be > > compatible with existing firmwares which don't support > > RP_MBOX_SHUTDOWN, and with existing firmwares which do. > > > Existing users of *TI Kernel*, not *Upstream Kernel*. This is > a miss from TI side not to have worked the feature > upstream first, and for the same reason upstream need not > care about maintaining backward compat with vendor > specific features. Sure. I hope that TI does better in the future. Unfortunately that doesn't help here, and it definitely doesn't change the fact that there are real users and real products out there which are built on and have shipped relying on the behaviours in TI's kernel, because the upstream support was broken. Patrick > Thanks, > Beleswar > > > > > That said, restarting remote processors on k3 was quite broken without > > the shutdown handshake, often leading to hangs/crashes requiring a > > full system reboot to recover. This is why I previously asked about > > recovery if the remoteproc crashes or is unable to handle the shutdown > > request. > > > > I suspect that most real world users who are actually restarting > > remoteprocs on k3 are already using the handshake with TI's kernel. > > > > IMHO it's probably fine to just send RP_MBOX_SHUTDOWN to firmwares > > which don't support it, and handle old firmwares via the same recovery > > path as new firmwares which have crashed. This would mean that > > upgrading a system with an old firmware to a new kernel will have an > > additional delay when shutting down a remoteproc, but realistically, > > this shutdown path was broken anyway. > > > > Patrick > > > >>> For older TI firmwares also, doing rproc_stop() resulted in those > >>> intermittent bugs as mentioned above. So we never really supported > >>> the stop() feature until now. > >>> > >>> Thanks, > >>> Beleswar > >>> > >>>> Thanks, > >>>> Mathieu > >>>> > >>>>> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, > >>>>> + 200, 2000); > >>>>> + if (ret) > >>>>> + return ret; > >>>>> + > >>>>> + return 0; > >>>>> +} > >>>>> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); > >>>>> + > >>>>> /* > >>>>> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a > >>>>> * generic module reset that powers on the device and allows the internal > >>>>> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); > >>>>> int k3_rproc_stop(struct rproc *rproc) > >>>>> { > >>>>> struct k3_rproc *kproc = rproc->priv; > >>>>> + int ret; > >>>>> + > >>>>> + ret = notify_shutdown_rproc(kproc); > >>>>> + if (ret) > >>>>> + return ret; > >>>>> > >>>>> return k3_rproc_reset(kproc); > >>>>> } > >>>>> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h > >>>>> index aee3c28dbe510..2a025f4894b82 100644 > >>>>> --- a/drivers/remoteproc/ti_k3_common.h > >>>>> +++ b/drivers/remoteproc/ti_k3_common.h > >>>>> @@ -22,6 +22,7 @@ > >>>>> #define REMOTEPROC_TI_K3_COMMON_H > >>>>> > >>>>> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) > >>>>> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 > >>>>> > >>>>> /** > >>>>> * struct k3_rproc_mem - internal memory structure > >>>>> @@ -92,6 +93,7 @@ struct k3_rproc { > >>>>> u32 ti_sci_id; > >>>>> struct mbox_chan *mbox; > >>>>> struct mbox_client client; > >>>>> + struct completion shutdown_complete; > >>>>> void *priv; > >>>>> }; > >>>>> > >>>>> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, > >>>>> void k3_mem_release(void *data); > >>>>> int k3_reserved_mem_init(struct k3_rproc *kproc); > >>>>> void k3_release_tsp(void *data); > >>>>> +bool is_core_in_wfi(struct k3_rproc *kproc); > >>>>> +int notify_shutdown_rproc(struct k3_rproc *kproc); > >>>>> #endif /* REMOTEPROC_TI_K3_COMMON_H */ > >>>>> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >>>>> index d6ceea6dc920e..156ae09d8ee25 100644 > >>>>> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >>>>> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >>>>> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) > >>>>> if (ret) > >>>>> return ret; > >>>>> > >>>>> + init_completion(&kproc->shutdown_complete); > >>>>> + > >>>>> ret = k3_rproc_of_get_memories(pdev, kproc); > >>>>> if (ret) > >>>>> return ret; > >>>>> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c > >>>>> index 3a11fd24eb52b..64d99071279b0 100644 > >>>>> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c > >>>>> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c > >>>>> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) > >>>>> if (ret) > >>>>> return ret; > >>>>> > >>>>> + init_completion(&kproc->shutdown_complete); > >>>>> + > >>>>> ret = k3_rproc_of_get_memories(pdev, kproc); > >>>>> if (ret) > >>>>> return ret; > >>>>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > >>>>> index 04f23295ffc10..8748dc6089cc2 100644 > >>>>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > >>>>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > >>>>> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > >>>>> struct k3_r5_cluster *cluster = core->cluster; > >>>>> int ret; > >>>>> > >>>>> + ret = notify_shutdown_rproc(kproc); > >>>>> + if (ret) > >>>>> + return ret; > >>>>> + > >>>>> /* halt all applicable cores */ > >>>>> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { > >>>>> list_for_each_entry(core, &cluster->cores, elem) { > >>>>> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) > >>>>> goto out; > >>>>> } > >>>>> > >>>>> + init_completion(&kproc->shutdown_complete); > >>>>> init_rmem: > >>>>> k3_r5_adjust_tcm_sizes(kproc); > >>>>> > >>>>> -- > >>>>> 2.34.1 > >>>>> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2026-01-16 8:27 ` Patrick Oppenlander @ 2026-01-19 4:51 ` Beleswar Prasad Padhi 0 siblings, 0 replies; 17+ messages in thread From: Beleswar Prasad Padhi @ 2026-01-19 4:51 UTC (permalink / raw) To: Patrick Oppenlander Cc: Mathieu Poirier, andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On 16/01/26 13:57, Patrick Oppenlander wrote: > On Fri, 16 Jan 2026 at 16:58, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: >> >> On 15/01/26 03:57, Patrick Oppenlander wrote: [...] >>>>>>> [...] >>>>>>> + >>>>>>> +/** >>>>>>> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown >>>>>>> + * @kproc: remote core pointer used for sending mbox msg >>>>>>> + * >>>>>>> + * This function sends the shutdown prepare message to remote processor and >>>>>>> + * waits for an ACK. Further, it checks if the remote processor has entered >>>>>>> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc >>>>>>> + * has relinquished its resources before asserting a reset, so the shutdown >>>>>>> + * happens cleanly. >>>>>>> + */ >>>>>>> +int notify_shutdown_rproc(struct k3_rproc *kproc) >>>>>>> +{ >>>>>>> + bool wfi_status = false; >>>>>>> + int ret; >>>>>>> + >>>>>>> + reinit_completion(&kproc->shutdown_complete); >>>>>>> + >>>>>>> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); >>>>>>> + if (ret < 0) { >>>>>>> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); >>>>>>> + return ret; >>>>>>> + } >>>>>>> + >>>>>>> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, >>>>>>> + msecs_to_jiffies(5000)); >>>>>>> + if (ret == 0) { >>>>>>> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", >>>>>>> + __func__); >>>>>>> + return -EBUSY; >>>>>>> + } >>>>>>> + >>>>>> Won't that create an issue on systems with an older FW that doesn't send a >>>>>> RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature >>>>>> needs to be backward compatible. >>>>> I feel it would be unsafe to just abruptly power off a core without some >>>>> handshake.. The core could be executing something, there could be >>>>> pending bus transactions leading to system hangs etc.. We start the >>>>> IPC mechanism with a handshake, so we should end it with a >>>>> handshake too.. And for firmwares that don't support this handshake, >>>>> IMO its better to reject the shutdown request. What do you think? >>>>> >>>> We can't affect the behavior of systems where old FW is coupled with a >>>> new kernel. If people want to address the bugs you referred to, they >>>> can update their FW as they see fit. As such things need to be >>>> backward compatible. NXP has recently implemented a handshake >>>> mechanism such as this, but to assert the readiness of a booting >>>> remote processor. They used the vendor specific resource table to >>>> store a flag that enables the handshake - I suggest using the same >>>> heuristic to implement this feature. >>> A flag in a resource table enabling the new behaviour could work, but >>> we would probably need another way to do the new thing, maybe with a >>> devicetree flag. >> >> That's hacky. Device tree is for hardware description only. We >> should not be putting SW required quirks in DT. It should be >> rightly placed in the vendor specific resource table. > There's plenty of places in devicetree such things are already done > (most stuff starting with "linux,", and plenty of others). Yeah, but we should not keep repeating the same mistake :) DT is for non-discoverable hardware. This sort of info can always be grabbed from querying firmware at runtime or from the resource table at build time. The Devicetree is not just used by Linux, but various other projects (bootloaders like U-Boot to count) which might not make use of those custom "Linux" properties we put here. Thanks, Beleswar > [...] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2026-01-14 22:27 ` Patrick Oppenlander 2026-01-16 5:58 ` Beleswar Prasad Padhi @ 2026-01-16 18:08 ` Mathieu Poirier 2026-01-21 21:34 ` Patrick Oppenlander 1 sibling, 1 reply; 17+ messages in thread From: Mathieu Poirier @ 2026-01-16 18:08 UTC (permalink / raw) To: Patrick Oppenlander Cc: Beleswar Prasad Padhi, andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On Wed, 14 Jan 2026 at 15:28, Patrick Oppenlander <patrick.oppenlander@gmail.com> wrote: > > On Thu, 15 Jan 2026 at 03:36, Mathieu Poirier > <mathieu.poirier@linaro.org> wrote: > > > > On Tue, 13 Jan 2026 at 09:37, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: > > > > > > Hi Mathieu, > > > > > > Sorry for the delay in response here. Somehow all the messages > > > in this thread ended up in spam. Didn't realize there were new > > > msgs until I looked up on lore. > > > > > > > I've been getting weird automated email replies from TI. > > > > > On 17/12/25 03:53, Mathieu Poirier wrote: > > > > Hi Beleswar, > > > > > > > > On Tue, Nov 25, 2025 at 02:07:46PM +0530, Beleswar Padhi wrote: > > > >> From: Richard Genoud <richard.genoud@bootlin.com> > > > >> > > > >> Introduce software IPC handshake between the host running Linux and the > > > >> remote processors to gracefully stop/reset the remote core. > > > >> > > > >> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox > > > >> message to the remotecore. > > > >> The remote core is expected to: > > > >> - relinquish all the resources acquired through Device Manager (DM) > > > >> - disable its interrupts > > > >> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK > > > >> - enter WFI state. > > > >> > > > >> Meanwhile, the K3 remoteproc driver does: > > > >> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core > > > >> - wait for the remoteproc to enter WFI state > > > >> - reset the remote core through device manager > > > >> > > > >> Based on work from: Hari Nagalla <hnagalla@ti.com> > > > >> > > > >> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> > > > >> [b-padhi@ti.com: Extend support to all rprocs] > > > >> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > > > >> --- > > > >> v2: Changelog: > > > >> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) > > > >> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has > > > >> entered WFI state. > > > >> 3. Convert return type of is_core_in_wfi() to bool. Works better with > > > >> readx_poll_timeout() condition. > > > >> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings > > > >> when void* is 64 bit. > > > >> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc > > > >> function. > > > >> 6. Updated commit message to fix minor typos and such. > > > >> > > > >> Link to v1: > > > >> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ > > > >> > > > >> Testing done: > > > >> 1. Tested Boot across all TI K3 EVM/SK boards. > > > >> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). > > > >> 4. Tested R5 rprocs can now be shutdown and powered back on > > > >> from userspace. > > > >> 3. Tested that each patch in the series generates no new > > > >> warnings/errors. > > > >> > > > >> drivers/remoteproc/omap_remoteproc.h | 9 ++- > > > >> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ > > > >> drivers/remoteproc/ti_k3_common.h | 4 ++ > > > >> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + > > > >> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + > > > >> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ > > > >> 6 files changed, 93 insertions(+), 1 deletion(-) > > > >> > > > >> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h > > > >> index 828e13256c023..c008f11fa2a43 100644 > > > >> --- a/drivers/remoteproc/omap_remoteproc.h > > > >> +++ b/drivers/remoteproc/omap_remoteproc.h > > > >> @@ -42,6 +42,11 @@ > > > >> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor > > > >> * on a suspend request > > > >> * > > > >> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor > > > >> + * > > > >> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a > > > >> + * shutdown request. The remote processor should be in WFI state short after. > > > >> + * > > > >> * Introduce new message definitions if any here. > > > >> * > > > >> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core > > > >> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { > > > >> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, > > > >> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, > > > >> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, > > > >> - RP_MBOX_END_MSG = 0xFFFFFF14, > > > >> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, > > > >> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, > > > >> + RP_MBOX_END_MSG = 0xFFFFFF16, > > > >> }; > > > >> > > > >> #endif /* _OMAP_RPMSG_H */ > > > >> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > > > >> index 56b71652e449f..5d469f65115c3 100644 > > > >> --- a/drivers/remoteproc/ti_k3_common.c > > > >> +++ b/drivers/remoteproc/ti_k3_common.c > > > >> @@ -18,7 +18,9 @@ > > > >> * Hari Nagalla <hnagalla@ti.com> > > > >> */ > > > >> > > > >> +#include <linux/delay.h> > > > >> #include <linux/io.h> > > > >> +#include <linux/iopoll.h> > > > >> #include <linux/mailbox_client.h> > > > >> #include <linux/module.h> > > > >> #include <linux/of_address.h> > > > >> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) > > > >> case RP_MBOX_ECHO_REPLY: > > > >> dev_info(dev, "received echo reply from %s\n", rproc->name); > > > >> break; > > > >> + case RP_MBOX_SHUTDOWN_ACK: > > > >> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); > > > >> + complete(&kproc->shutdown_complete); > > > >> + break; > > > >> default: > > > >> /* silently handle all other valid messages */ > > > >> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) > > > >> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) > > > >> } > > > >> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); > > > >> > > > >> +/** > > > >> + * is_core_in_wfi - Utility function to check core status > > > >> + * @kproc: remote core pointer used for checking core status > > > >> + * > > > >> + * This utility function is invoked by the shutdown sequence to ensure > > > >> + * the remote core is in wfi, before asserting a reset. > > > >> + */ > > > >> +bool is_core_in_wfi(struct k3_rproc *kproc) > > > >> +{ > > > >> + int ret; > > > >> + u64 boot_vec; > > > >> + u32 cfg, ctrl, stat; > > > >> + > > > >> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); > > > >> + if (ret) > > > >> + return false; > > > >> + > > > >> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); > > > >> +} > > > >> +EXPORT_SYMBOL_GPL(is_core_in_wfi); > > > >> + > > > >> +/** > > > >> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown > > > >> + * @kproc: remote core pointer used for sending mbox msg > > > >> + * > > > >> + * This function sends the shutdown prepare message to remote processor and > > > >> + * waits for an ACK. Further, it checks if the remote processor has entered > > > >> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc > > > >> + * has relinquished its resources before asserting a reset, so the shutdown > > > >> + * happens cleanly. > > > >> + */ > > > >> +int notify_shutdown_rproc(struct k3_rproc *kproc) > > > >> +{ > > > >> + bool wfi_status = false; > > > >> + int ret; > > > >> + > > > >> + reinit_completion(&kproc->shutdown_complete); > > > >> + > > > >> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); > > > >> + if (ret < 0) { > > > >> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); > > > >> + return ret; > > > >> + } > > > >> + > > > >> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, > > > >> + msecs_to_jiffies(5000)); > > > >> + if (ret == 0) { > > > >> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", > > > >> + __func__); > > > >> + return -EBUSY; > > > >> + } > > > >> + > > > > > > > > Won't that create an issue on systems with an older FW that doesn't send a > > > > RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature > > > > needs to be backward compatible. > > > > > > > > > I feel it would be unsafe to just abruptly power off a core without some > > > handshake.. The core could be executing something, there could be > > > pending bus transactions leading to system hangs etc.. We start the > > > IPC mechanism with a handshake, so we should end it with a > > > handshake too.. And for firmwares that don't support this handshake, > > > IMO its better to reject the shutdown request. What do you think? > > > > > > > We can't affect the behavior of systems where old FW is coupled with a > > new kernel. If people want to address the bugs you referred to, they > > can update their FW as they see fit. As such things need to be > > backward compatible. NXP has recently implemented a handshake > > mechanism such as this, but to assert the readiness of a booting > > remote processor. They used the vendor specific resource table to > > store a flag that enables the handshake - I suggest using the same > > heuristic to implement this feature. > > A flag in a resource table enabling the new behaviour could work, but > we would probably need another way to do the new thing, maybe with a > devicetree flag. Why? Because people are running TI's kernel, which > has had this feature since Feb 2025, and may want to migrate to a > mainline kernel. Those firmwares already use the handshake. > We can't expect upstream to be compatible with what is happening in vendor kernels, it simply doesn't scale. Moreover, a devicetree flag would mean two ways of supporting the same feature, which quickly becomes a maintenance nightmare. > If we want to be nice to existing users, we should provide a way to be > compatible with existing firmwares which don't support > RP_MBOX_SHUTDOWN, and with existing firmwares which do. > That is indeed what I suggested. > That said, restarting remote processors on k3 was quite broken without > the shutdown handshake, often leading to hangs/crashes requiring a > full system reboot to recover. This is why I previously asked about > recovery if the remoteproc crashes or is unable to handle the shutdown > request. > > I suspect that most real world users who are actually restarting > remoteprocs on k3 are already using the handshake with TI's kernel. > > IMHO it's probably fine to just send RP_MBOX_SHUTDOWN to firmwares > which don't support it, and handle old firmwares via the same recovery > path as new firmwares which have crashed. This would mean that > upgrading a system with an old firmware to a new kernel will have an > additional delay when shutting down a remoteproc, but realistically, > this shutdown path was broken anyway. > I'm sure there are systems where the extra delay will cause trouble, hence the need to add the flexibility to keep the original behavior when the remote processor isn't expecting a handshake at shutdown time. > Patrick > > > > For older TI firmwares also, doing rproc_stop() resulted in those > > > intermittent bugs as mentioned above. So we never really supported > > > the stop() feature until now. > > > > > > Thanks, > > > Beleswar > > > > > > > > > > > Thanks, > > > > Mathieu > > > > > > > >> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, > > > >> + 200, 2000); > > > >> + if (ret) > > > >> + return ret; > > > >> + > > > >> + return 0; > > > >> +} > > > >> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); > > > >> + > > > >> /* > > > >> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a > > > >> * generic module reset that powers on the device and allows the internal > > > >> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); > > > >> int k3_rproc_stop(struct rproc *rproc) > > > >> { > > > >> struct k3_rproc *kproc = rproc->priv; > > > >> + int ret; > > > >> + > > > >> + ret = notify_shutdown_rproc(kproc); > > > >> + if (ret) > > > >> + return ret; > > > >> > > > >> return k3_rproc_reset(kproc); > > > >> } > > > >> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h > > > >> index aee3c28dbe510..2a025f4894b82 100644 > > > >> --- a/drivers/remoteproc/ti_k3_common.h > > > >> +++ b/drivers/remoteproc/ti_k3_common.h > > > >> @@ -22,6 +22,7 @@ > > > >> #define REMOTEPROC_TI_K3_COMMON_H > > > >> > > > >> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) > > > >> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 > > > >> > > > >> /** > > > >> * struct k3_rproc_mem - internal memory structure > > > >> @@ -92,6 +93,7 @@ struct k3_rproc { > > > >> u32 ti_sci_id; > > > >> struct mbox_chan *mbox; > > > >> struct mbox_client client; > > > >> + struct completion shutdown_complete; > > > >> void *priv; > > > >> }; > > > >> > > > >> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, > > > >> void k3_mem_release(void *data); > > > >> int k3_reserved_mem_init(struct k3_rproc *kproc); > > > >> void k3_release_tsp(void *data); > > > >> +bool is_core_in_wfi(struct k3_rproc *kproc); > > > >> +int notify_shutdown_rproc(struct k3_rproc *kproc); > > > >> #endif /* REMOTEPROC_TI_K3_COMMON_H */ > > > >> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > > > >> index d6ceea6dc920e..156ae09d8ee25 100644 > > > >> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > > > >> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > > > >> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) > > > >> if (ret) > > > >> return ret; > > > >> > > > >> + init_completion(&kproc->shutdown_complete); > > > >> + > > > >> ret = k3_rproc_of_get_memories(pdev, kproc); > > > >> if (ret) > > > >> return ret; > > > >> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c > > > >> index 3a11fd24eb52b..64d99071279b0 100644 > > > >> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c > > > >> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c > > > >> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) > > > >> if (ret) > > > >> return ret; > > > >> > > > >> + init_completion(&kproc->shutdown_complete); > > > >> + > > > >> ret = k3_rproc_of_get_memories(pdev, kproc); > > > >> if (ret) > > > >> return ret; > > > >> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > > > >> index 04f23295ffc10..8748dc6089cc2 100644 > > > >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > > > >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > > > >> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > > > >> struct k3_r5_cluster *cluster = core->cluster; > > > >> int ret; > > > >> > > > >> + ret = notify_shutdown_rproc(kproc); > > > >> + if (ret) > > > >> + return ret; > > > >> + > > > >> /* halt all applicable cores */ > > > >> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { > > > >> list_for_each_entry(core, &cluster->cores, elem) { > > > >> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) > > > >> goto out; > > > >> } > > > >> > > > >> + init_completion(&kproc->shutdown_complete); > > > >> init_rmem: > > > >> k3_r5_adjust_tcm_sizes(kproc); > > > >> > > > >> -- > > > >> 2.34.1 > > > >> > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2026-01-16 18:08 ` Mathieu Poirier @ 2026-01-21 21:34 ` Patrick Oppenlander 0 siblings, 0 replies; 17+ messages in thread From: Patrick Oppenlander @ 2026-01-21 21:34 UTC (permalink / raw) To: Mathieu Poirier Cc: Beleswar Prasad Padhi, andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On Sat, 17 Jan 2026 at 05:09, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > On Wed, 14 Jan 2026 at 15:28, Patrick Oppenlander > <patrick.oppenlander@gmail.com> wrote: > > > > On Thu, 15 Jan 2026 at 03:36, Mathieu Poirier > > <mathieu.poirier@linaro.org> wrote: > > > > > > On Tue, 13 Jan 2026 at 09:37, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: > > > > > > > > Hi Mathieu, > > > > > > > > Sorry for the delay in response here. Somehow all the messages > > > > in this thread ended up in spam. Didn't realize there were new > > > > msgs until I looked up on lore. > > > > > > > > > > I've been getting weird automated email replies from TI. > > > > > > > On 17/12/25 03:53, Mathieu Poirier wrote: > > > > > Hi Beleswar, > > > > > > > > > > On Tue, Nov 25, 2025 at 02:07:46PM +0530, Beleswar Padhi wrote: > > > > >> From: Richard Genoud <richard.genoud@bootlin.com> > > > > >> > > > > >> Introduce software IPC handshake between the host running Linux and the > > > > >> remote processors to gracefully stop/reset the remote core. > > > > >> > > > > >> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox > > > > >> message to the remotecore. > > > > >> The remote core is expected to: > > > > >> - relinquish all the resources acquired through Device Manager (DM) > > > > >> - disable its interrupts > > > > >> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK > > > > >> - enter WFI state. > > > > >> > > > > >> Meanwhile, the K3 remoteproc driver does: > > > > >> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core > > > > >> - wait for the remoteproc to enter WFI state > > > > >> - reset the remote core through device manager > > > > >> > > > > >> Based on work from: Hari Nagalla <hnagalla@ti.com> > > > > >> > > > > >> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> > > > > >> [b-padhi@ti.com: Extend support to all rprocs] > > > > >> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > > > > >> --- > > > > >> v2: Changelog: > > > > >> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) > > > > >> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has > > > > >> entered WFI state. > > > > >> 3. Convert return type of is_core_in_wfi() to bool. Works better with > > > > >> readx_poll_timeout() condition. > > > > >> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings > > > > >> when void* is 64 bit. > > > > >> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc > > > > >> function. > > > > >> 6. Updated commit message to fix minor typos and such. > > > > >> > > > > >> Link to v1: > > > > >> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ > > > > >> > > > > >> Testing done: > > > > >> 1. Tested Boot across all TI K3 EVM/SK boards. > > > > >> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). > > > > >> 4. Tested R5 rprocs can now be shutdown and powered back on > > > > >> from userspace. > > > > >> 3. Tested that each patch in the series generates no new > > > > >> warnings/errors. > > > > >> > > > > >> drivers/remoteproc/omap_remoteproc.h | 9 ++- > > > > >> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ > > > > >> drivers/remoteproc/ti_k3_common.h | 4 ++ > > > > >> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + > > > > >> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + > > > > >> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ > > > > >> 6 files changed, 93 insertions(+), 1 deletion(-) > > > > >> > > > > >> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h > > > > >> index 828e13256c023..c008f11fa2a43 100644 > > > > >> --- a/drivers/remoteproc/omap_remoteproc.h > > > > >> +++ b/drivers/remoteproc/omap_remoteproc.h > > > > >> @@ -42,6 +42,11 @@ > > > > >> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor > > > > >> * on a suspend request > > > > >> * > > > > >> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor > > > > >> + * > > > > >> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a > > > > >> + * shutdown request. The remote processor should be in WFI state short after. > > > > >> + * > > > > >> * Introduce new message definitions if any here. > > > > >> * > > > > >> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core > > > > >> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { > > > > >> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, > > > > >> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, > > > > >> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, > > > > >> - RP_MBOX_END_MSG = 0xFFFFFF14, > > > > >> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, > > > > >> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, > > > > >> + RP_MBOX_END_MSG = 0xFFFFFF16, > > > > >> }; > > > > >> > > > > >> #endif /* _OMAP_RPMSG_H */ > > > > >> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > > > > >> index 56b71652e449f..5d469f65115c3 100644 > > > > >> --- a/drivers/remoteproc/ti_k3_common.c > > > > >> +++ b/drivers/remoteproc/ti_k3_common.c > > > > >> @@ -18,7 +18,9 @@ > > > > >> * Hari Nagalla <hnagalla@ti.com> > > > > >> */ > > > > >> > > > > >> +#include <linux/delay.h> > > > > >> #include <linux/io.h> > > > > >> +#include <linux/iopoll.h> > > > > >> #include <linux/mailbox_client.h> > > > > >> #include <linux/module.h> > > > > >> #include <linux/of_address.h> > > > > >> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) > > > > >> case RP_MBOX_ECHO_REPLY: > > > > >> dev_info(dev, "received echo reply from %s\n", rproc->name); > > > > >> break; > > > > >> + case RP_MBOX_SHUTDOWN_ACK: > > > > >> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); > > > > >> + complete(&kproc->shutdown_complete); > > > > >> + break; > > > > >> default: > > > > >> /* silently handle all other valid messages */ > > > > >> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) > > > > >> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) > > > > >> } > > > > >> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); > > > > >> > > > > >> +/** > > > > >> + * is_core_in_wfi - Utility function to check core status > > > > >> + * @kproc: remote core pointer used for checking core status > > > > >> + * > > > > >> + * This utility function is invoked by the shutdown sequence to ensure > > > > >> + * the remote core is in wfi, before asserting a reset. > > > > >> + */ > > > > >> +bool is_core_in_wfi(struct k3_rproc *kproc) > > > > >> +{ > > > > >> + int ret; > > > > >> + u64 boot_vec; > > > > >> + u32 cfg, ctrl, stat; > > > > >> + > > > > >> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); > > > > >> + if (ret) > > > > >> + return false; > > > > >> + > > > > >> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); > > > > >> +} > > > > >> +EXPORT_SYMBOL_GPL(is_core_in_wfi); > > > > >> + > > > > >> +/** > > > > >> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown > > > > >> + * @kproc: remote core pointer used for sending mbox msg > > > > >> + * > > > > >> + * This function sends the shutdown prepare message to remote processor and > > > > >> + * waits for an ACK. Further, it checks if the remote processor has entered > > > > >> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc > > > > >> + * has relinquished its resources before asserting a reset, so the shutdown > > > > >> + * happens cleanly. > > > > >> + */ > > > > >> +int notify_shutdown_rproc(struct k3_rproc *kproc) > > > > >> +{ > > > > >> + bool wfi_status = false; > > > > >> + int ret; > > > > >> + > > > > >> + reinit_completion(&kproc->shutdown_complete); > > > > >> + > > > > >> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); > > > > >> + if (ret < 0) { > > > > >> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); > > > > >> + return ret; > > > > >> + } > > > > >> + > > > > >> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, > > > > >> + msecs_to_jiffies(5000)); > > > > >> + if (ret == 0) { > > > > >> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", > > > > >> + __func__); > > > > >> + return -EBUSY; > > > > >> + } > > > > >> + > > > > > > > > > > Won't that create an issue on systems with an older FW that doesn't send a > > > > > RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature > > > > > needs to be backward compatible. > > > > > > > > > > > > I feel it would be unsafe to just abruptly power off a core without some > > > > handshake.. The core could be executing something, there could be > > > > pending bus transactions leading to system hangs etc.. We start the > > > > IPC mechanism with a handshake, so we should end it with a > > > > handshake too.. And for firmwares that don't support this handshake, > > > > IMO its better to reject the shutdown request. What do you think? > > > > > > > > > > We can't affect the behavior of systems where old FW is coupled with a > > > new kernel. If people want to address the bugs you referred to, they > > > can update their FW as they see fit. As such things need to be > > > backward compatible. NXP has recently implemented a handshake > > > mechanism such as this, but to assert the readiness of a booting > > > remote processor. They used the vendor specific resource table to > > > store a flag that enables the handshake - I suggest using the same > > > heuristic to implement this feature. > > > > A flag in a resource table enabling the new behaviour could work, but > > we would probably need another way to do the new thing, maybe with a > > devicetree flag. Why? Because people are running TI's kernel, which > > has had this feature since Feb 2025, and may want to migrate to a > > mainline kernel. Those firmwares already use the handshake. > > > > We can't expect upstream to be compatible with what is happening in > vendor kernels, it simply doesn't scale. Moreover, a devicetree flag > would mean two ways of supporting the same feature, which quickly > becomes a maintenance nightmare. > > > If we want to be nice to existing users, we should provide a way to be > > compatible with existing firmwares which don't support > > RP_MBOX_SHUTDOWN, and with existing firmwares which do. > > > > That is indeed what I suggested. Then we can't add a feature flag to the resource table as existing firmwares which expect RP_MBOX_SHUTDOWN won't have the flag set. > > That said, restarting remote processors on k3 was quite broken without > > the shutdown handshake, often leading to hangs/crashes requiring a > > full system reboot to recover. This is why I previously asked about > > recovery if the remoteproc crashes or is unable to handle the shutdown > > request. > > > > I suspect that most real world users who are actually restarting > > remoteprocs on k3 are already using the handshake with TI's kernel. > > > > IMHO it's probably fine to just send RP_MBOX_SHUTDOWN to firmwares > > which don't support it, and handle old firmwares via the same recovery > > path as new firmwares which have crashed. This would mean that > > upgrading a system with an old firmware to a new kernel will have an > > additional delay when shutting down a remoteproc, but realistically, > > this shutdown path was broken anyway. > > > > I'm sure there are systems where the extra delay will cause trouble, > hence the need to add the flexibility to keep the original behavior > when the remote processor isn't expecting a handshake at shutdown > time. Personally I think this is unlikely to cause problems as shutting down the remoteproc in this way really is very unreliable. We would be adding a small additional delay to an already buggy code path. Systems in the wild likely fall into two categories: 1. Remoteproc is started and never stopped. 2. Remoteproc already uses RP_MBOX_SHUTDOWN. Our product falls into the 2nd category. We ship firmwares which support RP_MBOX_SHUTDOWN, and use TI's kernel for now. We will switch to an LTS kernel at some point as we have an expected product life of at least 20 years. Changing our remoteproc firmwares to add an additional flag to the resource table may sound trivial, but may invalidate expensive compliance certifications as the firmware hashes will change. We can work around this by carrying an additional kernel patch, but even though it's a trivial thing to do, it still incurs a non-zero maintenance cost in the long term. Patrick > > Patrick > > > > > > For older TI firmwares also, doing rproc_stop() resulted in those > > > > intermittent bugs as mentioned above. So we never really supported > > > > the stop() feature until now. > > > > > > > > Thanks, > > > > Beleswar > > > > > > > > > > > > > > Thanks, > > > > > Mathieu > > > > > > > > > >> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, > > > > >> + 200, 2000); > > > > >> + if (ret) > > > > >> + return ret; > > > > >> + > > > > >> + return 0; > > > > >> +} > > > > >> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); > > > > >> + > > > > >> /* > > > > >> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a > > > > >> * generic module reset that powers on the device and allows the internal > > > > >> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); > > > > >> int k3_rproc_stop(struct rproc *rproc) > > > > >> { > > > > >> struct k3_rproc *kproc = rproc->priv; > > > > >> + int ret; > > > > >> + > > > > >> + ret = notify_shutdown_rproc(kproc); > > > > >> + if (ret) > > > > >> + return ret; > > > > >> > > > > >> return k3_rproc_reset(kproc); > > > > >> } > > > > >> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h > > > > >> index aee3c28dbe510..2a025f4894b82 100644 > > > > >> --- a/drivers/remoteproc/ti_k3_common.h > > > > >> +++ b/drivers/remoteproc/ti_k3_common.h > > > > >> @@ -22,6 +22,7 @@ > > > > >> #define REMOTEPROC_TI_K3_COMMON_H > > > > >> > > > > >> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) > > > > >> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 > > > > >> > > > > >> /** > > > > >> * struct k3_rproc_mem - internal memory structure > > > > >> @@ -92,6 +93,7 @@ struct k3_rproc { > > > > >> u32 ti_sci_id; > > > > >> struct mbox_chan *mbox; > > > > >> struct mbox_client client; > > > > >> + struct completion shutdown_complete; > > > > >> void *priv; > > > > >> }; > > > > >> > > > > >> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, > > > > >> void k3_mem_release(void *data); > > > > >> int k3_reserved_mem_init(struct k3_rproc *kproc); > > > > >> void k3_release_tsp(void *data); > > > > >> +bool is_core_in_wfi(struct k3_rproc *kproc); > > > > >> +int notify_shutdown_rproc(struct k3_rproc *kproc); > > > > >> #endif /* REMOTEPROC_TI_K3_COMMON_H */ > > > > >> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > > > > >> index d6ceea6dc920e..156ae09d8ee25 100644 > > > > >> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > > > > >> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > > > > >> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) > > > > >> if (ret) > > > > >> return ret; > > > > >> > > > > >> + init_completion(&kproc->shutdown_complete); > > > > >> + > > > > >> ret = k3_rproc_of_get_memories(pdev, kproc); > > > > >> if (ret) > > > > >> return ret; > > > > >> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c > > > > >> index 3a11fd24eb52b..64d99071279b0 100644 > > > > >> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c > > > > >> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c > > > > >> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) > > > > >> if (ret) > > > > >> return ret; > > > > >> > > > > >> + init_completion(&kproc->shutdown_complete); > > > > >> + > > > > >> ret = k3_rproc_of_get_memories(pdev, kproc); > > > > >> if (ret) > > > > >> return ret; > > > > >> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > > > > >> index 04f23295ffc10..8748dc6089cc2 100644 > > > > >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > > > > >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > > > > >> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > > > > >> struct k3_r5_cluster *cluster = core->cluster; > > > > >> int ret; > > > > >> > > > > >> + ret = notify_shutdown_rproc(kproc); > > > > >> + if (ret) > > > > >> + return ret; > > > > >> + > > > > >> /* halt all applicable cores */ > > > > >> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { > > > > >> list_for_each_entry(core, &cluster->cores, elem) { > > > > >> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) > > > > >> goto out; > > > > >> } > > > > >> > > > > >> + init_completion(&kproc->shutdown_complete); > > > > >> init_rmem: > > > > >> k3_r5_adjust_tcm_sizes(kproc); > > > > >> > > > > >> -- > > > > >> 2.34.1 > > > > >> > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2026-01-14 16:36 ` Mathieu Poirier 2026-01-14 22:27 ` Patrick Oppenlander @ 2026-01-16 5:41 ` Beleswar Prasad Padhi 2026-01-16 17:57 ` Mathieu Poirier 1 sibling, 1 reply; 17+ messages in thread From: Beleswar Prasad Padhi @ 2026-01-16 5:41 UTC (permalink / raw) To: Mathieu Poirier Cc: andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On 14/01/26 22:06, Mathieu Poirier wrote: > On Tue, 13 Jan 2026 at 09:37, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: >> Hi Mathieu, >> >> Sorry for the delay in response here. Somehow all the messages >> in this thread ended up in spam. Didn't realize there were new >> msgs until I looked up on lore. >> > I've been getting weird automated email replies from TI. > >> On 17/12/25 03:53, Mathieu Poirier wrote: >>> Hi Beleswar, >>> >>> On Tue, Nov 25, 2025 at 02:07:46PM +0530, Beleswar Padhi wrote: >>>> From: Richard Genoud <richard.genoud@bootlin.com> >>>> >>>> Introduce software IPC handshake between the host running Linux and the >>>> remote processors to gracefully stop/reset the remote core. >>>> >>>> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox >>>> message to the remotecore. >>>> The remote core is expected to: >>>> - relinquish all the resources acquired through Device Manager (DM) >>>> - disable its interrupts >>>> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK >>>> - enter WFI state. >>>> >>>> Meanwhile, the K3 remoteproc driver does: >>>> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core >>>> - wait for the remoteproc to enter WFI state >>>> - reset the remote core through device manager >>>> >>>> Based on work from: Hari Nagalla <hnagalla@ti.com> >>>> >>>> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> >>>> [b-padhi@ti.com: Extend support to all rprocs] >>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> >>>> --- >>>> v2: Changelog: >>>> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) >>>> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has >>>> entered WFI state. >>>> 3. Convert return type of is_core_in_wfi() to bool. Works better with >>>> readx_poll_timeout() condition. >>>> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings >>>> when void* is 64 bit. >>>> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc >>>> function. >>>> 6. Updated commit message to fix minor typos and such. >>>> >>>> Link to v1: >>>> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ >>>> >>>> Testing done: >>>> 1. Tested Boot across all TI K3 EVM/SK boards. >>>> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). >>>> 4. Tested R5 rprocs can now be shutdown and powered back on >>>> from userspace. >>>> 3. Tested that each patch in the series generates no new >>>> warnings/errors. >>>> >>>> drivers/remoteproc/omap_remoteproc.h | 9 ++- >>>> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ >>>> drivers/remoteproc/ti_k3_common.h | 4 ++ >>>> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + >>>> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + >>>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ >>>> 6 files changed, 93 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h >>>> index 828e13256c023..c008f11fa2a43 100644 >>>> --- a/drivers/remoteproc/omap_remoteproc.h >>>> +++ b/drivers/remoteproc/omap_remoteproc.h >>>> @@ -42,6 +42,11 @@ >>>> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor >>>> * on a suspend request >>>> * >>>> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor >>>> + * >>>> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a >>>> + * shutdown request. The remote processor should be in WFI state short after. >>>> + * >>>> * Introduce new message definitions if any here. >>>> * >>>> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core >>>> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { >>>> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, >>>> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, >>>> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, >>>> - RP_MBOX_END_MSG = 0xFFFFFF14, >>>> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, >>>> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, >>>> + RP_MBOX_END_MSG = 0xFFFFFF16, >>>> }; >>>> >>>> #endif /* _OMAP_RPMSG_H */ >>>> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c >>>> index 56b71652e449f..5d469f65115c3 100644 >>>> --- a/drivers/remoteproc/ti_k3_common.c >>>> +++ b/drivers/remoteproc/ti_k3_common.c >>>> @@ -18,7 +18,9 @@ >>>> * Hari Nagalla <hnagalla@ti.com> >>>> */ >>>> >>>> +#include <linux/delay.h> >>>> #include <linux/io.h> >>>> +#include <linux/iopoll.h> >>>> #include <linux/mailbox_client.h> >>>> #include <linux/module.h> >>>> #include <linux/of_address.h> >>>> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) >>>> case RP_MBOX_ECHO_REPLY: >>>> dev_info(dev, "received echo reply from %s\n", rproc->name); >>>> break; >>>> + case RP_MBOX_SHUTDOWN_ACK: >>>> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); >>>> + complete(&kproc->shutdown_complete); >>>> + break; >>>> default: >>>> /* silently handle all other valid messages */ >>>> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) >>>> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) >>>> } >>>> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); >>>> >>>> +/** >>>> + * is_core_in_wfi - Utility function to check core status >>>> + * @kproc: remote core pointer used for checking core status >>>> + * >>>> + * This utility function is invoked by the shutdown sequence to ensure >>>> + * the remote core is in wfi, before asserting a reset. >>>> + */ >>>> +bool is_core_in_wfi(struct k3_rproc *kproc) >>>> +{ >>>> + int ret; >>>> + u64 boot_vec; >>>> + u32 cfg, ctrl, stat; >>>> + >>>> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); >>>> + if (ret) >>>> + return false; >>>> + >>>> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); >>>> +} >>>> +EXPORT_SYMBOL_GPL(is_core_in_wfi); >>>> + >>>> +/** >>>> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown >>>> + * @kproc: remote core pointer used for sending mbox msg >>>> + * >>>> + * This function sends the shutdown prepare message to remote processor and >>>> + * waits for an ACK. Further, it checks if the remote processor has entered >>>> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc >>>> + * has relinquished its resources before asserting a reset, so the shutdown >>>> + * happens cleanly. >>>> + */ >>>> +int notify_shutdown_rproc(struct k3_rproc *kproc) >>>> +{ >>>> + bool wfi_status = false; >>>> + int ret; >>>> + >>>> + reinit_completion(&kproc->shutdown_complete); >>>> + >>>> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); >>>> + if (ret < 0) { >>>> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); >>>> + return ret; >>>> + } >>>> + >>>> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, >>>> + msecs_to_jiffies(5000)); >>>> + if (ret == 0) { >>>> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", >>>> + __func__); >>>> + return -EBUSY; >>>> + } >>>> + >>> Won't that create an issue on systems with an older FW that doesn't send a >>> RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature >>> needs to be backward compatible. >> >> I feel it would be unsafe to just abruptly power off a core without some >> handshake.. The core could be executing something, there could be >> pending bus transactions leading to system hangs etc.. We start the >> IPC mechanism with a handshake, so we should end it with a >> handshake too.. And for firmwares that don't support this handshake, >> IMO its better to reject the shutdown request. What do you think? >> > We can't affect the behavior of systems where old FW is coupled with a > new kernel. If people want to address the bugs you referred to, they > can update their FW as they see fit. As such things need to be > backward compatible. NXP has recently implemented a handshake > mechanism such as this, but to assert the readiness of a booting > remote processor. They used the vendor specific resource table to > store a flag that enables the handshake - I suggest using the same > heuristic to implement this feature. Fair... I have let the internal firmware teams know about this requirement. Once we have a compatible firmware, I will refresh v3 of this patch on Linux side. Thanks, Beleswar > >> For older TI firmwares also, doing rproc_stop() resulted in those >> intermittent bugs as mentioned above. So we never really supported >> the stop() feature until now. >> >> Thanks, >> Beleswar >> >>> Thanks, >>> Mathieu >>> >>>> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, >>>> + 200, 2000); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + return 0; >>>> +} >>>> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); >>>> + >>>> /* >>>> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a >>>> * generic module reset that powers on the device and allows the internal >>>> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); >>>> int k3_rproc_stop(struct rproc *rproc) >>>> { >>>> struct k3_rproc *kproc = rproc->priv; >>>> + int ret; >>>> + >>>> + ret = notify_shutdown_rproc(kproc); >>>> + if (ret) >>>> + return ret; >>>> >>>> return k3_rproc_reset(kproc); >>>> } >>>> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h >>>> index aee3c28dbe510..2a025f4894b82 100644 >>>> --- a/drivers/remoteproc/ti_k3_common.h >>>> +++ b/drivers/remoteproc/ti_k3_common.h >>>> @@ -22,6 +22,7 @@ >>>> #define REMOTEPROC_TI_K3_COMMON_H >>>> >>>> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) >>>> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 >>>> >>>> /** >>>> * struct k3_rproc_mem - internal memory structure >>>> @@ -92,6 +93,7 @@ struct k3_rproc { >>>> u32 ti_sci_id; >>>> struct mbox_chan *mbox; >>>> struct mbox_client client; >>>> + struct completion shutdown_complete; >>>> void *priv; >>>> }; >>>> >>>> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, >>>> void k3_mem_release(void *data); >>>> int k3_reserved_mem_init(struct k3_rproc *kproc); >>>> void k3_release_tsp(void *data); >>>> +bool is_core_in_wfi(struct k3_rproc *kproc); >>>> +int notify_shutdown_rproc(struct k3_rproc *kproc); >>>> #endif /* REMOTEPROC_TI_K3_COMMON_H */ >>>> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c >>>> index d6ceea6dc920e..156ae09d8ee25 100644 >>>> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c >>>> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c >>>> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) >>>> if (ret) >>>> return ret; >>>> >>>> + init_completion(&kproc->shutdown_complete); >>>> + >>>> ret = k3_rproc_of_get_memories(pdev, kproc); >>>> if (ret) >>>> return ret; >>>> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c >>>> index 3a11fd24eb52b..64d99071279b0 100644 >>>> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c >>>> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c >>>> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) >>>> if (ret) >>>> return ret; >>>> >>>> + init_completion(&kproc->shutdown_complete); >>>> + >>>> ret = k3_rproc_of_get_memories(pdev, kproc); >>>> if (ret) >>>> return ret; >>>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c >>>> index 04f23295ffc10..8748dc6089cc2 100644 >>>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c >>>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c >>>> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) >>>> struct k3_r5_cluster *cluster = core->cluster; >>>> int ret; >>>> >>>> + ret = notify_shutdown_rproc(kproc); >>>> + if (ret) >>>> + return ret; >>>> + >>>> /* halt all applicable cores */ >>>> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { >>>> list_for_each_entry(core, &cluster->cores, elem) { >>>> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) >>>> goto out; >>>> } >>>> >>>> + init_completion(&kproc->shutdown_complete); >>>> init_rmem: >>>> k3_r5_adjust_tcm_sizes(kproc); >>>> >>>> -- >>>> 2.34.1 >>>> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores 2026-01-16 5:41 ` Beleswar Prasad Padhi @ 2026-01-16 17:57 ` Mathieu Poirier 0 siblings, 0 replies; 17+ messages in thread From: Mathieu Poirier @ 2026-01-16 17:57 UTC (permalink / raw) To: Beleswar Prasad Padhi Cc: andersson, richard.genoud, afd, hnagalla, jm, u-kumar1, jan.kiszka, christophe.jaillet, linux-remoteproc, linux-kernel On Thu, 15 Jan 2026 at 22:41, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: > > > On 14/01/26 22:06, Mathieu Poirier wrote: > > On Tue, 13 Jan 2026 at 09:37, Beleswar Prasad Padhi <b-padhi@ti.com> wrote: > >> Hi Mathieu, > >> > >> Sorry for the delay in response here. Somehow all the messages > >> in this thread ended up in spam. Didn't realize there were new > >> msgs until I looked up on lore. > >> > > I've been getting weird automated email replies from TI. > > > >> On 17/12/25 03:53, Mathieu Poirier wrote: > >>> Hi Beleswar, > >>> > >>> On Tue, Nov 25, 2025 at 02:07:46PM +0530, Beleswar Padhi wrote: > >>>> From: Richard Genoud <richard.genoud@bootlin.com> > >>>> > >>>> Introduce software IPC handshake between the host running Linux and the > >>>> remote processors to gracefully stop/reset the remote core. > >>>> > >>>> Upon a stop request, remoteproc driver sends a RP_MBOX_SHUTDOWN mailbox > >>>> message to the remotecore. > >>>> The remote core is expected to: > >>>> - relinquish all the resources acquired through Device Manager (DM) > >>>> - disable its interrupts > >>>> - send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK > >>>> - enter WFI state. > >>>> > >>>> Meanwhile, the K3 remoteproc driver does: > >>>> - wait for the RP_MBOX_SHUTDOWN_ACK from the remote core > >>>> - wait for the remoteproc to enter WFI state > >>>> - reset the remote core through device manager > >>>> > >>>> Based on work from: Hari Nagalla <hnagalla@ti.com> > >>>> > >>>> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com> > >>>> [b-padhi@ti.com: Extend support to all rprocs] > >>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com> > >>>> --- > >>>> v2: Changelog: > >>>> 1. Extend graceful shutdown support for all rprocs (R5, DSP, M4) > >>>> 2. Halt core only if SHUTDOWN_ACK is received from rproc and it has > >>>> entered WFI state. > >>>> 3. Convert return type of is_core_in_wfi() to bool. Works better with > >>>> readx_poll_timeout() condition. > >>>> 4. Cast RP_MBOX_SHUTDOWN to uintptr_t to suppress compiler warnings > >>>> when void* is 64 bit. > >>>> 5. Wrapped Graceful shutdown code in the form of notify_shutdown_rproc > >>>> function. > >>>> 6. Updated commit message to fix minor typos and such. > >>>> > >>>> Link to v1: > >>>> https://lore.kernel.org/all/20240621150058.319524-5-richard.genoud@bootlin.com/ > >>>> > >>>> Testing done: > >>>> 1. Tested Boot across all TI K3 EVM/SK boards. > >>>> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK). > >>>> 4. Tested R5 rprocs can now be shutdown and powered back on > >>>> from userspace. > >>>> 3. Tested that each patch in the series generates no new > >>>> warnings/errors. > >>>> > >>>> drivers/remoteproc/omap_remoteproc.h | 9 ++- > >>>> drivers/remoteproc/ti_k3_common.c | 72 +++++++++++++++++++++++ > >>>> drivers/remoteproc/ti_k3_common.h | 4 ++ > >>>> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 + > >>>> drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 + > >>>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 5 ++ > >>>> 6 files changed, 93 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h > >>>> index 828e13256c023..c008f11fa2a43 100644 > >>>> --- a/drivers/remoteproc/omap_remoteproc.h > >>>> +++ b/drivers/remoteproc/omap_remoteproc.h > >>>> @@ -42,6 +42,11 @@ > >>>> * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor > >>>> * on a suspend request > >>>> * > >>>> + * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor > >>>> + * > >>>> + * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a > >>>> + * shutdown request. The remote processor should be in WFI state short after. > >>>> + * > >>>> * Introduce new message definitions if any here. > >>>> * > >>>> * @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core > >>>> @@ -59,7 +64,9 @@ enum omap_rp_mbox_messages { > >>>> RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11, > >>>> RP_MBOX_SUSPEND_ACK = 0xFFFFFF12, > >>>> RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13, > >>>> - RP_MBOX_END_MSG = 0xFFFFFF14, > >>>> + RP_MBOX_SHUTDOWN = 0xFFFFFF14, > >>>> + RP_MBOX_SHUTDOWN_ACK = 0xFFFFFF15, > >>>> + RP_MBOX_END_MSG = 0xFFFFFF16, > >>>> }; > >>>> > >>>> #endif /* _OMAP_RPMSG_H */ > >>>> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > >>>> index 56b71652e449f..5d469f65115c3 100644 > >>>> --- a/drivers/remoteproc/ti_k3_common.c > >>>> +++ b/drivers/remoteproc/ti_k3_common.c > >>>> @@ -18,7 +18,9 @@ > >>>> * Hari Nagalla <hnagalla@ti.com> > >>>> */ > >>>> > >>>> +#include <linux/delay.h> > >>>> #include <linux/io.h> > >>>> +#include <linux/iopoll.h> > >>>> #include <linux/mailbox_client.h> > >>>> #include <linux/module.h> > >>>> #include <linux/of_address.h> > >>>> @@ -69,6 +71,10 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data) > >>>> case RP_MBOX_ECHO_REPLY: > >>>> dev_info(dev, "received echo reply from %s\n", rproc->name); > >>>> break; > >>>> + case RP_MBOX_SHUTDOWN_ACK: > >>>> + dev_dbg(dev, "received shutdown_ack from %s\n", rproc->name); > >>>> + complete(&kproc->shutdown_complete); > >>>> + break; > >>>> default: > >>>> /* silently handle all other valid messages */ > >>>> if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG) > >>>> @@ -188,6 +194,67 @@ int k3_rproc_request_mbox(struct rproc *rproc) > >>>> } > >>>> EXPORT_SYMBOL_GPL(k3_rproc_request_mbox); > >>>> > >>>> +/** > >>>> + * is_core_in_wfi - Utility function to check core status > >>>> + * @kproc: remote core pointer used for checking core status > >>>> + * > >>>> + * This utility function is invoked by the shutdown sequence to ensure > >>>> + * the remote core is in wfi, before asserting a reset. > >>>> + */ > >>>> +bool is_core_in_wfi(struct k3_rproc *kproc) > >>>> +{ > >>>> + int ret; > >>>> + u64 boot_vec; > >>>> + u32 cfg, ctrl, stat; > >>>> + > >>>> + ret = ti_sci_proc_get_status(kproc->tsp, &boot_vec, &cfg, &ctrl, &stat); > >>>> + if (ret) > >>>> + return false; > >>>> + > >>>> + return (bool)(stat & PROC_BOOT_STATUS_FLAG_CPU_WFI); > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(is_core_in_wfi); > >>>> + > >>>> +/** > >>>> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown > >>>> + * @kproc: remote core pointer used for sending mbox msg > >>>> + * > >>>> + * This function sends the shutdown prepare message to remote processor and > >>>> + * waits for an ACK. Further, it checks if the remote processor has entered > >>>> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc > >>>> + * has relinquished its resources before asserting a reset, so the shutdown > >>>> + * happens cleanly. > >>>> + */ > >>>> +int notify_shutdown_rproc(struct k3_rproc *kproc) > >>>> +{ > >>>> + bool wfi_status = false; > >>>> + int ret; > >>>> + > >>>> + reinit_completion(&kproc->shutdown_complete); > >>>> + > >>>> + ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN); > >>>> + if (ret < 0) { > >>>> + dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret); > >>>> + return ret; > >>>> + } > >>>> + > >>>> + ret = wait_for_completion_timeout(&kproc->shutdown_complete, > >>>> + msecs_to_jiffies(5000)); > >>>> + if (ret == 0) { > >>>> + dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n", > >>>> + __func__); > >>>> + return -EBUSY; > >>>> + } > >>>> + > >>> Won't that create an issue on systems with an older FW that doesn't send a > >>> RP_MBOX_SHUDOWN_ACK message? Unless I'm missing something, this kind of feature > >>> needs to be backward compatible. > >> > >> I feel it would be unsafe to just abruptly power off a core without some > >> handshake.. The core could be executing something, there could be > >> pending bus transactions leading to system hangs etc.. We start the > >> IPC mechanism with a handshake, so we should end it with a > >> handshake too.. And for firmwares that don't support this handshake, > >> IMO its better to reject the shutdown request. What do you think? > >> > > We can't affect the behavior of systems where old FW is coupled with a > > new kernel. If people want to address the bugs you referred to, they > > can update their FW as they see fit. As such things need to be > > backward compatible. NXP has recently implemented a handshake > > mechanism such as this, but to assert the readiness of a booting > > remote processor. They used the vendor specific resource table to > > store a flag that enables the handshake - I suggest using the same > > heuristic to implement this feature. > > > Fair... I have let the internal firmware teams know about this > requirement. Once we have a compatible firmware, I will > refresh v3 of this patch on Linux side. Very well. > > Thanks, > Beleswar > > > > >> For older TI firmwares also, doing rproc_stop() resulted in those > >> intermittent bugs as mentioned above. So we never really supported > >> the stop() feature until now. > >> > >> Thanks, > >> Beleswar > >> > >>> Thanks, > >>> Mathieu > >>> > >>>> + ret = readx_poll_timeout(is_core_in_wfi, kproc, wfi_status, wfi_status, > >>>> + 200, 2000); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + return 0; > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(notify_shutdown_rproc); > >>>> + > >>>> /* > >>>> * The K3 DSP and M4 cores have a local reset that affects only the CPU, and a > >>>> * generic module reset that powers on the device and allows the internal > >>>> @@ -288,6 +355,11 @@ EXPORT_SYMBOL_GPL(k3_rproc_start); > >>>> int k3_rproc_stop(struct rproc *rproc) > >>>> { > >>>> struct k3_rproc *kproc = rproc->priv; > >>>> + int ret; > >>>> + > >>>> + ret = notify_shutdown_rproc(kproc); > >>>> + if (ret) > >>>> + return ret; > >>>> > >>>> return k3_rproc_reset(kproc); > >>>> } > >>>> diff --git a/drivers/remoteproc/ti_k3_common.h b/drivers/remoteproc/ti_k3_common.h > >>>> index aee3c28dbe510..2a025f4894b82 100644 > >>>> --- a/drivers/remoteproc/ti_k3_common.h > >>>> +++ b/drivers/remoteproc/ti_k3_common.h > >>>> @@ -22,6 +22,7 @@ > >>>> #define REMOTEPROC_TI_K3_COMMON_H > >>>> > >>>> #define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1) > >>>> +#define PROC_BOOT_STATUS_FLAG_CPU_WFI 0x00000002 > >>>> > >>>> /** > >>>> * struct k3_rproc_mem - internal memory structure > >>>> @@ -92,6 +93,7 @@ struct k3_rproc { > >>>> u32 ti_sci_id; > >>>> struct mbox_chan *mbox; > >>>> struct mbox_client client; > >>>> + struct completion shutdown_complete; > >>>> void *priv; > >>>> }; > >>>> > >>>> @@ -115,4 +117,6 @@ int k3_rproc_of_get_memories(struct platform_device *pdev, > >>>> void k3_mem_release(void *data); > >>>> int k3_reserved_mem_init(struct k3_rproc *kproc); > >>>> void k3_release_tsp(void *data); > >>>> +bool is_core_in_wfi(struct k3_rproc *kproc); > >>>> +int notify_shutdown_rproc(struct k3_rproc *kproc); > >>>> #endif /* REMOTEPROC_TI_K3_COMMON_H */ > >>>> diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >>>> index d6ceea6dc920e..156ae09d8ee25 100644 > >>>> --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >>>> +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > >>>> @@ -133,6 +133,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev) > >>>> if (ret) > >>>> return ret; > >>>> > >>>> + init_completion(&kproc->shutdown_complete); > >>>> + > >>>> ret = k3_rproc_of_get_memories(pdev, kproc); > >>>> if (ret) > >>>> return ret; > >>>> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c > >>>> index 3a11fd24eb52b..64d99071279b0 100644 > >>>> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c > >>>> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c > >>>> @@ -90,6 +90,8 @@ static int k3_m4_rproc_probe(struct platform_device *pdev) > >>>> if (ret) > >>>> return ret; > >>>> > >>>> + init_completion(&kproc->shutdown_complete); > >>>> + > >>>> ret = k3_rproc_of_get_memories(pdev, kproc); > >>>> if (ret) > >>>> return ret; > >>>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > >>>> index 04f23295ffc10..8748dc6089cc2 100644 > >>>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > >>>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > >>>> @@ -533,6 +533,10 @@ static int k3_r5_rproc_stop(struct rproc *rproc) > >>>> struct k3_r5_cluster *cluster = core->cluster; > >>>> int ret; > >>>> > >>>> + ret = notify_shutdown_rproc(kproc); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> /* halt all applicable cores */ > >>>> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) { > >>>> list_for_each_entry(core, &cluster->cores, elem) { > >>>> @@ -1129,6 +1133,7 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev) > >>>> goto out; > >>>> } > >>>> > >>>> + init_completion(&kproc->shutdown_complete); > >>>> init_rmem: > >>>> k3_r5_adjust_tcm_sizes(kproc); > >>>> > >>>> -- > >>>> 2.34.1 > >>>> ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-01-21 21:34 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-25 8:37 [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores Beleswar Padhi 2025-11-26 20:41 ` Patrick Oppenlander 2025-11-27 10:33 ` Beleswar Prasad Padhi 2025-11-27 15:17 ` Mathieu Poirier 2026-01-09 16:53 ` Mathieu Poirier 2026-01-13 16:52 ` Beleswar Prasad Padhi 2025-12-16 22:23 ` Mathieu Poirier 2026-01-13 16:37 ` Beleswar Prasad Padhi 2026-01-14 16:36 ` Mathieu Poirier 2026-01-14 22:27 ` Patrick Oppenlander 2026-01-16 5:58 ` Beleswar Prasad Padhi 2026-01-16 8:27 ` Patrick Oppenlander 2026-01-19 4:51 ` Beleswar Prasad Padhi 2026-01-16 18:08 ` Mathieu Poirier 2026-01-21 21:34 ` Patrick Oppenlander 2026-01-16 5:41 ` Beleswar Prasad Padhi 2026-01-16 17:57 ` Mathieu Poirier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox