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

[jacopo mondi <jacopo@jmondi.org>]

[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]

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.
>
> > --- 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.
>

This version is the one using te soc_camera framework which is
targeted for deprecation. The soc_camera based sensor drivers are
being ported to be regular v4l2 sensor drivers, hence the copy in
drivers/media/i2c/. The original versions in
drivers/media/i2c/soc_camera will be around for a little longer and
then possibly moved to staging or later removed.

Hope this clarifies.
Thanks
   j


> Other than your patch looks fine to me.
>
> Helmut

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]