All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Jacopo Mondi <jacopo@jmondi.org>
Cc: Jacopo Mondi <jacopo+renesas@jmondi.org>,
	kieran.bingham+renesas@ideasonboard.com,
	niklas.soderlund+renesas@ragnatech.se, geert@linux-m68k.org,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 09/16] media: i2c: rdacm21: Re-work OV10640 initialization
Date: Wed, 24 Feb 2021 22:30:28 +0200	[thread overview]
Message-ID: <YDa3ZEY1FFdxlCkl@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20210222151913.a4teevpafzxi2xlz@uno.localdomain>

Hi Jacopo,

On Mon, Feb 22, 2021 at 04:19:13PM +0100, Jacopo Mondi wrote:
> On Mon, Feb 22, 2021 at 03:27:25AM +0200, Laurent Pinchart wrote:
> > On Tue, Feb 16, 2021 at 06:41:39PM +0100, Jacopo Mondi wrote:
> > > The OV10640 image sensor reset and powerdown on signals are controlled
> >
> > s/ on//
> >
> > > by the embedded OV490 ISP. The current reset procedure does not respect
> > > the 1 millisecond power-up delay and releases the reset signal before
> > > the powerdown one.
> > >
> > > Fix the OV10640 power up sequence by releasing the powerdown signal,
> > > waiting the mandatory 1 millisecond power up delay and then releasing
> > > the reset signal. The reset delay is not characterized in the chip
> > > manual if not as "255 XVCLK + initialization". Wait for at least 3
> > > milliseconds to guarantee the SCCB bus is available.
> > >
> > > This commit fixes a sporadic start-up error triggered by a failure to
> > > read the OV10640 chip ID:
> > > rdacm21 8-0054: OV10640 ID mismatch: (0x01)
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  drivers/media/i2c/rdacm21.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/rdacm21.c b/drivers/media/i2c/rdacm21.c
> > > index b22a2ca5340b..c420a6b96879 100644
> > > --- a/drivers/media/i2c/rdacm21.c
> > > +++ b/drivers/media/i2c/rdacm21.c
> > > @@ -333,13 +333,15 @@ static int ov10640_initialize(struct rdacm21_device *dev)
> > >  {
> > >  	u8 val;
> > >
> > > -	/* Power-up OV10640 by setting RESETB and PWDNB pins high. */
> > > +	/* Power-up OV10640 by setting PWDNB and RESETB pins high. */
> > >  	ov490_write_reg(dev, OV490_GPIO_SEL0, OV490_GPIO0);
> > >  	ov490_write_reg(dev, OV490_GPIO_SEL1, OV490_SPWDN0);
> > >  	ov490_write_reg(dev, OV490_GPIO_DIRECTION0, OV490_GPIO0);
> > >  	ov490_write_reg(dev, OV490_GPIO_DIRECTION1, OV490_SPWDN0);
> > > -	ov490_write_reg(dev, OV490_GPIO_OUTPUT_VALUE0, OV490_GPIO0);
> > > +
> > >  	ov490_write_reg(dev, OV490_GPIO_OUTPUT_VALUE0, OV490_SPWDN0);
> >
> > Shouldn't this be OV490_GPIO_OUTPUT_VALUE1 ?
> 
> Ouch, yes it should
> 
> > > +	usleep_range(1500, 3000);
> > > +	ov490_write_reg(dev, OV490_GPIO_OUTPUT_VALUE0, OV490_GPIO0);
> >
> > I'm a bit puzzled by why this patch would improve the ID read issue,
> > given that it sets GPIO0 to 1, then sets GPIO0 to 1, compared to
> > previously setting GPIO0 to 1 following by setting GPIO0 to 1 :-) Maybe
> 
> :) it doesn't make things worse at least!
> 
> > it's the additional delay ? In any case, it would probably be a good
> > idea to perform additional tests after fixing this.
> 
> I think the additional delay plays indeed a role, as it's a
> requirement in the datasheet that was not respected, but now I'm dead
> scared to fix this and find out I've opened another can of worms..

With some luck it will be the opposite, and fixing this will make the
startup sequence much more reliable. Of course luck has been quite
scarce so far for anything even remotely related to GMSL, so I won't
hold my breath.

