* [PATCH] media: ov6650: Fix wrong register used for red control
@ 2011-09-12 11:25 Janusz Krzysztofik
2011-09-12 11:48 ` Guennadi Liakhovetski
0 siblings, 1 reply; 3+ messages in thread
From: Janusz Krzysztofik @ 2011-09-12 11:25 UTC (permalink / raw)
To: linux-media; +Cc: Guennadi Liakhovetski, Hans Verkuil, Mauro Carvalho Chehab
REG_BLUE has been used by mistake instead of REG_RED. Fix it.
While being at it, fix a few minor issues:
* with no "retrun ret;" at the end, there is no need to initialize ret
any longer,
* consequently use conditional expressions, not if...else constructs,
throughout ov6650_s_ctrl(),
* v4l2_ctrl_new_std_menu() max value of V4L2_EXPOSURE_MANUAL instead of
equivalent 1 looks more clear.
Created on top of "Converting soc_camera to the control framework"
series.
Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
drivers/media/video/ov6650.c | 16 +++++++---------
1 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/media/video/ov6650.c b/drivers/media/video/ov6650.c
index 089a4aa..c0709ee 100644
--- a/drivers/media/video/ov6650.c
+++ b/drivers/media/video/ov6650.c
@@ -310,7 +310,7 @@ static int ov6550_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd = &priv->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
uint8_t reg, reg2;
- int ret = 0;
+ int ret;
switch (ctrl->id) {
case V4L2_CID_AUTOGAIN:
@@ -342,7 +342,7 @@ static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
struct v4l2_subdev *sd = &priv->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- int ret = 0;
+ int ret;
switch (ctrl->id) {
case V4L2_CID_AUTOGAIN:
@@ -357,7 +357,7 @@ static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
if (!ret && !ctrl->val) {
ret = ov6650_reg_write(client, REG_BLUE, priv->blue->val);
if (!ret)
- ret = ov6650_reg_write(client, REG_BLUE,
+ ret = ov6650_reg_write(client, REG_RED,
priv->red->val);
}
return ret;
@@ -370,10 +370,8 @@ static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_BRIGHTNESS:
return ov6650_reg_write(client, REG_BRT, ctrl->val);
case V4L2_CID_EXPOSURE_AUTO:
- if (ctrl->val == V4L2_EXPOSURE_AUTO)
- ret = ov6650_reg_rmw(client, REG_COMB, COMB_AEC, 0);
- else
- ret = ov6650_reg_rmw(client, REG_COMB, 0, COMB_AEC);
+ ret = ov6650_reg_rmw(client, REG_COMB, ctrl->val ==
+ V4L2_EXPOSURE_AUTO ? COMB_AEC : 0, COMB_AEC);
if (!ret && ctrl->val == V4L2_EXPOSURE_MANUAL)
ret = ov6650_reg_write(client, REG_AECH,
priv->exposure->val);
@@ -993,8 +991,8 @@ static int ov6650_probe(struct i2c_client *client,
v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
V4L2_CID_BRIGHTNESS, 0, 0xff, 1, 0x80);
priv->autoexposure = v4l2_ctrl_new_std_menu(&priv->hdl,
- &ov6550_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
- V4L2_EXPOSURE_AUTO);
+ &ov6550_ctrl_ops, V4L2_CID_EXPOSURE_AUTO,
+ V4L2_EXPOSURE_MANUAL, 0, V4L2_EXPOSURE_AUTO);
priv->exposure = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
V4L2_CID_EXPOSURE, 0, 0xff, 1, DEF_AECH);
v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
--
1.7.3.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] media: ov6650: Fix wrong register used for red control
2011-09-12 11:25 [PATCH] media: ov6650: Fix wrong register used for red control Janusz Krzysztofik
@ 2011-09-12 11:48 ` Guennadi Liakhovetski
2011-09-12 15:09 ` Janusz Krzysztofik
0 siblings, 1 reply; 3+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-12 11:48 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: linux-media, Hans Verkuil, Mauro Carvalho Chehab
Hi Janusz
Thanks for the patch, but, since I anyway will have to re-roll my branch
on linuxtv, maybe I'll roll in your s/BLUE/RED/ hunk into the original
patch from Hans with a suitable
[jkrzyszt@tis.icnet.pl: fix a typo in the register name]
comment, and then add this your patch without that hunk and with an
amended description on top, would that be ok with you?
Thanks
Guennadi
On Mon, 12 Sep 2011, Janusz Krzysztofik wrote:
> REG_BLUE has been used by mistake instead of REG_RED. Fix it.
>
> While being at it, fix a few minor issues:
> * with no "retrun ret;" at the end, there is no need to initialize ret
> any longer,
> * consequently use conditional expressions, not if...else constructs,
> throughout ov6650_s_ctrl(),
> * v4l2_ctrl_new_std_menu() max value of V4L2_EXPOSURE_MANUAL instead of
> equivalent 1 looks more clear.
>
> Created on top of "Converting soc_camera to the control framework"
> series.
>
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> ---
> drivers/media/video/ov6650.c | 16 +++++++---------
> 1 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/video/ov6650.c b/drivers/media/video/ov6650.c
> index 089a4aa..c0709ee 100644
> --- a/drivers/media/video/ov6650.c
> +++ b/drivers/media/video/ov6650.c
> @@ -310,7 +310,7 @@ static int ov6550_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> struct v4l2_subdev *sd = &priv->subdev;
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> uint8_t reg, reg2;
> - int ret = 0;
> + int ret;
>
> switch (ctrl->id) {
> case V4L2_CID_AUTOGAIN:
> @@ -342,7 +342,7 @@ static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
> struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
> struct v4l2_subdev *sd = &priv->subdev;
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> - int ret = 0;
> + int ret;
>
> switch (ctrl->id) {
> case V4L2_CID_AUTOGAIN:
> @@ -357,7 +357,7 @@ static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
> if (!ret && !ctrl->val) {
> ret = ov6650_reg_write(client, REG_BLUE, priv->blue->val);
> if (!ret)
> - ret = ov6650_reg_write(client, REG_BLUE,
> + ret = ov6650_reg_write(client, REG_RED,
> priv->red->val);
> }
> return ret;
> @@ -370,10 +370,8 @@ static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
> case V4L2_CID_BRIGHTNESS:
> return ov6650_reg_write(client, REG_BRT, ctrl->val);
> case V4L2_CID_EXPOSURE_AUTO:
> - if (ctrl->val == V4L2_EXPOSURE_AUTO)
> - ret = ov6650_reg_rmw(client, REG_COMB, COMB_AEC, 0);
> - else
> - ret = ov6650_reg_rmw(client, REG_COMB, 0, COMB_AEC);
> + ret = ov6650_reg_rmw(client, REG_COMB, ctrl->val ==
> + V4L2_EXPOSURE_AUTO ? COMB_AEC : 0, COMB_AEC);
> if (!ret && ctrl->val == V4L2_EXPOSURE_MANUAL)
> ret = ov6650_reg_write(client, REG_AECH,
> priv->exposure->val);
> @@ -993,8 +991,8 @@ static int ov6650_probe(struct i2c_client *client,
> v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> V4L2_CID_BRIGHTNESS, 0, 0xff, 1, 0x80);
> priv->autoexposure = v4l2_ctrl_new_std_menu(&priv->hdl,
> - &ov6550_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
> - V4L2_EXPOSURE_AUTO);
> + &ov6550_ctrl_ops, V4L2_CID_EXPOSURE_AUTO,
> + V4L2_EXPOSURE_MANUAL, 0, V4L2_EXPOSURE_AUTO);
> priv->exposure = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> V4L2_CID_EXPOSURE, 0, 0xff, 1, DEF_AECH);
> v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> --
> 1.7.3.4
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] media: ov6650: Fix wrong register used for red control
2011-09-12 11:48 ` Guennadi Liakhovetski
@ 2011-09-12 15:09 ` Janusz Krzysztofik
0 siblings, 0 replies; 3+ messages in thread
From: Janusz Krzysztofik @ 2011-09-12 15:09 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-media, Hans Verkuil, Mauro Carvalho Chehab
Guennadi Liakhovetski wrote:
> Hi Janusz
>
> Thanks for the patch, but, since I anyway will have to re-roll my branch
> on linuxtv, maybe I'll roll in your s/BLUE/RED/ hunk into the original
> patch from Hans with a suitable
>
> [jkrzyszt@tis.icnet.pl: fix a typo in the register name]
>
> comment, and then add this your patch without that hunk and with an
> amended description on top, would that be ok with you?
Yeah, that's exactly what I thought could be more appropriate.
Thanks,
Janusz
>
> Thanks
> Guennadi
>
> On Mon, 12 Sep 2011, Janusz Krzysztofik wrote:
>
>> REG_BLUE has been used by mistake instead of REG_RED. Fix it.
>>
>> While being at it, fix a few minor issues:
>> * with no "retrun ret;" at the end, there is no need to initialize ret
>> any longer,
>> * consequently use conditional expressions, not if...else constructs,
>> throughout ov6650_s_ctrl(),
>> * v4l2_ctrl_new_std_menu() max value of V4L2_EXPOSURE_MANUAL instead of
>> equivalent 1 looks more clear.
>>
>> Created on top of "Converting soc_camera to the control framework"
>> series.
>>
>> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
>> ---
>> drivers/media/video/ov6650.c | 16 +++++++---------
>> 1 files changed, 7 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/media/video/ov6650.c b/drivers/media/video/ov6650.c
>> index 089a4aa..c0709ee 100644
>> --- a/drivers/media/video/ov6650.c
>> +++ b/drivers/media/video/ov6650.c
>> @@ -310,7 +310,7 @@ static int ov6550_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>> struct v4l2_subdev *sd = &priv->subdev;
>> struct i2c_client *client = v4l2_get_subdevdata(sd);
>> uint8_t reg, reg2;
>> - int ret = 0;
>> + int ret;
>>
>> switch (ctrl->id) {
>> case V4L2_CID_AUTOGAIN:
>> @@ -342,7 +342,7 @@ static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
>> struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
>> struct v4l2_subdev *sd = &priv->subdev;
>> struct i2c_client *client = v4l2_get_subdevdata(sd);
>> - int ret = 0;
>> + int ret;
>>
>> switch (ctrl->id) {
>> case V4L2_CID_AUTOGAIN:
>> @@ -357,7 +357,7 @@ static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
>> if (!ret && !ctrl->val) {
>> ret = ov6650_reg_write(client, REG_BLUE, priv->blue->val);
>> if (!ret)
>> - ret = ov6650_reg_write(client, REG_BLUE,
>> + ret = ov6650_reg_write(client, REG_RED,
>> priv->red->val);
>> }
>> return ret;
>> @@ -370,10 +370,8 @@ static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
>> case V4L2_CID_BRIGHTNESS:
>> return ov6650_reg_write(client, REG_BRT, ctrl->val);
>> case V4L2_CID_EXPOSURE_AUTO:
>> - if (ctrl->val == V4L2_EXPOSURE_AUTO)
>> - ret = ov6650_reg_rmw(client, REG_COMB, COMB_AEC, 0);
>> - else
>> - ret = ov6650_reg_rmw(client, REG_COMB, 0, COMB_AEC);
>> + ret = ov6650_reg_rmw(client, REG_COMB, ctrl->val ==
>> + V4L2_EXPOSURE_AUTO ? COMB_AEC : 0, COMB_AEC);
>> if (!ret && ctrl->val == V4L2_EXPOSURE_MANUAL)
>> ret = ov6650_reg_write(client, REG_AECH,
>> priv->exposure->val);
>> @@ -993,8 +991,8 @@ static int ov6650_probe(struct i2c_client *client,
>> v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
>> V4L2_CID_BRIGHTNESS, 0, 0xff, 1, 0x80);
>> priv->autoexposure = v4l2_ctrl_new_std_menu(&priv->hdl,
>> - &ov6550_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
>> - V4L2_EXPOSURE_AUTO);
>> + &ov6550_ctrl_ops, V4L2_CID_EXPOSURE_AUTO,
>> + V4L2_EXPOSURE_MANUAL, 0, V4L2_EXPOSURE_AUTO);
>> priv->exposure = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
>> V4L2_CID_EXPOSURE, 0, 0xff, 1, DEF_AECH);
>> v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
>> --
>> 1.7.3.4
>>
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> --
> 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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-09-12 15:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-12 11:25 [PATCH] media: ov6650: Fix wrong register used for red control Janusz Krzysztofik
2011-09-12 11:48 ` Guennadi Liakhovetski
2011-09-12 15:09 ` Janusz Krzysztofik
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.