linux-kernel - Re: [PATCH v1] media: i2c: imx219: fix msr access command sequence

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 9 Jun 2024 16:35:07 -0500
From: Adam Ford <aford173@...il.com>
To: Conor Dooley <conor@...nel.org>
Cc: linux-media@...r.kernel.org, Conor Dooley <conor.dooley@...rochip.com>, 
	Dave Stevenson <dave.stevenson@...pberrypi.com>, 
	Sakari Ailus <sakari.ailus@...ux.intel.com>, Mauro Carvalho Chehab <mchehab@...nel.org>, 
	Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	Andrey Konovalov <andrey.konovalov@...aro.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] media: i2c: imx219: fix msr access command sequence

On Fri, Jun 7, 2024 at 10:50 AM Conor Dooley <conor@...nel.org> wrote:
>
> From: Conor Dooley <conor.dooley@...rochip.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@...il.com>  #imx8mp-beacon-kit

adam

> Link: https://www.opensourceinstruments.com/Electronics/Data/IMX219PQ.pdf [1]
> Fixes: 8508455961d5 ("media: i2c: imx219: Split common registers from mode tables")
> Fixes: 1283b3b8f82b ("media: i2c: Add driver for Sony IMX219 sensor")
> Signed-off-by: Conor Dooley <conor.dooley@...rochip.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@...pberrypi.com>
> CC: Sakari Ailus <sakari.ailus@...ux.intel.com>
> CC: Mauro Carvalho Chehab <mchehab@...nel.org>
> CC: Adam Ford <aford173@...il.com>
> CC: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> CC: Andrey Konovalov <andrey.konovalov@...aro.org>
> CC: linux-media@...r.kernel.org
> CC: linux-kernel@...r.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_regs[] = {
>         { 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
>