Received-SPF: pass (google.com: domain of linux-kernel+bounces-207517-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240607-trimmer-pummel-b452ed15e103@spud>
In-Reply-To: <20240607-trimmer-pummel-b452ed15e103@spud>
From: Adam Ford <aford173@gmail.com>
Date: Sun, 9 Jun 2024 16:35:07 -0500
Message-ID: <CAHCN7xJgjRwBUwy9LgRObTUXUnp9dWDJD9Z9hnZbN9S1h1dR_A@mail.gmail.com>
Subject: Re: [PATCH v1] media: i2c: imx219: fix msr access command sequence
To: Conor Dooley <conor@kernel.org>
Cc: linux-media@vger.kernel.org, Conor Dooley <conor.dooley@microchip.com>, 
	Dave Stevenson <dave.stevenson@raspberrypi.com>, 
	Sakari Ailus <sakari.ailus@linux.intel.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, 
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>, 
	Andrey Konovalov <andrey.konovalov@linaro.org>, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Jun 7, 2024 at 10:50=E2=80=AFAM Conor Dooley <conor@kernel.org> wro=
te:
>
> From: Conor Dooley <conor.dooley@microchip.com>
>
> It was reported to me that the imx219 didn't work on one of our
> development kits partly because the access sequence is incorrect.
> The datasheet I could find [1] for this camera has the access sequence:
> Seq. No. Address (Hex) data
> 1        30EB          05
> 2        30EB          0C
> 3        300A          FF
> 4        300B          FF
> 5        30EB          05
> 6        30EB          09
>
> but the driver swaps the first two elements. Laurent pointed out on IRC
> that the original code used the correct sequence for 1920x1080 but the
> current sequence for 3280x2464 and 1640x1232. During refactoring of the
> init sequence the current order was used for all formats.
>
> Switch to using the documented sequence.
>


Tested-by:  Adam Ford <aford173@gmail.com>  #imx8mp-beacon-kit

adam

> Link: https://www.opensourceinstruments.com/Electronics/Data/IMX219PQ.pdf=
 [1]
> Fixes: 8508455961d5 ("media: i2c: imx219: Split common registers from mod=
e tables")
> Fixes: 1283b3b8f82b ("media: i2c: Add driver for Sony IMX219 sensor")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> I got the report of this third hand, I don't have a device and can't
> test this. I do wonder why the RPis get away with the sequence that
> seemingly doesn't work for the guy that reported this to me. My theory
> is either that they noticed the sequence was wrong while adding some
> other MSR access that is needed on this board while either cross
> checking the values written or because the other MSR accesses didn't
> take effect.
>
> CC: Dave Stevenson <dave.stevenson@raspberrypi.com>
> CC: Sakari Ailus <sakari.ailus@linux.intel.com>
> CC: Mauro Carvalho Chehab <mchehab@kernel.org>
> CC: Adam Ford <aford173@gmail.com>
> CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> CC: Andrey Konovalov <andrey.konovalov@linaro.org>
> CC: linux-media@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  drivers/media/i2c/imx219.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index 51ebf5453fce..e78a80b2bb2e 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -162,8 +162,8 @@ static const struct cci_reg_sequence imx219_common_re=
gs[] =3D {
>         { IMX219_REG_MODE_SELECT, 0x00 },       /* Mode Select */
>
>         /* To Access Addresses 3000-5fff, send the following commands */
> -       { CCI_REG8(0x30eb), 0x0c },
>         { CCI_REG8(0x30eb), 0x05 },
> +       { CCI_REG8(0x30eb), 0x0c },
>         { CCI_REG8(0x300a), 0xff },
>         { CCI_REG8(0x300b), 0xff },
>         { CCI_REG8(0x30eb), 0x05 },
> --
> 2.43.0
>