dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] drm/exynos: Bug fix and devm_* usage
@ 2012-11-23  3:41 Sachin Kamat
  2012-11-23  3:41 ` [PATCH 1/5] drm/exynos: Use devm_clk_get in exynos_drm_fimd.c Sachin Kamat
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Sachin Kamat @ 2012-11-23  3:41 UTC (permalink / raw)
  To: dri-devel; +Cc: patches, sachin.kamat

First 4 patches use devm_* APIs for simpler code and cleanup.
Last one fixes a potential bug.

Series is build tested and based on the exynos-drm-next branch of the
following tree:
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

Sachin Kamat (5):
  drm/exynos: Use devm_clk_get in exynos_drm_fimd.c
  drm/exynos: Use devm_gpio_request in exynos_hdmi.c
  drm/exynos: Use devm_clk_get in exynos_mixer.c
  drm/exynos: Use devm_clk_get in exynos_drm_g2d.c
  drm/exynos: Fix potential NULL pointer dereference

 drivers/gpu/drm/exynos/exynos_drm_fimd.c |    9 ++-------
 drivers/gpu/drm/exynos/exynos_drm_g2d.c  |    6 ++----
 drivers/gpu/drm/exynos/exynos_hdmi.c     |    8 ++------
 drivers/gpu/drm/exynos/exynos_mixer.c    |   20 +++++---------------
 4 files changed, 11 insertions(+), 32 deletions(-)

-- 
1.7.4.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/5] drm/exynos: Use devm_clk_get in exynos_drm_fimd.c
  2012-11-23  3:41 [PATCH 0/5] drm/exynos: Bug fix and devm_* usage Sachin Kamat
@ 2012-11-23  3:41 ` Sachin Kamat
  2012-11-23  6:40   ` Inki Dae
  2012-11-23  3:41 ` [PATCH 2/5] drm/exynos: Use devm_gpio_request in exynos_hdmi.c Sachin Kamat
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Sachin Kamat @ 2012-11-23  3:41 UTC (permalink / raw)
  To: dri-devel; +Cc: patches, sachin.kamat

devm_clk_get is device managed and makes error handling and exit code
simpler.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index ad04edd..57851cc 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -846,14 +846,14 @@ static int __devinit fimd_probe(struct platform_device *pdev)
 	if (!ctx)
 		return -ENOMEM;
 
-	ctx->bus_clk = clk_get(dev, "fimd");
+	ctx->bus_clk = devm_clk_get(dev, "fimd");
 	if (IS_ERR(ctx->bus_clk)) {
 		dev_err(dev, "failed to get bus clock\n");
 		ret = PTR_ERR(ctx->bus_clk);
 		goto err_clk_get;
 	}
 
-	ctx->lcd_clk = clk_get(dev, "sclk_fimd");
+	ctx->lcd_clk = devm_clk_get(dev, "sclk_fimd");
 	if (IS_ERR(ctx->lcd_clk)) {
 		dev_err(dev, "failed to get lcd clock\n");
 		ret = PTR_ERR(ctx->lcd_clk);
@@ -918,11 +918,9 @@ static int __devinit fimd_probe(struct platform_device *pdev)
 
 err_clk:
 	clk_disable(ctx->lcd_clk);
-	clk_put(ctx->lcd_clk);
 
 err_bus_clk:
 	clk_disable(ctx->bus_clk);
-	clk_put(ctx->bus_clk);
 
 err_clk_get:
 	return ret;
@@ -949,9 +947,6 @@ static int __devexit fimd_remove(struct platform_device *pdev)
 out:
 	pm_runtime_disable(dev);
 
-	clk_put(ctx->lcd_clk);
-	clk_put(ctx->bus_clk);
-
 	return 0;
 }
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/5] drm/exynos: Use devm_gpio_request in exynos_hdmi.c
  2012-11-23  3:41 [PATCH 0/5] drm/exynos: Bug fix and devm_* usage Sachin Kamat
  2012-11-23  3:41 ` [PATCH 1/5] drm/exynos: Use devm_clk_get in exynos_drm_fimd.c Sachin Kamat
@ 2012-11-23  3:41 ` Sachin Kamat
  2012-11-23  6:51   ` Inki Dae
  2012-11-23  3:41 ` [PATCH 3/5] drm/exynos: Use devm_clk_get in exynos_mixer.c Sachin Kamat
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Sachin Kamat @ 2012-11-23  3:41 UTC (permalink / raw)
  To: dri-devel; +Cc: patches, sachin.kamat

