[PATCH 1/3] [media] s5p-tv: Fix incorrect usage of IS_ERR_OR_NULL in hdmi_drv.c

[PATCH 1/3] [media] s5p-tv: Fix incorrect usage of IS_ERR_OR_NULL in hdmi_drv.c

[Sachin Kamat]

NULL check on clocks obtained using common clock APIs should not
be done. Use IS_ERR only.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/s5p-tv/hdmi_drv.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c b/drivers/media/platform/s5p-tv/hdmi_drv.c
index 4e86626..b3344cb 100644
--- a/drivers/media/platform/s5p-tv/hdmi_drv.c
+++ b/drivers/media/platform/s5p-tv/hdmi_drv.c
@@ -765,15 +765,15 @@ static void hdmi_resources_cleanup(struct hdmi_device *hdev)
 		regulator_bulk_free(res->regul_count, res->regul_bulk);
 	/* kfree is NULL-safe */
 	kfree(res->regul_bulk);
-	if (!IS_ERR_OR_NULL(res->hdmiphy))
+	if (!IS_ERR(res->hdmiphy))
 		clk_put(res->hdmiphy);
-	if (!IS_ERR_OR_NULL(res->sclk_hdmiphy))
+	if (!IS_ERR(res->sclk_hdmiphy))
 		clk_put(res->sclk_hdmiphy);
-	if (!IS_ERR_OR_NULL(res->sclk_pixel))
+	if (!IS_ERR(res->sclk_pixel))
 		clk_put(res->sclk_pixel);
-	if (!IS_ERR_OR_NULL(res->sclk_hdmi))
+	if (!IS_ERR(res->sclk_hdmi))
 		clk_put(res->sclk_hdmi);
-	if (!IS_ERR_OR_NULL(res->hdmi))
+	if (!IS_ERR(res->hdmi))
 		clk_put(res->hdmi);
 	memset(res, 0, sizeof(*res));
 }
-- 
1.7.9.5

[PATCH 2/3] [media] s5p-tv: Fix incorrect usage of IS_ERR_OR_NULL in mixer_drv.c

[Sachin Kamat]

NULL check on clocks obtained using common clock APIs should not
be done. Use IS_ERR only.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/s5p-tv/mixer_drv.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/s5p-tv/mixer_drv.c b/drivers/media/platform/s5p-tv/mixer_drv.c
index 5733033..8dd8b88 100644
--- a/drivers/media/platform/s5p-tv/mixer_drv.c
+++ b/drivers/media/platform/s5p-tv/mixer_drv.c
@@ -222,15 +222,15 @@ static void mxr_release_clocks(struct mxr_device *mdev)
 {
 	struct mxr_resources *res = &mdev->res;
 
-	if (!IS_ERR_OR_NULL(res->sclk_dac))
+	if (!IS_ERR(res->sclk_dac))
 		clk_put(res->sclk_dac);
-	if (!IS_ERR_OR_NULL(res->sclk_hdmi))
+	if (!IS_ERR(res->sclk_hdmi))
 		clk_put(res->sclk_hdmi);
-	if (!IS_ERR_OR_NULL(res->sclk_mixer))
+	if (!IS_ERR(res->sclk_mixer))
 		clk_put(res->sclk_mixer);
-	if (!IS_ERR_OR_NULL(res->vp))
+	if (!IS_ERR(res->vp))
 		clk_put(res->vp);
-	if (!IS_ERR_OR_NULL(res->mixer))
+	if (!IS_ERR(res->mixer))
 		clk_put(res->mixer);
 }
 
-- 
1.7.9.5

[PATCH 3/3] [media] s5p-tv: Fix incorrect usage of IS_ERR_OR_NULL in mixer_video.c

[Sachin Kamat]

vb2_dma_contig_init_ctx does not return NULL. Use IS_ERR()
instead.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/s5p-tv/mixer_video.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-tv/mixer_video.c b/drivers/media/platform/s5p-tv/mixer_video.c
index ef0efdf..25319de 100644
--- a/drivers/media/platform/s5p-tv/mixer_video.c
+++ b/drivers/media/platform/s5p-tv/mixer_video.c
@@ -81,7 +81,7 @@ int mxr_acquire_video(struct mxr_device *mdev,
 	}
 
 	mdev->alloc_ctx = vb2_dma_contig_init_ctx(mdev->dev);
