2023-09-29 13:36:01

by Dave Stevenson

[permalink] [raw]
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Hi Krzysztof

On Fri, 29 Sept 2023 at 11:53, Krzysztof Hałasa <[email protected]> wrote:
>
> Hi,
>
> I am debugging certain IMX290 and IMX462 I2C failures on an NXP
> i.MX6MP based board and there seem to be interesting "feature" here:
> Sony IMX290 and IMX462 image sensors apparently terminate I2C transfers
> after 2^18 of their master clock cycles. For example, with a typical
> 37.125 MHz clock oscillator, this means about 7 ms. In practice, I'm
> barely able to read a block of 128 registers (at I2C 400 kHz clock
> rate).
>
> I think the sensors simply disconnect from the bus after 2^18 clock
> cycles (starting from the first START condition, the repeated STARTs
> don't reset the timeout). This means, in case of a READ operation, the
> data read by the CPU after the timeout contains all bits set to 1.
> i.MX8MP detects arbitration losses, so if the SDA change to 1 happens on
> high clock value (but not on ACK bit), the whole transfer is aborted.
> Otherwise, the ending bytes are simply set to FF (and the last non-FF
> may be corrupted partially).
>
> The problem is 7 ms is a short time and on a non-real time system even
> a simple non-DMA I2C register writes can take as much time. This causes
> driver failures.

What's your requirement to be able to read back so many registers at once?

Whilst potentially useful during development, most sensor drivers
write specific registers but never read anything other than
potentially a model ID register during probe.
Large block writes are generally not possible as at least the IMX290
datasheet does state "Do not perform communication to addresses not
listed in the Register Map", and there are numerous reserved blocks
throughout the map.
As far as I know there's no OTP on these sensors which would be about
the only reason I can think of for needing to read large blocks.

I currently can't see a reason for a sensor driver to be needing to do
these big reads, so how can it cause a driver failure?

Scanning the IMX290 datasheet further, there is a slightly worrying
paragraph in "Register Communication Timing (I2C)":
"In I2C communication system, communication can be performed excluding
during the period when communication is prohibited from the falling
edge of XVS to 6H after (1H period)."
and the diagram shows a 1 line period 6 lines after XVS which is
marked "communication prohibited period". Ouch!
I've never experienced issues with I2C comms to these modules, but is
it possible that you're hitting this period with your longer I2C
transactions? A logic analyser looking at XVS, XHS, and the I2C might
give you some clues. Or are you not streaming when you're doing these
transactions?
IMX462 appears to list a similar restriction.

Dave

> Needless to say, the datasheets know nothing about the feature.
>
> Any thoughts?
> --
> Krzysztof "Chris" Hałasa
>
> Sieć Badawcza Łukasiewicz
> Przemysłowy Instytut Automatyki i Pomiarów PIAP
> Al. Jerozolimskie 202, 02-486 Warszawa


2023-10-03 12:59:30

by Krzysztof Hałasa

[permalink] [raw]
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Hi Dave,

thanks for your mail:

> What's your requirement to be able to read back so many registers at once?

Ah, I'm debugging I/O errors while trying to use the sensors, it was
only an example how to reliably trigger the problem.

> I currently can't see a reason for a sensor driver to be needing to do
> these big reads, so how can it cause a driver failure?

I2C doesn't specify a time limit for requests. Even doing basic things -
like enabling streaming - I sometimes get I/O errors ("arbitration
lost"). This is i.MX6MP platform using PIO-style hardware master I2C.
While the CPU has some DMA capability for I2C, it's apparently not used
in this case - maybe because the transfers are shorter than the
threshold.

BTW the "arbitration lost" means here a STOP was detected by the CPU.
This happens when the sensor stops pulling down SDA on timeout, in case
SCL was high at the time (SDA 0->1 with SCL high is technically STOP,
but there is only one master here - CPU - and only one I2C device - the
sensor, so it isn't a real STOP, it's just a sensor disconnection from
the I2C bus).

With 37.125 MHz master clock the time limit is ca. 7 ms - it's easy for
a short transfer to take more time, even on an otherwise idle system and
even with 400 kHz I2C clock.

It happens in init, while enabling streaming, while setting parameters
while streaming. Everytime a transfer crosses the magic ca. 7.06xx ms
time. No transfer can take more time than this value, I always get
a (false) STOP or the data being read becomes all ones.

> Scanning the IMX290 datasheet further, there is a slightly worrying
> paragraph in "Register Communication Timing (I2C)":
> "In I2C communication system, communication can be performed excluding
> during the period when communication is prohibited from the falling
> edge of XVS to 6H after (1H period)."
> and the diagram shows a 1 line period 6 lines after XVS which is
> marked "communication prohibited period". Ouch!

Right, I noticed this, but I don't think it's related. The issue can be
seen in idle as well.

> I've never experienced issues with I2C comms to these modules, but is
> it possible that you're hitting this period with your longer I2C
> transactions?

I'm just hitting a seemingly hard limit with long transfers (albeit it
may be quite short in terms of bytes - times between bytes are just long
due to scheduling and PIO-style byte-sized CPU access).

> A logic analyser looking at XVS, XHS, and the I2C might
> give you some clues.

TBH I didn't try to connect to XVS and XHS. My IMX290 modules don't even
have these signals (apparently - there are "unused" connections).
I just observed that the timeout is 2^18 master cycles from the first
START. There it very little jitter here (on an oscilloscope - a fraction
of a microsecond). I haven't counted the exact master clock cycles #,
should I?

> Or are you not streaming when you're doing these
> transactions?

That's the case, too.

> IMX462 appears to list a similar restriction.
--
Krzysztof "Chris" Hałasa

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

2023-10-10 09:46:55

by Krzysztof Hałasa

[permalink] [raw]
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Hi,

I'm back to IMX290 and IMX462 sensors running on i.MX8MP (and not on
some nonexistent sort of i.MX6 as I previously wrote). The timeout thing
happens at circa 2^18 + 8 or + 9 master clocks after the regular START
(not repeated START). With MCLK = 37.125 MHz this amounts to ca. 7 ms.

