Re: [PATCH v2 8/9] media: ov2740: Add a sleep after resetting the sensor

[Hao Yao <hao.yao@xxxxxxxxx>]

Hi Bingbu, Hans,

On 2023/11/27 12:15, Bingbu Cao wrote:
> Hans,
>
> On 11/26/23 10:15 PM, Hans de Goede wrote:
> > Split the resetting of the sensor out of the link_freq_config reg_list
> > and add a delay after this.
> >
> > This hopefully fixes the stream sometimes not starting, this was
> > taken from the ov2740 sensor driver in the out of tree IPU6 driver:
>
> Thanks for your patch.
>
> I don't know the details for ov2740 here, we met some similar issues
> with another OminiVision camera sensor, it is somehow related to the
> OTP read. Unfortunately, we didn't find the root-cause.
>
> Maybe you can remove the OTP read to check, I think the OTP is useless
> if I don't forget anything.
>
> Hao, do you have any details for this issue?

Are we talking about this Github issue?
https://github.com/intel/ipu6-drivers/issues/187

I remembered that I added sleep after the power on/off. As for software 
resetting, I encountered similar issue when I worked on HM2170 sensor. 
It hangs if we transfer I2C messages immediately after software reset. 
Even OV2740 document didn't say anything about delay after software 
reset, I think it's worth a try.

I didn't use the OTP feature so I didn't look into it on OV2740.


Best Regards,
Hao Yao

> > https://github.com/intel/ipu6-drivers/
> >
> > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > ---
> >   drivers/media/i2c/ov2740.c | 11 +++++++++--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> > index 8f5c33f68d42..a49c065c6cf4 100644
> > --- a/drivers/media/i2c/ov2740.c
> > +++ b/drivers/media/i2c/ov2740.c
> > @@ -128,7 +128,6 @@ struct ov2740_mode {
> >   };
> >   
> >   static const struct ov2740_reg mipi_data_rate_720mbps[] = {
> > -	{0x0103, 0x01},
> >   	{0x0302, 0x4b},
> >   	{0x030d, 0x4b},
> >   	{0x030e, 0x02},
> > @@ -137,7 +136,6 @@ static const struct ov2740_reg mipi_data_rate_720mbps[] = {
> >   };
> >   
> >   static const struct ov2740_reg mipi_data_rate_360mbps[] = {
> > -	{0x0103, 0x01},
> >   	{0x0302, 0x4b},
> >   	{0x0303, 0x01},
> >   	{0x030d, 0x4b},
> > @@ -935,6 +933,15 @@ static int ov2740_start_streaming(struct ov2740 *ov2740)
> >   	if (ov2740->nvm)
> >   		ov2740_load_otp_data(ov2740->nvm);
> >   
> > +	/* Reset the sensor */
> > +	ret = ov2740_write_reg(ov2740, 0x0103, 1, 0x01);
> > +	if (ret) {
> > +		dev_err(&client->dev, "failed to reset\n");
> > +		return ret;
> > +	}
> > +
> > +	usleep_range(10000, 15000);
> > +
> >   	link_freq_index = ov2740->cur_mode->link_freq_index;
> >   	reg_list = &link_freq_configs[link_freq_index].reg_list;
> >   	ret = ov2740_write_reg_list(ov2740, reg_list);