Re: [PATCH 1/4] remoteproc: add common wc-ioremap carveout callbacks

[Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>]

On 5/11/26 23:18, Ben Levinsky wrote:
> Several remoteproc drivers open-code the same ioremap_wc() and
> iounmap() callbacks for carveout mappings. Add subsystem-private
> helpers in remoteproc_internal.h so those drivers can share the same
> implementation.
> 
> Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
> ---
>   drivers/remoteproc/remoteproc_internal.h | 26 +++++++++++++++++++++++-
>   1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
> index 0a5e15744b1d..3724a47a9748 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -12,8 +12,9 @@
>   #ifndef REMOTEPROC_INTERNAL_H
>   #define REMOTEPROC_INTERNAL_H
>   
> -#include <linux/irqreturn.h>
>   #include <linux/firmware.h>
> +#include <linux/io.h>
> +#include <linux/irqreturn.h>
>   
>   struct rproc;
>   
> @@ -122,6 +123,29 @@ rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...);
>   void rproc_add_rvdev(struct rproc *rproc, struct rproc_vdev *rvdev);
>   void rproc_remove_rvdev(struct rproc_vdev *rvdev);
>   
> +static inline int rproc_mem_entry_ioremap_wc(struct rproc *rproc,
> +					     struct rproc_mem_entry *mem)
> +{
> +	void __iomem *va;
> +
> +	va = ioremap_wc(mem->dma, mem->len);
> +	if (!va)
> +		return -ENOMEM;

Could you add error message here to help for debug

+		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
+			&mem->dma, mem->len);
> +
> +	mem->va = (__force void *)va;
> +	mem->is_iomem = true;

HHere, you set mem->is_iomem, but this is not done in platform drivers.

It seems better to add this in a separate commit after patch 2/4, with 
an explanation of why it needs to be set.

Regards,
Arnaud

> +
> +	return 0;
> +}
> +
> +static inline int rproc_mem_entry_iounmap(struct rproc *rproc,
> +					  struct rproc_mem_entry *mem)
> +{
> +	iounmap((__force __iomem void *)mem->va);
> +
> +	return 0;
> +}
> +
>   static inline int rproc_prepare_device(struct rproc *rproc)
>   {
>   	if (rproc->ops->prepare)