Using imx290.c and the i2c-imx.c I2C driver (NXP's version 5.15, but it
doesn't seem to differ significantly from mainstream):

wait_queue_head_t queue;

i2c_imx_trx_complete()
wait_event_timeout(queue, ..., HZ / 10);

i2c_imx_master_isr()
wake_up(&queue);

Sensor register write example, I've added a couple of obvious printk()
calls in the interrupt handler.

First adding some load to each CPU core:
for n in 1 2 3 4; do while :; do :; done & done

Then the actual sensor register write:
Sending I2C DEVSEL byte:
[86.968267] <i2c_imx_write> write slave address: addr=0x34

Completed:
[86.968306] <i2c_imx_isr> 0xA2 0xF8 <<<<<< ACK
[86.968311] <i2c_imx_master_isr> 0xA2
[86.980361] <i2c_imx_trx_complete> TRX complete

It took 12 ms to wake up, way too long for IMX462 to remain on the I2C
bus, so the following byte TX fails.

The question is: what can be done about it?
- should I make the camera control thread real time?
- or maybe the I2C driver should be modified?
- or maybe imx290 register accesses should be somehow atomic?
--
Krzysztof "Chris" Hałasa

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

2023-10-11 09:12:41

by Krzysztof Hałasa

[permalink] [raw]
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

2023-10-11 09:51:23

by Krzysztof Hałasa

[permalink] [raw]
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Hi,

adding more people to Cc: as this is more general stuff than my specific
image sensor setup.

Is there any reason for the following (meta) patch to not be applied?

Currently, every i.MX8MP atomic I2C transfer starts with 100 us delay
(just after the START condition). At 400 kHz bus (384 kHz or whatever),
this is equivalent to several tens of bits. Is this delay needed?

This is on NXP 5.15 branch, but it seems the mainline is identical here.

With this patch, the 1-byte (quasi) atomic image sensor register reads
(16-bit address + 8-bit value) are down to ca. 160 us, and writes take
120 us.

It seems one bit on the bus takes ca. 2.66 us (hardware), and the delay
between consecutive bytes is ca. 4.82 us (I guess CPU takes a fair share
of this). This is i.MX8MP @ apparently 1200 MHz (1600 MHz with freq
scaler).

Fire away.
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -534,xx +534,xx @@
static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy, bool atomic)
{
unsigned long orig_jiffies = jiffies;
unsigned int temp;

while (1) {
temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);

/* check for arbitration lost */
if (temp & I2SR_IAL) {
i2c_imx_clear_irq(i2c_imx, I2SR_IAL);
return -EAGAIN;
}

if (for_busy && (temp & I2SR_IBB)) {
i2c_imx->stopped = 0;
break;
}
if (!for_busy && !(temp & I2SR_IBB)) {
i2c_imx->stopped = 1;
break;
}
if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
dev_dbg(&i2c_imx->adapter.dev,
"<%s> I2C bus is busy\n", __func__);
return -ETIMEDOUT;
}
if (atomic)
- udelay(100);
+ udelay(1);
else
schedule();
}

return 0;
}
--
Krzysztof "Chris" Hałasa

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

2023-10-11 10:44:11

by Stefan Lengfeld

[permalink] [raw]
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Hi Krzysztof,

> Currently, every i.MX8MP atomic I2C transfer starts with 100 us delay
> (just after the START condition). At 400 kHz bus (384 kHz or whatever),
> this is equivalent to several tens of bits. Is this delay needed?

