Re: [PATCH 2/4] remoteproc: switch exact-match drivers to wc-ioremap callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Implicit and undocumented behavioral change in memory access semantics, and introduced inconsistency.
--

commit 33b97865cf4d72ce26fd26e8b0209b38daaf1222
Author: Ben Levinsky <ben.levinsky@amd.com>

remoteproc: switch exact-match drivers to wc-ioremap callbacks

Replace the exact-match carveout map and unmap callbacks in the
existing remoteproc drivers with the common wc-ioremap helpers. Leave
the zynqmp R5 TCM callbacks alone because they also clear the mapped
memory and are not exact matches.

> diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
> index 008741af9f11c..2d9f14fbef1da 100644
> --- a/drivers/remoteproc/imx_dsp_rproc.c
> +++ b/drivers/remoteproc/imx_dsp_rproc.c
[ ... ]
> @@ -700,8 +674,10 @@ static int imx_dsp_rproc_add_carveout(struct imx_dsp_rproc *priv)
>  
>  		/* Register memory region */
>  		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");
> +					   att->size, da,
> +					   rproc_mem_entry_ioremap_wc,
> +					   rproc_mem_entry_iounmap,
> +					   "dsp_mem");

Does replacing the custom callbacks with rproc_mem_entry_ioremap_wc()
unintentionally alter the memory access semantics for these drivers?

The new rproc_mem_entry_ioremap_wc() helper explicitly sets
mem->is_iomem = true. The previous custom callbacks like
imx_dsp_rproc_mem_alloc() called ioremap_wc() but left mem->is_iomem as
false.

Setting is_iomem = true causes the ELF loader and coredump routines to
switch from standard memcpy() and memset() to I/O accessors like
memcpy_toio() and memset_io().

While this is likely safer for I/O mapped memory, this behavioral change
does not appear to be documented in the commit message. Should the commit
message document this shift in memory access semantics?

> Leave the zynqmp R5 TCM callbacks alone because they also clear the mapped
> memory and are not exact matches.

By leaving the ZynqMP R5 TCM callbacks untouched, does this create an
inconsistency in how the remoteproc framework handles I/O mapped memory?

The ZynqMP R5 TCM callbacks will continue to map memory using ioremap_wc()
without setting is_iomem = true. Would it be better to update the ZynqMP
callbacks to also set is_iomem so the framework accesses I/O memory
consistently across all drivers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511211841.284809-1-ben.levinsky@amd.com?part=2