On Fri, 24 Aug 2012, Anatolij Gustschin wrote: > Hi Guennadi, > > On Fri, 24 Aug 2012 13:22:18 +0200 (CEST) > Guennadi Liakhovetski <g.liakhovetski@xxxxxx> wrote: > ... > > > --- a/drivers/media/i2c/soc_camera/mt9v022.c > > > +++ b/drivers/media/i2c/soc_camera/mt9v022.c > > > @@ -274,9 +274,9 @@ static int mt9v022_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a) > > > if (ret & 1) /* Autoexposure */ > > > ret = reg_write(client, mt9v022->reg->max_total_shutter_width, > > > rect.height + mt9v022->y_skip_top + 43); > > > - else > > > - ret = reg_write(client, MT9V022_TOTAL_SHUTTER_WIDTH, > > > - rect.height + mt9v022->y_skip_top + 43); > > > + else /* Set to the manually controlled value */ > > > + ret = v4l2_ctrl_s_ctrl(mt9v022->exposure, > > > + mt9v022->exposure->val); > > > > But why do we have to write it here at all then? Autoexposure can be off > > only if the user has set exposure manually, using V4L2_CID_EXPOSURE_AUTO. > > In this case MT9V022_TOTAL_SHUTTER_WIDTH already contains the correct > > value. Why do we have to set it again? Maybe just adding a comment, > > explaining the above, would suffice? > > Actually we do not have to write it here, yes. Should I remove the shutter > register setting here entirely? And add a comment explaining, why? Remove it from the "else" clause, yes, please. And a comment would be good! Thanks Guennadi --- 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
