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] [day] [month] [year] [list]

Message-ID: <m3y1g6st6a.fsf@t19.piap.pl>
Date:   Fri, 13 Oct 2023 12:39:09 +0200
From:   Krzysztof Hałasa <khalasa@...p.pl>
To:     Stefan Lengfeld <stefan@...gfeld.xyz>
Cc:     linux-media <linux-media@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Dave Stevenson <dave.stevenson@...pberrypi.com>,
        Oleksij Rempel <linux@...pel-privat.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>, linux-i2c@...r.kernel.org
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Hi Stefan,

> Maybe you just hit a corner case or a bug, that can be avoid, in the I2C
> device.  Maybe check with the manufacturer directly?

I don't have direct contact at Sony, I guess I can try to escalate this
through the part supplier, but I won't hold my breath.

> Do you know the I2C repeated start feature [1]? This allows to batch together
> multiple I2C read/writes in a single transfer. And in the best case, this
> transfer is executed in one go without a delay in between. At least in the
> kernel it's guaranteed that no other driver can go in between with another
> transfer.

Sure, imx290.c sensor driver use repeated STARTs. In fact, it only makes
things worse.

The timeout counter seems to start with the regular START (falling edge
of SDA), repeated STARTs don't reset it. After 2^18 + 8 or + 9 MCLK
cycles, the sensor simply disconnects from the bus, generating
pseudo-STOP if it was in the middle of its 0 bit (0->1 SDA change while
SCL high) or starting sending pseudo-1 bits otherwise (0xFF data on read
or negative ACK on write). This way repeated START -> longer transfer ->
higher probability of failure. Not that it really matters.

I don't know about in-sensor race conditions, for example on WRITE to
the chip, when the ACK it interrupted by the timeout (this can be
detected by the CPU, but not reliably, depending on actual timings).

OTOH with my "always use atomic xfers with these sensors" hack to the
i.MX I2C driver, it seems to work correctly (at least as far as I2C is
concerned).

I wonder if we could/should add some special handling of these sensors
in the mainline kernel. local_irq_save() and the atomic path do the
trick, but it would have to be done in all I2C drivers (or at least in
ones used with these sensors). If no other devices need such treatment,
well... Everyone can (possibly) use a non-official hack.

Thanks for your input,
-- 
Krzysztof "Chris" Hałasa

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