linux-kernel - Re: Sony IMX290/462 image sensors I2C xfer peculiarity

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]

Message-ID: <m3o7h5tthf.fsf@t19.piap.pl>
Date:   Wed, 11 Oct 2023 11:10:20 +0200
From:   Krzysztof Hałasa <khalasa@...p.pl>
To:     linux-media <linux-media@...r.kernel.org>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        Dave Stevenson <dave.stevenson@...pberrypi.com>
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Hi,

For the record, I think I will use the following. This way reading
a register from Sony IMX290 and IMX462 image sensors (which behave
exactly the same on I2C bus) takes up to ca. 270 us, and writing
a register takes up to ca. 230 us. While still slow, this means the
sensor won't disconnect itself from the I2C bus on 7 ms timeout
(I wonder if the timeout is 3.5 ms on sensors running at 74.250 MHz).

The times are on i.MX8MP at 1600 MHz.

0x30a50000 and 0x30ae0000 are I2C controllers connected to the
IMX290/462 sensors.

Not very nice, but works. Obviously, long transfers (like reading 0x100
registers at once) won't work (0xC0 seems fine), but the sensor driver
only does 1-byte reads/writes (resp. 5 or 4 bytes on the bus).

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 97455283ef0cf..ffffb0d50ebd9 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -275,6 +275,7 @@ struct imx_i2c_struct {
 
 	struct i2c_client	*slave;
 	enum i2c_slave_event last_slave_event;
+	bool			no_sleep;
 };
 
 static const struct imx_i2c_hwdata imx1_i2c_hwdata = {
@@ -1249,8 +1250,15 @@ static int i2c_imx_xfer_common(struct i2c_adapter *adapter,
 	unsigned int i, temp;
 	int result;
 	bool is_lastmsg = false;
+	bool really_atomic = atomic;
 	struct imx_i2c_struct *i2c_imx = i2c_get_adapdata(adapter);
+	unsigned long flags;
 
+	if (i2c_imx->no_sleep)
+		atomic = true; // IMX290 and IMX462 sensors disconnect after ca. 2^18 MCLK cycles
+
+	if (atomic)
+		local_irq_save(flags);
 	/* Start I2C transfer */
 	result = i2c_imx_start(i2c_imx, atomic);
 	if (result) {
@@ -1258,8 +1266,12 @@ static int i2c_imx_xfer_common(struct i2c_adapter *adapter,
 		 * Bus recovery uses gpiod_get_value_cansleep() which is not
 		 * allowed within atomic context.
 		 */
-		if (!atomic && i2c_imx->adapter.bus_recovery_info) {
+		if (!really_atomic && i2c_imx->adapter.bus_recovery_info) {
+			if (atomic)
+				local_irq_restore(flags);
 			i2c_recover_bus(&i2c_imx->adapter);
+			if (atomic)
+				local_irq_save(flags);
 			result = i2c_imx_start(i2c_imx, atomic);
 		}
 	}
@@ -1318,6 +1330,8 @@ static int i2c_imx_xfer_common(struct i2c_adapter *adapter,
 fail0:
 	/* Stop I2C transfer */
 	i2c_imx_stop(i2c_imx, atomic);
+	if (atomic)
+		local_irq_restore(flags);
 
 	dev_dbg(&i2c_imx->adapter.dev, "<%s> exit with: %s: %d\n", __func__,
 		(result < 0) ? "error" : "success msg",
@@ -1627,6 +1641,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
 	i2c_imx->adapter.nr		= pdev->id;
 	i2c_imx->adapter.dev.of_node	= pdev->dev.of_node;
 	i2c_imx->base			= base;
+	i2c_imx->no_sleep		= phy_addr == 0x30a50000 || phy_addr == 0x30ae0000; // IMX290/462 hack
 	ACPI_COMPANION_SET(&i2c_imx->adapter.dev, ACPI_COMPANION(&pdev->dev));
 
 	/* Get I2C clock */

-- 
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa