Hi Sakari,
Thank you for the review.
<snip>
On 22-Jan-25 9:32 AM, Sakari Ailus wrote:
>> +static int ov02c10_power_off(struct device *dev)
>> +{
>> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
>> + struct ov02c10 *ov02c10 = to_ov02c10(sd);
>> + int ret = 0;
>> +
>> + gpiod_set_value_cansleep(ov02c10->reset, 1);
>> + gpiod_set_value_cansleep(ov02c10->handshake, 0);
>
> What's the handshake GPIO for? The sensor datasheet does not document it.
This is the same handshake GPIO as for which I started a discussion here:
https://lore.kernel.org/linux-media/59f672c3-6d87-4ec7-9b7f-f44fe2cce934@xxxxxxxxxx/
Since the Dell XPS 9x40 devices on which this sensor is use the iVSC chip
I do not believe that handshake GPIO support is necessary in this driver,
so the code to get the handshake GPIO and to set its value can simply be
dropped for the next verison of this patch.
<snip>
>> + * after handshake triggered. We set 25ms as a safe value and wait
>> + * for a stable version FW.
>> + */
>> + msleep_interruptible(25);
>
> How is this related to a lattice MIPI aggregator btw.? This driver is for
> an Omnivision sensor.
Again, see:
https://lore.kernel.org/linux-media/59f672c3-6d87-4ec7-9b7f-f44fe2cce934@xxxxxxxxxx/
For this driver this can simply be changed to:
usleep_range(1000, 1100);
Regards,
Hans
