Hi Helmut, On Tue, Sep 25, 2018 at 08:33:29AM +0200, Helmut Grohne wrote: > 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. I'd say "yes". This is the only sub-device driver that puts a default other than the bounds there. Its source is SoC camera as you see below. > > > --- 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 -- Kind regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx
