Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
inodentry opened this issue Dec 30, 2020 · 5 comments
Open

Comments

@inodentry
Copy link

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!

@TheZoq2
Copy link
Member

TheZoq2 commented Jan 2, 2021

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
Copy link
Member

TheZoq2 commented Jan 2, 2021

I pushed 7f33ced which adds docs now

@inodentry
Copy link
Author

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
Copy link
Author

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);
}

@TheZoq2
Copy link
Member

TheZoq2 commented Jan 23, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants