Re: [PATCH v2] media: i2c: Add OV05C10 camera sensor driver

["Nirujogi, Pratap" <pnirujog@amd.com>]

Hi Laurent,

Please see below v4l2-compliance test reports for:

(a) ov05c10 device /dev/v4l-subdev0
(b) amd_isp_capture device /dev/video0


Compliance test for device /dev/v4l-subdev0:
--------------------------------------------

atg@isp-pv-linux:~/v4l-utils/build/utils/v4l2-compliance$ sudo 
./v4l2-compliance -u 0
v4l2-compliance 1.29.0-5348, 64 bits, 64-bit time_t
v4l2-compliance SHA: 75e3f0e2c2cb 2025-03-17 18:12:17

Compliance test for device /dev/v4l-subdev0:

Driver Info:
         Driver version   : 6.14.0
         Capabilities     : 0x00000000
         Client Capabilities: 0x0000000000000002
interval-uses-which
Required ioctls:
         test VIDIOC_SUDBEV_QUERYCAP: OK
         test invalid ioctls: OK

Allow for multiple opens:
         test second /dev/v4l-subdev0 open: OK
         test VIDIOC_SUBDEV_QUERYCAP: OK
         test for unlimited opens: OK

Debug ioctls:
         test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
         test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
         test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
         test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
         test VIDIOC_ENUMAUDIO: OK (Not Supported)
         test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
         test VIDIOC_G/S_AUDIO: OK (Not Supported)
         Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
         test VIDIOC_G/S_MODULATOR: OK (Not Supported)
         test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
         test VIDIOC_ENUMAUDOUT: OK (Not Supported)
         test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
         test VIDIOC_G/S_AUDOUT: OK (Not Supported)
         Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
         test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
         test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
         test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
         test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
         test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
         test VIDIOC_QUERYCTRL: OK
         test VIDIOC_G/S_CTRL: OK
         test VIDIOC_G/S/TRY_EXT_CTRLS: OK
         test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
         test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
         Standard Controls: 11 Private Controls: 0

Format ioctls:
         test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
         test VIDIOC_G/S_PARM: OK (Not Supported)
         test VIDIOC_G_FBUF: OK (Not Supported)
         test VIDIOC_G_FMT: OK (Not Supported)
         test VIDIOC_TRY_FMT: OK (Not Supported)
         test VIDIOC_S_FMT: OK (Not Supported)
         test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
         test Cropping: OK (Not Supported)
         test Composing: OK (Not Supported)
         test Scaling: OK (Not Supported)

Codec ioctls:
         test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
         test VIDIOC_G_ENC_INDEX: OK (Not Supported)
         test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
         test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
         test CREATE_BUFS maximum buffers: OK
         test VIDIOC_REMOVE_BUFS: OK
         test VIDIOC_EXPBUF: OK (Not Supported)
         test Requests: OK (Not Supported)
         test blocking wait: OK (Not Supported)

Total for device /dev/v4l-subdev0: 46, Succeeded: 46, Failed: 0, Warnings: 0
atg@isp-pv-linux:~/v4l-utils/build/utils/v4l2-compliance$
---

Compliance test for amd_isp_capture device /dev/video0:
-------------------------------------------------------
atg@isp-pv-linux:~/test$ sudo ./v4l2-compliance -d /dev/video0
[sudo] password for atg:
v4l2-compliance 1.29.0-5348, 64 bits, 64-bit time_t
v4l2-compliance SHA: 75e3f0e2c2cb 2025-03-17 18:12:17

Compliance test for amd_isp_capture device /dev/video0:

Driver Info:
         Driver name      : amd_isp_capture
         Card type        : amd_isp_capture
         Bus info         : platform:amd_isp_capture
         Driver version   : 6.14.0
         Capabilities     : 0xa4200001
                 Video Capture
                 I/O MC
                 Streaming
                 Extended Pix Format
                 Device Capabilities
         Device Caps      : 0x24200001
                 Video Capture
                 I/O MC
                 Streaming
                 Extended Pix Format
Media Driver Info:
         Driver name      : amd_isp_capture
         Model            : amd_isp41_mdev
         Serial           :
         Bus info         : platform:amd_isp_capture
         Media version    : 6.14.0
         Hardware revision: 0x00000000 (0)
         Driver version   : 6.14.0
Interface Info:
         ID               : 0x03000003
         Type             : V4L Video
Entity Info:
         ID               : 0x00000001 (1)
         Name             : Preview
         Function         : V4L2 I/O
         Pad 0x01000002   : 0: Sink
           Link 0x02000007: from remote pad 0x1000006 of entity 'ov05c10 
99-0010' (Camera Sensor): Data, Enabled, Immutable

Required ioctls:
         test MC information (see 'Media Driver Info' above): OK
         test VIDIOC_QUERYCAP: OK
         test invalid ioctls: OK

