remoteproc: Use of_reserved_mem_region_* functions for "memory-region"

Use the newly added of_reserved_mem_region_to_resource() and of_reserved_mem_region_count() functions to handle "memory-region" properties.

The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in of_reserved_mem_lookup() is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is just of_reserved_mem_region_to_resource() which is checked for an error.

Acked-by Arnaud Pouliquen <[email protected]>

Tested-by Peng Fan <[email protected]> # i.MX93-11x11-EVK for imx_rproc.c

Signed-off-by Rob Herring (Arm) <[email protected]>

---
v4:
- Rebase on v6.17-rc1. qcom_q6v5_pas.c conflicted needing s/adsp/pas/

v3:
- Rebase on v6.16-rc1. Move TI K3 changes to new common file.
- Fix double increment of "i" in xlnx_r5

v2:
- Use strstarts instead of strcmp for resource names as they include the unit-address.
- Drop the unit-address from resource name for imx and st drivers

---
 drivers/remoteproc/imx_dsp_rproc.c        | 45 ++++++--------
 drivers/remoteproc/imx_rproc.c            | 68 ++++++++------------
 drivers/remoteproc/qcom_q6v5_adsp.c       | 24 +++-----
 drivers/remoteproc/qcom_q6v5_mss.c        | 60 ++++++------------
 drivers/remoteproc/qcom_q6v5_pas.c        | 75 +++++++++--------------
 drivers/remoteproc/qcom_q6v5_wcss.c       | 25 +++-----
 drivers/remoteproc/qcom_wcnss.c           | 23 +++----
 drivers/remoteproc/rcar_rproc.c           | 36 +++++------
 drivers/remoteproc/st_remoteproc.c        | 41 ++++++-------
 drivers/remoteproc/stm32_rproc.c          | 44 ++++++-------
 drivers/remoteproc/ti_k3_common.c         | 28 ++++-----
 drivers/remoteproc/ti_k3_dsp_remoteproc.c |  2 +-
 drivers/remoteproc/ti_k3_r5_remoteproc.c  |  2 +-
 drivers/remoteproc/xlnx_r5_remoteproc.c   | 51 ++++++---------
 14 files changed, 204 insertions(+), 320 deletions(-)

drivers/remoteproc/qcom_q6v5_adsp.c +0 -6

