On 22/01/18 11:26, Sakari Ailus wrote:
> Hi Hans,
>
> On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>
>> 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 <hans.verkuil@xxxxxxxxx>
>> ---
>> drivers/media/i2c/mt9v011.c | 29 +++++++++----------
>> drivers/media/i2c/ov6650.c | 33 ++++++++++------------
>> drivers/media/i2c/ov7670.c | 26 +++++++++--------
>> drivers/media/i2c/ov7740.c | 29 ++++++++-----------
>> drivers/media/i2c/tvp514x.c | 37 +++++++++++--------------
>> 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, 137 insertions(+), 144 deletions(-)
>>
>> diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
>> index 5e29064fae91..0e0bcc8b67ca 100644
>> --- a/drivers/media/i2c/mt9v011.c
>> +++ b/drivers/media/i2c/mt9v011.c
>> @@ -364,33 +364,30 @@ 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)
>> + if (ival->pad)
>> return -EINVAL;
>
> The pad number checks are already present in v4l2-subdev.c. Do you think
> we'll need them in drivers as well?
>
> It's true that another driver could mis-use this interface. In that case
> I'd introduce a wrapper to the op rather than introduce the check in every
> driver.
I'm not that keen on introducing wrappers for an op. I wouldn't actually know
how to implement that cleanly. Since the pad check is subdev driver specific,
and the overhead of a wrapper is almost certainly higher than just doing this
check I feel it is OK to do this.
>
>>
>> - memset(cp, 0, sizeof(struct v4l2_captureparm));
>> - cp->capability = V4L2_CAP_TIMEPERFRAME;
>> + memset(ival->reserved, 0, sizeof(ival->reserved));
>> calc_fps(sd,
>> - &cp->timeperframe.numerator,
>> - &cp->timeperframe.denominator);
>> + &ival->interval.numerator,
>> + &ival->interval.denominator);
>>
>> return 0;
>> }
Regards,
Hans