devm_gpio_request is device managed and makes error handling and exit code
simpler.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 59839cc..3fe2d61 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -2428,7 +2428,7 @@ static int __devinit hdmi_probe(struct platform_device *pdev)
 		goto err_resource;
 	}
 
-	ret = gpio_request(hdata->hpd_gpio, "HPD");
+	ret = devm_gpio_request(&pdev->dev, hdata->hpd_gpio, "HPD");
 	if (ret) {
 		DRM_ERROR("failed to request HPD gpio\n");
 		goto err_resource;
@@ -2438,7 +2438,7 @@ static int __devinit hdmi_probe(struct platform_device *pdev)
 	if (i2c_add_driver(&ddc_driver)) {
 		DRM_ERROR("failed to register ddc i2c driver\n");
 		ret = -ENOENT;
-		goto err_gpio;
+		goto err_resource;
 	}
 
 	hdata->ddc_port = hdmi_ddc;
@@ -2501,8 +2501,6 @@ err_hdmiphy:
 	i2c_del_driver(&hdmiphy_driver);
 err_ddc:
 	i2c_del_driver(&ddc_driver);
-err_gpio:
-	gpio_free(hdata->hpd_gpio);
 err_resource:
 	hdmi_resources_cleanup(hdata);
 err_data:
@@ -2522,8 +2520,6 @@ static int __devexit hdmi_remove(struct platform_device *pdev)
 	free_irq(hdata->internal_irq, hdata);
 	free_irq(hdata->external_irq, hdata);
 
-	gpio_free(hdata->hpd_gpio);
-
 	hdmi_resources_cleanup(hdata);
 
 	/* hdmiphy i2c driver */
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/5] drm/exynos: Use devm_clk_get in exynos_mixer.c
  2012-11-23  3:41 [PATCH 0/5] drm/exynos: Bug fix and devm_* usage Sachin Kamat
  2012-11-23  3:41 ` [PATCH 1/5] drm/exynos: Use devm_clk_get in exynos_drm_fimd.c Sachin Kamat
  2012-11-23  3:41 ` [PATCH 2/5] drm/exynos: Use devm_gpio_request in exynos_hdmi.c Sachin Kamat
@ 2012-11-23  3:41 ` Sachin Kamat
  2012-11-23  7:02   ` Inki Dae
  2012-11-23  3:41 ` [PATCH 4/5] drm/exynos: Use devm_clk_get in exynos_drm_g2d.c Sachin Kamat
  2012-11-23  3:41 ` [PATCH 5/5] drm/exynos: Fix potential NULL pointer dereference Sachin Kamat
  4 siblings, 1 reply; 13+ messages in thread
From: Sachin Kamat @ 2012-11-23  3:41 UTC (permalink / raw)
  To: dri-devel; +Cc: patches, sachin.kamat

