diff mbox series

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

Message ID 20231126141517.7534-9-hdegoede@redhat.com (mailing list archive)
State Superseded
Delegated to: Sakari Ailus
Headers
Series media: ov2740: reset GPIO, clk and 180 MHz link-frequency support |

Commit Message

Hans de Goede Nov. 26, 2023, 2:15 p.m. UTC 
  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:

https://github.com/intel/ipu6-drivers/

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov2740.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Bingbu Cao Nov. 27, 2023, 4:15 a.m. UTC | #1 
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?

> 
> https://github.com/intel/ipu6-drivers/
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  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);
>
Hao Yao Nov. 27, 2023, 6:39 a.m. UTC | #2 
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@redhat.com>
>> ---
>>   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);
>>
>
diff mbox series

Patch

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);