On Mon, Sep 24, 2018 at 04:42:27PM +0200, Sakari Ailus wrote: > --- a/drivers/media/i2c/mt9t112.c > +++ b/drivers/media/i2c/mt9t112.c > @@ -888,12 +888,6 @@ static int mt9t112_get_selection(struct v4l2_subdev *sd, > sel->r.width = MAX_WIDTH; > sel->r.height = MAX_HEIGHT; > return 0; > - case V4L2_SEL_TGT_CROP_DEFAULT: > - sel->r.left = 0; > - sel->r.top = 0; > - sel->r.width = VGA_WIDTH; > - sel->r.height = VGA_HEIGHT; > - return 0; > case V4L2_SEL_TGT_CROP: > sel->r = priv->frame; > return 0; Together with the change in soc_scale_crop.c, this constitutes an (unintentional?) behaviour change. It was formerly reporting 640x480 and will now be reporting 2048x1536. I cannot tell whether that is reasonable. > --- a/drivers/media/i2c/soc_camera/mt9t112.c > +++ b/drivers/media/i2c/soc_camera/mt9t112.c > @@ -884,12 +884,6 @@ static int mt9t112_get_selection(struct v4l2_subdev *sd, > sel->r.width = MAX_WIDTH; > sel->r.height = MAX_HEIGHT; > return 0; > - case V4L2_SEL_TGT_CROP_DEFAULT: > - sel->r.left = 0; > - sel->r.top = 0; > - sel->r.width = VGA_WIDTH; > - sel->r.height = VGA_HEIGHT; > - return 0; > case V4L2_SEL_TGT_CROP: > sel->r = priv->frame; > return 0; This one looks duplicate. Is there a good reason to have two drivers for mt9t112? This is lilely out of scope for the patch. Cced Jacopo Mondi as he introduced the copy. Other than your patch looks fine to me. Helmut