On Wed, Dec 17, 2025 at 02:57:24PM +0200, Daniel Baluta wrote: > On Wed, Dec 17, 2025 at 12:13 AM Mathieu Poirier > <[email protected]> wrote: > > > > Good day, > > > > On Wed, Dec 10, 2025 at 05:49:06PM +0200, Daniel Baluta wrote: > > > After commit 67a7bc7f0358 ("remoteproc: Use of reserved_mem_region_* > > > functions for "memory-region"") following commands with > > > imx-dsp-rproc started to fail: > > > > > > $ echo zephyr.elf > /sys/class/remoteproc/remoteproc0/firmware > > > $ echo start > /sys/class/remoteproc/remoteproc0/state > > > $ echo stop > /sys/class/remoteproc/remoteproc0/state > > > $ echo start > /sys/class/remoteproc/remoteproc0/state #! This fails > > > -sh: echo: write error: Device or resource busy > > > > > > This happens because aforementioned commit replaced devm_ioremap_wc with > > > devm_ioremap_resource_wc which will "reserve" the memory region with the > > > first start and then will fail at the second start if the memory > > > region is already reserved. > > > > > > Even partially reverting the faulty commit won't fix the > > > underlying issue because we map the address in prepare() but we never > > > unmap it at unprepare(), so we will keep leaking memory regions. > > > > > > So, lets use alloc() and release() callbacks for memory carveout > > > handling. This will nicely map() the memory region at prepare() time > > > and unmap() it at unprepare(). > > > > > > Fixes: 67a7bc7f0358 ("remoteproc: Use of_reserved_mem_region_* functions > > > for "memory-region"") > > > Signed-off-by: Daniel Baluta <[email protected]> > > > --- > > > Changes since v1: > > > * > > > https://lore.kernel.org/imx/[email protected]/T/#t > > > * took a different approach and instead of partially reverting the > > > faulty patch, used alloc() and release() callbacks to handle memory > > > region mapping. > > > drivers/remoteproc/imx_dsp_rproc.c | 50 ++++++++++++++++++++---------- > > > 1 file changed, 33 insertions(+), 17 deletions(-) > > > > > > diff --git a/drivers/remoteproc/imx_dsp_rproc.c > > > b/drivers/remoteproc/imx_dsp_rproc.c > > > index 5130a35214c9..83468558e634 100644 > > > --- a/drivers/remoteproc/imx_dsp_rproc.c > > > +++ b/drivers/remoteproc/imx_dsp_rproc.c > > > @@ -644,6 +644,32 @@ static void imx_dsp_rproc_free_mbox(struct > > > imx_dsp_rproc *priv) > > > mbox_free_channel(priv->rxdb_ch); > > > } > > > > > > +static int imx_dsp_rproc_mem_alloc(struct rproc *rproc, > > > + struct rproc_mem_entry *mem) > > > +{ > > > + struct device *dev = rproc->dev.parent; > > > + void *va; > > > + > > > + va = ioremap_wc(mem->dma, mem->len); > > > + if (!va) { > > > + dev_err(dev, "Unable to map memory region: %pa+%zx\n", > > > + &mem->dma, mem->len); > > > + return -ENOMEM; > > > + } > > > + > > > + mem->va = va; > > > + > > > + return 0; > > > +} > > > + > > > +static int imx_dsp_rproc_mem_release(struct rproc *rproc, > > > + struct rproc_mem_entry *mem) > > > +{ > > > + iounmap(mem->va); > > > + > > > + return 0; > > > +} > > > + > > > /** > > > * imx_dsp_rproc_add_carveout() - request mailbox channels > > > * @priv: private data pointer > > > @@ -659,7 +685,6 @@ static int imx_dsp_rproc_add_carveout(struct > > > imx_dsp_rproc *priv) > > > struct device *dev = rproc->dev.parent; > > > struct device_node *np = dev->of_node; > > > struct rproc_mem_entry *mem; > > > - void __iomem *cpu_addr; > > > int a, i = 0; > > > u64 da; > > > > > > @@ -673,15 +698,10 @@ static int imx_dsp_rproc_add_carveout(struct > > > imx_dsp_rproc *priv) > > > if (imx_dsp_rproc_sys_to_da(priv, att->sa, att->size, &da)) > > > return -EINVAL; > > > > > > - cpu_addr = devm_ioremap_wc(dev, att->sa, att->size); > > > - if (!cpu_addr) { > > > - dev_err(dev, "failed to map memory %p\n", &att->sa); > > > - return -ENOMEM; > > > - } > > > - > > > /* Register memory region */ > > > - mem = rproc_mem_entry_init(dev, (void __force *)cpu_addr, > > > (dma_addr_t)att->sa, > > > - att->size, da, NULL, NULL, > > > "dsp_mem"); > > > + mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)att->sa, > > > + att->size, da, > > > imx_dsp_rproc_mem_alloc, > > > + imx_dsp_rproc_mem_release, > > > "dsp_mem"); > > > > Was there a reason you kept those here rather than moving them to probe() as > > Iuliana suggested? Note that I would be fine with this solution since this > > is > > how it was before, but if we have to go through a refactoring we may as well > > take those things into account. > > Tried moving imx_dsp_rproc_add_carveout at probe() time but it doesn't work > because stop() will clean the carveout list and then the next start() will > fail. > > at probe() > imx_dsp_rproc_add_carveout > -> rproc_add_carveout (adds allocated carveout to the list). > > at start(): > -> first start OK > > at stop() > -> rproc_shutdown > -> rproc_stop > -> rproc_resource_cleanup ;//cleans up careveout allocations > > at next start() > -> CRASH
I have applied this patch. Thanks, Mathieu
