Re: [PATCH 1/1] v4l: Remove support for crop default target in subdev drivers

[Helmut Grohne <helmut.grohne@intenta.de>]

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