-	if (IS_ERR_OR_NULL(mdev->alloc_ctx)) {
+	if (IS_ERR(mdev->alloc_ctx)) {
 		mxr_err(mdev, "could not acquire vb2 allocator\n");
 		goto fail_v4l2_dev;
 	}
-- 
1.7.9.5

Re: [PATCH 1/3] [media] s5p-tv: Fix incorrect usage of IS_ERR_OR_NULL in hdmi_drv.c

[Sylwester Nawrocki]

On 04/29/2013 11:24 AM, Sachin Kamat wrote:
> NULL check on clocks obtained using common clock APIs should not
> be done. Use IS_ERR only.
>
> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
> ---
>   drivers/media/platform/s5p-tv/hdmi_drv.c |   10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c b/drivers/media/platform/s5p-tv/hdmi_drv.c
> index 4e86626..b3344cb 100644
> --- a/drivers/media/platform/s5p-tv/hdmi_drv.c
> +++ b/drivers/media/platform/s5p-tv/hdmi_drv.c
> @@ -765,15 +765,15 @@ static void hdmi_resources_cleanup(struct hdmi_device *hdev)
>   		regulator_bulk_free(res->regul_count, res->regul_bulk);
>   	/* kfree is NULL-safe */
>   	kfree(res->regul_bulk);
> -	if (!IS_ERR_OR_NULL(res->hdmiphy))
> +	if (!IS_ERR(res->hdmiphy))
>   		clk_put(res->hdmiphy);
> -	if (!IS_ERR_OR_NULL(res->sclk_hdmiphy))
> +	if (!IS_ERR(res->sclk_hdmiphy))
>   		clk_put(res->sclk_hdmiphy);
> -	if (!IS_ERR_OR_NULL(res->sclk_pixel))
> +	if (!IS_ERR(res->sclk_pixel))
>   		clk_put(res->sclk_pixel);
> -	if (!IS_ERR_OR_NULL(res->sclk_hdmi))
> +	if (!IS_ERR(res->sclk_hdmi))
>   		clk_put(res->sclk_hdmi);
> -	if (!IS_ERR_OR_NULL(res->hdmi))
> +	if (!IS_ERR(res->hdmi))
>   		clk_put(res->hdmi);
>   	memset(res, 0, sizeof(*res));
>   }

I think this patch is incomplete. You need to ensure all the clock pointers
are initially set to an invalid value. This is currently done with 
memsetting
whole hdmi_resource structure to 0. But now some ERR_PTR() value needs to be
used instead. Same applies to your subsequent patch.

Thanks,
Sylwester

Re: [PATCH 1/3] [media] s5p-tv: Fix incorrect usage of IS_ERR_OR_NULL in hdmi_drv.c

[Sachin Kamat]

Hi Sylwester,

On 1 May 2013 15:02, Sylwester Nawrocki <sylvester.nawrocki@gmail.com> wrote:
> On 04/29/2013 11:24 AM, Sachin Kamat wrote:
>>
>> NULL check on clocks obtained using common clock APIs should not
>> be done. Use IS_ERR only.
>>
>> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
>> ---
>>   drivers/media/platform/s5p-tv/hdmi_drv.c |   10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c
>> b/drivers/media/platform/s5p-tv/hdmi_drv.c
>> index 4e86626..b3344cb 100644
>> --- a/drivers/media/platform/s5p-tv/hdmi_drv.c
>> +++ b/drivers/media/platform/s5p-tv/hdmi_drv.c
>> @@ -765,15 +765,15 @@ static void hdmi_resources_cleanup(struct
>> hdmi_device *hdev)
>>                 regulator_bulk_free(res->regul_count, res->regul_bulk);
>>         /* kfree is NULL-safe */
>>         kfree(res->regul_bulk);
>> -       if (!IS_ERR_OR_NULL(res->hdmiphy))
>> +       if (!IS_ERR(res->hdmiphy))
>>                 clk_put(res->hdmiphy);
>> -       if (!IS_ERR_OR_NULL(res->sclk_hdmiphy))
>> +       if (!IS_ERR(res->sclk_hdmiphy))
>>                 clk_put(res->sclk_hdmiphy);
>> -       if (!IS_ERR_OR_NULL(res->sclk_pixel))
>> +       if (!IS_ERR(res->sclk_pixel))
>>                 clk_put(res->sclk_pixel);
>> -       if (!IS_ERR_OR_NULL(res->sclk_hdmi))
>> +       if (!IS_ERR(res->sclk_hdmi))
>>                 clk_put(res->sclk_hdmi);
>> -       if (!IS_ERR_OR_NULL(res->hdmi))
>> +       if (!IS_ERR(res->hdmi))
>>                 clk_put(res->hdmi);
>>         memset(res, 0, sizeof(*res));
>>   }
>
>
> I think this patch is incomplete. You need to ensure all the clock pointers
> are initially set to an invalid value. This is currently done with
> memsetting
> whole hdmi_resource structure to 0. But now some ERR_PTR() value needs to be
> used instead. Same applies to your subsequent patch.

Yes you are right.
Initialised the clocks to invalid value and sent v2 of both these patches.

-- 
With warm regards,
Sachin