Re: [PATCH v4 1/6] remoteproc: qcom: fix sparse warnings for __iomem annotated mem_region

[Mukesh Ojha]

On Thu, Jun 18, 2026 at 03:53:22PM +0200, Konrad Dybcio wrote:
> On 4/30/26 9:12 PM, Mukesh Ojha wrote:
> > The firmware memory regions in qcom_adsp, qcom_pas and qcom_wcnss are
> > mapped using devm_ioremap_wc() / devm_ioremap_resource_wc(), which
> > return void __iomem *. However, the mem_region (and dtb_mem_region)
> > fields in the respective driver structs were declared as plain void *,
> > causing sparse to flag address space mismatches:
> > 
> > qcom_q6v5_adsp.c:639:26: warning: incorrect type in assignment (different address spaces)
> > qcom_q6v5_adsp.c:639:26:    expected void *mem_region
> > qcom_q6v5_adsp.c:639:26:    got void [noderef] __iomem *
> > qcom_q6v5_pas.c:141:45: warning: incorrect type in argument 2 (different address spaces)
> > qcom_q6v5_pas.c:141:45:    expected void const volatile [noderef] __iomem *src
> > qcom_q6v5_pas.c:141:45:    got void *
> > qcom_q6v5_pas.c:637:25: warning: incorrect type in assignment (different address spaces)
> > qcom_q6v5_pas.c:637:25:    expected void *mem_region
> > qcom_q6v5_pas.c:637:25:    got void [noderef] __iomem *
> > qcom_q6v5_pas.c:654:29: warning: incorrect type in assignment (different address spaces)
> > qcom_q6v5_pas.c:654:29:    expected void *dtb_mem_region
> > qcom_q6v5_pas.c:654:29:    got void [noderef] __iomem *
> > qcom_wcnss.c:540:27: warning: incorrect type in assignment (different address spaces)
> > qcom_wcnss.c:540:27:    expected void *mem_region
> > qcom_wcnss.c:540:27:    got void [noderef] __iomem *
> > 
> > Fix this by annotating the struct fields with __iomem to correctly
> > reflect the address space of the underlying mapping.
> > 
> > These regions are subsequently passed to qcom_mdt_load(),
> > qcom_mdt_load_no_init() and qcom_mdt_pas_load(), all of which take
> > void * and use plain memcpy()/memset() internally to write firmware
> > segments into the region. This is intentional and safe: the mappings
> > are write-combining (WC), which on arm64 permits bulk CPU stores
> > without requiring the memcpy_toio()/memset_io() accessors. Changing
> > the MDT loader API to accept void __iomem * would be a more invasive
> > change and would affect callers.
> 
> Zooming out a bit more, should we even be ioremapping these regions
> in the first place? Are they not just RAM/Normal Memory in arm parlance?

Yes, ioremap may not be required. Don't know why it was kept in the
initial implementation, as for most Qualcomm SoCs this is going to be
normal RAM and not SRAM or something, while other implementations like
qcom_q6v5_mss still use memremap with MEMREMAP_WC.

> 
> Would switching to devm_memremap(, MEMREMAP_WC) (like we do in RMTFS
> today) be an even better solution here?

This looks fine and I do not see any immediate impact; however, I would
like to hear from Bjorn on this, if there is any history..

-- 
-Mukesh Ojha