Allow for multiple opens:
         test second /dev/video0 open: OK
         test VIDIOC_QUERYCAP: OK
         test VIDIOC_G/S_PRIORITY: OK
         test for unlimited opens: OK

Debug ioctls:
         test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
         test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
         test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
         test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
         test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
         test VIDIOC_ENUMAUDIO: OK (Not Supported)
         test VIDIOC_G/S/ENUMINPUT: OK
         test VIDIOC_G/S_AUDIO: OK (Not Supported)
         Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
         test VIDIOC_G/S_MODULATOR: OK (Not Supported)
         test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
         test VIDIOC_ENUMAUDOUT: OK (Not Supported)
         test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
         test VIDIOC_G/S_AUDOUT: OK (Not Supported)
         Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
         test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
         test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
         test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
         test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
         test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
         test VIDIOC_QUERYCTRL: OK (Not Supported)
         test VIDIOC_G/S_CTRL: OK (Not Supported)
         test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
         test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
         test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
         Standard Controls: 0 Private Controls: 0

Format ioctls (Input 0):
         test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
         test VIDIOC_G/S_PARM: OK
         test VIDIOC_G_FBUF: OK (Not Supported)
         test VIDIOC_G_FMT: OK
         test VIDIOC_TRY_FMT: OK
         test VIDIOC_S_FMT: OK
         test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
         test Cropping: OK (Not Supported)
         test Composing: OK (Not Supported)
         test Scaling: OK (Not Supported)

Codec ioctls (Input 0):
         test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
         test VIDIOC_G_ENC_INDEX: OK (Not Supported)
         test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
         test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
         test CREATE_BUFS maximum buffers: OK
         test VIDIOC_REMOVE_BUFS: OK
         test VIDIOC_EXPBUF: OK
         test Requests: OK (Not Supported)
         test blocking wait: OK

Total for amd_isp_capture device /dev/video0: 49, Succeeded: 49, Failed: 
0, Warnings: 0
atg@isp-pv-linux:~/test$
---

Thanks,
Pratap

On 4/1/2025 9:20 PM, Laurent Pinchart wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Mon, Mar 31, 2025 at 03:17:22PM -0400, Nirujogi, Pratap wrote:
>> On 3/28/2025 9:18 PM, Bryan O'Donoghue wrote:
>>> On 28/03/2025 21:42, Pratap Nirujogi wrote:
>>>> From: Bin Du <Bin.Du@amd.com>
>>>
>>>> +static const struct i2c_device_id ov05c10_id[] = {
>>>> +     {"ov05c10", 0 },
>>>> +     { }
>>>> +};
>>>
>>> There's an IPU6/IPU7 version of this driver.
>>>
>>> https://github.com/intel/ipu6-drivers/blob/master/drivers/media/i2c/
>>> ov05c10.c
>>>
>>> Perhaps you could import the Intel ACPI name contained in there too.
>>>
>> sure, will add Intel ACPI names too in V3. To be specific, I'm going to
>> add the below table in addition to the existing "struct i2c_device_id
>> ov05c10_id[]" table:
>>
>> static const struct acpi_device_id ov05c10_acpi_ids[] = {
>>        { "OVTI05C1" },
>>        {}
>> };
> 
> You could drop the i2c_device_id table if you added an OF device ID
> table, but you'll need DT bindings for that. Sakari, any best practice
> rule in this area ?
> 

Sure, will wait for Sakari's feedback. We will also check if the driver 
can work using acpi_device_id match instead of i2c_device_id.

>>>> +
>>>> +MODULE_DEVICE_TABLE(i2c, ov05c10_id);
>>>> +
>>>> +static struct i2c_driver ov05c10_i2c_driver = {
>>>> +     .driver = {
>>>> +             .name = DRV_NAME,
>>>> +             .pm = pm_ptr(&ov05c10_pm_ops),
>>>> +     },
>>>> +     .id_table = ov05c10_id,
>>>> +     .probe = ov05c10_probe,
>>>> +     .remove = ov05c10_remove,
>>>> +};
>>>> +
>>>> +module_i2c_driver(ov05c10_i2c_driver);
>>>> +
>>>> +MODULE_AUTHOR("Pratap Nirujogi <pratap.nirujogi@amd.com>");
>>>> +MODULE_AUTHOR("Venkata Narendra Kumar Gutta <vengutta@amd.com>");
>>>> +MODULE_AUTHOR("Bin Du <bin.du@amd.com>");
>>>> +MODULE_DESCRIPTION("OmniVision OV05C1010 sensor driver");
>>>> +MODULE_LICENSE("GPL v2");
>>>
>>> Why v2 ? Checkpatch will complain about v2 and BTW the IPU6 driver above
>>> is GPL not GPL v2.
>>
>> sure, will replace "GPL v2" with "GPL" in V3.
> 
> --
> Regards,
> 
> Laurent Pinchart