[PATCH v2 0/4] [media] exynos-gsc: Some fixes

[PATCH v2 0/4] [media] exynos-gsc: Some fixes

[Sachin Kamat]

Changes since v1:
Removed the label 'err' from function gsc_clk_get as suggested
by Sylwester Nawrocki <s.nawrocki@samsung.com> in patch 3/4.
Other patches remain the same.

Patch series build tested and based on samsung/for_v3.8 branch of
git://linuxtv.org/snawrocki/media.git.

Sachin Kamat (4):
  [media] exynos-gsc: Rearrange error messages for valid prints
  [media] exynos-gsc: Remove gsc_clk_put call from gsc_clk_get
  [media] exynos-gsc: Use devm_clk_get()
  [media] exynos-gsc: Fix checkpatch warning in gsc-m2m.c

 drivers/media/platform/exynos-gsc/gsc-core.c |   21 ++++++++-------------
 drivers/media/platform/exynos-gsc/gsc-m2m.c  |    2 +-
 2 files changed, 9 insertions(+), 14 deletions(-)

-- 
1.7.4.1

[PATCH v2 1/4] [media] exynos-gsc: Rearrange error messages for valid prints

[Sachin Kamat]

In case of clk_prepare failure, the function gsc_clk_get also prints
"failed to get clock" which is not correct. Hence move the error
messages to their respective blocks. While at it, also renamed the labels
meaningfully.

Cc: Shaik Ameer Basha <shaik.ameer@samsung.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/exynos-gsc/gsc-core.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
index 6d6f65d..45bcfa7 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1017,25 +1017,26 @@ static int gsc_clk_get(struct gsc_dev *gsc)
 	dev_dbg(&gsc->pdev->dev, "gsc_clk_get Called\n");
 
 	gsc->clock = clk_get(&gsc->pdev->dev, GSC_CLOCK_GATE_NAME);
-	if (IS_ERR(gsc->clock))
-		goto err_print;
+	if (IS_ERR(gsc->clock)) {
+		dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n",
+			GSC_CLOCK_GATE_NAME);
+		goto err_clk_get;
+	}
 
 	ret = clk_prepare(gsc->clock);
 	if (ret < 0) {
+		dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n",
+			GSC_CLOCK_GATE_NAME);
 		clk_put(gsc->clock);
 		gsc->clock = NULL;
-		goto err;
+		goto err_clk_prepare;
 	}
 
 	return 0;
 
-err:
-	dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n",
-					GSC_CLOCK_GATE_NAME);
+err_clk_prepare:
 	gsc_clk_put(gsc);
-err_print:
-	dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n",
-					GSC_CLOCK_GATE_NAME);
+err_clk_get:
 	return -ENXIO;
 }
 
-- 
1.7.4.1

[PATCH v2 2/4] [media] exynos-gsc: Remove gsc_clk_put call from gsc_clk_get

[Sachin Kamat]

Since this function just returns (since gsc->clock is NULL),
remove it and make the exit code simpler.

Cc: Shaik Ameer Basha <shaik.ameer@samsung.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/exynos-gsc/gsc-core.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
index 45bcfa7..99ee1a9 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1020,7 +1020,7 @@ static int gsc_clk_get(struct gsc_dev *gsc)
 	if (IS_ERR(gsc->clock)) {
 		dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n",
 			GSC_CLOCK_GATE_NAME);
-		goto err_clk_get;
+		goto err;
 	}
 
 	ret = clk_prepare(gsc->clock);
@@ -1029,14 +1029,12 @@ static int gsc_clk_get(struct gsc_dev *gsc)
 			GSC_CLOCK_GATE_NAME);
 		clk_put(gsc->clock);
 		gsc->clock = NULL;
-		goto err_clk_prepare;
+		goto err;
 	}
 
 	return 0;
 
-err_clk_prepare:
-	gsc_clk_put(gsc);
-err_clk_get:
+err:
 	return -ENXIO;
 }
 
-- 
1.7.4.1