I cannot answer whether the delay is needed for atomic transfer or not.
But I can give a bit of context for I2C atomic transfers in general.

These where only introduced for a very narrow and special uses shutting
down the device/power with external PMICs in the kernel's shutdown
handlers.

E.g. the see code in the file 'drivers/i2c/i2c-core.h':

/*
* We only allow atomic transfers for very late communication, e.g. to access a
* PMIC when powering down. Atomic transfers are a corner case and not for
* generic use!
*/
static inline bool i2c_in_atomic_xfer_mode(void)
{
return system_state > SYSTEM_RUNNING && irqs_disabled();
}

So I wonder why this delay is a problem for your described you case.
The e-mail title talks about an image sensor. Why and in which use case
this sensor needs an atomic transfer?

My understand is that an ordinary I2C device would just use normal (and
sleepable) I2C transfers while the device is in use.

Kind regards,
Stefan

On Wed, Oct 11, 2023 at 11:50:12AM +0200, Krzysztof Hałasa wrote:
> Hi,
>
> adding more people to Cc: as this is more general stuff than my specific
> image sensor setup.
>
> Is there any reason for the following (meta) patch to not be applied?
>
> Currently, every i.MX8MP atomic I2C transfer starts with 100 us delay
> (just after the START condition). At 400 kHz bus (384 kHz or whatever),
> this is equivalent to several tens of bits. Is this delay needed?
>
> This is on NXP 5.15 branch, but it seems the mainline is identical here.
>
> With this patch, the 1-byte (quasi) atomic image sensor register reads
> (16-bit address + 8-bit value) are down to ca. 160 us, and writes take
> 120 us.
>
> It seems one bit on the bus takes ca. 2.66 us (hardware), and the delay
> between consecutive bytes is ca. 4.82 us (I guess CPU takes a fair share
> of this). This is i.MX8MP @ apparently 1200 MHz (1600 MHz with freq
> scaler).
>
> Fire away.
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -534,xx +534,xx @@
> static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy, bool atomic)
> {
> unsigned long orig_jiffies = jiffies;
> unsigned int temp;
>
> while (1) {
> temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
>
> /* check for arbitration lost */
> if (temp & I2SR_IAL) {
> i2c_imx_clear_irq(i2c_imx, I2SR_IAL);
> return -EAGAIN;
> }
>
> if (for_busy && (temp & I2SR_IBB)) {
> i2c_imx->stopped = 0;
> break;
> }
> if (!for_busy && !(temp & I2SR_IBB)) {
> i2c_imx->stopped = 1;
> break;
> }
> if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
> dev_dbg(&i2c_imx->adapter.dev,
> "<%s> I2C bus is busy\n", __func__);
> return -ETIMEDOUT;
> }
> if (atomic)
> - udelay(100);
> + udelay(1);
> else
> schedule();
> }
>
> return 0;
> }
> --
> Krzysztof "Chris" Hałasa
>
> Sieć Badawcza Łukasiewicz
> Przemysłowy Instytut Automatyki i Pomiarów PIAP
> Al. Jerozolimskie 202, 02-486 Warszawa
>

2023-10-11 11:26:18

by Krzysztof Hałasa

[permalink] [raw]
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Stefan,

> I cannot answer whether the delay is needed for atomic transfer or not.
> But I can give a bit of context for I2C atomic transfers in general.
>
> These where only introduced for a very narrow and special uses shutting
> down the device/power with external PMICs in the kernel's shutdown
> handlers.

Well, I guess I'm abusing this code a bit.

The problem is I use Sony IMX290 and IMX462 image sensors, and they have
an apparently hard-coded timeout of about 2^18 their master clock cycles
(= ca. 7 ms with my setup). After the timeout they simply disconnect
from the I2C bus. Of course, this isn't mentioned in the docs.
Unfortunately, "normal" I2C accesses take frequently more than those
7 ms (mostly due to scheduling when all CPU cores are in use). So I
hacked the IMX I2C driver a bit and now all accesses to the sensor use
the atomic paths and local_irq_save() (inside the driver only).

> My understand is that an ordinary I2C device would just use normal (and
> sleepable) I2C transfers while the device is in use.

You are spot-on here :-) Now I use IMX 290 and 462.

OTOH I wonder if such issues are limited to those sensors only.

Thanks for your immediate response,
--
Krzysztof "Chris" Hałasa

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

2023-10-11 12:01:56

by Alexander Stein

[permalink] [raw]
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Hi,

