circle icon indicating copy to clipboard operation
circle copied to clipboard

Questions about I2C slave

Open toptensoftware opened this issue 1 year ago • 33 comments

Where can I find some more information about how i2c slave works? For example:

  • I'm looking at the i2cping example, and it looks like the slave is polling for updates. Is it possible to get an interrupt notification instead of polling?
  • If a read asks for more bytes than available does it block until that many bytes have been read, or does it return immediately with whatever is currently available?
  • Is there any buffering on the received data and if so what size is the buffer?

toptensoftware avatar May 29 '23 23:05 toptensoftware

The Circle I2C slave driver is simple polling driver. The I2C slave peripheral in the Raspberry Pi supports interrupts, but this is not used here. When more bytes are requested than are actually sent by the master, the received bytes are returned immediately in the buffer with the byte count returned by Read() smaller than the given buffer size. The peripheral has a FIFO. I guess it can buffer 16 bytes, but this is not specified in the hardware specification, which is here (page 160-171).

rsta2 avatar May 30 '23 08:05 rsta2

Thanks Rene, that's useful info. Hopefully polling will be enough for now.

toptensoftware avatar May 30 '23 12:05 toptensoftware

Did this work for you or do we need an interrupt driven I2C slave?

rsta2 avatar Jun 15 '23 13:06 rsta2

Hey Rene,

I haven't had a chance to look at it properly yet (I've been stuck trying to get a DPI LCD display panel working). If it's not too hard to implement I'd almost certainly use it. My main concern about polling it is I'll be running an OpenGL render loop too which I think throttles the loop to the frame rate - not sure if that'd be fast enough for polling i2c.

Brad

toptensoftware avatar Jun 15 '23 14:06 toptensoftware

Hi Brad, OK, let me know, when polling I2C is not fast enough.

Rene

rsta2 avatar Jun 15 '23 15:06 rsta2

Hi Rene,

I'm finally getting around to integrating this with my OpenGL render loop and have hit a blocking issue - it seems CI2CSlave::Read() blocks if nothing has been received. I've tried just reading a single byte and it doesn't return until the first byte is received.

How do I poll and just read what's available and return immediately if there's nothing?

Brad

toptensoftware avatar Jun 21 '23 13:06 toptensoftware

fwiw, not sure if this correct but I got this working by replacing the first while loop in CI2CSlave::Read with this:

	while (nCount > 0 && (read32 (ARM_BSC_SPI_SLAVE_FR) & FR_RXFE) == 0)
	{
		if (read32 (ARM_BSC_SPI_SLAVE_RSR) & RSR_OE)
		{
			nResult = -1;

			break;
		}

		*pData++ = read32 (ARM_BSC_SPI_SLAVE_DR) & DR_DATA__MASK;

		nResult++;
		nCount--;
	}

toptensoftware avatar Jun 21 '23 13:06 toptensoftware

Hi Brad, yes, you are right, it blocks and there is no way out with the current implementation. Please give me two days for an update, which is also in sync with sample/16-i2cping.

Rene

rsta2 avatar Jun 21 '23 20:06 rsta2

Thanks Rene. No rush as I can work with my temp hack until you get it sorted.

toptensoftware avatar Jun 21 '23 23:06 toptensoftware

I added a timeout parameter to CI2CSlave::Read() and CI2CSlave::Write() as a more generic solution. When you specify TimeoutNone as 3rd parameter, you should get the same behavior, as suggested by yourself. By default the methods will block, when no transfer occurs, which is similar to the previous behavior. It's on the develop branch.

rsta2 avatar Jun 22 '23 16:06 rsta2

Awesome, thanks Rene. I'll check it out but might be a few days... I've got some other high priority work that's popped up but hope to get back to this soon.

toptensoftware avatar Jun 22 '23 23:06 toptensoftware

Thanks Brad for info. No problem.

rsta2 avatar Jun 23 '23 09:06 rsta2

Hi Rene,

I'm back working on my bare metal Pi project again and I'm having issues with I2C - receiving corrupted packets and occasional lock ups. Some questions:

  • How hard would it be to update CI2CSlave to support an interrupt mode that moves incoming data to a temporary buffer to make it more resilient in case the program is busy and doesn't poll quickly enough?
  • Is there any way I can check if there's been I2C data dropped/lost and/or monitor the state of the I2C port?
  • Are there any other timeouts or control over the slave mode? I2C lockups are usually because the slave is holding the data line and the master can't get control again. I think that might be happening in my case and wondering if there's anything I can do to release it. I'm still investigating what's causing the lock up, but it'd be good to have a way to force the slave to let go on a timeout (if possible).

Anything that can be done to make this more reliable would be much appreciated (at the end of the day I need something rock solid).

Brad

toptensoftware avatar Sep 20 '23 08:09 toptensoftware

Hi Brad,

it should be possible to extend the I2C slave receiver with an IRQ mode. The hardware spec. (pg. 160) mentions a RXINTR, but because I never used it so far, it requires some experimenting. There is also an overrun error interrupt (OEINTR), which could be monitored. There is a BRK (break current operation) bit in the Control register, which according to the spec. stops the current operation and clears the FIFOs.

