Compile tested against linux-next tree (20121224).
Sachin Kamat (10): drm/exynos: Use devm_kzalloc in exynos_drm_ipp.c drm/exynos: Remove explicit freeing using devm_* APIs in exynos_drm_fimc.c drm/exynos: Remove redundant NULL check drm/exynos: Use devm_clk_get in exynos_drm_fimc.c drm/exynos: Remove unnecessary devm_* freeing APIs in exynos_drm_rotator.c drm/exynos: Remove redundant NULL check in exynos_drm_rotator.c drm/exynos: Use devm_clk_get in exynos_drm_rotator.c drm/exynos: Remove explicit freeing using devm_* APIs in exynos_drm_gsc.c drm/exynos: Remove redundant NULL check in exynos_drm_gsc.c drm/exynos: Use devm_clk_get in exynos_drm_gsc.c
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 78 ++++++--------------------- drivers/gpu/drm/exynos/exynos_drm_gsc.c | 30 ++--------- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 9 +--- drivers/gpu/drm/exynos/exynos_drm_rotator.c | 25 ++------- 4 files changed, 28 insertions(+), 114 deletions(-)
devm_kzalloc makes the code simpler by eliminating the need for explicit freeing.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 9 ++------- 1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index 49eebe9..441b719 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -1895,7 +1895,7 @@ static int __devinit ipp_probe(struct platform_device *pdev) struct exynos_drm_subdrv *subdrv; int ret;
- ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); if (!ctx) return -ENOMEM;
@@ -1916,8 +1916,7 @@ static int __devinit ipp_probe(struct platform_device *pdev) ctx->event_workq = create_singlethread_workqueue("ipp_event"); if (!ctx->event_workq) { dev_err(dev, "failed to create event workqueue\n"); - ret = -EINVAL; - goto err_clear; + return -EINVAL; }
/* @@ -1958,8 +1957,6 @@ err_cmd_workq: destroy_workqueue(ctx->cmd_workq); err_event_workq: destroy_workqueue(ctx->event_workq); -err_clear: - kfree(ctx); return ret; }
@@ -1985,8 +1982,6 @@ static int __devexit ipp_remove(struct platform_device *pdev) destroy_workqueue(ctx->cmd_workq); destroy_workqueue(ctx->event_workq);
- kfree(ctx); - return 0; }
devm_* APIs are device managed and get freed automatically when the device detaches. Thus explicit freeing is not needed. This saves some code.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 30 +++++++++--------------------- 1 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index 61ea242..ab75150 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1742,64 +1742,58 @@ static int __devinit fimc_probe(struct platform_device *pdev) ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc"); if (IS_ERR(ctx->sclk_fimc_clk)) { dev_err(dev, "failed to get src fimc clock.\n"); - ret = PTR_ERR(ctx->sclk_fimc_clk); - goto err_ctx; + return PTR_ERR(ctx->sclk_fimc_clk); } clk_enable(ctx->sclk_fimc_clk);
ctx->fimc_clk = clk_get(dev, "fimc"); if (IS_ERR(ctx->fimc_clk)) { dev_err(dev, "failed to get fimc clock.\n"); - ret = PTR_ERR(ctx->fimc_clk); clk_disable(ctx->sclk_fimc_clk); clk_put(ctx->sclk_fimc_clk); - goto err_ctx; + return PTR_ERR(ctx->fimc_clk); }
ctx->wb_clk = clk_get(dev, "pxl_async0"); if (IS_ERR(ctx->wb_clk)) { dev_err(dev, "failed to get writeback a clock.\n"); - ret = PTR_ERR(ctx->wb_clk); clk_disable(ctx->sclk_fimc_clk); clk_put(ctx->sclk_fimc_clk); clk_put(ctx->fimc_clk); - goto err_ctx; + return PTR_ERR(ctx->wb_clk); }
ctx->wb_b_clk = clk_get(dev, "pxl_async1"); if (IS_ERR(ctx->wb_b_clk)) { dev_err(dev, "failed to get writeback b clock.\n"); - ret = PTR_ERR(ctx->wb_b_clk); clk_disable(ctx->sclk_fimc_clk); clk_put(ctx->sclk_fimc_clk); clk_put(ctx->fimc_clk); clk_put(ctx->wb_clk); - goto err_ctx; + return PTR_ERR(ctx->wb_b_clk); }
parent_clk = clk_get(dev, ddata->parent_clk);
if (IS_ERR(parent_clk)) { dev_err(dev, "failed to get parent clock.\n"); - ret = PTR_ERR(parent_clk); clk_disable(ctx->sclk_fimc_clk); clk_put(ctx->sclk_fimc_clk); clk_put(ctx->fimc_clk); clk_put(ctx->wb_clk); clk_put(ctx->wb_b_clk); - goto err_ctx; + return PTR_ERR(parent_clk); }
if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) { dev_err(dev, "failed to set parent.\n"); - ret = -EINVAL; clk_put(parent_clk); clk_disable(ctx->sclk_fimc_clk); clk_put(ctx->sclk_fimc_clk); clk_put(ctx->fimc_clk); clk_put(ctx->wb_clk); clk_put(ctx->wb_b_clk); - goto err_ctx; + return -EINVAL; }
clk_put(parent_clk); @@ -1825,7 +1819,7 @@ static int __devinit fimc_probe(struct platform_device *pdev) if (!res) { dev_err(dev, "failed to request irq resource.\n"); ret = -ENOENT; - goto err_get_regs; + goto err_clk; }
ctx->irq = res->start; @@ -1833,7 +1827,7 @@ static int __devinit fimc_probe(struct platform_device *pdev) IRQF_ONESHOT, "drm_fimc", ctx); if (ret < 0) { dev_err(dev, "failed to request irq.\n"); - goto err_get_regs; + goto err_clk; }
/* context initailization */ @@ -1879,15 +1873,12 @@ err_ippdrv_register: pm_runtime_disable(dev); err_get_irq: free_irq(ctx->irq, ctx); -err_get_regs: - devm_iounmap(dev, ctx->regs); err_clk: clk_put(ctx->sclk_fimc_clk); clk_put(ctx->fimc_clk); clk_put(ctx->wb_clk); clk_put(ctx->wb_b_clk); -err_ctx: - devm_kfree(dev, ctx); + return ret; }
@@ -1905,15 +1896,12 @@ static int __devexit fimc_remove(struct platform_device *pdev) pm_runtime_disable(dev);
free_irq(ctx->irq, ctx); - devm_iounmap(dev, ctx->regs);
clk_put(ctx->sclk_fimc_clk); clk_put(ctx->fimc_clk); clk_put(ctx->wb_clk); clk_put(ctx->wb_b_clk);
- devm_kfree(dev, ctx); - return 0; }
devm_request_and_ioremap API checks for NULL. Hence explicit NULL check is not necessary. Saves some code.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index ab75150..972aa70 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1801,12 +1801,6 @@ static int __devinit fimc_probe(struct platform_device *pdev)
/* resource memory */ ctx->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!ctx->regs_res) { - dev_err(dev, "failed to find registers.\n"); - ret = -ENOENT; - goto err_clk; - } - ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res); if (!ctx->regs) { dev_err(dev, "failed to map registers.\n");
This eliminates the need for explicit clk_put and makes the cleanup and exit path code simpler.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 46 ++++++----------------------- 1 files changed, 10 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index 972aa70..c4006b8 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1739,64 +1739,50 @@ static int __devinit fimc_probe(struct platform_device *pdev) platform_get_device_id(pdev)->driver_data;
/* clock control */ - ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc"); + ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc"); if (IS_ERR(ctx->sclk_fimc_clk)) { dev_err(dev, "failed to get src fimc clock.\n"); return PTR_ERR(ctx->sclk_fimc_clk); } clk_enable(ctx->sclk_fimc_clk);
- ctx->fimc_clk = clk_get(dev, "fimc"); + ctx->fimc_clk = devm_clk_get(dev, "fimc"); if (IS_ERR(ctx->fimc_clk)) { dev_err(dev, "failed to get fimc clock.\n"); clk_disable(ctx->sclk_fimc_clk); - clk_put(ctx->sclk_fimc_clk); return PTR_ERR(ctx->fimc_clk); }
- ctx->wb_clk = clk_get(dev, "pxl_async0"); + ctx->wb_clk = devm_clk_get(dev, "pxl_async0"); if (IS_ERR(ctx->wb_clk)) { dev_err(dev, "failed to get writeback a clock.\n"); clk_disable(ctx->sclk_fimc_clk); - clk_put(ctx->sclk_fimc_clk); - clk_put(ctx->fimc_clk); return PTR_ERR(ctx->wb_clk); }
- ctx->wb_b_clk = clk_get(dev, "pxl_async1"); + ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1"); if (IS_ERR(ctx->wb_b_clk)) { dev_err(dev, "failed to get writeback b clock.\n"); clk_disable(ctx->sclk_fimc_clk); - clk_put(ctx->sclk_fimc_clk); - clk_put(ctx->fimc_clk); - clk_put(ctx->wb_clk); return PTR_ERR(ctx->wb_b_clk); }
- parent_clk = clk_get(dev, ddata->parent_clk); + parent_clk = devm_clk_get(dev, ddata->parent_clk);
if (IS_ERR(parent_clk)) { dev_err(dev, "failed to get parent clock.\n"); clk_disable(ctx->sclk_fimc_clk); - clk_put(ctx->sclk_fimc_clk); - clk_put(ctx->fimc_clk); - clk_put(ctx->wb_clk); - clk_put(ctx->wb_b_clk); return PTR_ERR(parent_clk); }
if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) { dev_err(dev, "failed to set parent.\n"); - clk_put(parent_clk); + devm_clk_put(dev, parent_clk); clk_disable(ctx->sclk_fimc_clk); - clk_put(ctx->sclk_fimc_clk); - clk_put(ctx->fimc_clk); - clk_put(ctx->wb_clk); - clk_put(ctx->wb_b_clk); return -EINVAL; }
- clk_put(parent_clk); + devm_clk_put(dev, parent_clk); clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate);
/* resource memory */ @@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct platform_device *pdev) ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res); if (!ctx->regs) { dev_err(dev, "failed to map registers.\n"); - ret = -ENXIO; - goto err_clk; + return -ENXIO; }
/* resource irq */ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { dev_err(dev, "failed to request irq resource.\n"); - ret = -ENOENT; - goto err_clk; + return -ENOENT; }
ctx->irq = res->start; @@ -1821,7 +1805,7 @@ static int __devinit fimc_probe(struct platform_device *pdev) IRQF_ONESHOT, "drm_fimc", ctx); if (ret < 0) { dev_err(dev, "failed to request irq.\n"); - goto err_clk; + return ret; }
/* context initailization */ @@ -1867,11 +1851,6 @@ err_ippdrv_register: pm_runtime_disable(dev); err_get_irq: free_irq(ctx->irq, ctx); -err_clk: - clk_put(ctx->sclk_fimc_clk); - clk_put(ctx->fimc_clk); - clk_put(ctx->wb_clk); - clk_put(ctx->wb_b_clk);
return ret; } @@ -1891,11 +1870,6 @@ static int __devexit fimc_remove(struct platform_device *pdev)
free_irq(ctx->irq, ctx);
- clk_put(ctx->sclk_fimc_clk); - clk_put(ctx->fimc_clk); - clk_put(ctx->wb_clk); - clk_put(ctx->wb_b_clk); - return 0; }
2012/12/24 Sachin Kamat sachin.kamat@linaro.org
This eliminates the need for explicit clk_put and makes the cleanup and exit path code simpler.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 46 ++++++----------------------- 1 files changed, 10 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index 972aa70..c4006b8 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1739,64 +1739,50 @@ static int __devinit fimc_probe(struct platform_device *pdev) platform_get_device_id(pdev)->driver_data;
/* clock control */
ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc");
ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc"); if (IS_ERR(ctx->sclk_fimc_clk)) { dev_err(dev, "failed to get src fimc clock.\n"); return PTR_ERR(ctx->sclk_fimc_clk); } clk_enable(ctx->sclk_fimc_clk);
ctx->fimc_clk = clk_get(dev, "fimc");
ctx->fimc_clk = devm_clk_get(dev, "fimc"); if (IS_ERR(ctx->fimc_clk)) { dev_err(dev, "failed to get fimc clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk); return PTR_ERR(ctx->fimc_clk); }
ctx->wb_clk = clk_get(dev, "pxl_async0");
ctx->wb_clk = devm_clk_get(dev, "pxl_async0"); if (IS_ERR(ctx->wb_clk)) { dev_err(dev, "failed to get writeback a clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk); return PTR_ERR(ctx->wb_clk); }
ctx->wb_b_clk = clk_get(dev, "pxl_async1");
ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1"); if (IS_ERR(ctx->wb_b_clk)) { dev_err(dev, "failed to get writeback b clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk); return PTR_ERR(ctx->wb_b_clk); }
parent_clk = clk_get(dev, ddata->parent_clk);
parent_clk = devm_clk_get(dev, ddata->parent_clk); if (IS_ERR(parent_clk)) { dev_err(dev, "failed to get parent clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return PTR_ERR(parent_clk); } if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) { dev_err(dev, "failed to set parent.\n");
clk_put(parent_clk);
devm_clk_put(dev, parent_clk);
remove the above call. is there any reason that devm_clk_put should be called here?
clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return -EINVAL; }
clk_put(parent_clk);
devm_clk_put(dev, parent_clk);
ditto.
clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate); /* resource memory */
@@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct platform_device *pdev) ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res); if (!ctx->regs) { dev_err(dev, "failed to map registers.\n");
ret = -ENXIO;
goto err_clk;
return -ENXIO; } /* resource irq */ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { dev_err(dev, "failed to request irq resource.\n");
ret = -ENOENT;
goto err_clk;
return -ENOENT; } ctx->irq = res->start;
@@ -1821,7 +1805,7 @@ static int __devinit fimc_probe(struct platform_device *pdev) IRQF_ONESHOT, "drm_fimc", ctx); if (ret < 0) { dev_err(dev, "failed to request irq.\n");
goto err_clk;
return ret; } /* context initailization */
@@ -1867,11 +1851,6 @@ err_ippdrv_register: pm_runtime_disable(dev); err_get_irq: free_irq(ctx->irq, ctx); -err_clk:
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return ret;
} @@ -1891,11 +1870,6 @@ static int __devexit fimc_remove(struct platform_device *pdev)
free_irq(ctx->irq, ctx);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk);
return 0;
}
-- 1.7.4.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wednesday, 26 December 2012, Inki Dae inki.dae@samsung.com wrote:
2012/12/24 Sachin Kamat sachin.kamat@linaro.org
This eliminates the need for explicit clk_put and makes the cleanup and exit path code simpler.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 46
++++++-----------------------
1 files changed, 10 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 972aa70..c4006b8 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1739,64 +1739,50 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
platform_get_device_id(pdev)->driver_data; /* clock control */
ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc");
ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc"); if (IS_ERR(ctx->sclk_fimc_clk)) { dev_err(dev, "failed to get src fimc clock.\n"); return PTR_ERR(ctx->sclk_fimc_clk); } clk_enable(ctx->sclk_fimc_clk);
ctx->fimc_clk = clk_get(dev, "fimc");
ctx->fimc_clk = devm_clk_get(dev, "fimc"); if (IS_ERR(ctx->fimc_clk)) { dev_err(dev, "failed to get fimc clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk); return PTR_ERR(ctx->fimc_clk); }
ctx->wb_clk = clk_get(dev, "pxl_async0");
ctx->wb_clk = devm_clk_get(dev, "pxl_async0"); if (IS_ERR(ctx->wb_clk)) { dev_err(dev, "failed to get writeback a clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk); return PTR_ERR(ctx->wb_clk); }
ctx->wb_b_clk = clk_get(dev, "pxl_async1");
ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1"); if (IS_ERR(ctx->wb_b_clk)) { dev_err(dev, "failed to get writeback b clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk); return PTR_ERR(ctx->wb_b_clk); }
parent_clk = clk_get(dev, ddata->parent_clk);
parent_clk = devm_clk_get(dev, ddata->parent_clk); if (IS_ERR(parent_clk)) { dev_err(dev, "failed to get parent clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return PTR_ERR(parent_clk); } if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) { dev_err(dev, "failed to set parent.\n");
clk_put(parent_clk);
devm_clk_put(dev, parent_clk);
remove the above call. is there any reason that devm_clk_put should be
called here?
Devm resources are freed/released automatically only when the device detaches. In the above case the clock was released explicitly (for whatever reasons) earlier. Hence i have used devm call to keep the code logic same as i do not know the behavior if this clock is 'put' when the device detaches instead of here.
clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return -EINVAL; }
clk_put(parent_clk);
devm_clk_put(dev, parent_clk);
ditto.
clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate); /* resource memory */
@@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res); if (!ctx->regs) { dev_err(dev, "failed to map registers.\n");
ret = -ENXIO;
goto err_clk;
return -ENXIO; } /* resource irq */ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { dev_err(dev, "failed to request irq resource.\n");
ret = -ENOENT;
goto err_clk;
return -ENOENT; } ctx->irq = res->start;
@@ -1821,7 +1805,7 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
IRQF_ONESHOT, "drm_fimc", ctx); if (ret < 0) { dev_err(dev, "failed to request irq.\n");
goto err_clk;
return ret; } /* context initailization */
@@ -1867,11 +1851,6 @@ err_ippdrv_register: pm_runtime_disable(dev); err_get_irq: free_irq(ctx->irq, ctx); -err_clk:
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return ret;
} @@ -1891,11 +1870,6 @@ static int __devexit fimc_remove(struct
platform_device *pdev)
free_irq(ctx->irq, ctx);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk);
return 0;
}
-- 1.7.4.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
2012/12/26 Sachin Kamat sachin.kamat@linaro.org
On Wednesday, 26 December 2012, Inki Dae inki.dae@samsung.com wrote:
2012/12/24 Sachin Kamat sachin.kamat@linaro.org
This eliminates the need for explicit clk_put and makes the cleanup and exit path code simpler.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 46
++++++-----------------------
1 files changed, 10 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 972aa70..c4006b8 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1739,64 +1739,50 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
platform_get_device_id(pdev)->driver_data; /* clock control */
ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc");
ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc"); if (IS_ERR(ctx->sclk_fimc_clk)) { dev_err(dev, "failed to get src fimc clock.\n"); return PTR_ERR(ctx->sclk_fimc_clk); } clk_enable(ctx->sclk_fimc_clk);
ctx->fimc_clk = clk_get(dev, "fimc");
ctx->fimc_clk = devm_clk_get(dev, "fimc"); if (IS_ERR(ctx->fimc_clk)) { dev_err(dev, "failed to get fimc clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk); return PTR_ERR(ctx->fimc_clk); }
ctx->wb_clk = clk_get(dev, "pxl_async0");
ctx->wb_clk = devm_clk_get(dev, "pxl_async0"); if (IS_ERR(ctx->wb_clk)) { dev_err(dev, "failed to get writeback a clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk); return PTR_ERR(ctx->wb_clk); }
ctx->wb_b_clk = clk_get(dev, "pxl_async1");
ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1"); if (IS_ERR(ctx->wb_b_clk)) { dev_err(dev, "failed to get writeback b clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk); return PTR_ERR(ctx->wb_b_clk); }
parent_clk = clk_get(dev, ddata->parent_clk);
parent_clk = devm_clk_get(dev, ddata->parent_clk); if (IS_ERR(parent_clk)) { dev_err(dev, "failed to get parent clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return PTR_ERR(parent_clk); } if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) { dev_err(dev, "failed to set parent.\n");
clk_put(parent_clk);
devm_clk_put(dev, parent_clk);
remove the above call. is there any reason that devm_clk_put should be
called here?
Devm resources are freed/released automatically only when the device detaches. In the above case the clock was released explicitly (for whatever reasons) earlier. Hence i have used devm call to keep the code logic same as i do not know the behavior if this clock is 'put' when the device detaches instead of here.
If probe is failed, devm resources are released by devres_release_all(). So that is unnecessary call. Remove it.
clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return -EINVAL; }
clk_put(parent_clk);
devm_clk_put(dev, parent_clk);
ditto.
clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate); /* resource memory */
@@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res); if (!ctx->regs) { dev_err(dev, "failed to map registers.\n");
ret = -ENXIO;
goto err_clk;
return -ENXIO; } /* resource irq */ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { dev_err(dev, "failed to request irq resource.\n");
ret = -ENOENT;
goto err_clk;
return -ENOENT; } ctx->irq = res->start;
@@ -1821,7 +1805,7 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
IRQF_ONESHOT, "drm_fimc", ctx); if (ret < 0) { dev_err(dev, "failed to request irq.\n");
goto err_clk;
return ret; } /* context initailization */
@@ -1867,11 +1851,6 @@ err_ippdrv_register: pm_runtime_disable(dev); err_get_irq: free_irq(ctx->irq, ctx); -err_clk:
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return ret;
} @@ -1891,11 +1870,6 @@ static int __devexit fimc_remove(struct
platform_device *pdev)
free_irq(ctx->irq, ctx);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk);
return 0;
}
-- 1.7.4.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
-- With warm regards, Sachin
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 27 December 2012 15:43, Inki Dae inki.dae@samsung.com wrote:
2012/12/26 Sachin Kamat sachin.kamat@linaro.org
On Wednesday, 26 December 2012, Inki Dae inki.dae@samsung.com wrote:
2012/12/24 Sachin Kamat sachin.kamat@linaro.org
This eliminates the need for explicit clk_put and makes the cleanup and exit path code simpler.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 46 ++++++----------------------- 1 files changed, 10 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index 972aa70..c4006b8 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1739,64 +1739,50 @@ static int __devinit fimc_probe(struct platform_device *pdev) platform_get_device_id(pdev)->driver_data;
/* clock control */
ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc");
ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc"); if (IS_ERR(ctx->sclk_fimc_clk)) { dev_err(dev, "failed to get src fimc clock.\n"); return PTR_ERR(ctx->sclk_fimc_clk); } clk_enable(ctx->sclk_fimc_clk);
ctx->fimc_clk = clk_get(dev, "fimc");
ctx->fimc_clk = devm_clk_get(dev, "fimc"); if (IS_ERR(ctx->fimc_clk)) { dev_err(dev, "failed to get fimc clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk); return PTR_ERR(ctx->fimc_clk); }
ctx->wb_clk = clk_get(dev, "pxl_async0");
ctx->wb_clk = devm_clk_get(dev, "pxl_async0"); if (IS_ERR(ctx->wb_clk)) { dev_err(dev, "failed to get writeback a clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk); return PTR_ERR(ctx->wb_clk); }
ctx->wb_b_clk = clk_get(dev, "pxl_async1");
ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1"); if (IS_ERR(ctx->wb_b_clk)) { dev_err(dev, "failed to get writeback b clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk); return PTR_ERR(ctx->wb_b_clk); }
parent_clk = clk_get(dev, ddata->parent_clk);
parent_clk = devm_clk_get(dev, ddata->parent_clk); if (IS_ERR(parent_clk)) { dev_err(dev, "failed to get parent clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return PTR_ERR(parent_clk); } if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) { dev_err(dev, "failed to set parent.\n");
clk_put(parent_clk);
devm_clk_put(dev, parent_clk);
remove the above call. is there any reason that devm_clk_put should be called here?
Devm resources are freed/released automatically only when the device detaches. In the above case the clock was released explicitly (for whatever reasons) earlier. Hence i have used devm call to keep the code logic same as i do not know the behavior if this clock is 'put' when the device detaches instead of here.
If probe is failed, devm resources are released by devres_release_all(). So that is unnecessary call. Remove it.
In this case you are right. It is not required. I will remove it. But in the below case it is not part of cleanup, so I will keep it. What is your opinion about it?
clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return -EINVAL; }
clk_put(parent_clk);
devm_clk_put(dev, parent_clk);
ditto.
clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate); /* resource memory */
@@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct platform_device *pdev) ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res); if (!ctx->regs) { dev_err(dev, "failed to map registers.\n");
ret = -ENXIO;
goto err_clk;
return -ENXIO; } /* resource irq */ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { dev_err(dev, "failed to request irq resource.\n");
ret = -ENOENT;
goto err_clk;
return -ENOENT; } ctx->irq = res->start;
@@ -1821,7 +1805,7 @@ static int __devinit fimc_probe(struct platform_device *pdev) IRQF_ONESHOT, "drm_fimc", ctx); if (ret < 0) { dev_err(dev, "failed to request irq.\n");
goto err_clk;
return ret; } /* context initailization */
@@ -1867,11 +1851,6 @@ err_ippdrv_register: pm_runtime_disable(dev); err_get_irq: free_irq(ctx->irq, ctx); -err_clk:
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return ret;
} @@ -1891,11 +1870,6 @@ static int __devexit fimc_remove(struct platform_device *pdev)
free_irq(ctx->irq, ctx);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk);
return 0;
}
-- 1.7.4.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
-- With warm regards, Sachin
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wednesday, 26 December 2012, Inki Dae inki.dae@samsung.com wrote:
2012/12/24 Sachin Kamat sachin.kamat@linaro.org
This eliminates the need for explicit clk_put and makes the cleanup and exit path code simpler.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 46
++++++-----------------------
1 files changed, 10 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 972aa70..c4006b8 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1739,64 +1739,50 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
platform_get_device_id(pdev)->driver_data; /* clock control */
ctx->sclk_fimc_clk = clk_get(dev, "sclk_fimc");
ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc"); if (IS_ERR(ctx->sclk_fimc_clk)) { dev_err(dev, "failed to get src fimc clock.\n"); return PTR_ERR(ctx->sclk_fimc_clk); } clk_enable(ctx->sclk_fimc_clk);
ctx->fimc_clk = clk_get(dev, "fimc");
ctx->fimc_clk = devm_clk_get(dev, "fimc"); if (IS_ERR(ctx->fimc_clk)) { dev_err(dev, "failed to get fimc clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk); return PTR_ERR(ctx->fimc_clk); }
ctx->wb_clk = clk_get(dev, "pxl_async0");
ctx->wb_clk = devm_clk_get(dev, "pxl_async0"); if (IS_ERR(ctx->wb_clk)) { dev_err(dev, "failed to get writeback a clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk); return PTR_ERR(ctx->wb_clk); }
ctx->wb_b_clk = clk_get(dev, "pxl_async1");
ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1"); if (IS_ERR(ctx->wb_b_clk)) { dev_err(dev, "failed to get writeback b clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk); return PTR_ERR(ctx->wb_b_clk); }
parent_clk = clk_get(dev, ddata->parent_clk);
parent_clk = devm_clk_get(dev, ddata->parent_clk); if (IS_ERR(parent_clk)) { dev_err(dev, "failed to get parent clock.\n"); clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return PTR_ERR(parent_clk); } if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) { dev_err(dev, "failed to set parent.\n");
clk_put(parent_clk);
devm_clk_put(dev, parent_clk);
remove the above call. is there any reason that devm_clk_put should be
called here?
Devm resources are freed/released automatically only when the device detaches. In the above case the clock was released explicitly (for whatever reasons) earlier. Hence i have used devm call to keep the code logic same as i do not know the behavior if this clock is 'put' when the device detaches instead of here.
clk_disable(ctx->sclk_fimc_clk);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return -EINVAL; }
clk_put(parent_clk);
devm_clk_put(dev, parent_clk);
ditto.
clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate); /* resource memory */
@@ -1804,16 +1790,14 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res); if (!ctx->regs) { dev_err(dev, "failed to map registers.\n");
ret = -ENXIO;
goto err_clk;
return -ENXIO; } /* resource irq */ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { dev_err(dev, "failed to request irq resource.\n");
ret = -ENOENT;
goto err_clk;
return -ENOENT; } ctx->irq = res->start;
@@ -1821,7 +1805,7 @@ static int __devinit fimc_probe(struct
platform_device *pdev)
IRQF_ONESHOT, "drm_fimc", ctx); if (ret < 0) { dev_err(dev, "failed to request irq.\n");
goto err_clk;
return ret; } /* context initailization */
@@ -1867,11 +1851,6 @@ err_ippdrv_register: pm_runtime_disable(dev); err_get_irq: free_irq(ctx->irq, ctx); -err_clk:
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk); return ret;
} @@ -1891,11 +1870,6 @@ static int __devexit fimc_remove(struct
platform_device *pdev)
free_irq(ctx->irq, ctx);
clk_put(ctx->sclk_fimc_clk);
clk_put(ctx->fimc_clk);
clk_put(ctx->wb_clk);
clk_put(ctx->wb_b_clk);
return 0;
}
-- 1.7.4.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
devm_* APIs are device managed and get freed automatically when the device detaches. Thus explicit freeing is not needed. This saves some code.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_rotator.c | 18 ++++-------------- 1 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c index 1c23660..0f168449 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c +++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c @@ -657,29 +657,26 @@ static int __devinit rotator_probe(struct platform_device *pdev) rot->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!rot->regs_res) { dev_err(dev, "failed to find registers\n"); - ret = -ENOENT; - goto err_get_resource; + return -ENOENT; }
rot->regs = devm_request_and_ioremap(dev, rot->regs_res); if (!rot->regs) { dev_err(dev, "failed to map register\n"); - ret = -ENXIO; - goto err_get_resource; + return -ENXIO; }
rot->irq = platform_get_irq(pdev, 0); if (rot->irq < 0) { dev_err(dev, "failed to get irq\n"); - ret = rot->irq; - goto err_get_irq; + return rot->irq; }
ret = request_threaded_irq(rot->irq, NULL, rotator_irq_handler, IRQF_ONESHOT, "drm_rotator", rot); if (ret < 0) { dev_err(dev, "failed to request irq\n"); - goto err_get_irq; + return ret; }
rot->clock = clk_get(dev, "rotator"); @@ -723,10 +720,6 @@ err_ippdrv_register: clk_put(rot->clock); err_clk_get: free_irq(rot->irq, rot); -err_get_irq: - devm_iounmap(dev, rot->regs); -err_get_resource: - devm_kfree(dev, rot); return ret; }
@@ -743,9 +736,6 @@ static int __devexit rotator_remove(struct platform_device *pdev) clk_put(rot->clock);
free_irq(rot->irq, rot); - devm_iounmap(dev, rot->regs); - - devm_kfree(dev, rot);
return 0; }
devm_request_and_ioremap API checks for NULL. Hence explicit NULL check is not necessary. Saves some code.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_rotator.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c index 0f168449..4505163 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c +++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c @@ -655,11 +655,6 @@ static int __devinit rotator_probe(struct platform_device *pdev) platform_get_device_id(pdev)->driver_data;
rot->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!rot->regs_res) { - dev_err(dev, "failed to find registers\n"); - return -ENOENT; - } - rot->regs = devm_request_and_ioremap(dev, rot->regs_res); if (!rot->regs) { dev_err(dev, "failed to map register\n");
This eliminates the need for explicit clk_put and makes the cleanup and exit path code simpler.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_rotator.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c index 4505163..484c6bd 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c +++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c @@ -674,7 +674,7 @@ static int __devinit rotator_probe(struct platform_device *pdev) return ret; }
- rot->clock = clk_get(dev, "rotator"); + rot->clock = devm_clk_get(dev, "rotator"); if (IS_ERR_OR_NULL(rot->clock)) { dev_err(dev, "failed to get clock\n"); ret = PTR_ERR(rot->clock); @@ -712,7 +712,6 @@ static int __devinit rotator_probe(struct platform_device *pdev) err_ippdrv_register: devm_kfree(dev, ippdrv->prop_list); pm_runtime_disable(dev); - clk_put(rot->clock); err_clk_get: free_irq(rot->irq, rot); return ret; @@ -728,7 +727,6 @@ static int __devexit rotator_remove(struct platform_device *pdev) exynos_drm_ippdrv_unregister(ippdrv);
pm_runtime_disable(dev); - clk_put(rot->clock);
free_irq(rot->irq, rot);
devm_* APIs are device managed and get freed automatically when the device detaches. Thus explicit freeing is not needed. This saves some code.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_gsc.c | 15 +++------------ 1 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c index 5639353..3b47a7d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c @@ -1699,8 +1699,7 @@ static int __devinit gsc_probe(struct platform_device *pdev) ctx->gsc_clk = clk_get(dev, "gscl"); if (IS_ERR(ctx->gsc_clk)) { dev_err(dev, "failed to get gsc clock.\n"); - ret = PTR_ERR(ctx->gsc_clk); - goto err_ctx; + return PTR_ERR(ctx->gsc_clk); }
/* resource memory */ @@ -1723,7 +1722,7 @@ static int __devinit gsc_probe(struct platform_device *pdev) if (!res) { dev_err(dev, "failed to request irq resource.\n"); ret = -ENOENT; - goto err_get_regs; + goto err_clk; }
ctx->irq = res->start; @@ -1731,7 +1730,7 @@ static int __devinit gsc_probe(struct platform_device *pdev) IRQF_ONESHOT, "drm_gsc", ctx); if (ret < 0) { dev_err(dev, "failed to request irq.\n"); - goto err_get_regs; + goto err_clk; }
/* context initailization */ @@ -1775,12 +1774,8 @@ err_ippdrv_register: pm_runtime_disable(dev); err_get_irq: free_irq(ctx->irq, ctx); -err_get_regs: - devm_iounmap(dev, ctx->regs); err_clk: clk_put(ctx->gsc_clk); -err_ctx: - devm_kfree(dev, ctx); return ret; }
@@ -1798,12 +1793,8 @@ static int __devexit gsc_remove(struct platform_device *pdev) pm_runtime_disable(dev);
free_irq(ctx->irq, ctx); - devm_iounmap(dev, ctx->regs); - clk_put(ctx->gsc_clk);
- devm_kfree(dev, ctx); - return 0; }
devm_request_and_ioremap API checks for NULL. Hence explicit NULL check is not necessary. Saves some code.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_gsc.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c index 3b47a7d..e21a0d9 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c @@ -1704,12 +1704,6 @@ static int __devinit gsc_probe(struct platform_device *pdev)
/* resource memory */ ctx->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!ctx->regs_res) { - dev_err(dev, "failed to find registers.\n"); - ret = -ENOENT; - goto err_clk; - } - ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res); if (!ctx->regs) { dev_err(dev, "failed to map registers.\n");
This eliminates the need for explicit clk_put and makes the cleanup and exit path code simpler.
Cc: Eunchul Kim chulspro.kim@samsung.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_gsc.c | 13 ++++--------- 1 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c index e21a0d9..0497e90 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c @@ -1696,7 +1696,7 @@ static int __devinit gsc_probe(struct platform_device *pdev) return -ENOMEM;
/* clock control */ - ctx->gsc_clk = clk_get(dev, "gscl"); + ctx->gsc_clk = devm_clk_get(dev, "gscl"); if (IS_ERR(ctx->gsc_clk)) { dev_err(dev, "failed to get gsc clock.\n"); return PTR_ERR(ctx->gsc_clk); @@ -1707,16 +1707,14 @@ static int __devinit gsc_probe(struct platform_device *pdev) ctx->regs = devm_request_and_ioremap(dev, ctx->regs_res); if (!ctx->regs) { dev_err(dev, "failed to map registers.\n"); - ret = -ENXIO; - goto err_clk; + return -ENXIO; }
/* resource irq */ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { dev_err(dev, "failed to request irq resource.\n"); - ret = -ENOENT; - goto err_clk; + return -ENOENT; }
ctx->irq = res->start; @@ -1724,7 +1722,7 @@ static int __devinit gsc_probe(struct platform_device *pdev) IRQF_ONESHOT, "drm_gsc", ctx); if (ret < 0) { dev_err(dev, "failed to request irq.\n"); - goto err_clk; + return ret; }
/* context initailization */ @@ -1768,8 +1766,6 @@ err_ippdrv_register: pm_runtime_disable(dev); err_get_irq: free_irq(ctx->irq, ctx); -err_clk: - clk_put(ctx->gsc_clk); return ret; }
@@ -1787,7 +1783,6 @@ static int __devexit gsc_remove(struct platform_device *pdev) pm_runtime_disable(dev);
free_irq(ctx->irq, ctx); - clk_put(ctx->gsc_clk);
return 0; }
dri-devel@lists.freedesktop.org