sipeed / bl602-hal

Hardware Abstract Layer for BL602 RISC-V WiFi + BLE SoC in embedded Rust
Other
74 stars 14 forks source link

i2c try_write does not block correctly #23

Closed 9names closed 3 years ago

9names commented 3 years ago

If you send multiple writes back-to-back, the first write succeeds but the subsequent writes do not block or return errors. Further writes succeed if you manually insert delays between calls to try_write. try_write is supposed to be a blocking API, so any crates that take an i2c instance will fail to work correctly

Here's a simple test case, writing the first pair of initialization values to an MCP23017 I2C IO expander

let sda = parts.pin1.into_i2c_sda();
let scl = parts.pin2.into_i2c_scl();
let mut i2c = hal::i2c::I2c::i2c(
    dp.I2C,
    (scl, sda),
    100_000u32.Hz(),
    clocks,
    );

// This data is sent and ack'd
let mut two_bytes:[u8;2] = [0;2];
if !i2c.try_write(0x20, &two_bytes).is_ok() {
  serial.write_str("err writing\r\n").ok();
}

// This data is lost
two_bytes = [0x1,0x0];
if !i2c.try_write(0x20, &two_bytes).is_ok() {
  serial.write_str("err writing 2\r\n").ok();
} 
bjoernQ commented 3 years ago

this should be looked into after #22 is merged

most likely it's because the blocking is on cr_i2c_bus_busy_clr which is definitely the wrong bit - should be sts_i2c_bus_busy

additionally it seems that cr_i2c_m_en shouldn't be set before something was written to i2c_fifo_wdata before

bjoernQ commented 3 years ago

Created PR #24 which hopefully fixes your problem.

I couldn't reproduce what I said about i2c_fifo_wdata so it probably wasn't real.

I tried to interface to an SSD103 and it works for me on this branch and also everything looks fine using a logic analyzer.

I2C_SSD1306_BL602

9names commented 3 years ago

This is much better, but still not working correctly for me. I'm obviously hitting the very short timeouts in the driver https://github.com/bjoernQ/bl602-hal/blob/5f55b1889e6834b5657d43be12ca1b3bff152f5d/src/i2c.rs#L151 and that's something I could work with, but we're not correctly reporting when we should have returned an error - maybe we aren't clearing enough flags/internal state after an error? Rather than have you guess at the problems, I've put some code in a branch for you to try: https://github.com/9names/bl602-rust-example/tree/i2c_testing I write 8 pairs of bytes to the device. I've added a very short delay between transactions. If you make it 10x longer, all the bytes get sent. With the current delay, the first 4 pairs get sent, the last 4 do not (according to my logic analyzer) i2c.try_write reports success without sending the 6th and 8th pairs of bytes.

Starting i2c test
writing success
writing success
writing success
writing success
err Timeout
writing success
err Timeout
writing success
Finished i2c test
bjoernQ commented 3 years ago

I am really puzzled about this. I tried your code (had to change the address since I don't have any i2c slave with address 0x20) and it works

Starting i2c test<\r><\n>
writing success<\r><\n>
writing success<\r><\n>
writing success<\r><\n>
writing success<\r><\n>
writing success<\r><\n>
writing success<\r><\n>
writing success<\r><\n>
writing success<\r><\n>
Finished i2c test<\r><\n>

Also in Saleae it looks fine: saleae

Here is the export: i2c.csv

But you are totally right that once it gets into an error state it won't recover. On my branch I pushed some changes:

But I can't really test it since I don't get into the situation. I will try a bit more but maybe these changes already change things to the better.

9names commented 3 years ago

I gave it a quick test and it still failed, but it could be because cargo didn't bring in your changes. Agreed that it's very hard to fix something when you can't reproduce it. I think it would be best to merge https://github.com/sipeed/bl602-hal/pull/24 and I try to diagnose what's going wrong.