neorv32_twi_start_trans() does not work with prsc>1

[andkae]

Hi Stephan,

at the moment i do some tests with the TWI. My belonging C code looks like follows:

/** Includes **/
/* Standard libs */
#include <string.h>     // memset

/* Processor */
#include <neorv32.h>
#include <neorv32_twi.h>


/**
 *  main
 */
int main()
{
    /** Variables **/

    /* init UART */
    neorv32_uart0_setup(115200, PARITY_NONE, FLOW_CONTROL_NONE);

    /* entry message */
    neorv32_uart0_print("Interfaces I2C Core of NEORV32\n");

    /* setup I2C */
    neorv32_twi_disable();
        // neorv32_twi_setup(uint8_t prsc)
    neorv32_twi_setup(4);
    neorv32_twi_mack_enable();
    neorv32_twi_enable();

    /* write to I2C */
    neorv32_twi_start_trans(0x55);
    neorv32_twi_generate_stop();

    while ( 1 ) { } // inifinite loop
    return 0;
}

For asserting the transfer i use the NEORV32 function neorv32_twi_start_trans. For small prescaler values like 1 works this very well.

PRSC=1:

For PRSC=4 is only the starbit sended and then happends nothing

I did some investigation, the issue seems like follows, if a higher PRSC value is used, then is the FSM in the startbit state arbiter=101 and the write of ths address - in my case 0x55 - is skipped:

One possible solution could be to poll the busy flag, this would lead to an change in neorv32_twi_start_trans() and the IRQ driven I2C would not work anymore.

What are your thoughts?

BR,
Andreas

[stnolting]

Hey Andreas,

you need to generate a start conditions first before you can do actual TWI transfers. So you need to call neorv32_twi_generate_start first:

    /* write to I2C */
    neorv32_twi_generate_start(); // generate START conditions before triggering actual transfers
    neorv32_twi_start_trans(0x55);
    neorv32_twi_generate_stop();

I just had a quick look at the TWI documentation - and it seems like the documentation is not really clear here. I need to fix that! 😉

[stnolting]

I just realized I mixed up neorv32_twi_start_trans() and neorv32_twi_trans() - my fault! 😅
I will have a look at this!

[andkae]

if i use this code snippet:

    /* write to I2C */
    neorv32_twi_generate_start();
    while ( 0 != neorv32_twi_busy() ) {
        asm ("nop");
    }
    neorv32_twi_trans(0x55);
    while ( 0 != neorv32_twi_busy() ) {
        asm ("nop");
    }
    neorv32_twi_generate_stop();

then works the I2C very fine:

[stnolting]

First things first: the TWI controller works as expected on real hardware.

This is a great example of a hardware-vs-simulation mismatch due to the lack of a dedicate hardware reset (a related problem is described in #330).

The TWI module uses bit 31 of the status and control register to indicate a "BUSY" state. For example, this bis is set when generating a START condition (like in your example). The according C functions polls the busy bit to find out when the START condition has been successfully generated.

Now the problem: bit 30 of the TWI control and status register contains the state of the last ACK/NACK that has been received. This bit is undefined as there has not been any ACK/NACK yet plus the source register of this signal has no defined hardware reset.

The CPU fetches the whole 32-bit of the status and control register into some register in the CPU's register file for further inspection. The check in the polling loop evaluates to a "branch if less than zero" as the compiler is smart enough to see that the busy flag is basically the sign bit of a 32-bit value.

The according VHDL comparison in the CPU is:

  cmp(cmp_less_c)  <= '1' when (signed(cmp_opx) < signed(cmp_opy)) else '0'; -- signed or unsigned comparison

As bit 30 is undefined this piece of VHDL code returns an incorrect result making the program abort the polling loop - falsely assuming that the busy flag has cleared. In summary, this messes up the TWI transaction.

if i use this code snippet:

In contrast, this works fine as the compiler will use a "branch if zero" instruction here using a different piece of VHDL code:

  cmp(cmp_equal_c) <= '1' when (rs1_i = rs2_i) else '0';

In this situation, this code returns the correct result making the polling work fine.

[andkae]

Hi Stephan, thanks for your explanation. What could be the solution, providing a HW reset into the HDL sources, or using the SW workaround snippet?

Thanks for you support!

BR,
Andreas

[stnolting]

Your fix is a great workaround for now, but I think a dedicated hardware reset for all IO/peripheral devices would be a better and permanent solution. Otherwise we might get trouble with this again in other places. I think I'll start working on that reset issue on another branch. Then you can have a look, too, and we can make sure to eliminate all those hardware-vs-simulation mismatch issues.

[andkae]

Hi Stephan, it works with #334 ! Thanks for the really fast providing the fix

    /* write to I2C */
    neorv32_twi_start_trans(0x55);
    neorv32_twi_generate_stop();

the belonging waveform with global reset

BR,
Andreas

[stnolting]

Great to hear! You have used the modified version from #334, right?

[andkae]

yes #334 was used