* [PATCH] media: ov5640: fix MIPI power sequence
@ 2020-10-07 14:43 Hugues Fruchet
2020-10-07 14:43 ` Hugues Fruchet
0 siblings, 1 reply; 5+ messages in thread
From: Hugues Fruchet @ 2020-10-07 14:43 UTC (permalink / raw)
To: Lad Prabhakar, Jacopo Mondi, Steve Longerbeam, Sakari Ailus,
Hans Verkuil, Mauro Carvalho Chehab
Cc: linux-media, linux-stm32, Hugues Fruchet, Alain Volmat
fixes: b1751ae652fb ("media: i2c: ov5640: Separate out mipi configuration from s_power")
Fix ov5640_write_reg()return value unchecked at power off.
Reformat code to keep register access below the register description.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
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;
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH] media: ov5640: fix MIPI power sequence 2020-10-07 14:43 [PATCH] media: ov5640: fix MIPI power sequence Hugues Fruchet @ 2020-10-07 14:43 ` Hugues Fruchet 2020-10-07 15:01 ` Sakari Ailus 0 siblings, 1 reply; 5+ messages in thread From: Hugues Fruchet @ 2020-10-07 14:43 UTC (permalink / raw) To: Lad Prabhakar, Jacopo Mondi, Steve Longerbeam, Sakari Ailus, Hans Verkuil, Mauro Carvalho Chehab Cc: linux-media, linux-stm32, Hugues Fruchet, Alain Volmat fixes: b1751ae652fb ("media: i2c: ov5640: Separate out mipi configuration from s_power") Fix ov5640_write_reg()return value unchecked at power off. Reformat code to keep register access below the register description. Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com> Change-Id: I12db67c416c3d63eadee400a3c89aaf48c5b1469 --- 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; -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] media: ov5640: fix MIPI power sequence 2020-10-07 14:43 ` Hugues Fruchet @ 2020-10-07 15:01 ` Sakari Ailus 2020-10-07 15:09 ` Jacopo Mondi 0 siblings, 1 reply; 5+ messages in thread From: Sakari Ailus @ 2020-10-07 15:01 UTC (permalink / raw) To: Hugues Fruchet Cc: Lad Prabhakar, Jacopo Mondi, Steve Longerbeam, Hans Verkuil, Mauro Carvalho Chehab, linux-media, linux-stm32, Alain Volmat 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@st.com> > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: ov5640: fix MIPI power sequence 2020-10-07 15:01 ` Sakari Ailus @ 2020-10-07 15:09 ` Jacopo Mondi 2020-10-08 8:58 ` Hugues FRUCHET 0 siblings, 1 reply; 5+ messages in thread From: Jacopo Mondi @ 2020-10-07 15:09 UTC (permalink / raw) To: Sakari Ailus Cc: Hugues Fruchet, Lad Prabhakar, Steve Longerbeam, Hans Verkuil, Mauro Carvalho Chehab, linux-media, linux-stm32, Alain Volmat Hi Hugues, Sakari, On Wed, Oct 07, 2020 at 06:01:03PM +0300, Sakari Ailus wrote: > 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. Agreed, I think it's best to continue instead of bailing out as we're in a power-off path > > > > > Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com> > > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: ov5640: fix MIPI power sequence 2020-10-07 15:09 ` Jacopo Mondi @ 2020-10-08 8:58 ` Hugues FRUCHET 0 siblings, 0 replies; 5+ messages in thread From: Hugues FRUCHET @ 2020-10-08 8:58 UTC (permalink / raw) To: Jacopo Mondi, Sakari Ailus Cc: Lad Prabhakar, Steve Longerbeam, Hans Verkuil, Mauro Carvalho Chehab, linux-media, linux-stm32, Alain VOLMAT Hi Jacopo, Sakari, As you want, let's drop this patch. BR, Hugues. On 10/7/20 5:09 PM, Jacopo Mondi wrote: > Hi Hugues, Sakari, > > On Wed, Oct 07, 2020 at 06:01:03PM +0300, Sakari Ailus wrote: >> 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. > > Agreed, I think it's best to continue instead of bailing out as we're > in a power-off path > >> >>> >>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com> >>> 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-08 8:59 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-07 14:43 [PATCH] media: ov5640: fix MIPI power sequence Hugues Fruchet 2020-10-07 14:43 ` Hugues Fruchet 2020-10-07 15:01 ` Sakari Ailus 2020-10-07 15:09 ` Jacopo Mondi 2020-10-08 8:58 ` Hugues FRUCHET
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).