Andrew, Hari and Beleswar - please test this on your side and get back to me with the results. Thanks, Mathieu On Fri, Jun 27, 2025 at 11:42:33PM +0200, Christophe JAILLET wrote: > IF an error occurs after a successful k3_rproc_request_mbox() call, > mbox_free_channel() should be called to avoid a leak. > > Such a call is missing in the error handing path of k3_dsp_rproc_probe(). > It is also missing both in the error handling path of k3_m4_rproc_probe() > and in (in-existent) corresponding remove function. > > Switch to managed resources to avoid these leaks and simplify the code. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> > --- > Compile tested only. > > This is an update of [1]. > The code has been heavily refactored recently with things moved to > ti_k3_common.c > > This new version also fixes a leak in k3_m4_rproc_probe(). In this file, > mbox_free_channel() was missing. > > Being very different from the v3, I've removed the previous review tags. > > [1]: https://lore.kernel.org/all/591e219df99da6f02c9d402f7854bc3ab23e76f9.1726328417.git.christophe.jaillet@xxxxxxxxxx/ > --- > drivers/remoteproc/ti_k3_common.c | 12 +++++++++++- > drivers/remoteproc/ti_k3_dsp_remoteproc.c | 2 -- > drivers/remoteproc/ti_k3_r5_remoteproc.c | 2 -- > 3 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c > index d4f20900f33b..7a9c3fb80fec 100644 > --- a/drivers/remoteproc/ti_k3_common.c > +++ b/drivers/remoteproc/ti_k3_common.c > @@ -155,6 +155,13 @@ int k3_rproc_release(struct k3_rproc *kproc) > } > EXPORT_SYMBOL_GPL(k3_rproc_release); > > +static void k3_rproc_free_channel(void *data) > +{ > + struct k3_rproc *kproc = data; > + > + mbox_free_channel(kproc->mbox); > +} > + > int k3_rproc_request_mbox(struct rproc *rproc) > { > struct k3_rproc *kproc = rproc->priv; > @@ -173,6 +180,10 @@ int k3_rproc_request_mbox(struct rproc *rproc) > return dev_err_probe(dev, PTR_ERR(kproc->mbox), > "mbox_request_channel failed\n"); > > + ret = devm_add_action_or_reset(dev, k3_rproc_free_channel, kproc); > + if (ret) > + return ret; > + > /* > * Ping the remote processor, this is only for sanity-sake for now; > * there is no functional effect whatsoever. > @@ -183,7 +194,6 @@ int k3_rproc_request_mbox(struct rproc *rproc) > ret = mbox_send_message(kproc->mbox, (void *)RP_MBOX_ECHO_REQUEST); > if (ret < 0) { > dev_err(dev, "mbox_send_message failed (%pe)\n", ERR_PTR(ret)); > - mbox_free_channel(kproc->mbox); > return ret; > } > > diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > index 7a72933bd403..d6ceea6dc920 100644 > --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c > @@ -175,8 +175,6 @@ static void k3_dsp_rproc_remove(struct platform_device *pdev) > if (ret) > dev_err(dev, "failed to detach proc (%pe)\n", ERR_PTR(ret)); > } > - > - mbox_free_channel(kproc->mbox); > } > > static const struct k3_rproc_mem_data c66_mems[] = { > diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > index ca5ff280d2dc..04f23295ffc1 100644 > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > @@ -1206,8 +1206,6 @@ static void k3_r5_cluster_rproc_exit(void *data) > return; > } > } > - > - mbox_free_channel(kproc->mbox); > } > } > > -- > 2.50.0 >
