[PATCH RFC 0/1] Support CMA regions

[PATCH RFC 0/1] Support CMA regions

[Shun-yi Wang]

From: "shun-yi.wang" <shun-yi.wang@mediatek.com>

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.

In the DTS (Device Tree Source), we may have several memory regions 
with different names, e.g.,
&scp {
	...
	memory-region = <&mem_reserved_1>, <&mem_reserved_2>;
};

mem_reserved_1: xxx-xxx-region {
	...
};
mem_reserved_2: xxx-xxx-cma-region {
	...
};

When the name of the region contains "cma", a corresponding heap 
allocator is added by cma_heap_add().
However, we are unsure if using the name "cma" as an identifier
is a good practice.
I sincerely hope that you can provide me some suggestions, thanks.
Especially within the segment of '#ifdef CONFIG_DMA_BUF_HEAPS_CMA'

Note: The cma_heap_add() is introduced from this patch:
dma-buf: heaps: Introduce cma_heap_add() for non-default CMA heap

shun-yi.wang (1):
  remoteproc: mediatek: Support reserved CMA regions

 drivers/remoteproc/mtk_scp.c | 38 ++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

-- 
2.18.0

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

[Shun-yi Wang]

From: "shun-yi.wang" <shun-yi.wang@mediatek.com>

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@mediatek.com>
---
 drivers/remoteproc/mtk_scp.c | 38 ++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

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

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@mediatek.com> wrote:
>
> From: "shun-yi.wang" <shun-yi.wang@mediatek.com>
>
> 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@mediatek.com>
> ---
>  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
>