Re: [PATCH v2] media: i2c: add ov01a10 image sensor driver

[Bingbu Cao <bingbu.cao@linux.intel.com>]

Kieran,

Thanks for your review.

On 4/20/23 7:55 PM, Kieran Bingham wrote:
> Hi Bingbu,
> 
> Running this through my sensor driver check script still highlights some
> missing features:
> 
> Selection Controls and fwnode properties (will become mandatory):
>  - V4L2_SEL_TGT_CROP_DEFAULT       : --------
>  - V4L2_SEL_TGT_CROP               : --------
>  - V4L2_SEL_TGT_CROP_BOUNDS        : --------
>  - .get_selection                  : --------
>  - v4l2_ctrl_new_fwnode_properties : found
> 
> Could you consider adding these controls please?

Yes, I will add them in v3.

> 
> Please see https://git.libcamera.org/libcamera/libcamera.git/tree/Documentation/sensor_driver_requirements.rst#n77:
> 
> The sensor driver should implement support for the V4L2 Selection API,
> specifically it should implement support for the
> `VIDIOC_SUBDEV_G_SELECTION` ioctl with support for the following selection
> targets:
> 
> .. VIDIOC_SUBDEV_G_SELECTION:
> 
> * `V4L2_SEL_TGT_CROP_BOUNDS` to report the readable pixel array area size
> * `V4L2_SEL_TGT_CROP_DEFAULT` to report the active pixel array area size
> * `V4L2_SEL_TGT_CROP` to report the analogue selection rectangle
> 
> - https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/vidioc-subdev-g-selection.html
> - https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/v4l2-selection-targets.html
> 
> --
> Regards
> 
> Kieran
> 
> 
> Quoting bingbu.cao@intel.com (2023-04-20 05:22:13)
>> From: Bingbu Cao <bingbu.cao@intel.com>
>>
>> Add v4l2 device driver for OmniVision ov01a10 image sensor, ov01a10
>> image sensor can deliver 1280x800 resolution BGGR10 images at 60 fps.
>>
>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>> Signed-off-by: Yating Wang <yating.wang@intel.com>
>> ---
>> Changes from v1:
>>  - Create ORIENTATION and ROTATION controls if fwnode properties exists
>>  - Move sensor PLL related settings into mipi_data_rate_720mbps[]
>>  - Use v4l2 subdev active state API and remove private mutex
>>  - Add support for vertical and horizontal flip v4l2 control
>>  - Update the copyright header comment format
>>  - Remove some unused macros
>>  - Add entry in MAINTAINERS
>>
>> ---

..snip..

-- 
Best regards,
Bingbu Cao