I2C mode rework

General
1

I2C uses an ever so slightly modified version of the PICO SDK I2C PIO program. I’ve been reworking it a bit because it’s kind of obnoxious to deal with.


;do_nack:
;    jmp y-- entry_point        ; Continue if NAK was expected
;    irq wait 0 rel             ; Otherwise stop, ask for help

do_byte:
    set x, 8                   ; Loop 9 times
bitloop:
    out pins, 1         [7] ; Serialise write data (all-ones if reading)
    nop             side 0b1 [2] ; SCL rising edge
    wait 1 pin, 1          [4] ; Allow clock to be stretched, in pins: 0=sda, 1=scl
    in pins, 1             [7] ; Sample read data in middle of SCL pulse
    jmp x-- bitloop side 0b0 [7] ; SCL falling edge

    ; Handle ACK pulse
    ;out pins, 1         [7] ; On reads, we provide the ACK.
    ;nop             side 0b1 [7] ; SCL rising edge
    ;wait 1 pin, 1          [7] ; Allow clock to be stretched, in pins: 0=sda, 1=scl

    ;jmp pin do_nack side 0b0 [1] ; Test SDA for ACK=0/NAK=1, fall through if ACK, 1 jump pin defined in init
    mov isr, null              ; Reset the input bit counter

This is the business end. At Handle ACK pulse the program tries to test for the expected ACK/NACK of a write operation, and fails with an interrupt.

This is great if you want to queue up a bunch of data to send and then just forget about it. For us though, it makes things overly complicated.

I’ve commented out the Handle ACK pulse chunk, as well as do_nack. I increased the bitloop to 9 cycles. Now it brute forces through the ACK/NACK and we’ll handle the error checking in code.

    wait 1 pin, 1          [4] ; Allow clock to be stretched, in pins: 0=sda, 1=scl

Clock stretching is enabled in this PIO program. This is another source of confusion I’ve seen a few times. This will be split into two programs with and without clock stretching, to be selected from the configuration menu.

1 Like
2

image
image814×199 9.38 KB

image
image792×263 8.12 KB

Previously, the I2C mode would halt execution and display an error after the write address (0b10) was not ACKed. Now it will power through and just acknowledge the NACK.

This is more consistent with BPv3.x UX, but it depends on the user understanding that NACK means nobody answered. I feel this is more robust and more in line with how I expect the interface to work.

2 Likes
3

image
image820×482 17.2 KB

We now have a program for clock stretching and a program without. Selection is though the mode config menu, default is off/disabled.

Next, the I2C low level “drivers” need to be completely reworked. They are a bit of a mess from dealing with error detection in the PIO, so a good clean is in order. This will continue tomorrow.

image
image816×433 24.2 KB

I2C also updated to use the new mode settings display in the info screen. This is coming to all modes as FALA support is verified in each mode.

3 Likes
4 
typedef enum {
    HWI2C_OK = 0,
    HWI2C_NACK = 1,
    HWI2C_TIMEOUT = 2
} hwi2c_status_t;

There is a new type enum for passing around i2c status. The mode, demos, drivers, etc have all been reworked to use it.

image
image820×436 20.7 KB

Reads and writes appear to be correct. The scan command works.

image

This bug is a head scratcher though: the first time the mode starts, there is a constant waveform on the clock pin. After changing modes it goes away.

In the past this was caused by PIO program bugs or SDK bugs, but it was working fine yesterday with no change to the initialization code. Hum.

1 Like
5 
    if(clock_stretch) {
        pio_config.program = &i2c_clock_stretch_program;
        pio_config.offset = pio_add_program(pio_config.pio, pio_config.program);
        i2c_clock_stretch_program_init(pio_config.pio, pio_config.sm, pio_config.offset, sda, scl, dir_sda, dir_scl, baudrate);
    } else {
        pio_config.program = &i2c_program;
        pio_config.offset = pio_add_program(pio_config.pio, pio_config.program);
        i2c_program_init(pio_config.pio, pio_config.sm, pio_config.offset, sda, scl, dir_sda, dir_scl, baudrate);
    }

Easier than I thought. The program was initialized before it was loaded. This fixes everything.

Clock stretching works to the extent that it doesn’t error out. However, I’m going to have to find a device that supports clock stretching and see how it works.

I will do a bit more testing, especially of the various demos, and then push.

3 Likes
6

This might be confusing for a beginner. Does it make sense to indicate in a subtle way that there was a implied NACK response, i.e. a change in color/dimmed or lower case letters, etc? Or better documentation?

1 Like
7

Docs of course. Rework of the syntax compiler won’t come until post v1.0

It is coming though.

1 Like
8

I2C rework is complete and tested. I pushed to main and you can find the update in the latest autobuild.

I’m extremely pleased with the updates, I2C feels much more solid now:

  • No timeout errors (unless a real timeout while clock stretching)
  • No more “I2C bus errors” generated by the PIO program being too “cute”
  • No longer need a violent recovery from failure sequence to shake the PIO loose during “I2C bus errors”
  • Repeated starts should work again
  • Note: if no device responds with an ACK, the bus transaction will no longer abort
1 Like