devm_clk_get is device managed and makes error handling and exit code
simpler.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/gpu/drm/exynos/exynos_mixer.c |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 0d3ed28..88fcb40 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -962,14 +962,14 @@ static int __devinit mixer_resources_init(struct exynos_drm_hdmi_context *ctx,
 
 	spin_lock_init(&mixer_res->reg_slock);
 
-	mixer_res->mixer = clk_get(dev, "mixer");
+	mixer_res->mixer = devm_clk_get(dev, "mixer");
 	if (IS_ERR_OR_NULL(mixer_res->mixer)) {
 		dev_err(dev, "failed to get clock 'mixer'\n");
 		ret = -ENODEV;
 		goto fail;
 	}
 
-	mixer_res->sclk_hdmi = clk_get(dev, "sclk_hdmi");
+	mixer_res->sclk_hdmi = devm_clk_get(dev, "sclk_hdmi");
 	if (IS_ERR_OR_NULL(mixer_res->sclk_hdmi)) {
 		dev_err(dev, "failed to get clock 'sclk_hdmi'\n");
 		ret = -ENODEV;
@@ -1008,10 +1008,6 @@ static int __devinit mixer_resources_init(struct exynos_drm_hdmi_context *ctx,
 	return 0;
 
 fail:
-	if (!IS_ERR_OR_NULL(mixer_res->sclk_hdmi))
-		clk_put(mixer_res->sclk_hdmi);
-	if (!IS_ERR_OR_NULL(mixer_res->mixer))
-		clk_put(mixer_res->mixer);
 	return ret;
 }
 
@@ -1024,19 +1020,19 @@ static int __devinit vp_resources_init(struct exynos_drm_hdmi_context *ctx,
 	struct resource *res;
 	int ret;
 
-	mixer_res->vp = clk_get(dev, "vp");
+	mixer_res->vp = devm_clk_get(dev, "vp");
 	if (IS_ERR_OR_NULL(mixer_res->vp)) {
 		dev_err(dev, "failed to get clock 'vp'\n");
 		ret = -ENODEV;
 		goto fail;
 	}
-	mixer_res->sclk_mixer = clk_get(dev, "sclk_mixer");
+	mixer_res->sclk_mixer = devm_clk_get(dev, "sclk_mixer");
 	if (IS_ERR_OR_NULL(mixer_res->sclk_mixer)) {
 		dev_err(dev, "failed to get clock 'sclk_mixer'\n");
 		ret = -ENODEV;
 		goto fail;
 	}
-	mixer_res->sclk_dac = clk_get(dev, "sclk_dac");
+	mixer_res->sclk_dac = devm_clk_get(dev, "sclk_dac");
 	if (IS_ERR_OR_NULL(mixer_res->sclk_dac)) {
 		dev_err(dev, "failed to get clock 'sclk_dac'\n");
 		ret = -ENODEV;
@@ -1064,12 +1060,6 @@ static int __devinit vp_resources_init(struct exynos_drm_hdmi_context *ctx,
 	return 0;
 
 fail:
-	if (!IS_ERR_OR_NULL(mixer_res->sclk_dac))
-		clk_put(mixer_res->sclk_dac);
-	if (!IS_ERR_OR_NULL(mixer_res->sclk_mixer))
-		clk_put(mixer_res->sclk_mixer);
-	if (!IS_ERR_OR_NULL(mixer_res->vp))
-		clk_put(mixer_res->vp);
 	return ret;
 }
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/5] drm/exynos: Use devm_clk_get in exynos_drm_g2d.c
  2012-11-23  3:41 [PATCH 0/5] drm/exynos: Bug fix and devm_* usage Sachin Kamat
                   ` (2 preceding siblings ...)
  2012-11-23  3:41 ` [PATCH 3/5] drm/exynos: Use devm_clk_get in exynos_mixer.c Sachin Kamat
@ 2012-11-23  3:41 ` Sachin Kamat
  2012-11-23  7:06   ` Inki Dae
  2012-11-23  3:41 ` [PATCH 5/5] drm/exynos: Fix potential NULL pointer dereference Sachin Kamat
  4 siblings, 1 reply; 13+ messages in thread
From: Sachin Kamat @ 2012-11-23  3:41 UTC (permalink / raw)
  To: dri-devel; +Cc: patches, sachin.kamat

devm_clk_get is device managed and makes error handling and exit code
simpler.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index a9002ad..c1054cb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -1125,7 +1125,7 @@ static int __devinit g2d_probe(struct platform_device *pdev)
 	mutex_init(&g2d->cmdlist_mutex);
 	mutex_init(&g2d->runqueue_mutex);
 
-	g2d->gate_clk = clk_get(dev, "fimg2d");
+	g2d->gate_clk = devm_clk_get(dev, "fimg2d");
 	if (IS_ERR(g2d->gate_clk)) {
 		dev_err(dev, "failed to get gate clock\n");
 		ret = PTR_ERR(g2d->gate_clk);
@@ -1181,7 +1181,6 @@ static int __devinit g2d_probe(struct platform_device *pdev)
 
 err_put_clk:
 	pm_runtime_disable(dev);
-	clk_put(g2d->gate_clk);
 err_destroy_workqueue:
 	destroy_workqueue(g2d->g2d_workq);
 err_destroy_slab:
@@ -1202,7 +1201,6 @@ static int __devexit g2d_remove(struct platform_device *pdev)
 	}
 
 	pm_runtime_disable(&pdev->dev);
-	clk_put(g2d->gate_clk);
 
 	g2d_fini_cmdlist(g2d);
 	destroy_workqueue(g2d->g2d_workq);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/5] drm/exynos: Fix potential NULL pointer dereference
  2012-11-23  3:41 [PATCH 0/5] drm/exynos: Bug fix and devm_* usage Sachin Kamat
                   ` (3 preceding siblings ...)
  2012-11-23  3:41 ` [PATCH 4/5] drm/exynos: Use devm_clk_get in exynos_drm_g2d.c Sachin Kamat
@ 2012-11-23  3:41 ` Sachin Kamat
  2012-11-23  7:08   ` Inki Dae
  4 siblings, 1 reply; 13+ messages in thread
From: Sachin Kamat @ 2012-11-23  3:41 UTC (permalink / raw)
  To: dri-devel; +Cc: patches, sachin.kamat

Pointer was being dereferenced after freeing.

Fixes the following error:
drivers/gpu/drm/exynos/exynos_drm_g2d.c:323 g2d_userptr_put_dma_addr() error:
dereferencing freed memory 'g2d_userptr'

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index c1054cb..6ffa076 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -319,8 +319,8 @@ out:
 	g2d_userptr->sgt = NULL;
 
 	kfree(g2d_userptr->pages);
-	kfree(g2d_userptr);
 	g2d_userptr->pages = NULL;
+	kfree(g2d_userptr);
 	g2d_userptr = NULL;
 }
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/5] drm/exynos: Use devm_clk_get in exynos_drm_fimd.c
  2012-11-23  3:41 ` [PATCH 1/5] drm/exynos: Use devm_clk_get in exynos_drm_fimd.c Sachin Kamat
@ 2012-11-23  6:40   ` Inki Dae
  2012-11-23  6:43     ` Sachin Kamat
  0 siblings, 1 reply; 13+ messages in thread
From: Inki Dae @ 2012-11-23  6:40 UTC (permalink / raw)
  To: 'Sachin Kamat', dri-devel; +Cc: patches



> -----Original Message-----
> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
> Sent: Friday, November 23, 2012 12:42 PM
> To: dri-devel@lists.freedesktop.org
> Cc: inki.dae@samsung.com; jy0922.shim@samsung.com; airlied@linux.ie;
> sachin.kamat@linaro.org; patches@linaro.org
> Subject: [PATCH 1/5] drm/exynos: Use devm_clk_get in exynos_drm_fimd.c
> 
> devm_clk_get is device managed and makes error handling and exit code
> simpler.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |    9 ++-------
>  1 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index ad04edd..57851cc 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -846,14 +846,14 @@ static int __devinit fimd_probe(struct
> platform_device *pdev)
>  	if (!ctx)
>  		return -ENOMEM;
> 
> -	ctx->bus_clk = clk_get(dev, "fimd");
> +	ctx->bus_clk = devm_clk_get(dev, "fimd");
>  	if (IS_ERR(ctx->bus_clk)) {
>  		dev_err(dev, "failed to get bus clock\n");
>  		ret = PTR_ERR(ctx->bus_clk);
>  		goto err_clk_get;
>  	}
> 
> -	ctx->lcd_clk = clk_get(dev, "sclk_fimd");
> +	ctx->lcd_clk = devm_clk_get(dev, "sclk_fimd");
>  	if (IS_ERR(ctx->lcd_clk)) {
>  		dev_err(dev, "failed to get lcd clock\n");
>  		ret = PTR_ERR(ctx->lcd_clk);
> @@ -918,11 +918,9 @@ static int __devinit fimd_probe(struct
> platform_device *pdev)
> 
>  err_clk:
>  	clk_disable(ctx->lcd_clk);
> -	clk_put(ctx->lcd_clk);

clk_disable shouldn't be called here so let's cleanup this also to reduce
unnecessary cleanup commits.

> 
>  err_bus_clk:
>  	clk_disable(ctx->bus_clk);
> -	clk_put(ctx->bus_clk);
> 

Ditto.

Thanks,
Inki Dae

>  err_clk_get:
>  	return ret;
> @@ -949,9 +947,6 @@ static int __devexit fimd_remove(struct
> platform_device *pdev)
>  out:
>  	pm_runtime_disable(dev);
> 
> -	clk_put(ctx->lcd_clk);
> -	clk_put(ctx->bus_clk);
> -
>  	return 0;
>  }
> 
> --
> 1.7.4.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/5] drm/exynos: Use devm_clk_get in exynos_drm_fimd.c
  2012-11-23  6:40   ` Inki Dae
@ 2012-11-23  6:43     ` Sachin Kamat
  0 siblings, 0 replies; 13+ messages in thread
From: Sachin Kamat @ 2012-11-23  6:43 UTC (permalink / raw)
  To: Inki Dae; +Cc: dri-devel, patches

On 23 November 2012 12:10, Inki Dae <inki.dae@samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
>> Sent: Friday, November 23, 2012 12:42 PM
>> To: dri-devel@lists.freedesktop.org
>> Cc: inki.dae@samsung.com; jy0922.shim@samsung.com; airlied@linux.ie;
>> sachin.kamat@linaro.org; patches@linaro.org
>> Subject: [PATCH 1/5] drm/exynos: Use devm_clk_get in exynos_drm_fimd.c
>>
>> devm_clk_get is device managed and makes error handling and exit code
>> simpler.
>>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |    9 ++-------
>>  1 files changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index ad04edd..57851cc 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -846,14 +846,14 @@ static int __devinit fimd_probe(struct
>> platform_device *pdev)
>>       if (!ctx)
>>               return -ENOMEM;
>>
>> -     ctx->bus_clk = clk_get(dev, "fimd");
>> +     ctx->bus_clk = devm_clk_get(dev, "fimd");
>>       if (IS_ERR(ctx->bus_clk)) {
>>               dev_err(dev, "failed to get bus clock\n");
>>               ret = PTR_ERR(ctx->bus_clk);
>>               goto err_clk_get;
>>       }
>>
>> -     ctx->lcd_clk = clk_get(dev, "sclk_fimd");
>> +     ctx->lcd_clk = devm_clk_get(dev, "sclk_fimd");
>>       if (IS_ERR(ctx->lcd_clk)) {
>>               dev_err(dev, "failed to get lcd clock\n");
>>               ret = PTR_ERR(ctx->lcd_clk);
>> @@ -918,11 +918,9 @@ static int __devinit fimd_probe(struct
>> platform_device *pdev)
>>
>>  err_clk:
>>       clk_disable(ctx->lcd_clk);
>> -     clk_put(ctx->lcd_clk);
>
> clk_disable shouldn't be called here so let's cleanup this also to reduce
> unnecessary cleanup commits.
>
>>
>>  err_bus_clk:
>>       clk_disable(ctx->bus_clk);
>> -     clk_put(ctx->bus_clk);
>>
>
> Ditto.

OK. I will remove it. Do you want me to re-send only this patch or the
entire series?

>
> Thanks,
> Inki Dae
>
>>  err_clk_get:
>>       return ret;
>> @@ -949,9 +947,6 @@ static int __devexit fimd_remove(struct
>> platform_device *pdev)
>>  out:
>>       pm_runtime_disable(dev);
>>
>> -     clk_put(ctx->lcd_clk);
>> -     clk_put(ctx->bus_clk);
>> -
>>       return 0;
>>  }
>>
>> --
>> 1.7.4.1
>



-- 
With warm regards,
Sachin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 2/5] drm/exynos: Use devm_gpio_request in exynos_hdmi.c
  2012-11-23  3:41 ` [PATCH 2/5] drm/exynos: Use devm_gpio_request in exynos_hdmi.c Sachin Kamat
@ 2012-11-23  6:51   ` Inki Dae
  2012-11-23  7:03     ` Sachin Kamat
  0 siblings, 1 reply; 13+ messages in thread
From: Inki Dae @ 2012-11-23  6:51 UTC (permalink / raw)
  To: 'Sachin Kamat', dri-devel; +Cc: patches



> -----Original Message-----
> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
> Sent: Friday, November 23, 2012 12:42 PM
> To: dri-devel@lists.freedesktop.org
> Cc: inki.dae@samsung.com; jy0922.shim@samsung.com; airlied@linux.ie;
> sachin.kamat@linaro.org; patches@linaro.org
> Subject: [PATCH 2/5] drm/exynos: Use devm_gpio_request in exynos_hdmi.c
> 
> devm_gpio_request is device managed and makes error handling and exit code
> simpler.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c |    8 ++------
>  1 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 59839cc..3fe2d61 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -2428,7 +2428,7 @@ static int __devinit hdmi_probe(struct
> platform_device *pdev)
>  		goto err_resource;
>  	}
> 
> -	ret = gpio_request(hdata->hpd_gpio, "HPD");
> +	ret = devm_gpio_request(&pdev->dev, hdata->hpd_gpio, "HPD");
>  	if (ret) {
>  		DRM_ERROR("failed to request HPD gpio\n");
>  		goto err_resource;
> @@ -2438,7 +2438,7 @@ static int __devinit hdmi_probe(struct
> platform_device *pdev)
>  	if (i2c_add_driver(&ddc_driver)) {
>  		DRM_ERROR("failed to register ddc i2c driver\n");
>  		ret = -ENOENT;
> -		goto err_gpio;
> +		goto err_resource;
>  	}
> 
>  	hdata->ddc_port = hdmi_ddc;
> @@ -2501,8 +2501,6 @@ err_hdmiphy:
>  	i2c_del_driver(&hdmiphy_driver);
>  err_ddc:
>  	i2c_del_driver(&ddc_driver);
> -err_gpio:
> -	gpio_free(hdata->hpd_gpio);
>  err_resource:
>  	hdmi_resources_cleanup(hdata);

With cleanup to hdmi_resources_init, we can remove hdmi_resource_cleanup
function. So could you please re-send updated patches?

You've been submitting patches into too small pieces.
Because they are trivial enough and in the same context, please merge this
kind of patches as long as they are related with the same class or topics. I
think you can combine them. :)

Thanks,
Inki Dae

>  err_data:
> @@ -2522,8 +2520,6 @@ static int __devexit hdmi_remove(struct
> platform_device *pdev)
>  	free_irq(hdata->internal_irq, hdata);
>  	free_irq(hdata->external_irq, hdata);
> 
> -	gpio_free(hdata->hpd_gpio);
> -
>  	hdmi_resources_cleanup(hdata);
> 
>  	/* hdmiphy i2c driver */
> --
> 1.7.4.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 3/5] drm/exynos: Use devm_clk_get in exynos_mixer.c
  2012-11-23  3:41 ` [PATCH 3/5] drm/exynos: Use devm_clk_get in exynos_mixer.c Sachin Kamat
@ 2012-11-23  7:02   ` Inki Dae
  0 siblings, 0 replies; 13+ messages in thread
From: Inki Dae @ 2012-11-23  7:02 UTC (permalink / raw)
  To: 'Sachin Kamat', dri-devel; +Cc: patches



> -----Original Message-----
> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
> Sent: Friday, November 23, 2012 12:42 PM
> To: dri-devel@lists.freedesktop.org
> Cc: inki.dae@samsung.com; jy0922.shim@samsung.com; airlied@linux.ie;
> sachin.kamat@linaro.org; patches@linaro.org
> Subject: [PATCH 3/5] drm/exynos: Use devm_clk_get in exynos_mixer.c
> 
> devm_clk_get is device managed and makes error handling and exit code
> simpler.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c |   20 +++++---------------
>  1 files changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 0d3ed28..88fcb40 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -962,14 +962,14 @@ static int __devinit mixer_resources_init(struct
> exynos_drm_hdmi_context *ctx,
> 
>  	spin_lock_init(&mixer_res->reg_slock);
> 
> -	mixer_res->mixer = clk_get(dev, "mixer");
> +	mixer_res->mixer = devm_clk_get(dev, "mixer");
>  	if (IS_ERR_OR_NULL(mixer_res->mixer)) {
>  		dev_err(dev, "failed to get clock 'mixer'\n");
>  		ret = -ENODEV;

Just return ret;

>  		goto fail;
>  	}
> 
> -	mixer_res->sclk_hdmi = clk_get(dev, "sclk_hdmi");
> +	mixer_res->sclk_hdmi = devm_clk_get(dev, "sclk_hdmi");
>  	if (IS_ERR_OR_NULL(mixer_res->sclk_hdmi)) {
>  		dev_err(dev, "failed to get clock 'sclk_hdmi'\n");
>  		ret = -ENODEV;

Ditto.

> @@ -1008,10 +1008,6 @@ static int __devinit mixer_resources_init(struct
> exynos_drm_hdmi_context *ctx,
>  	return 0;
> 
>  fail:
> -	if (!IS_ERR_OR_NULL(mixer_res->sclk_hdmi))
> -		clk_put(mixer_res->sclk_hdmi);
> -	if (!IS_ERR_OR_NULL(mixer_res->mixer))
> -		clk_put(mixer_res->mixer);
>  	return ret;
>  }
> 
> @@ -1024,19 +1020,19 @@ static int __devinit vp_resources_init(struct
> exynos_drm_hdmi_context *ctx,
>  	struct resource *res;
>  	int ret;
> 
> -	mixer_res->vp = clk_get(dev, "vp");
> +	mixer_res->vp = devm_clk_get(dev, "vp");
>  	if (IS_ERR_OR_NULL(mixer_res->vp)) {
>  		dev_err(dev, "failed to get clock 'vp'\n");
>  		ret = -ENODEV;
>  		goto fail;

Just return ret;

>  	}
> -	mixer_res->sclk_mixer = clk_get(dev, "sclk_mixer");
> +	mixer_res->sclk_mixer = devm_clk_get(dev, "sclk_mixer");
>  	if (IS_ERR_OR_NULL(mixer_res->sclk_mixer)) {
>  		dev_err(dev, "failed to get clock 'sclk_mixer'\n");
>  		ret = -ENODEV;
>  		goto fail;

Ditto.

>  	}
> -	mixer_res->sclk_dac = clk_get(dev, "sclk_dac");
> +	mixer_res->sclk_dac = devm_clk_get(dev, "sclk_dac");
>  	if (IS_ERR_OR_NULL(mixer_res->sclk_dac)) {
>  		dev_err(dev, "failed to get clock 'sclk_dac'\n");
>  		ret = -ENODEV;
> @@ -1064,12 +1060,6 @@ static int __devinit vp_resources_init(struct
> exynos_drm_hdmi_context *ctx,
>  	return 0;
> 
>  fail:
> -	if (!IS_ERR_OR_NULL(mixer_res->sclk_dac))
> -		clk_put(mixer_res->sclk_dac);
> -	if (!IS_ERR_OR_NULL(mixer_res->sclk_mixer))
> -		clk_put(mixer_res->sclk_mixer);
> -	if (!IS_ERR_OR_NULL(mixer_res->vp))
> -		clk_put(mixer_res->vp);
>  	return ret;

And remove this.

>  }
> 
> --
> 1.7.4.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] drm/exynos: Use devm_gpio_request in exynos_hdmi.c
  2012-11-23  6:51   ` Inki Dae
@ 2012-11-23  7:03     ` Sachin Kamat
  0 siblings, 0 replies; 13+ messages in thread
From: Sachin Kamat @ 2012-11-23  7:03 UTC (permalink / raw)
  To: Inki Dae; +Cc: dri-devel, patches

On 23 November 2012 12:21, Inki Dae <inki.dae@samsung.com> wrote:
[snip]

> With cleanup to hdmi_resources_init, we can remove hdmi_resource_cleanup
> function. So could you please re-send updated patches?

OK. I will update and re-send.

>
> You've been submitting patches into too small pieces.
> Because they are trivial enough and in the same context, please merge this
> kind of patches as long as they are related with the same class or topics. I
> think you can combine them. :)

Generally, the accepted practice is to combine patches if they do
similar changes in the same file.
Else, separate patches is preferred as it makes things easier for
reviewers and maintainer by improving readability and less rework
effort.
Since you have specifically requested for combining the patches I will
squash patches related to clk_get together and resend the series.

>
> Thanks,
> Inki Dae
>


-- 
With warm regards,
Sachin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 4/5] drm/exynos: Use devm_clk_get in exynos_drm_g2d.c
  2012-11-23  3:41 ` [PATCH 4/5] drm/exynos: Use devm_clk_get in exynos_drm_g2d.c Sachin Kamat
@ 2012-11-23  7:06   ` Inki Dae
  0 siblings, 0 replies; 13+ messages in thread
From: Inki Dae @ 2012-11-23  7:06 UTC (permalink / raw)
  To: 'Sachin Kamat', dri-devel; +Cc: patches

Applied.

Thanks,
Inki Dae

> -----Original Message-----
> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
> Sent: Friday, November 23, 2012 12:42 PM
> To: dri-devel@lists.freedesktop.org
> Cc: inki.dae@samsung.com; jy0922.shim@samsung.com; airlied@linux.ie;
> sachin.kamat@linaro.org; patches@linaro.org
> Subject: [PATCH 4/5] drm/exynos: Use devm_clk_get in exynos_drm_g2d.c
> 
> devm_clk_get is device managed and makes error handling and exit code
> simpler.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index a9002ad..c1054cb 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -1125,7 +1125,7 @@ static int __devinit g2d_probe(struct
> platform_device *pdev)
>  	mutex_init(&g2d->cmdlist_mutex);
>  	mutex_init(&g2d->runqueue_mutex);
> 
> -	g2d->gate_clk = clk_get(dev, "fimg2d");
> +	g2d->gate_clk = devm_clk_get(dev, "fimg2d");
>  	if (IS_ERR(g2d->gate_clk)) {
>  		dev_err(dev, "failed to get gate clock\n");
>  		ret = PTR_ERR(g2d->gate_clk);
> @@ -1181,7 +1181,6 @@ static int __devinit g2d_probe(struct
> platform_device *pdev)
> 
>  err_put_clk:
>  	pm_runtime_disable(dev);
> -	clk_put(g2d->gate_clk);
>  err_destroy_workqueue:
>  	destroy_workqueue(g2d->g2d_workq);
>  err_destroy_slab:
> @@ -1202,7 +1201,6 @@ static int __devexit g2d_remove(struct
> platform_device *pdev)
>  	}
> 
>  	pm_runtime_disable(&pdev->dev);
> -	clk_put(g2d->gate_clk);
> 
>  	g2d_fini_cmdlist(g2d);
>  	destroy_workqueue(g2d->g2d_workq);
> --
> 1.7.4.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 5/5] drm/exynos: Fix potential NULL pointer dereference
  2012-11-23  3:41 ` [PATCH 5/5] drm/exynos: Fix potential NULL pointer dereference Sachin Kamat
@ 2012-11-23  7:08   ` Inki Dae
  0 siblings, 0 replies; 13+ messages in thread
From: Inki Dae @ 2012-11-23  7:08 UTC (permalink / raw)
  To: 'Sachin Kamat', dri-devel; +Cc: patches

This was my missing point. Applied.

Thanks,
Inki Dae

> -----Original Message-----
> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
> Sent: Friday, November 23, 2012 12:42 PM
> To: dri-devel@lists.freedesktop.org
> Cc: inki.dae@samsung.com; jy0922.shim@samsung.com; airlied@linux.ie;
> sachin.kamat@linaro.org; patches@linaro.org
> Subject: [PATCH 5/5] drm/exynos: Fix potential NULL pointer dereference
> 
> Pointer was being dereferenced after freeing.
> 
> Fixes the following error:
> drivers/gpu/drm/exynos/exynos_drm_g2d.c:323 g2d_userptr_put_dma_addr()
> error:
> dereferencing freed memory 'g2d_userptr'
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index c1054cb..6ffa076 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -319,8 +319,8 @@ out:
>  	g2d_userptr->sgt = NULL;
> 
>  	kfree(g2d_userptr->pages);
> -	kfree(g2d_userptr);
>  	g2d_userptr->pages = NULL;
> +	kfree(g2d_userptr);
>  	g2d_userptr = NULL;
>  }
> 
> --
> 1.7.4.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-11-23  7:08 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-23  3:41 [PATCH 0/5] drm/exynos: Bug fix and devm_* usage Sachin Kamat
2012-11-23  3:41 ` [PATCH 1/5] drm/exynos: Use devm_clk_get in exynos_drm_fimd.c Sachin Kamat
2012-11-23  6:40   ` Inki Dae
2012-11-23  6:43     ` Sachin Kamat
2012-11-23  3:41 ` [PATCH 2/5] drm/exynos: Use devm_gpio_request in exynos_hdmi.c Sachin Kamat
2012-11-23  6:51   ` Inki Dae
2012-11-23  7:03     ` Sachin Kamat
2012-11-23  3:41 ` [PATCH 3/5] drm/exynos: Use devm_clk_get in exynos_mixer.c Sachin Kamat
2012-11-23  7:02   ` Inki Dae
2012-11-23  3:41 ` [PATCH 4/5] drm/exynos: Use devm_clk_get in exynos_drm_g2d.c Sachin Kamat
2012-11-23  7:06   ` Inki Dae
2012-11-23  3:41 ` [PATCH 5/5] drm/exynos: Fix potential NULL pointer dereference Sachin Kamat
2012-11-23  7:08   ` Inki Dae

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).