[PATCH v2 0/3] Simplify with devm_platform_ioremap_resource_byname()

[PATCH v2 0/3] Simplify with devm_platform_ioremap_resource_byname()

[Zhang Zekun]

Use devm_platform_ioremap_resource_byname() to simplify code logic.
No functional change here.

v2: Add other cleanups in the remoteproc subsystem

Zhang Zekun (3):
  remoteproc: Use devm_platform_ioremap_resource_byname() helper
    function
  remoteproc: da8xx: Simplify with
    devm_platform_ioremap_resource_byname()
  remoteporc: ingenic: Simplify with
    devm_platform_ioremap_resource_byname()

 drivers/remoteproc/da8xx_remoteproc.c | 10 ++--------
 drivers/remoteproc/ingenic_rproc.c    |  3 +--
 drivers/remoteproc/st_slim_rproc.c    |  6 ++----
 3 files changed, 5 insertions(+), 14 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] remoteproc: Use devm_platform_ioremap_resource_byname() helper function

[Zhang Zekun]

platform_get_resource_byname() and devm_ioremap_resource() can be
replaced by devm_platform_ioremap_resource_byname(), which can
simplify the code logic a bit, No functional change here.

Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
---
 drivers/remoteproc/st_slim_rproc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/st_slim_rproc.c b/drivers/remoteproc/st_slim_rproc.c
index d17719384c16..5412beb0a692 100644
--- a/drivers/remoteproc/st_slim_rproc.c
+++ b/drivers/remoteproc/st_slim_rproc.c
@@ -259,16 +259,14 @@ struct st_slim_rproc *st_slim_rproc_alloc(struct platform_device *pdev,
 		slim_rproc->mem[i].size = resource_size(res);
 	}
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "slimcore");
-	slim_rproc->slimcore = devm_ioremap_resource(dev, res);
+	slim_rproc->slimcore = devm_platform_ioremap_resource_byname(pdev, "slimcore");
 	if (IS_ERR(slim_rproc->slimcore)) {
 		dev_err(&pdev->dev, "failed to ioremap slimcore IO\n");
 		err = PTR_ERR(slim_rproc->slimcore);
 		goto err;
 	}
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "peripherals");
-	slim_rproc->peri = devm_ioremap_resource(dev, res);
+	slim_rproc->peri = devm_platform_ioremap_resource_byname(pdev, "peripherals");
 	if (IS_ERR(slim_rproc->peri)) {
 		dev_err(&pdev->dev, "failed to ioremap peripherals IO\n");
 		err = PTR_ERR(slim_rproc->peri);
-- 
2.17.1

[PATCH v2 2/3] remoteproc: da8xx: Simplify with devm_platform_ioremap_resource_byname()

[Zhang Zekun]

platform_get_resource_byname() and devm_ioremap_resource() can be
replaced by devm_platform_ioremap_resource_byname(), which can
simplify the code logic a bit, No functional change here.

Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
---
 drivers/remoteproc/da8xx_remoteproc.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index 9041a0e07fb2..8770d0cf1255 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -239,8 +239,6 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 	struct da8xx_rproc *drproc;
 	struct rproc *rproc;
 	struct irq_data *irq_data;
-	struct resource *bootreg_res;
-	struct resource *chipsig_res;
 	struct clk *dsp_clk;
 	struct reset_control *dsp_reset;
 	void __iomem *chipsig;
@@ -258,15 +256,11 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	bootreg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-						   "host1cfg");
-	bootreg = devm_ioremap_resource(dev, bootreg_res);
+	bootreg = devm_platform_ioremap_resource_byname(pdev, "host1cfg");
 	if (IS_ERR(bootreg))
 		return PTR_ERR(bootreg);
 
-	chipsig_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-						   "chipsig");
-	chipsig = devm_ioremap_resource(dev, chipsig_res);
+	chipsig = devm_platform_ioremap_resource_byname(pdev, "chipsig");
 	if (IS_ERR(chipsig))
 		return PTR_ERR(chipsig);
 
-- 
2.17.1

[PATCH v2 3/3] remoteporc: ingenic: Simplify with devm_platform_ioremap_resource_byname()

[Zhang Zekun]

platform_get_resource_byname() and devm_ioremap_resource() can be
replaced by devm_platform_ioremap_resource_byname(), which can
simplify the code logic a bit, No functional change here.

Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
---
 drivers/remoteproc/ingenic_rproc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/remoteproc/ingenic_rproc.c b/drivers/remoteproc/ingenic_rproc.c
index 9902cce28692..1b78d8ddeacf 100644
--- a/drivers/remoteproc/ingenic_rproc.c
+++ b/drivers/remoteproc/ingenic_rproc.c
@@ -183,8 +183,7 @@ static int ingenic_rproc_probe(struct platform_device *pdev)
 	vpu->dev = &pdev->dev;
 	platform_set_drvdata(pdev, vpu);
 
