Use modify instead of write for clearing sr1 #298
Conversation
|
I think that all errors should be additionally cleared before a transaction begins. |
|
I think this code is not optimal. You read sr1 twice. Before comparison and when do modify. |
src/i2c.rs
Outdated
| @@ -220,16 +220,16 @@ macro_rules! wait_for_flag { | |||
| let sr1 = $i2c.sr1.read(); | |||
|
|
|||
| if sr1.berr().bit_is_set() { | |||
| $i2c.sr1.write(|w| w.berr().clear_bit()); | |||
| $i2c.sr1.modify(|_r, w| w.berr().clear_bit()); | |||
There was a problem hiding this comment.
| $i2c.sr1.modify(|_r, w| w.berr().clear_bit()); | |
| $i2c.sr1.write(|w| w | |
| .bits(0xffff) | |
| .berr().clear_bit() | |
| ); |
Something like this
There was a problem hiding this comment.
In theory, one should use an all-one mask instead of the previous register value, because the register value can be changed during the read-modify-write sequence and this code will end up clearing bits it wasn't supposed to clear.
TheZoq2
force-pushed
the
fix_i2c_sr_clear
branch
2 times, most recently
from
c0408b6
to
10b1b8d
Compare
|
Updated the PR with the suggested changes, status flags should now be cleared before starting a new transaction, and only the error we actually noted is cleared. I hope the unsafe I had to add is correct, writing to read only registers should have no effect. We also don't have a way to read more than one error flag, perhaps that's something we should add at some point |
TheZoq2
force-pushed
the
fix_i2c_sr_clear
branch
from
10b1b8d
to
d805d28
Compare
As per the discussion in #282
This seems better, but it also leaves the possibility of some flags not being cleared if multiple flags are raised at the same time. Is that something that can even happen, and how should we deal with it then?
CC: @Disasm