Re: [PATCH RFC 1/1] remoteproc: mediatek: Support reserved CMA regions

[Mathieu Poirier]

On Wed, 10 Jul 2024 at 02:42, Shun-yi Wang <shun-yi.wang@xxxxxxxxxxxx> wrote:
>
> From: "shun-yi.wang" <shun-yi.wang@xxxxxxxxxxxx>
>
> In order to reserve specific Contiguous Memory Allocator (CMA) regions
> for hardware use. When the name of the reserved region contains "cma",
> then a corresponding CMA heap is added.
>
> Signed-off-by: shun-yi.wang <shun-yi.wang@xxxxxxxxxxxx>
> ---
>  drivers/remoteproc/mtk_scp.c | 38 ++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
>

I'm not sure what to do with this patch...  Is it a superset of your
other patch [1]?  And if so why is it labelled as an RFC?

Please read the documentation on submitting patches [2] and subscribe
to the remoteproc mailing list to give you an idea of the patch
workflow that is expected.

Thanks,
Mathieu

[1]. [PATCH 1/1] remoteproc: mediatek: Support multiple reserved memory regions
[2]. https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst

> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index e744c07507ee..ca0a4a52ece9 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -4,6 +4,7 @@
>
>  #include <asm/barrier.h>
>  #include <linux/clk.h>
> +#include <linux/dma-heap.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/err.h>
>  #include <linux/interrupt.h>
> @@ -1006,22 +1007,43 @@ EXPORT_SYMBOL_GPL(scp_mapping_dm_addr);
>
>  static int scp_map_memory_region(struct mtk_scp *scp)
>  {
> -       int ret;
> +       int ret, i, err;
>         const struct mtk_scp_sizes_data *scp_sizes;
> +       struct device_node *node = scp->dev->of_node;
> +       struct of_phandle_iterator it;
> +
> +       i = 0;
> +       of_for_each_phandle(&it, err, node, "memory-region", NULL, 0) {
> +               ret = of_reserved_mem_device_init_by_idx(scp->dev, node, i);
> +
> +               if (ret) {
> +                       dev_err(scp->dev, "failed to assign memory-region: %s\n",
> +                               it.node->name);
> +                       of_node_put(it.node);
> +                       return -ENOMEM;
> +               }
> +
> +#ifdef CONFIG_DMABUF_HEAPS_CMA
> +               if (strstr(it.node->name, "cma")) {
> +                       /* Reserved cma memory region */
> +                       ret = dma_heap_add_cma(scp->dev);
> +                       if (ret) {
> +                               dev_err(scp->dev, "Failed to add reserved cma");
> +                               of_node_put(it.node);
> +                               return ret;
> +                       }
> +               }
> +#endif /* CONFIG_DMABUF_HEAPS_CMA */
>
> -       ret = of_reserved_mem_device_init(scp->dev);
> +               i++;
> +       }
>
>         /* reserved memory is optional. */
> -       if (ret == -ENODEV) {
> +       if (!i) {
>                 dev_info(scp->dev, "skipping reserved memory initialization.");
>                 return 0;
>         }
>
> -       if (ret) {
> -               dev_err(scp->dev, "failed to assign memory-region: %d\n", ret);
> -               return -ENOMEM;
> -       }
> -
>         /* Reserved SCP code size */
>         scp_sizes = scp->data->scp_sizes;
>         scp->cpu_addr = dma_alloc_coherent(scp->dev, scp_sizes->max_dram_size,
> --
> 2.18.0
>