Re: [PATCHv5 06/14] remoteproc/omap: Initialize and assign reserved memory node

From: Tero Kristo
Date: Thu Jan 30 2020 - 14:42:27 EST


On 30/01/2020 21:20, Andrew F. Davis wrote:
On 1/30/20 2:18 PM, Tero Kristo wrote:
On 30/01/2020 20:11, Andrew F. Davis wrote:
On 1/16/20 8:53 AM, Tero Kristo wrote:
From: Suman Anna <s-anna@xxxxxx>

The reserved memory nodes are not assigned to platform devices by
default in the driver core to avoid the lookup for every platform
device and incur a penalty as the real users are expected to be
only a few devices.

OMAP remoteproc devices fall into the above category and the OMAP
remoteproc driver _requires_ specific CMA pools to be assigned
for each device at the moment to align on the location of the
vrings and vring buffers in the RTOS-side firmware images. So,


Same comment as before, this is a firmware issue for only some firmwares
that do not handle being assigned vring locations correctly and instead
hard-code them.

I believe we discussed this topic in length in previous version but
there was no conclusion on it.

The commit desc might be a bit misleading, we are not actually forced to
use specific CMA buffers, as we use IOMMU to map these to device
addresses. For example IPU1/IPU2 use internally exact same memory
addresses, iommu is used to map these to specific CMA buffer.

CMA buffers are mostly used so that we get aligned large chunk of memory
which can be mapped properly with the limited IOMMU OMAP family of chips
have. Not sure if there is any sane way to get this done in any other
manner.



Why not use the default CMA area?

I think using default CMA area getting the actual memory block is not guaranteed and might fail. There are other users for the memory, and it might get fragmented at the very late phase we are grabbing the memory (omap remoteproc driver probe time.) Some chunks we need are pretty large.

I believe I could experiment with this a bit though and see, or Suman could maybe provide feedback why this was designed initially like this and why this would not be a good idea.

-Tero


Andrew


-Tero


This is not a requirement of the remote processor itself and so it
should not fail to probe if a specific memory carveout isn't given.


use the of_reserved_mem_device_init/release() API appropriately
to assign the corresponding reserved memory region to the OMAP
remoteproc device. Note that only one region per device is
allowed by the framework.

Signed-off-by: Suman Anna <s-anna@xxxxxx>
Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
Reviewed-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
---
v5: no changes

 drivers/remoteproc/omap_remoteproc.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/omap_remoteproc.c
b/drivers/remoteproc/omap_remoteproc.c
index 0846839b2c97..194303b860b2 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/err.h>
 #include <linux/of_device.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
 #include <linux/remoteproc.h>
@@ -480,14 +481,22 @@ static int omap_rproc_probe(struct
platform_device *pdev)
ÂÂÂÂÂ if (ret)
ÂÂÂÂÂÂÂÂÂ goto free_rproc;
 + ret = of_reserved_mem_device_init(&pdev->dev);
+ÂÂÂ if (ret) {
+ÂÂÂÂÂÂÂ dev_err(&pdev->dev, "device does not have specific CMA
pool\n");
+ÂÂÂÂÂÂÂ goto free_rproc;
+ÂÂÂ }
+
ÂÂÂÂÂ platform_set_drvdata(pdev, rproc);
 Â ret = rproc_add(rproc);
ÂÂÂÂÂ if (ret)
-ÂÂÂÂÂÂÂ goto free_rproc;
+ÂÂÂÂÂÂÂ goto release_mem;
 Â return 0;
 +release_mem:
+ÂÂÂ of_reserved_mem_device_release(&pdev->dev);
 free_rproc:
ÂÂÂÂÂ rproc_free(rproc);
ÂÂÂÂÂ return ret;
@@ -499,6 +508,7 @@ static int omap_rproc_remove(struct
platform_device *pdev)
 Â rproc_del(rproc);
ÂÂÂÂÂ rproc_free(rproc);
+ÂÂÂ of_reserved_mem_device_release(&pdev->dev);
 Â return 0;
 }


--

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki