Hi Prabhakar, As discussed separately I would prefer to better understand issue before going to this patch. Nevertheless I have some remarks in code in case we'll need it at the end.
On 9/4/20 10:18 PM, Lad Prabhakar wrote: > Keep the sensor in software power down mode and wake up only in > ov5640_set_stream_dvp() callback. > > Signed-off-by: Lad Prabhakar <[email protected]> > Reviewed-by: Biju Das <[email protected]> > Tested-by: Jacopo Mondi <[email protected]> > --- > drivers/media/i2c/ov5640.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > index 2fe4a7ac0592..880fde73a030 100644 > --- a/drivers/media/i2c/ov5640.c > +++ b/drivers/media/i2c/ov5640.c > @@ -34,6 +34,8 @@ > #define OV5640_REG_SYS_RESET02 0x3002 > #define OV5640_REG_SYS_CLOCK_ENABLE02 0x3006 > #define OV5640_REG_SYS_CTRL0 0x3008 > +#define OV5640_REG_SYS_CTRL0_SW_PWDN 0x42 > +#define OV5640_REG_SYS_CTRL0_SW_PWUP 0x02 For the time being this section was only referring to registers addresses and bit details was explained into a comment right before affectation, see OV5640_REG_IO_MIPI_CTRL00 for example. > #define OV5640_REG_CHIP_ID 0x300a > #define OV5640_REG_IO_MIPI_CTRL00 0x300e > #define OV5640_REG_PAD_OUTPUT_ENABLE01 0x3017 > @@ -1120,6 +1122,12 @@ static int ov5640_load_regs(struct ov5640_dev *sensor, > val = regs->val; > mask = regs->mask; > > + /* remain in power down mode for DVP */ > + if (regs->reg_addr == OV5640_REG_SYS_CTRL0 && > + val == OV5640_REG_SYS_CTRL0_SW_PWUP && > + sensor->ep.bus_type != V4L2_MBUS_CSI2_DPHY) > + continue; > + I understand that more or less register OV5640_REG_SYS_CTRL0 (0x3008) has been partially removed from big init sequence: for power-up part, but power-dwn remains at very beginning of sequence. We should completely remove 0x3008 from init sequence, including power-dwn, and introduce a new function ov5640_sw_powerdown(on/off) that should be called at the right place instead. > if (mask) > ret = ov5640_mod_reg(sensor, reg_addr, mask, val); > else > @@ -1297,9 +1305,14 @@ static int ov5640_set_stream_dvp(struct ov5640_dev > *sensor, bool on) > * PAD OUTPUT ENABLE 02 > * - [7:2]: D[5:0] output enable > */ > - return ov5640_write_reg(sensor, > - OV5640_REG_PAD_OUTPUT_ENABLE02, > - on ? 0xfc : 0); > + ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE02, > + on ? 0xfc : 0); > + if (ret) > + return ret; > + > + return ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ? > + OV5640_REG_SYS_CTRL0_SW_PWUP : > + OV5640_REG_SYS_CTRL0_SW_PWDN); > } > > static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on) > BR, Hugues.