index 94af77baa7a1..a5b7cbb8fe07 100644
@@ -625,26 +625,20 @@ static int adsp_init_mmio(struct qcom_adsp *adsp,
 
 static int adsp_alloc_memory_region(struct qcom_adsp *adsp)
 {
-	struct reserved_mem *rmem = NULL;
-	struct device_node *node;
-
-	node = of_parse_phandle(adsp->dev->of_node, "memory-region", 0);
-	if (node)
-		rmem = of_reserved_mem_lookup(node);

drivers/remoteproc/qcom_q6v5_adsp.c +4 -6

 	}
 
-	adsp->mem_phys = adsp->mem_reloc = rmem->base;
-	adsp->mem_size = rmem->size;
-	adsp->mem_region = devm_ioremap_wc(adsp->dev,
-				adsp->mem_phys, adsp->mem_size);
+	adsp->mem_phys = adsp->mem_reloc = res.start;
+	adsp->mem_size = resource_size(&res);
+	adsp->mem_region = devm_ioremap_resource_wc(adsp->dev, &res);
 	if (!adsp->mem_region) {
-		dev_err(adsp->dev, "unable to map memory region: %pa+%zx\n",
-			&rmem->base, adsp->mem_size);
+		dev_err(adsp->dev, "unable to map memory region: %pR\n", &res);
 		return -EBUSY;
 	}
 

drivers/remoteproc/qcom_q6v5_mss.c +15 -31

index 0c0199fb0e68..0fea5f91dd1c 100644
@@ -1961,8 +1961,8 @@ static int q6v5_init_reset(struct q6v5 *qproc)
 static int q6v5_alloc_memory_region(struct q6v5 *qproc)
 {
 	struct device_node *child;
-	struct reserved_mem *rmem;
-	struct device_node *node;
+	struct resource res;
+	int ret;
 
 	/*
 	 * In the absence of mba/mpss sub-child, extract the mba and mpss

@@ -1970,71 +1970,49 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
 	 */
 	child = of_get_child_by_name(qproc->dev->of_node, "mba");
 	if (!child) {
-		node = of_parse_phandle(qproc->dev->of_node,
-					"memory-region", 0);
+		ret = of_reserved_mem_region_to_resource(qproc->dev->of_node, 0, &res);
 	} else {
-		node = of_parse_phandle(child, "memory-region", 0);
+		ret = of_reserved_mem_region_to_resource(child, 0, &res);
 		of_node_put(child);
 	}
 
-	if (!node) {
-		dev_err(qproc->dev, "no mba memory-region specified\n");
-		return -EINVAL;
-	}
-
-	rmem = of_reserved_mem_lookup(node);
-	of_node_put(node);
-	if (!rmem) {
+	if (ret) {
 		dev_err(qproc->dev, "unable to resolve mba region\n");
-		return -EINVAL;
+		return ret;
 	}
 
-	qproc->mba_phys = rmem->base;
-	qproc->mba_size = rmem->size;
+	qproc->mba_phys = res.start;
+	qproc->mba_size = resource_size(&res);
 
 	if (!child) {
-		node = of_parse_phandle(qproc->dev->of_node,
-					"memory-region", 1);
+		ret = of_reserved_mem_region_to_resource(qproc->dev->of_node, 1, &res);
 	} else {
 		child = of_get_child_by_name(qproc->dev->of_node, "mpss");
-		node = of_parse_phandle(child, "memory-region", 0);
+		ret = of_reserved_mem_region_to_resource(child, 0, &res);
 		of_node_put(child);
 	}
 
-	if (!node) {
-		dev_err(qproc->dev, "no mpss memory-region specified\n");
-		return -EINVAL;
-	}
-
-	rmem = of_reserved_mem_lookup(node);
-	of_node_put(node);
-	if (!rmem) {
+	if (ret) {
 		dev_err(qproc->dev, "unable to resolve mpss region\n");
-		return -EINVAL;
+		return ret;
 	}
 
-	qproc->mpss_phys = qproc->mpss_reloc = rmem->base;
-	qproc->mpss_size = rmem->size;
+	qproc->mpss_phys = qproc->mpss_reloc = res.start;
+	qproc->mpss_size = resource_size(&res);
 
 	if (!child) {
-		node = of_parse_phandle(qproc->dev->of_node, "memory-region", 2);
+		ret = of_reserved_mem_region_to_resource(qproc->dev->of_node, 2, &res);

drivers/remoteproc/qcom_q6v5_mss.c +2 -8

-	rmem = of_reserved_mem_lookup(node);
-	if (!rmem) {
-		dev_err(qproc->dev, "unable to resolve metadata region\n");
-		return -EINVAL;
-	}
-
-	qproc->mdata_phys = rmem->base;
-	qproc->mdata_size = rmem->size;
+	qproc->mdata_phys = res.start;
+	qproc->mdata_size = resource_size(&res);
 
 	return 0;
 }

drivers/remoteproc/qcom_q6v5_pas.c +0 -0

index 02e29171cbbe..b3f7209289a6 100644

drivers/remoteproc/qcom_q6v5_pas.c +2 -2

 {
 	struct qcom_pas *pas = rproc->priv;
 	int total_offset;

@@ -149,7 +149,7 @@ static void qcom_pas_minidump(struct rproc *rproc)
 }
 
 static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
-			       size_t pd_count)
+			   size_t pd_count)
 {
 	int ret;
 	int i;

@@ -176,7 +176,7 @@ static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
 };
 
 static void qcom_pas_pds_disable(struct qcom_pas *pas, struct device **pds,
-				 size_t pd_count)
+			     size_t pd_count)
 {
 	int i;
 

Quoted 61 lines · Rob Herring (Arm)

Use the newly added of_reserved_mem_region_to_resource() and of_reserved_mem_region_count() functions to handle "memory-region" properties.

The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in of_reserved_mem_lookup() is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is just of_reserved_mem_region_to_resource() which is checked for an error.

Acked-by Arnaud Pouliquen <[email protected]>

Tested-by Peng Fan <[email protected]> # i.MX93-11x11-EVK for imx_rproc.c

Signed-off-by Rob Herring (Arm) <[email protected]>

---
v4:
- Rebase on v6.17-rc1. qcom_q6v5_pas.c conflicted needing s/adsp/pas/

v3:
- Rebase on v6.16-rc1. Move TI K3 changes to new common file.
- Fix double increment of "i" in xlnx_r5

v2:
- Use strstarts instead of strcmp for resource names as they include the unit-address.
- Drop the unit-address from resource name for imx and st drivers

---
 drivers/remoteproc/imx_dsp_rproc.c        | 45 ++++++--------
 drivers/remoteproc/imx_rproc.c            | 68 ++++++++------------
 drivers/remoteproc/qcom_q6v5_adsp.c       | 24 +++-----
 drivers/remoteproc/qcom_q6v5_mss.c        | 60 ++++++------------
 drivers/remoteproc/qcom_q6v5_pas.c        | 75 +++++++++--------------
 drivers/remoteproc/qcom_q6v5_wcss.c       | 25 +++-----
 drivers/remoteproc/qcom_wcnss.c           | 23 +++----
 drivers/remoteproc/rcar_rproc.c           | 36 +++++------
 drivers/remoteproc/st_remoteproc.c        | 41 ++++++-------
 drivers/remoteproc/stm32_rproc.c          | 44 ++++++-------
 drivers/remoteproc/ti_k3_common.c         | 28 ++++-----
 drivers/remoteproc/ti_k3_dsp_remoteproc.c |  2 +-
 drivers/remoteproc/ti_k3_r5_remoteproc.c  |  2 +-
 drivers/remoteproc/xlnx_r5_remoteproc.c   | 51 ++++++---------
 14 files changed, 204 insertions(+), 320 deletions(-)

drivers/remoteproc/qcom_q6v5_adsp.c +2 -7

index 94af77baa7a1..a5b7cbb8fe07 100644
@@ -625,26 +625,20 @@ static int adsp_init_mmio(struct qcom_adsp *adsp,
 
 static int adsp_alloc_memory_region(struct qcom_adsp *adsp)
 {
-     struct reserved_mem *rmem = NULL;
-     struct device_node *node;
-
-     node = of_parse_phandle(adsp->dev->of_node, "memory-region", 0);
-     if (node)
-             rmem = of_reserved_mem_lookup(node);
-     of_node_put(node);
+     int ret;
+     struct resource res;
 

drivers/remoteproc/qcom_q6v5_adsp.c static int adsp_init_mmio(struct qcom_adsp *adsp, · line 634

Quoted 6 lines

drivers/remoteproc/qcom_q6v5_adsp.c +3 -2

-     if (!rmem) {
+     ret = of_reserved_mem_region_to_resource(adsp->dev->of_node, 0, &res);
+     if (!ret) {
              dev_err(adsp->dev, "unable to resolve memory-region\n");
-             return -EINVAL;
+             return ret;

This looks strange. Shouldn't it be if (ret) { ?

Quoted 104 lines

drivers/remoteproc/qcom_q6v5_adsp.c +4 -6

      }
 
-     adsp->mem_phys = adsp->mem_reloc = rmem->base;
-     adsp->mem_size = rmem->size;
-     adsp->mem_region = devm_ioremap_wc(adsp->dev,
-                             adsp->mem_phys, adsp->mem_size);
+     adsp->mem_phys = adsp->mem_reloc = res.start;
+     adsp->mem_size = resource_size(&res);
+     adsp->mem_region = devm_ioremap_resource_wc(adsp->dev, &res);
      if (!adsp->mem_region) {
-             dev_err(adsp->dev, "unable to map memory region: %pa+%zx\n",
-                     &rmem->base, adsp->mem_size);
+             dev_err(adsp->dev, "unable to map memory region: %pR\n", &res);
              return -EBUSY;
      }
 

drivers/remoteproc/qcom_q6v5_mss.c +16 -32

index 0c0199fb0e68..0fea5f91dd1c 100644
@@ -1961,8 +1961,8 @@ static int q6v5_init_reset(struct q6v5 *qproc)
 static int q6v5_alloc_memory_region(struct q6v5 *qproc)
 {
      struct device_node *child;
-     struct reserved_mem *rmem;
-     struct device_node *node;
+     struct resource res;
+     int ret;
 
      /*
       * In the absence of mba/mpss sub-child, extract the mba and mpss

@@ -1970,71 +1970,49 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
       */
      child = of_get_child_by_name(qproc->dev->of_node, "mba");
      if (!child) {
-             node = of_parse_phandle(qproc->dev->of_node,
-                                     "memory-region", 0);
+             ret = of_reserved_mem_region_to_resource(qproc->dev->of_node, 0, &res);
      } else {
-             node = of_parse_phandle(child, "memory-region", 0);
+             ret = of_reserved_mem_region_to_resource(child, 0, &res);
              of_node_put(child);
      }
 
-     if (!node) {
-             dev_err(qproc->dev, "no mba memory-region specified\n");
-             return -EINVAL;
-     }
-
-     rmem = of_reserved_mem_lookup(node);
-     of_node_put(node);
-     if (!rmem) {
+     if (ret) {
              dev_err(qproc->dev, "unable to resolve mba region\n");
-             return -EINVAL;
+             return ret;
      }
 
-     qproc->mba_phys = rmem->base;
-     qproc->mba_size = rmem->size;
+     qproc->mba_phys = res.start;
+     qproc->mba_size = resource_size(&res);
 
      if (!child) {
-             node = of_parse_phandle(qproc->dev->of_node,
-                                     "memory-region", 1);
+             ret = of_reserved_mem_region_to_resource(qproc->dev->of_node, 1, &res);
      } else {
              child = of_get_child_by_name(qproc->dev->of_node, "mpss");
-             node = of_parse_phandle(child, "memory-region", 0);
+             ret = of_reserved_mem_region_to_resource(child, 0, &res);
              of_node_put(child);
      }
 
-     if (!node) {
-             dev_err(qproc->dev, "no mpss memory-region specified\n");
-             return -EINVAL;
-     }
-
-     rmem = of_reserved_mem_lookup(node);
-     of_node_put(node);
-     if (!rmem) {
+     if (ret) {
              dev_err(qproc->dev, "unable to resolve mpss region\n");
-             return -EINVAL;
+             return ret;
      }
 
-     qproc->mpss_phys = qproc->mpss_reloc = rmem->base;
-     qproc->mpss_size = rmem->size;
+     qproc->mpss_phys = qproc->mpss_reloc = res.start;
+     qproc->mpss_size = resource_size(&res);
 
      if (!child) {
-             node = of_parse_phandle(qproc->dev->of_node, "memory-region", 2);
+             ret = of_reserved_mem_region_to_resource(qproc->dev->of_node, 2, &res);
      } else {
              child = of_get_child_by_name(qproc->dev->of_node, "metadata");
-             node = of_parse_phandle(child, "memory-region", 0);
+             ret = of_reserved_mem_region_to_resource(child, 0, &res);
              of_node_put(child);
      }
 

drivers/remoteproc/qcom_q6v5_mss.c static int q6v5_alloc_memory_region(struct q6v5 *qproc) · line 2012

Quoted 3 lines

drivers/remoteproc/qcom_q6v5_mss.c +1 -1

-     if (!node)
+     if (ret)
              return 0;

Shouldn't we differentiate between an absent region (OK) and an error during parse.

Quoted 19 lines

drivers/remoteproc/qcom_q6v5_mss.c +2 -8

-     rmem = of_reserved_mem_lookup(node);
-     if (!rmem) {
-             dev_err(qproc->dev, "unable to resolve metadata region\n");
-             return -EINVAL;
-     }
-
-     qproc->mdata_phys = rmem->base;
-     qproc->mdata_size = rmem->size;
+     qproc->mdata_phys = res.start;
+     qproc->mdata_size = resource_size(&res);
 
      return 0;
 }

drivers/remoteproc/qcom_q6v5_pas.c +0 -0

index 02e29171cbbe..b3f7209289a6 100644
@@ -121,7 +121,7 @@ struct qcom_pas {
 

drivers/remoteproc/qcom_q6v5_pas.c struct qcom_pas { · line 124

Quoted 4 lines

drivers/remoteproc/qcom_q6v5_pas.c +1 -1

 static void qcom_pas_segment_dump(struct rproc *rproc,
                                struct rproc_dump_segment *segment,
-                               void *dest, size_t offset, size_t size)
+                    void *dest, size_t offset, size_t size)

Irrelevant? (and two next chunks)

Use the newly added of_reserved_mem_region_to_resource() and of_reserved_mem_region_count() functions to handle "memory-region" properties.

The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in of_reserved_mem_lookup() is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is just of_reserved_mem_region_to_resource() which is checked for an error.

Acked-by Arnaud Pouliquen <[email protected]>

Tested-by Peng Fan <[email protected]> # i.MX93-11x11-EVK for imx_rproc.c

Signed-off-by Rob Herring (Arm) <[email protected]>

drivers/remoteproc/rcar_rproc.c +12 -19

@@ -52,41 +52,33 @@ static int rcar_rproc_prepare(struct rproc *rproc)
 {
        struct device *dev = rproc->dev.parent;
        struct device_node *np = dev->of_node;
-       struct of_phandle_iterator it;
        struct rproc_mem_entry *mem;
-       struct reserved_mem *rmem;
+       int i = 0;
        u32 da;
 
        /* Register associated reserved memory regions */
-       of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
-       while (of_phandle_iterator_next(&it) == 0) {
-
-               rmem = of_reserved_mem_lookup(it.node);
-               if (!rmem) {
-                       of_node_put(it.node);
-                       dev_err(&rproc->dev,
-                               "unable to acquire memory-region\n");
-                       return -EINVAL;
-               }
+       while (1) {
+               struct resource res;
+               int ret;
+
+               ret = of_reserved_mem_region_to_resource(np, i++, &res);
+               if (ret)
+                       return 0;
 
-               if (rmem->base > U32_MAX) {
-                       of_node_put(it.node);
+               if (res.start > U32_MAX)
                        return -EINVAL;
-               }
 
                /* No need to translate pa to da, R-Car use same map */
-               da = rmem->base;
+               da = res.start;
                mem = rproc_mem_entry_init(dev, NULL,
-                                          rmem->base,
-                                          rmem->size, da,
+                                          res.start,
+                                          resource_size(&res), da,
                                           rcar_rproc_mem_alloc,
                                           rcar_rproc_mem_release,
-                                          it.node->name);