I2C NACK being ignored by ioctl I2C_RDWR

[jdf-advanced]

Describe the bug

I'm using C to interface with some I2C devices, but I've come across a corner case that I think could cause really infrequent issues. I want to see if a community has spotted a way round it, or if I'm doing something wrong.

I'm using ioctl with I2C_RDWR to do a read of a register using a repeated start. The ideal procedure is this:

[START CONDITION]
[SEND DEVICE ADDR + W]
[RECEIVE ACK]
[SEND REGISTER ADDR]
[RECEIVE ACK]
[START CONDITION]
[SEND DEVICE ADDR + R]
[RECEIVE ACK]
loop {
  [RECEIVE DATA]
  [SEND ACK]
}
[STOP CONDITION]

If the first ACK is instead a NACK (i.e. the slave doesn't respond to it's address) I'd expect the master (the pi) to send a STOP condition and cease the transmission. Instead it proceeds to do a repeated start and try the read anyway.

If the initial NACK is caused by the slave device being busy, but it's not busy by the time the read is attempted, this could result in invalid data being received, and possibly unintended behaviour from the slave.

It could cause issues if a slave device performs some sort of state change (clearing interrupts, kicking off the next ADC sample conversion etc.) when a read occurs. As far as the slave is concerned, it may have just done a perfectly valid read, but the master has ignored the data.

Steps to reproduce the behaviour

To reliably simulate this happening, I've been doing the write to a device that doesn't exist, and then a read from a device that does. Here's the code:

file_i2c = open("/dev/i2c-1", O_RDWR); 

uint8_t inbuf[20];
uint8_t outbuf[20];
struct i2c_rdwr_ioctl_data packets;
struct i2c_msg messages[2];

outbuf[0] = 0x40;
messages[0].addr = 0x36;
messages[0].flags = 0;
messages[0].len = 1;
messages[0].buf = outbuf;

messages[1].addr = 0x49;
messages[1].flags = I2C_M_RD;
messages[1].len = 4;
messages[1].buf = inbuf;

packets.msgs = messages;
packets.nmsgs = 2;

int result = ioctl(file_i2c, I2C_RDWR, &packets);

This is the logic analyser's output of what's happening:

ioctl does report the error. In the above case result would be -1, so we know it's failing. However, the read still shouldn't happen.

Device (s)

Raspberry Pi CM4, Raspberry Pi CM4 Lite

System

I'm using a custom busybox build so I don't have /etc/rpi-issue or vcgencmd, and haven't tested this using raspberry pi OS yet.

The kernel version for busybox is:

Linux secure 5.15.72-v7l #1 SMP Fri Jan 27 13:03:09 GMT 2023 armv7l GNU/Linux

Logs

No response

Additional context

No response

[pelwell]

Thank you for the clear report. I think I understand and agree with what you say, but can you point me at any piece of Linux I2C documentation to support your assertion that the read should not proceed?

[6by9]

Forum thread - https://forums.raspberrypi.com/viewtopic.php?t=350180

I'd had a brief look yesterday, and the error flag appears not to be reported by the hardware until the end of the complete transaction. I'm not sure there's much that can be done about that one.

[pelwell]

We take a dim view of cross-posting. It's a shame that this was sanctioned cross-posting, but that the followup comments weren't echoed here.

[jdf-advanced]

I can't find anything that conclusively says it shouldn't proceed, but I can't find anything that says it should either.

I'd suggest that since the spec is ambiguous, choosing the safer option and aborting the transaction upon the first NACK of an address would be the desired behaviour.

The closest I can get to determining a desired behaviour from the kernel docs is the description of the I2C_M_IGNORE_NAK flag:

Normally message is interrupted immediately if there is [NA] from the client. Setting this flag treats any [NA] as [A], and all of message is sent. These messages may still fail to SCL lo->hi timeout.

Now the specification doesn't seem to specify what it means by message, so in that sense it's ambiguous. But the driver uses message to mean each section of a transfer separated by start conditions. So the question we then have to ask is this: If a message within a transfer fails, should the rest of the transfer be aborted?

That said, if @6by9 is correct, this is all a bit of a moot point because this particular behaviour may be determined by the hardware.

[jdf-advanced]

@pelwell Apologies for the cross posting, I was not aware that it was frowned upon, or the etiquette around copied discussion.

I see you've closed the issue as completed. Can I ask why?

[6by9]

By not including a reference to the forum thread when opening this issue, pelwell will have gone over the same steps that I had already done and reported on that the forums. A simple link to the previous discussion stops that duplication of effort.

If you had already opened this issue before I'd investigated, then my comments would have been on here and not on the forums.

Reading the code again, I suspect the delayed ERR report is inherent as the interrupt is TXW for TX FIFO needs writing. That most likely occurs before the ACK/NACK phase has even happened. This is sort of also implied by the comment at https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/i2c/busses/i2c-bcm2835.c#L321-L332 too.

[pelwell]

After thinking about this for a while I've made some progress:

1. Adding a delay at the start of the ISR makes it work. The delay could be tuned to the clock rate and made conditional on it being the first interrupt of a write transaction.
2. Setting INTD and pre-filling the fifo for writes (to avoid the initial TXW) also works, but needs further work in the ISR because INTD no longer means all complete.

I feel there should be a solution with one of these approaches - I prefer 2 - but it will take a lot of testing until I feel comfortable with it.

[jdf-advanced]

Awesome, thanks very much both, and sorry once again for not linking the forum post and issue. Let me know if I and when can help with regards to testing.

[pelwell]

There's now a PR implementing option 1 - see #5434.

[pelwell]

To install a trial kernel with the updated module, run sudo rpi-update pulls/5434.