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

[Andrew F. Davis]

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?

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