Am Mittwoch, 11. Oktober 2023, 13:25:55 CEST schrieb Krzysztof Hałasa:
> Stefan,
>
> > I cannot answer whether the delay is needed for atomic transfer or not.
> > But I can give a bit of context for I2C atomic transfers in general.
> >
> > These where only introduced for a very narrow and special uses shutting
> > down the device/power with external PMICs in the kernel's shutdown
> > handlers.
>
> Well, I guess I'm abusing this code a bit.
>
> The problem is I use Sony IMX290 and IMX462 image sensors, and they have
> an apparently hard-coded timeout of about 2^18 their master clock cycles
> (= ca. 7 ms with my setup). After the timeout they simply disconnect
> from the I2C bus. Of course, this isn't mentioned in the docs.
> Unfortunately, "normal" I2C accesses take frequently more than those
> 7 ms (mostly due to scheduling when all CPU cores are in use). So I
> hacked the IMX I2C driver a bit and now all accesses to the sensor use
> the atomic paths and local_irq_save() (inside the driver only).

I assume that the master clock is running independently to I2C from the SoC
the sensor is attached to. Your calculations indicate you are assuming ~400kHz
I2C clock frequency.
But nothing is preventing that sensor from running on a 100kHz I2C bus. Even
this "atomic" hack will not be sufficient in that case.

Best regards,
Alexander

> > My understand is that an ordinary I2C device would just use normal (and
> > sleepable) I2C transfers while the device is in use.
>
> You are spot-on here :-) Now I use IMX 290 and 462.
>
> OTOH I wonder if such issues are limited to those sensors only.
>
> Thanks for your immediate response,


--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/



2023-10-11 12:18:45

by Krzysztof Hałasa

[permalink] [raw]
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Hi Alexander,

Alexander Stein <[email protected]> writes:

> I assume that the master clock is running independently to I2C from the SoC
> the sensor is attached to. Your calculations indicate you are assuming ~400kHz
> I2C clock frequency.

This is correct.

> But nothing is preventing that sensor from running on a 100kHz I2C bus. Even
> this "atomic" hack will not be sufficient in that case.

It will be sufficient. Even if all times are 4x longer (they shouldn't
since the CPU won't get slower), they wouldn't reach, say, 1200 us.
There would still be quite a lot of a margin (the timeout is 7 ms).
Even with the faster MCLK (these sensors can operate on 37.125 and on
74.250 MHz) and IF the timeout is still 2^18 in the latter case (meaning
3.5 ms), it would most probably work.

Of course, the atomic hack is not meant for general consumption (at
least for now in its current shape), only the udelay() reduction
(100 -> 1) could probably go in.
--
Krzysztof "Chris" Hałasa

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

2023-10-12 22:18:22

by Stefan Lengfeld

[permalink] [raw]
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity

Hi Chris,

> > My understand is that an ordinary I2C device would just use normal (and
> > sleepable) I2C transfers while the device is in use.
>
> You are spot-on here :-) Now I use IMX 290 and 462.
>
> OTOH I wonder if such issues are limited to those sensors only.

Hmm, yes. I know no other I2C device that has these timeout issues. (*)

> The problem is I use Sony IMX290 and IMX462 image sensors, and they have
> an apparently hard-coded timeout of about 2^18 their master clock cycles
> (= ca. 7 ms with my setup). After the timeout they simply disconnect
> from the I2C bus. Of course, this isn't mentioned in the docs.

hmm. I have no idea about this sensor and your setup. So I can just give hints:

This timeout seems strange. If this 7 ms timeout is required, it would mean
that I2C masters require to fullfill real-time/deadline requirements. For
"small" I2C master in microcontrolles this seems ok-ish, but for general
operating systems real-time requirements are hard. The real-time patches for
linux just landed recently and it still requires fine tuning the system for the
required deadlines.

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

> Unfortunately, "normal" I2C accesses take frequently more than those
> 7 ms (mostly due to scheduling when all CPU cores are in use).

Yes, correctly. There are multiple cases in which I2C transactions to the same
device can be preempted/delayed: A busy system, as you said, or when some other driver
in the kernel accesses another I2C device on the same bus. This will lock the
bus/I2C adapter for the duration of its transfer.

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.

Kind regards,
Stefan

[1]: https://www.i2c-bus.org/repeated-start-condition/

(*) Fun answer: Actually external watchdogs have timeouts. But the timeout
duration is in the range of seconds, not milliseconds. And timeout expiration
is expected (in error cases ;-).

2023-10-13 07:17:38

by Wolfram Sang

[permalink] [raw]
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity


> 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.

If the HW does rep_start properly, it is even guaranteed on the bus
because the bus is never seen as free by other participants. Check
"START and STOP" conditions in the I2C specs.


Attachments:
(No filename) (517.00 B)
signature.asc (849.00 B)
Download all attachments

2023-10-13 10:39:49

by Krzysztof Hałasa

[permalink] [raw]
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