[PATCH v2 3/4] [media] exynos-gsc: Use devm_clk_get()

[Sachin Kamat]

devm_clk_get() is a device managed function and makes error handling
a bit simpler.

Cc: Shaik Ameer Basha <shaik.ameer@samsung.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/exynos-gsc/gsc-core.c |   13 +++----------
 1 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
index 99ee1a9..5a285b2 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1006,8 +1006,6 @@ static void gsc_clk_put(struct gsc_dev *gsc)
 		return;
 
 	clk_unprepare(gsc->clock);
-	clk_put(gsc->clock);
-	gsc->clock = NULL;
 }
 
 static int gsc_clk_get(struct gsc_dev *gsc)
@@ -1016,26 +1014,21 @@ static int gsc_clk_get(struct gsc_dev *gsc)
 
 	dev_dbg(&gsc->pdev->dev, "gsc_clk_get Called\n");
 
-	gsc->clock = clk_get(&gsc->pdev->dev, GSC_CLOCK_GATE_NAME);
+	gsc->clock = devm_clk_get(&gsc->pdev->dev, GSC_CLOCK_GATE_NAME);
 	if (IS_ERR(gsc->clock)) {
 		dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n",
 			GSC_CLOCK_GATE_NAME);
-		goto err;
+		return PTR_ERR(gsc->clock);
 	}
 
 	ret = clk_prepare(gsc->clock);
 	if (ret < 0) {
 		dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n",
 			GSC_CLOCK_GATE_NAME);
-		clk_put(gsc->clock);
-		gsc->clock = NULL;
-		goto err;
+		return ret;
 	}
 
 	return 0;
-
-err:
-	return -ENXIO;
 }
 
 static int gsc_m2m_suspend(struct gsc_dev *gsc)
-- 
1.7.4.1

[PATCH v2 4/4] [media] exynos-gsc: Fix checkpatch warning in gsc-m2m.c

[Sachin Kamat]

Fixes the following warning:
WARNING: space prohibited between function name and open parenthesis '('
FILE: media/platform/exynos-gsc/gsc-m2m.c:606:
	ctx = kzalloc(sizeof (*ctx), GFP_KERNEL);

Cc: Shaik Ameer Basha <shaik.ameer@samsung.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/exynos-gsc/gsc-m2m.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index 39dff20..10036d6 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -603,7 +603,7 @@ static int gsc_m2m_open(struct file *file)
 	if (mutex_lock_interruptible(&gsc->lock))
 		return -ERESTARTSYS;
 
-	ctx = kzalloc(sizeof (*ctx), GFP_KERNEL);
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
 	if (!ctx) {
 		ret = -ENOMEM;
 		goto unlock;
-- 
1.7.4.1

Re: [PATCH v2 0/4] [media] exynos-gsc: Some fixes

[Shaik Ameer Basha]

Hi Sylwester,

I tested this patch series. Looks good to me.

Thanks,
Shaik Ameer Basha

On Fri, Nov 23, 2012 at 4:34 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> Changes since v1:
> Removed the label 'err' from function gsc_clk_get as suggested
> by Sylwester Nawrocki <s.nawrocki@samsung.com> in patch 3/4.
> Other patches remain the same.
>
> Patch series build tested and based on samsung/for_v3.8 branch of
> git://linuxtv.org/snawrocki/media.git.
>
> Sachin Kamat (4):
>   [media] exynos-gsc: Rearrange error messages for valid prints
>   [media] exynos-gsc: Remove gsc_clk_put call from gsc_clk_get
>   [media] exynos-gsc: Use devm_clk_get()
>   [media] exynos-gsc: Fix checkpatch warning in gsc-m2m.c
>
>  drivers/media/platform/exynos-gsc/gsc-core.c |   21 ++++++++-------------
>  drivers/media/platform/exynos-gsc/gsc-m2m.c  |    2 +-
>  2 files changed, 9 insertions(+), 14 deletions(-)
>
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/4] [media] exynos-gsc: Some fixes

