diff mbox series

[v2,3/3] media: ov5640: Honor power on time in init_setting

Message ID 20221219143056.4070-4-j-luthra@ti.com
State New
Headers show
Series media: ov5640: Fix power up sequence delays | expand

Commit Message

Jai Luthra Dec. 19, 2022, 2:30 p.m. UTC
From: Nishanth Menon <nm@ti.com>

OV5640 Datasheet[1] Figures 2-3 and 2-4 indicate the timing sequences
that is expected during various initialization steps. Note the power
on time includes t0 + t1 + t2 >= 5ms, delay for poweron.

As indicated in section 2.8, the PWDN assertion can either be via
external pin control OR via the register 0x3008 bit 6 (see table 7-1 in
[1])

[1] https://cdn.sparkfun.com/datasheets/Sensors/LightImaging/OV5640_datasheet.pdf

Fixes: 19a81c1426c1 ("[media] add Omnivision OV5640 sensor driver")
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 drivers/media/i2c/ov5640.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Jacopo Mondi Dec. 22, 2022, 6:24 p.m. UTC | #1
Hi Jai

On Mon, Dec 19, 2022 at 08:00:56PM +0530, Jai Luthra wrote:
> From: Nishanth Menon <nm@ti.com>
>
> OV5640 Datasheet[1] Figures 2-3 and 2-4 indicate the timing sequences
> that is expected during various initialization steps. Note the power
> on time includes t0 + t1 + t2 >= 5ms, delay for poweron.
>
> As indicated in section 2.8, the PWDN assertion can either be via
> external pin control OR via the register 0x3008 bit 6 (see table 7-1 in
> [1])
>
> [1] https://cdn.sparkfun.com/datasheets/Sensors/LightImaging/OV5640_datasheet.pdf
>
> Fixes: 19a81c1426c1 ("[media] add Omnivision OV5640 sensor driver")
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  drivers/media/i2c/ov5640.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 7b0ff04a2c93..0ea8691ecded 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -551,7 +551,7 @@ static const struct reg_value ov5640_init_setting[] = {
>  	{0x583b, 0x28, 0, 0}, {0x583c, 0x42, 0, 0}, {0x583d, 0xce, 0, 0},
>  	{0x5025, 0x00, 0, 0}, {0x3a0f, 0x30, 0, 0}, {0x3a10, 0x28, 0, 0},
>  	{0x3a1b, 0x30, 0, 0}, {0x3a1e, 0x26, 0, 0}, {0x3a11, 0x60, 0, 0},
> -	{0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3c00, 0x04, 0, 300},
> +	{0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 5}, {0x3c00, 0x04, 0, 300},
>  };
>
>  static const struct reg_value ov5640_setting_low_res[] = {
> @@ -1735,9 +1735,12 @@ static int ov5640_set_autogain(struct ov5640_dev *sensor, bool on)
>
>  static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
>  {
> -	return ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ?
> -				OV5640_REG_SYS_CTRL0_SW_PWUP :
> -				OV5640_REG_SYS_CTRL0_SW_PWDN);
> +	int ret;
> +	ret = ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ?
> +			       OV5640_REG_SYS_CTRL0_SW_PWUP :
> +			       OV5640_REG_SYS_CTRL0_SW_PWDN);
> +	usleep_range(5000, 6000);

Do you need to delay also when !on ?

> +	return ret;
>  }
>
>  static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on)
> --
> 2.17.1
>
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 7b0ff04a2c93..0ea8691ecded 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -551,7 +551,7 @@  static const struct reg_value ov5640_init_setting[] = {
 	{0x583b, 0x28, 0, 0}, {0x583c, 0x42, 0, 0}, {0x583d, 0xce, 0, 0},
 	{0x5025, 0x00, 0, 0}, {0x3a0f, 0x30, 0, 0}, {0x3a10, 0x28, 0, 0},
 	{0x3a1b, 0x30, 0, 0}, {0x3a1e, 0x26, 0, 0}, {0x3a11, 0x60, 0, 0},
-	{0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3c00, 0x04, 0, 300},
+	{0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 5}, {0x3c00, 0x04, 0, 300},
 };
 
 static const struct reg_value ov5640_setting_low_res[] = {
@@ -1735,9 +1735,12 @@  static int ov5640_set_autogain(struct ov5640_dev *sensor, bool on)
 
 static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
 {
-	return ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ?
-				OV5640_REG_SYS_CTRL0_SW_PWUP :
-				OV5640_REG_SYS_CTRL0_SW_PWDN);
+	int ret;
+	ret = ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ?
+			       OV5640_REG_SYS_CTRL0_SW_PWUP :
+			       OV5640_REG_SYS_CTRL0_SW_PWDN);
+	usleep_range(5000, 6000);
+	return ret;
 }
 
 static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on)