Re: [PATCH V2 5/5] remoteproc: imx_rproc: change to ioremap_wc for dram

On Wed, Jul 14, 2021 at 1:06 AM Mathieu Poirier
[off-list ref] wrote:

On Wed, Jul 07, 2021 at 05:40:33PM +0800, Dong Aisheng wrote:

quoted

DRAM is not io memory, so changed to ioremap_wc. This is also
aligned with core io accessories. e.g. memcpy/memset and cpu direct
access.

Cc: Bjorn Andersson <redacted>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Fixes: 5e4c1243071d ("remoteproc: imx_rproc: support remote cores booted before Linux Kernel")
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
v1->v2:
 * new patch

It's a new patch and yet Peng's RB tag is already on it...

quoted

---
 drivers/remoteproc/imx_rproc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index ff620688fad9..4ae416ba5080 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c

@@ -597,7 +597,7 @@ static int imx_rproc_addr_init(struct imx_rproc *priv,
                      break;

              /* Not use resource version, because we might share region */
-             priv->mem[b].cpu_addr = devm_ioremap(&pdev->dev, res.start, resource_size(&res));
+             priv->mem[b].cpu_addr = devm_ioremap_wc(&pdev->dev, res.start, resource_size(&res));

How was it working before?  Will it really work for all platforms and was this
extensively tested?

Here it is only used for accessing resource tables in DRAM which is
published by M core.

Why it works before is because:
1. the default memory access in remoteproc core (.e.g memcpy or direct
access by pointer
by CPU) seems also work well even it's device memory type mapped by
devm_ioremap.
e.g. cpu direct access
offset = rproc->table_ptr->offset[i]

2.  It will not work with meset() in rproc_elf_load_segments() which
has cache operations internally.
e.g. arch/arm64/lib/memset.S
However, it's lucky that for IMX cases, the resource table in DRAM are
currently used by early boot
(e.g. uboot/scfw loading M4 firmware), no chance to run into
rproc_elf_load_segments(),
so no issues so far.

Then the question is should we change the mapping type of resource
table mem to normal
memory (ioremap_wc) as remoteproc core are using normal memory accessories?

I guess we should do that, that's how this patch comes out.

Regards
Aisheng

Peng - I will need an explicit reply from you that you are in agreement with
this change.  I will also need you to review patch 01 and 02 of this set.

Thanks,
Mathieu

quoted

              if (!priv->mem[b].cpu_addr) {
                      dev_err(dev, "failed to remap %pr\n", &res);
                      return -ENOMEM;
--
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel