Re: [PATCH v2 1/3] sur40: properly report a single frame rate of 60 FPS

[Florian Echtler <floe@butterbrot.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1428 bytes --]

Hello Hans,

On 05.07.2016 08:41, Hans Verkuil wrote:
> On 05/31/2016 10:15 PM, Florian Echtler wrote:
>> The device hardware is always running at 60 FPS, so report this both via
>> PARM_IOCTL and ENUM_FRAMEINTERVALS.
>>
>> Signed-off-by: Martin Kaltenbrunner <modin@yuri.at>
>> Signed-off-by: Florian Echtler <floe@butterbrot.org>
>> ---
>>  drivers/input/touchscreen/sur40.c | 20 ++++++++++++++++++--
>>  1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> @@ -880,6 +893,9 @@ static const struct v4l2_ioctl_ops sur40_video_ioctl_ops = {
>>  	.vidioc_enum_framesizes = sur40_vidioc_enum_framesizes,
>>  	.vidioc_enum_frameintervals = sur40_vidioc_enum_frameintervals,
>>  
>> +	.vidioc_g_parm = sur40_ioctl_parm,
>> +	.vidioc_s_parm = sur40_ioctl_parm,
> 
> Why is s_parm added when you can't change the framerate?

Oh, I thought it's mandatory to always have s_parm if you have g_parm
(even if it always returns the same values).

> Same questions for the
> enum_frameintervals function: it doesn't hurt to have it, but if there is only
> one unchangeable framerate, then it doesn't make much sense.

If you don't have enum_frameintervals, how would you find out about the
framerate otherwise? Is g_parm itself enough already for all userspace
tools?

> Sorry, missed this when I reviewed this the first time around.

No problem.

Best, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]