cancel
Showing results for 
Search instead for 
Did you mean: 

Proofing I2C slave against too short reads: outputs old value from TXDR as soon as master clocks again - how to prevent?

SKled.1
Senior II

I have this scenario:

My stm32f0 implements a I2C slave, and I have a raspberrypi with i2ctools as a master.

I define a bunch of 16bit wide registers to access in the stm32.

Now I wanted to test what happens if the master only reads 1 byte instead of 2 bytes, to make the stm32 firmware robust against wrong accesses.

What I see is this:

  • on a logic analyzer, all looks correct: I see a NACK from master on the 1st (and only) R data byte, and then a STOP
  • in the stm32, however, the interrupt order is, on the first attempt of reading only 1 byte:
    • [...] TXIS, TXIS, NACK, STOP.
  • i.e. I get a second TXIS before the NACK, as apparently the TXDR is empty more quickly than the NACK from master is registered
  • wich means my code has no way (?) of telling that this happened and it will write the second byte into TXDR
    • which is not clocked by the master, as it wants only 1 byte
  • the second time I read, again only 1 byte, with the master, the NACK interrupt comes first, I guess because this time TXDR is not empty anymore...
  • Now if I read a proper 16bit word again with master: it is apparent that the bytes are shifted in history, so to speak, by one.
  • i.e. from the 1st time that the TXDR was filled with a 2nd byte but the slave won't clock it out afterwards, started a chain of "you get the previous byte, not the current one"

How can this be fixed?

Is there a way to force the TX to be "empty" without it being clocked/read?

That would help at least for the 2nd time only 1 byte is read and this becomes detectable because the NACK IRQ comes first.

Or can this somehow prevented to happen in the first place?

The "condensed" code snippet below contains the 2 handler functions of my i2c state machine that are involved in the above scenario. They are called by the i2c interrupt handler. The first state, "...SendData..." is active after the ReStart condition (2nd time the ADDR interrupt occurred, this time with ISR.DIR==1 i.e. read), and it's now about to write data to the master - so txBufPos is set to 0 and the TX interrupt is enabled right before changing to that state. (it's disabled again in that state given a condition, see below)

static void handle_state_SendDataBytesToMaster(I2cSlave* m)
{
    if (LL_I2C_IsActiveFlag_NACK(m->periph)) // NOTE - this didn't really work, as the 2nd TXIS arrives earlier than the NACK
    {   // EARLY STOP : Abort. If a master reads only one byte instead of two and then stops, we must not expect to send any next bytes.
        LL_I2C_DisableIT_TX( m->periph );
        m->state = I2cSlaveState_WaitForMasterReadFinalization;
    }
    else if (LL_I2C_IsActiveFlag_TXIS(m->periph))
    {	const typeof(m->txBufPos) newTxPos = m->txBufPos + 1;
        if (newTxPos >= sizeof(m->currRegisterVal)) // Expected number of bytes done with this one?
        {	LL_I2C_DisableIT_TX( m->periph );
            m->state = I2cSlaveState_WaitForMasterReadFinalization;
        }
        m->periph->TXDR = 0xff & (m->currRegisterVal >> (8 * m->txBufPos)); // put next byte into the TX register
        m->txBufPos = newTxPos;
    }
}
 
 
static void handle_state_WaitForMasterReadFinalization(I2cSlave* m)
{
    if (LL_I2C_IsActiveFlag_NACK(m->periph))
    {	LL_I2C_ClearFlag_NACK( m->periph );
    }
    else if (LL_I2C_IsActiveFlag_STOP(m->periph))
    {	LL_I2C_ClearFlag_STOP( m->periph );
        m->state = I2cSlaveState_Idle;
    }
}

1 REPLY 1
SKled.1
Senior II

Ok. Since I was not using TXE in the ISR register but only TXIE, I overlooked that little bit:

From the reference manual, 26.7.7 Interrupt and status register (I2C_ISR):

Bit 0 TXE: Transmit data register empty (transmitters)

This bit is set by hardware when the I2C_TXDR register is empty. It is cleared when the next

data to be sent is written in the I2C_TXDR register.

This bit can be written to ‘1’ by software in order to flush the transmit data register I2C_TXDR.

Note: This bit is set by hardware when PE=0.

Interesting. So the only change I made for now was to add a line writing the TXE bit, in the STOP IRQ handler.

That seems to do the trick, now I can make arbitrarily composed sequences of reads from the master with different numbers of "bad" and "good" reads, where bad means reading 1 byte only with i2cget -y 1 <myaddr> <myreg> b, and the device never getting into a weird state.

static void handle_state_WaitForMasterReadFinalization(I2cSlave* m)
{
    if (LL_I2C_IsActiveFlag_NACK(m->periph))
    {	LL_I2C_ClearFlag_NACK( m->periph );
    }
    else if (LL_I2C_IsActiveFlag_STOP(m->periph))
    {	LL_I2C_ClearFlag_STOP( m->periph );
        m->state = I2cSlaveState_Idle;
 
        // HACK: flush TXDR, whether the master correctly read 2 bytes, or 1 byte. In the latter case, our earlier writing to TXDR the 2nd byte that the master won't clock, this removes it again so it won't be read by the master "next round".
        m->periph->ISR |= LL_I2C_ISR_TXE;
    }
}