Hi Hugues, On Wed, Oct 07, 2020 at 04:43:18PM +0200, Hugues Fruchet wrote: > fixes: b1751ae652fb ("media: i2c: ov5640: Separate out mipi configuration from s_power") Fixes: ... And preferrably before Sob line. > > Fix ov5640_write_reg()return value unchecked at power off. > Reformat code to keep register access below the register description. If there's an error, I wouldn't stop turning things off, but just proceed. The caller will ignore the error in that case anyway. This changes with the patch. > > Signed-off-by: Hugues Fruchet <hugues.fruchet@xxxxxx> > Change-Id: I12db67c416c3d63eadee400a3c89aaf48c5b1469 This was probably not intended to be here. > --- > drivers/media/i2c/ov5640.c | 17 ++++++----------- > 1 file changed, 6 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > index 8d0254d..f34e62e 100644 > --- a/drivers/media/i2c/ov5640.c > +++ b/drivers/media/i2c/ov5640.c > @@ -1940,14 +1940,6 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on) > { > int ret; > > - if (!on) { > - /* Reset MIPI bus settings to their default values. */ > - ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, 0x58); > - ov5640_write_reg(sensor, OV5640_REG_MIPI_CTRL00, 0x04); > - ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00, 0x00); > - return 0; > - } > - > /* > * Power up MIPI HS Tx and LS Rx; 2 data lanes mode > * > @@ -1958,7 +1950,8 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on) > * [3] = 0 : Power up MIPI LS Rx > * [2] = 0 : MIPI interface disabled > */ > - ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, 0x40); > + ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, > + on ? 0x40 : 0x58); > if (ret) > return ret; > > @@ -1969,7 +1962,8 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on) > * [5] = 1 : Gate clock when 'no packets' > * [2] = 1 : MIPI bus in LP11 when 'no packets' > */ > - ret = ov5640_write_reg(sensor, OV5640_REG_MIPI_CTRL00, 0x24); > + ret = ov5640_write_reg(sensor, OV5640_REG_MIPI_CTRL00, > + on ? 0x24 : 0x04); > if (ret) > return ret; > > @@ -1981,7 +1975,8 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on) > * [5] = 1 : MIPI data lane 1 in LP11 when 'sleeping' > * [4] = 1 : MIPI clock lane in LP11 when 'sleeping' > */ > - ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00, 0x70); > + ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00, > + on ? 0x70 : 0x00); > if (ret) > return ret; > -- Kind regards, Sakari Ailus