[Sylwester Nawrocki]

Thanks Shaik,

Sachin, I've applied the last patch fixing the checkpatch.pl warning.

As for the remaining three, can you please squash them, together
with following patch

 From cb7b42d2089206c8134fa195c0d1f4145fcb4f72 Mon Sep 17 00:00:00 2001
From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
Date: Sun, 25 Nov 2012 14:16:02 +0100
Subject: [PATCH] exynos-gsc: Correct clock handling

Make sure there is no unbalanced clk_unprepare call and add missing
clock release in the driver's remove() callback.

Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
---
  drivers/media/platform/exynos-gsc/gsc-core.c |    9 +++++----
  1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 5a285b2..0c22ad5 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1002,10 +1002,8 @@ static void *gsc_get_drv_data(struct 
platform_device *pdev)

  static void gsc_clk_put(struct gsc_dev *gsc)
  {
-	if (IS_ERR_OR_NULL(gsc->clock))
-		return;
-
-	clk_unprepare(gsc->clock);
+	if (!IS_ERR(gsc->clock))
+		clk_unprepare(gsc->clock);
  }

  static int gsc_clk_get(struct gsc_dev *gsc)
@@ -1025,6 +1023,7 @@ static int gsc_clk_get(struct gsc_dev *gsc)
  	if (ret < 0) {
  		dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n",
  			GSC_CLOCK_GATE_NAME);
+		gsc->clock = ERR_PTR(-EINVAL);
  		return ret;
  	}

@@ -1097,6 +1096,7 @@ static int gsc_probe(struct platform_device *pdev)
  	init_waitqueue_head(&gsc->irq_queue);
  	spin_lock_init(&gsc->slock);
  	mutex_init(&gsc->lock);
+	gsc->clock = ERR_PTR(-EINVAL);

  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  	gsc->regs = devm_request_and_ioremap(dev, res);
@@ -1160,6 +1160,7 @@ static int __devexit gsc_remove(struct 
platform_device *pdev)

  	vb2_dma_contig_cleanup_ctx(gsc->alloc_ctx);
  	pm_runtime_disable(&pdev->dev);
+	gsc_clk_put(gsc);

  	dev_dbg(&pdev->dev, "%s driver unloaded\n", pdev->name);
  	return 0;
-- 
1.7.4.1

And then maybe create 2 patches out of it, one adding missing gsc_clk_put()
in the gsc_remove() callback and the other converting to devm_clk_get() ?

I don't like how one of these patches adds something that is mostly removed
in subsequent one. This is just unneeded churn that makes the changes more
difficult to follow.

Thanks,
Sylwester

On 11/25/2012 08:00 AM, Shaik Ameer Basha wrote:
> Hi Sylwester,
>
> I tested this patch series. Looks good to me.
>
> Thanks,
> Shaik Ameer Basha
>
> On Fri, Nov 23, 2012 at 4:34 PM, Sachin Kamat<sachin.kamat@linaro.org>  wrote:
>> Changes since v1:
>> Removed the label 'err' from function gsc_clk_get as suggested
>> by Sylwester Nawrocki<s.nawrocki@samsung.com>  in patch 3/4.
>> Other patches remain the same.
>>
>> Patch series build tested and based on samsung/for_v3.8 branch of
>> git://linuxtv.org/snawrocki/media.git.
>>
>> Sachin Kamat (4):
>>    [media] exynos-gsc: Rearrange error messages for valid prints
>>    [media] exynos-gsc: Remove gsc_clk_put call from gsc_clk_get
>>    [media] exynos-gsc: Use devm_clk_get()
>>    [media] exynos-gsc: Fix checkpatch warning in gsc-m2m.c
>>
>>   drivers/media/platform/exynos-gsc/gsc-core.c |   21 ++++++++-------------
>>   drivers/media/platform/exynos-gsc/gsc-m2m.c  |    2 +-
>>   2 files changed, 9 insertions(+), 14 deletions(-)
>>
>> --
>> 1.7.4.1