From patchwork Wed Feb 14 16:03:05 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mauro Carvalho Chehab X-Patchwork-Id: 10219349 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 847346055C for ; Wed, 14 Feb 2018 16:04:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 755E520499 for ; Wed, 14 Feb 2018 16:04:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 69CF227FA5; Wed, 14 Feb 2018 16:04:43 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B2281290D9 for ; Wed, 14 Feb 2018 16:03:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031946AbeBNQDO (ORCPT ); Wed, 14 Feb 2018 11:03:14 -0500 Received: from osg.samsung.com ([64.30.133.232]:37149 "EHLO osg.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031553AbeBNQDM (ORCPT ); Wed, 14 Feb 2018 11:03:12 -0500 Received: from localhost (localhost [127.0.0.1]) by osg.samsung.com (Postfix) with ESMTP id 407182EAE7; Wed, 14 Feb 2018 08:03:11 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at dev.s-opensource.com Received: from osg.samsung.com ([127.0.0.1]) by localhost (localhost [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yaDgp3WS5PrI; Wed, 14 Feb 2018 08:03:10 -0800 (PST) Received: from vento.lan (unknown [179.183.107.230]) by osg.samsung.com (Postfix) with ESMTPSA id 8CDFE2EADE; Wed, 14 Feb 2018 08:03:08 -0800 (PST) Date: Wed, 14 Feb 2018 14:03:05 -0200 From: Mauro Carvalho Chehab To: Hans Verkuil Cc: linux-media@vger.kernel.org, Sakari Ailus , Jacopo Mondi , Hans Verkuil Subject: Re: [PATCHv2 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs Message-ID: <20180214140257.1bfd266f@vento.lan> In-Reply-To: <20180122123125.24709-3-hverkuil@xs4all.nl> References: <20180122123125.24709-1-hverkuil@xs4all.nl> <20180122123125.24709-3-hverkuil@xs4all.nl> Organization: Samsung X-Mailer: Claws Mail 3.15.1-dirty (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Em Mon, 22 Jan 2018 13:31:18 +0100 Hans Verkuil escreveu: > From: Hans Verkuil > > Convert all g/s_parm calls to g/s_frame_interval. This allows us > to remove the g/s_parm ops since those are a duplicate of > g/s_frame_interval. > > Signed-off-by: Hans Verkuil > --- > drivers/media/i2c/mt9v011.c | 31 +++++++------------- > drivers/media/i2c/ov6650.c | 35 +++++++++------------- > drivers/media/i2c/ov7670.c | 24 +++++++-------- > drivers/media/i2c/ov7740.c | 31 +++++++------------- > drivers/media/i2c/tvp514x.c | 39 +++++++++---------------- > drivers/media/i2c/vs6624.c | 29 +++++++----------- > drivers/media/platform/atmel/atmel-isc.c | 10 ++----- > drivers/media/platform/atmel/atmel-isi.c | 12 ++------ > drivers/media/platform/blackfin/bfin_capture.c | 14 +++------ > drivers/media/platform/marvell-ccic/mcam-core.c | 12 ++++---- > drivers/media/platform/soc_camera/soc_camera.c | 10 ++++--- > drivers/media/platform/via-camera.c | 4 +-- > drivers/media/usb/em28xx/em28xx-video.c | 36 +++++++++++++++++++---- > 13 files changed, 122 insertions(+), 165 deletions(-) > > diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c > index 5e29064fae91..3e23c5b0de1f 100644 > --- a/drivers/media/i2c/mt9v011.c > +++ b/drivers/media/i2c/mt9v011.c > @@ -364,33 +364,24 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd, > return 0; > } > > -static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms) > +static int mt9v011_g_frame_interval(struct v4l2_subdev *sd, > + struct v4l2_subdev_frame_interval *ival) > { > - struct v4l2_captureparm *cp = &parms->parm.capture; > - > - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) > - return -EINVAL; > - > - memset(cp, 0, sizeof(struct v4l2_captureparm)); > - cp->capability = V4L2_CAP_TIMEPERFRAME; > + memset(ival->reserved, 0, sizeof(ival->reserved)); Hmm.. why to repeat memset everywhere? If the hole idea is to stop abusing, the best would be to do, instead: (same applies to s_frame_interval). > calc_fps(sd, > - &cp->timeperframe.numerator, > - &cp->timeperframe.denominator); > + &ival->interval.numerator, > + &ival->interval.denominator); > > return 0; > } > > -static int mt9v011_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms) > +static int mt9v011_s_frame_interval(struct v4l2_subdev *sd, > + struct v4l2_subdev_frame_interval *ival) > { > - struct v4l2_captureparm *cp = &parms->parm.capture; > - struct v4l2_fract *tpf = &cp->timeperframe; > + struct v4l2_fract *tpf = &ival->interval; > u16 speed; > > - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) > - return -EINVAL; > - if (cp->extendedmode != 0) > - return -EINVAL; > - Hmm... why are you removing those sanity checks everywhere? The core doesn't do it. All the above comments also apply to the other files modified by this patch. > + memset(ival->reserved, 0, sizeof(ival->reserved)); > speed = calc_speed(sd, tpf->numerator, tpf->denominator); > > mt9v011_write(sd, R0A_MT9V011_CLK_SPEED, speed); > @@ -469,8 +460,8 @@ static const struct v4l2_subdev_core_ops mt9v011_core_ops = { > }; > > static const struct v4l2_subdev_video_ops mt9v011_video_ops = { > - .g_parm = mt9v011_g_parm, > - .s_parm = mt9v011_s_parm, > + .g_frame_interval = mt9v011_g_frame_interval, > + .s_frame_interval = mt9v011_s_frame_interval, > }; > > static const struct v4l2_subdev_pad_ops mt9v011_pad_ops = { > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > index 8975d16b2b24..3f962dae7534 100644 > --- a/drivers/media/i2c/ov6650.c > +++ b/drivers/media/i2c/ov6650.c > @@ -201,7 +201,7 @@ struct ov6650 { > struct v4l2_rect rect; /* sensor cropping window */ > unsigned long pclk_limit; /* from host */ > unsigned long pclk_max; /* from resolution and format */ > - struct v4l2_fract tpf; /* as requested with s_parm */ > + struct v4l2_fract tpf; /* as requested with s_frame_interval */ > u32 code; > enum v4l2_colorspace colorspace; > }; > @@ -723,42 +723,33 @@ static int ov6650_enum_mbus_code(struct v4l2_subdev *sd, > return 0; > } > > -static int ov6650_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms) > +static int ov6650_g_frame_interval(struct v4l2_subdev *sd, > + struct v4l2_subdev_frame_interval *ival) > { > struct i2c_client *client = v4l2_get_subdevdata(sd); > struct ov6650 *priv = to_ov6650(client); > - struct v4l2_captureparm *cp = &parms->parm.capture; > > - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) > - return -EINVAL; > - > - memset(cp, 0, sizeof(*cp)); > - cp->capability = V4L2_CAP_TIMEPERFRAME; > - cp->timeperframe.numerator = GET_CLKRC_DIV(to_clkrc(&priv->tpf, > + memset(ival->reserved, 0, sizeof(ival->reserved)); > + ival->interval.numerator = GET_CLKRC_DIV(to_clkrc(&priv->tpf, > priv->pclk_limit, priv->pclk_max)); > - cp->timeperframe.denominator = FRAME_RATE_MAX; > + ival->interval.denominator = FRAME_RATE_MAX; > > dev_dbg(&client->dev, "Frame interval: %u/%u s\n", > - cp->timeperframe.numerator, cp->timeperframe.denominator); > + ival->interval.numerator, ival->interval.denominator); Hmm... not sure if a debug is needed here. Yet, if this is needed, IMHO, it would make mroe sense to move it to the core. Thanks, Mauro diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index c5639817db34..b18b418c080f 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -350,6 +350,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) if (fi->pad >= sd->entity.num_pads) return -EINVAL; + memset(fi->reserved, 0, sizeof(ival->reserved)); return v4l2_subdev_call(sd, video, g_frame_interval, arg); }