| Message ID | 20240906113405.92782-4-zhangzekun11@huawei.com (mailing list archive) |
|---|---|
| State | Queued |
| Headers | show |
| Series | Simplify with devm_platform_ioremap_resource_byname() | expand |
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 >
在 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
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
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);
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(-)