Re: [PATCH 8/9] ov9740: avoid calling ov9740_res_roundup() twice
On 05/03/2015 10:47 PM, Guennadi Liakhovetski wrote:
> Hi Hans,
>
> On Sun, 3 May 2015, Hans Verkuil wrote:
>
>> From: Hans Verkuil
>>
>> Simplify ov9740_s_fmt.
>>
>> Signed-off-by: Hans Verkuil
>> Reported-by: Guennadi Liakhovetski
>> ---
>> drivers/media/i2c/soc_camera/ov9740.c | 18 +-
>> 1 file changed, 1 insertion(+), 17 deletions(-)
>>
>> diff --git a/drivers/media/i2c/soc_camera/ov9740.c
>> b/drivers/media/i2c/soc_camera/ov9740.c
>> index 03a7fc7..61a8e18 100644
>> --- a/drivers/media/i2c/soc_camera/ov9740.c
>> +++ b/drivers/media/i2c/soc_camera/ov9740.c
>> @@ -673,20 +673,8 @@ static int ov9740_s_fmt(struct v4l2_subdev *sd,
>> {
>> struct i2c_client *client = v4l2_get_subdevdata(sd);
>> struct ov9740_priv *priv = to_ov9740(sd);
>> -enum v4l2_colorspace cspace;
>> -u32 code = mf->code;
>> int ret;
>>
>> -ov9740_res_roundup(&mf->width, &mf->height);
>> -
>> -switch (code) {
>> -case MEDIA_BUS_FMT_YUYV8_2X8:
>> -cspace = V4L2_COLORSPACE_SRGB;
>> -break;
>> -default:
>> -return -EINVAL;
>> -}
>> -
>
> ov9740_s_fmt() is also called from ov9740_s_power(), so, don't we have to
> do this simplification the other way round - remove redundant code from
> ov9740_set_fmt() instead?
Yes, but s_power is also calling ov9740_res_roundup() and it sets mf->code and
mf->colorspace to the correct values. So this code in s_fmt is a duplicate
for both s_power and for set_fmt. It can't be removed from set_fmt either
since it is needed for the TRY_FORMAT case anyway.
So IMHO it makes sense to remove it from s_fmt.
Regards,
Hans
>
> Thanks
> Guennadi
>
>> ret = ov9740_reg_write_array(client, ov9740_defaults,
>> ARRAY_SIZE(ov9740_defaults));
>> if (ret < 0)
>> @@ -696,11 +684,7 @@ static int ov9740_s_fmt(struct v4l2_subdev *sd,
>> if (ret < 0)
>> return ret;
>>
>> -mf->code= code;
>> -mf->colorspace = cspace;
>> -
>> -memcpy(&priv->current_mf, mf, sizeof(struct v4l2_mbus_framefmt));
>> -
>> +priv->current_mf = *mf;
>> return ret;
>> }
>>
>> --
>> 2.1.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/9] ov9740: avoid calling ov9740_res_roundup() twice
Hi Hans,
On Sun, 3 May 2015, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Simplify ov9740_s_fmt.
>
> Signed-off-by: Hans Verkuil
> Reported-by: Guennadi Liakhovetski
> ---
> drivers/media/i2c/soc_camera/ov9740.c | 18 +-
> 1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/media/i2c/soc_camera/ov9740.c
> b/drivers/media/i2c/soc_camera/ov9740.c
> index 03a7fc7..61a8e18 100644
> --- a/drivers/media/i2c/soc_camera/ov9740.c
> +++ b/drivers/media/i2c/soc_camera/ov9740.c
> @@ -673,20 +673,8 @@ static int ov9740_s_fmt(struct v4l2_subdev *sd,
> {
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> struct ov9740_priv *priv = to_ov9740(sd);
> - enum v4l2_colorspace cspace;
> - u32 code = mf->code;
> int ret;
>
> - ov9740_res_roundup(&mf->width, &mf->height);
> -
> - switch (code) {
> - case MEDIA_BUS_FMT_YUYV8_2X8:
> - cspace = V4L2_COLORSPACE_SRGB;
> - break;
> - default:
> - return -EINVAL;
> - }
> -
ov9740_s_fmt() is also called from ov9740_s_power(), so, don't we have to
do this simplification the other way round - remove redundant code from
ov9740_set_fmt() instead?
Thanks
Guennadi
> ret = ov9740_reg_write_array(client, ov9740_defaults,
>ARRAY_SIZE(ov9740_defaults));
> if (ret < 0)
> @@ -696,11 +684,7 @@ static int ov9740_s_fmt(struct v4l2_subdev *sd,
> if (ret < 0)
> return ret;
>
> - mf->code= code;
> - mf->colorspace = cspace;
> -
> - memcpy(&priv->current_mf, mf, sizeof(struct v4l2_mbus_framefmt));
> -
> + priv->current_mf = *mf;
> return ret;
> }
>
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] ov9740: avoid calling ov9740_res_roundup() twice
From: Hans Verkuil
Simplify ov9740_s_fmt.
Signed-off-by: Hans Verkuil
Reported-by: Guennadi Liakhovetski
---
drivers/media/i2c/soc_camera/ov9740.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/media/i2c/soc_camera/ov9740.c
b/drivers/media/i2c/soc_camera/ov9740.c
index 03a7fc7..61a8e18 100644
--- a/drivers/media/i2c/soc_camera/ov9740.c
+++ b/drivers/media/i2c/soc_camera/ov9740.c
@@ -673,20 +673,8 @@ static int ov9740_s_fmt(struct v4l2_subdev *sd,
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov9740_priv *priv = to_ov9740(sd);
- enum v4l2_colorspace cspace;
- u32 code = mf->code;
int ret;
- ov9740_res_roundup(&mf->width, &mf->height);
-
- switch (code) {
- case MEDIA_BUS_FMT_YUYV8_2X8:
- cspace = V4L2_COLORSPACE_SRGB;
- break;
- default:
- return -EINVAL;
- }
-
ret = ov9740_reg_write_array(client, ov9740_defaults,
ARRAY_SIZE(ov9740_defaults));
if (ret < 0)
@@ -696,11 +684,7 @@ static int ov9740_s_fmt(struct v4l2_subdev *sd,
if (ret < 0)
return ret;
- mf->code= code;
- mf->colorspace = cspace;
-
- memcpy(&priv->current_mf, mf, sizeof(struct v4l2_mbus_framefmt));
-
+ priv->current_mf = *mf;
return ret;
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
