stm32f1xx-hal icon indicating copy to clipboard operation
stm32f1xx-hal copied to clipboard

i2c timeouts not working by default (due to disabled DWT cycle counter)

Open inodentry opened this issue 4 years ago • 5 comments

I am using some I2C sensors that are not 100% reliable and sometimes the connection breaks.

I2C timeouts were not working, regardless of the values I provided to the builder functions when initializing the peripheral, causing my program to occasionally hang indefinitely when trying to use I2C, with no way out (other than reset or interrupt). I spent many hours in frustration, trying to figure out why this was happening.

After digging through the I2C code in this crate, I discovered that the blocking I2C implementation internally relies on the cycle counter from the DWT to implement timeouts. However, that cycle counter starts out disabled by default and must be explicitly enabled!

This was not documented anywhere!

Why does this crate not automatically take care of enabling the DWT cycle counter (that it relies on) when using blocking I2C? This should be fixed. All the APIs should automatically configure the hardware state they need.

If not, at the very least, it should be documented.

Just adding this one line to my program fixed everything and now my code works without issue:

core.DWT.enable_cycle_counter()

However, I had no way of knowing that I needed this! I spent many hours trying to figure it out!

inodentry avatar Dec 30 '20 12:12 inodentry

Oh wow, that's quite an oversight! I'll make a note in the docs about this though we probably should change the API to either ensure that the DWT is on, use a different timer, or perhaps not do timeouts in the blocking i2c impl which seems to be what the f4 hal does

TheZoq2 avatar Jan 02 '21 10:01 TheZoq2

I pushed 7f33ced which adds docs now

TheZoq2 avatar Jan 02 '21 10:01 TheZoq2

or perhaps not do timeouts in the blocking i2c impl which seems to be what the f4 hal does

Timeouts are very useful functionality. Please don't remove them from this API without providing another easy-to-use alternative.

inodentry avatar Jan 02 '21 13:01 inodentry

Actually, this is not all. There is even more to this issue.

By manually enabling the cycle counter, like I described above, it works, but only when the debugger is connected. If I reset or power cycle the microcontroller without the debugger being connected, timeouts still hang like before.

After some investigation, it seems that the DWT hardware is completely disabled by default, and the debugger enables it for us. To make it work without the debugger, I needed a way to enable it manually.

I found that it is a bit in the DEMCR register, which controls the various ARM Cortex M debug units. However, this register is currently not exposed via a Rust API at all (I could not find anything in this crate, or in the cortex-m crate).

For now, I got the raw address of the register from the ARM Cortex M documentation and did it myself using unsafe / raw pointer access. It works, but a proper wrapper API for it should be added (I guess it would go in cortex-m, as it is part of the core).

Here is the code I wrote to enable the DWT:

let demcr = 0xE000EDFC as *mut u32;
unsafe {
    let old = demcr.read_volatile();
    let new = old | 0x01000000;
    demcr.write_volatile(new);
}

inodentry avatar Jan 15 '21 17:01 inodentry

Oh right, that's what I remembered which is why I suggested we should get rid of/rewrite the blocking i2c to use timer. Depending on debug hardware feels dirty and clearly has a bunch of pitfalls.

TheZoq2 avatar Jan 23 '21 09:01 TheZoq2