Re: [PATCH 1/5] V4L2: I2C: ML86V7667 video decoder driver

[Vladimir Barinov <vladimir.barinov@cogentembedded.com>]

Hi Sylwester,

Thank you for the review.

Sylwester Nawrocki wrote
>
>> +static struct v4l2_subdev_video_ops ml86v7667_subdev_video_ops = {
>> +    .querystd = ml86v7667_querystd,
>> +    .g_input_status = ml86v7667_g_input_status,
>> +    .enum_mbus_fmt = ml86v7667_enum_mbus_fmt,
>> +    .try_mbus_fmt = ml86v7667_try_mbus_fmt,
>> +    .g_mbus_fmt = ml86v7667_g_mbus_fmt,
>> +    .s_mbus_fmt = ml86v7667_s_mbus_fmt,
>> +    .cropcap = ml86v7667_cropcap,
>> +    .g_mbus_config = ml86v7667_g_mbus_config,
>> +};
>
> Why do you need .cropcap when there is no s_crop/g_crop ops ? Is it
> only for pixel aspect ratio ?
Yes it is.
>
> Also, new drivers are supposed to use the selections API instead
> (set/get_selection ops). However this requires pad level ops support
> in your host driver, hence might be a bigger issue.
Yes, since the host driver (rcar_vin is already send for review) is 
designed for soc-camera layer that does not use the pad level ops but 
needs at least cropcap to get the pixel aspect ratio.
The soc-camera has it's own set/get_selection in soc_camera_host_ops and 
it is possible to use it like all i2c/soc_camera sensors can/do.
But we would not like to design the ml86v7667 as soc-camera device since 
it will not be available for all non soc-camera v4l2 hosts.

Regards,
Vladimir