Received-SPF: pass (google.com: domain of linux-kernel+bounces-206425-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Conor Dooley <conor@kernel.org>
To: linux-media@vger.kernel.org
Cc: conor@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>,
	Adam Ford <aford173@gmail.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Andrey Konovalov <andrey.konovalov@linaro.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v1] media: i2c: imx219: fix msr access command sequence
Date: Fri,  7 Jun 2024 16:50:23 +0100
Message-ID: <20240607-trimmer-pummel-b452ed15e103@spud>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

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.

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@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_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