Race Condition in I2C Slave

[progvault]

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);

pico-sdk/src/rp2_common/pico_i2c_slave/i2c_slave.c

Lines 56 to 79 in 6a7db34

	void i2c_slave_init(i2c_inst_t *i2c, uint8_t address, i2c_slave_handler_t handler) {
	assert(i2c == i2c0 || i2c == i2c1);
	assert(handler != NULL);
	uint i2c_index = i2c_hw_index(i2c);
	i2c_slave_t *slave = &i2c_slaves[i2c_index];
	slave->handler = handler;
	// Note: The I2C slave does clock stretching implicitly after a RD_REQ, while the Tx FIFO is empty.
	// There is also an option to enable clock stretching while the Rx FIFO is full, but we leave it
	// disabled since the Rx FIFO should never fill up (unless slave->handler() is way too slow).
	i2c_set_slave_mode(i2c, true, address);
	i2c_hw_t *hw = i2c_get_hw(i2c);
	// unmask necessary interrupts
	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;
	// enable interrupt for current core
	uint num = I2C0_IRQ + i2c_index;
	irq_set_exclusive_handler(num, i2c_slave_irq_handler);
	irq_set_enabled(num, true);
	}

[peterharperuk]

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?

[peterharperuk]

Closing this as I can't reproduce a problem and there's no response from the reported. Tried running the master slave example on two cores. Master keeps transmitting until slave responds. Added delays after i2c_set_slave_mode.