I'm not fully aware of your use case. Can you please give me more info about how you want to use the I2C slave, so that I can prepare a test program and define the class interface.

Thanks,

Rene

rsta2 avatar Sep 20 '23 15:09 rsta2

Emailed you.

toptensoftware avatar Sep 21 '23 02:09 toptensoftware

For reference, here's a good description of i2c lockup, prevention and recovery:

https://pebblebay.com/i2c-lock-up-prevention-and-recovery/

toptensoftware avatar Sep 21 '23 04:09 toptensoftware

So that it is not lost, attached there is a slightly modified sample/16-i2cping/slave, which contains a I2C slave driver in the files i2cslaveirq.*. The I2C receiver in this driver is IRQ driven and should not "loose" bytes any more. The receiver is always on after Initialize() and can receive packets in the background. It can buffer up to 4096 bytes. When you call Read(), the recently received packet(s) will be returned. If no packet has been received, it immediately returns 0. On error a negative value is returned. The Write() method is mostly unmodified.

i2c-slave-irq.zip

rsta2 avatar Feb 05 '24 15:02 rsta2

While porting the I2C master driver for the Raspberry Pi 5, I discovered this. How it seems, the I2C bus recover must be done by the I2C master, which is implemented here for Linux. The slave cannot do this.

rsta2 avatar Feb 05 '24 15:02 rsta2

Thanks Rene.. Unfortunately I've had to put my project on hold for the moment due to other commitments but this will be useful when I get back to it.

toptensoftware avatar Feb 05 '24 23:02 toptensoftware

No problem Brad. Understood.

rsta2 avatar Feb 06 '24 05:02 rsta2

Hello, I had a similar need recently whereby I was spending a good share of the CPU continuously polling an I2C slave device (I was too lazy to solder its interrupt GPIO). So I wrote an I2CMasterIRQ class - it's probably not 100% complete but it works in my case. Here it is, if interested ? i2c-master-irq.zip

sebastienNEC avatar Feb 28 '24 21:02 sebastienNEC

Thanks @sebastienNEC! I think, @toptensoftware needs an I2C slave on the Raspberry Pi and runs the master on an other type of board. But perhaps your work can be used to add an IRQ-driven I2C master to Circle. What do you think has to be added/changed on your driver to prepare it for that purpose?

rsta2 avatar Feb 29 '24 09:02 rsta2

Not much needs changed I would think, just a bit of sanity-check on your side :-) Right now, my driver has these limitations:

  • It only supports the Write-Then-Read operation, i.e. not individual Read or Write operations. It's not hard to add but it might end up not being used because most I2C slave boards expose a Write/Read register-based "API".

  • It assumes that the number of bytes read/written fits into the I2C's FIFO. I just noticed btw that I am not checking this for the read buffer, so it needs added. Then again, this is not a limitation for usual I2C slave devices.

Also, I might have been a bit lax with the checks on the Status register in CI2CMasterIRQ::InterruptHandler(). I am only checking the 3 more important indicator bits but one should probably have a look at other status bits.

sebastienNEC avatar Feb 29 '24 09:02 sebastienNEC

Oh, one important extra limitation: due to the asynchronous nature, it's possible/realistic that users could make a second call to CI2CMasterIRQ::StartWriteRead() while a first transaction is not done yet. For example when using this class for two different I2C slaves. This would of course completely fail, but I am not safe-checking this right now in the code. It's just a matter of checking the m_nStatus flag at the start of the method, but that's not in the code I shared above :)

sebastienNEC avatar Feb 29 '24 09:02 sebastienNEC

I can of course do these modifications if you'd like, or feel free to do them on your side as part of checking the code if you prefer :)

sebastienNEC avatar Feb 29 '24 09:02 sebastienNEC

It would be a great help, if you would do the necessary modifications. It's also easier for you to do it, because it's your driver and you are familiar with it. So yes, please do!

rsta2 avatar Feb 29 '24 10:02 rsta2

Will do ! One question: the driver shares quite a bit of code with the regular CI2CMaster class. Is it ok to inherit from that class, in order not to duplicate code ? What's the general policy in circle ?

sebastienNEC avatar Feb 29 '24 10:02 sebastienNEC

Thanks! I would think OOP here: If the IRQ-driven I2C master "is a" I2C-polling-master (which is the class CI2CMaster) then I would inherit it from the polling-master. But it isn't. So I wouldn't do that. There could be a common I2C-master base class for both classes, but I think their interfaces are different and I don't want to make it too complicated. The general policy is: Keep it simple! ;) Where inheritance really gives an advantage, just use it. But I wouldn't use it to safe some code here. I think, it's not that much.

rsta2 avatar Feb 29 '24 11:02 rsta2

Ok I've implemented the changes above, I think it's cleaner now. i2c-master-irq.zip

sebastienNEC avatar Mar 01 '24 20:03 sebastienNEC

Thank you! This looks good. I will test it this evening.

rsta2 avatar Mar 04 '24 10:03 rsta2