search

raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.79k stars 953 forks source link

Race Condition in I2C Slave #1669

Open progvault opened 8 months ago

progvault commented 8 months ago

I encountered a race condition in the I2C Slave: https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_i2c_slave/i2c_slave.c#L56-L79

If there is active traffic on the bus from the master (i.e. device address retries) during initialization, when the slave is enabled before the interrupts are unmasked, the slave will acknowledge the address but the RD_REQ won't assert (stays low). This is because the RD_REQ bit wasn't unmasked when the slave was enabled. The result is a indefinitely stalled bus because RD_REQ is low and the TX FIFO remains empty. Or in put another way, the RD_REQ interrupt never fires so nothing gets loaded into the TX FIFO.

Might be better to unmask the interrupts, then enable the slave. Like this

hw->intr_mask = I2C_IC_INTR_MASK_M_RX_FULL_BITS | I2C_IC_INTR_MASK_M_RD_REQ_BITS | I2C_IC_RAW_INTR_STAT_TX_ABRT_BITS | I2C_IC_INTR_MASK_M_STOP_DET_BITS | I2C_IC_INTR_MASK_M_START_DET_BITS;

i2c_set_slave_mode(i2c, true, address);

https://github.com/raspberrypi/pico-sdk/blob/6a7db34ff63345a7badec79ebea3aaef1712f374/src/rp2_common/pico_i2c_slave/i2c_slave.c#L56-L79

peterharperuk commented 3 weeks ago

Sorry for the rather late response, but can you give us any indication how we might be able to reproduce this race condition? In particular - what kind of device is the master?