-	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aux");
-	vpu->aux_base = devm_ioremap_resource(dev, mem);
+	vpu->aux_base = devm_platform_ioremap_resource_byname(pdev, "aux");
 	if (IS_ERR(vpu->aux_base)) {
 		dev_err(dev, "Failed to ioremap\n");
 		return PTR_ERR(vpu->aux_base);
-- 
2.17.1

Re: [PATCH v2 3/3] remoteporc: ingenic: Simplify with devm_platform_ioremap_resource_byname()

[Mathieu Poirier]

You still have missed several instances.  I am dropping this set.

On Fri, Sep 06, 2024 at 07:34:05PM +0800, Zhang Zekun wrote:
> platform_get_resource_byname() and devm_ioremap_resource() can be
> replaced by devm_platform_ioremap_resource_byname(), which can
> simplify the code logic a bit, No functional change here.
> 
> Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
> ---
>  drivers/remoteproc/ingenic_rproc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/ingenic_rproc.c b/drivers/remoteproc/ingenic_rproc.c
> index 9902cce28692..1b78d8ddeacf 100644
> --- a/drivers/remoteproc/ingenic_rproc.c
> +++ b/drivers/remoteproc/ingenic_rproc.c
> @@ -183,8 +183,7 @@ static int ingenic_rproc_probe(struct platform_device *pdev)
>  	vpu->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, vpu);
>  
> -	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aux");
> -	vpu->aux_base = devm_ioremap_resource(dev, mem);
> +	vpu->aux_base = devm_platform_ioremap_resource_byname(pdev, "aux");
>  	if (IS_ERR(vpu->aux_base)) {
>  		dev_err(dev, "Failed to ioremap\n");
>  		return PTR_ERR(vpu->aux_base);
> -- 
> 2.17.1
> 

Re: [PATCH v2 3/3] remoteporc: ingenic: Simplify with devm_platform_ioremap_resource_byname()

[zhangzekun (A)]

在 2024/9/7 0:00, Mathieu Poirier 写道:
> 
> You still have missed several instances.  I am dropping this set.
> 
Hi, Mathieu,

I have checked the subsystem again and does not find the missing 
instances that can make such conversion.

Instance like this would require storing the resource size or the 
res->start, so we can not conversion likt that:

------------------------code start---------------------------------
da8xx_remoteproc.c:

res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
                                    mem_names[i]);
drproc->mem[i].cpu_addr = devm_ioremap_resource(dev, res);
if (IS_ERR(drproc->mem[i].cpu_addr)) {
         dev_err(dev, "failed to parse and map %s memory\n",
                 mem_names[i]);
         return PTR_ERR(drproc->mem[i].cpu_addr);
}
drproc->mem[i].bus_addr = res->start;
drproc->mem[i].dev_addr =
                 res->start & DA8XX_RPROC_LOCAL_ADDRESS_MASK;
drproc->mem[i].size = resource_size(res);

------------------------------------------------------------------

I have thought about adding a 
devm_platform_get_and_ioremap_resource_byname() to make conversion for 
these instances, but the function name seems to be too long...


For other instance like below, we will have code logic broken, because 
devm_platform_ioremap_resource_byname() will return error if res is NULL:

---------------------------code start------------------------------
mtk_scp.c:

  /* l1tcm is an optional memory region */
  res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "l1tcm");
  if (res) {
          scp_cluster->l1tcm_base = devm_ioremap_resource(dev, res);
          if (IS_ERR(scp_cluster->l1tcm_base))
                  return dev_err_probe(dev, 
PTR_ERR(scp_cluster->l1tcm_base),
                                       "Failed to map l1tcm memory\n");

          scp_cluster->l1tcm_size = resource_size(res);
          scp_cluster->l1tcm_phys = res->start;
  }
-------------------------------------------------------------------

Best Regards,
Zekun

Re: [PATCH v2 3/3] remoteporc: ingenic: Simplify with devm_platform_ioremap_resource_byname()

[Mathieu Poirier]

On Fri, 6 Sept 2024 at 21:00, zhangzekun (A) <zhangzekun11@huawei.com> wrote:
>
>
>
> 在 2024/9/7 0:00, Mathieu Poirier 写道:
> >
> > You still have missed several instances.  I am dropping this set.
> >
> Hi, Mathieu,
>
> I have checked the subsystem again and does not find the missing
> instances that can make such conversion.
>
> Instance like this would require storing the resource size or the
> res->start, so we can not conversion likt that:
>
> ------------------------code start---------------------------------
> da8xx_remoteproc.c:
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>                                     mem_names[i]);
> drproc->mem[i].cpu_addr = devm_ioremap_resource(dev, res);
> if (IS_ERR(drproc->mem[i].cpu_addr)) {
>          dev_err(dev, "failed to parse and map %s memory\n",
>                  mem_names[i]);
>          return PTR_ERR(drproc->mem[i].cpu_addr);
> }
> drproc->mem[i].bus_addr = res->start;
> drproc->mem[i].dev_addr =
>                  res->start & DA8XX_RPROC_LOCAL_ADDRESS_MASK;
> drproc->mem[i].size = resource_size(res);
>
> ------------------------------------------------------------------
>
> I have thought about adding a
> devm_platform_get_and_ioremap_resource_byname() to make conversion for
> these instances, but the function name seems to be too long...
>
>
> For other instance like below, we will have code logic broken, because
> devm_platform_ioremap_resource_byname() will return error if res is NULL:
>
> ---------------------------code start------------------------------
> mtk_scp.c:
>
>   /* l1tcm is an optional memory region */
>   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "l1tcm");
>   if (res) {
>           scp_cluster->l1tcm_base = devm_ioremap_resource(dev, res);
>           if (IS_ERR(scp_cluster->l1tcm_base))
>                   return dev_err_probe(dev,
> PTR_ERR(scp_cluster->l1tcm_base),
>                                        "Failed to map l1tcm memory\n");
>
>           scp_cluster->l1tcm_size = resource_size(res);
>           scp_cluster->l1tcm_phys = res->start;
>   }
> -------------------------------------------------------------------
>

I see your point and I applied your patches.

Thanks,
Mathieu