> > >  	usleep_range(3000, 5000);
> > >
> > >  	/* Read OV10640 ID to test communications. */

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2021-02-24 20:32 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-16 17:41 [PATCH 00/16] media: i2c: GMSL reliability improvements Jacopo Mondi
2021-02-16 17:41 ` [PATCH 01/16] media: i2c: rdacm20: Enable noise immunity Jacopo Mondi
2021-02-17 12:55   ` Kieran Bingham
2021-02-22  0:49     ` Laurent Pinchart
2021-02-22 14:59       ` Jacopo Mondi
2021-02-24 20:35         ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 02/16] media: i2c: rdacm20: Embedded 'serializer' field Jacopo Mondi
2021-02-17 12:56   ` Kieran Bingham
2021-02-22  0:58   ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 03/16] media: i2c: rdacm20: Replace goto with a loop Jacopo Mondi
2021-02-17 13:01   ` Kieran Bingham
2021-02-22  1:05     ` Laurent Pinchart
2021-02-22 15:06       ` Jacopo Mondi
2021-02-24 20:27         ` Laurent Pinchart
2021-02-25  8:56           ` Kieran Bingham
2021-02-16 17:41 ` [PATCH 04/16] media: i2c: rdacm20: Report camera module name Jacopo Mondi
2021-02-17 13:02   ` Kieran Bingham
2021-02-22  1:06   ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 05/16] media: i2c: rdacm20: Check return values Jacopo Mondi
2021-02-17 13:08   ` Kieran Bingham
2021-02-22  1:09   ` Laurent Pinchart
2021-02-22 15:08     ` Jacopo Mondi
2021-02-16 17:41 ` [PATCH 06/16] media: i2c: rdacm20: Re-work ov10635 reset Jacopo Mondi
2021-02-17 13:22   ` Kieran Bingham
2021-02-22  1:15   ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 07/16] media: i2c: rdacm2x: Fix wake up delay Jacopo Mondi
2021-02-17  8:02   ` Geert Uytterhoeven
2021-02-17 13:33   ` Kieran Bingham
2021-02-22  1:18     ` Laurent Pinchart
2021-02-22 15:11       ` Jacopo Mondi
2021-02-24 20:29         ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 08/16] media: i2c: max9286: Adjust parameters indent Jacopo Mondi
2021-02-17 13:36   ` Kieran Bingham
2021-02-22  1:20   ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 09/16] media: i2c: rdacm21: Re-work OV10640 initialization Jacopo Mondi
2021-02-17  8:06   ` Geert Uytterhoeven
2021-02-17 13:55   ` Kieran Bingham
2021-02-22  1:27   ` Laurent Pinchart
2021-02-22 15:19     ` Jacopo Mondi
2021-02-24 20:30       ` Laurent Pinchart [this message]
2021-02-16 17:41 ` [PATCH 10/16] media: i2c: max9286: Rename reverse_channel_mv Jacopo Mondi
2021-02-17  8:07   ` Geert Uytterhoeven
2021-02-18 15:06   ` Kieran Bingham
2021-02-22  1:29   ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 11/16] media: i2c: max9286: Cache channel amplitude Jacopo Mondi
2021-02-17  8:08   ` Geert Uytterhoeven
2021-02-18 15:39   ` Kieran Bingham
2021-02-22  1:33   ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 12/16] media: i2c: max9286: Define high " Jacopo Mondi
2021-02-18 15:39   ` Kieran Bingham
2021-02-22  1:35   ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 13/16] media: i2c: rdacm2x: Implement .init() subdev op Jacopo Mondi
2021-02-17  8:17   ` Geert Uytterhoeven
2021-02-18 16:13   ` Kieran Bingham
2021-02-22  1:52     ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 14/16] media: i2c: max9286: Initialize remotes when bound Jacopo Mondi
2021-02-17  8:19   ` Geert Uytterhoeven
2021-02-18 16:00   ` Kieran Bingham
2021-02-22  2:03   ` Laurent Pinchart
2021-02-16 17:41 ` [PATCH 15/16] media: i2c: max9286: Rework comments in .bound() Jacopo Mondi
2021-02-18 16:03   ` Kieran Bingham
2021-02-16 17:41 ` [PATCH 16/16] media: i2c: gmsl: Use 339Kbps I2C bit-rate Jacopo Mondi
2021-02-17  8:19   ` Geert Uytterhoeven
2021-02-18 16:07   ` Kieran Bingham
2021-02-22  2:06     ` Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YDa3ZEY1FFdxlCkl@pendragon.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=geert@linux-m68k.org \
    --cc=jacopo+renesas@jmondi.org \
    --cc=jacopo@jmondi.org \
    --cc=kieran.bingham+renesas@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --subject='Re: [PATCH 09/16] media: i2c: rdacm21: Re-work OV10640 initialization' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.