| Message ID | 20231126141517.7534-9-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: ov2740: reset GPIO, clk and 180 MHz link-frequency support | expand |
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); >
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 --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);
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(-)