212 i2c fix broken timeout handler (Meson) #241
Conversation
omeh-a
added
drivers
omeh-a
force-pushed
the
212-i2c-fix-broken-timeout-handler
branch
3 times, most recently
from
773571e to
6dd5973
Compare
|
What happens with the example now? I imagine we get thousands of error responses in the client now? Is that not messing anything up in the clients? |
(referring to just the PN532 part of the I2C example). |
Yeah pretty much. These timeouts occur because of how the PN532 abuses NACKs and it's not the driver's fault. It doesn't mess anything up, although I've noticed there is some kind of weird cross-talk issue when the ds3231 and pn532 are enabled at the same time that I am trying to fix now. |
|
Converting to draft pending fixing the crosstalk issue mentioned above |
omeh-a
force-pushed
the
212-i2c-fix-broken-timeout-handler
branch
4 times, most recently
from
cbefc96 to
65ef3fb
Compare
omeh-a
force-pushed
the
212-i2c-fix-broken-timeout-handler
branch
from
65ef3fb to
8f5cb2c
Compare
|
Little block of code for token return got yeeted by the patch generated by the style checker somehow?? Adding back now. |
omeh-a
force-pushed
the
212-i2c-fix-broken-timeout-handler
branch
from
8f5cb2c to
225a98f
Compare
| // Max number of timeout IRQs before giving up. For some devices such as the pn532, | ||
| // these arrive constantly in normal operation. As a result we need a high threshold | ||
| // before acting. | ||
| #define TIMEOUT_MAX 100000 |
There was a problem hiding this comment.
Seems weird to work-around broken hardware in the driver rather than the client.
There was a problem hiding this comment.
I might be missing something but here it seems we're just hacking around the fact that the PN532 generates a lot of timeouts.
There was a problem hiding this comment.
The real issue is that we don't know why the timeouts get generated. The main thing we need this driver to do is to avoid locking up in the case that an actual defective device ACKs and then stops. Since the I2C hardware doesn't stop when an IRQ arrives, it's really hard to reason about whether any particular timeout is authentic, so I figured that just counting them is an OK fix since we know that a large quantity continuously discounts the possibility of a weird device.
Since we have no way to know why the IRQs fire, we can't really figure out why the PN532 causes it. IMO it's not worth leaving this driver in a state where it can lock up because of that
There was a problem hiding this comment.
(as in the timeout IRQs are undocumented - the actual condition that makes them fire is unclear to me even when looking at the logic analyser output)
omeh-a
force-pushed
the
212-i2c-fix-broken-timeout-handler
branch
3 times, most recently
from
976a037 to
ace86f5
Compare
Signed-off-by: Matt Rossouw <[email protected]>
omeh-a
force-pushed
the
212-i2c-fix-broken-timeout-handler
branch
from
ace86f5 to
328df63
Compare
This commit adds a timeout handler to discard requests if a timeout is reported by the I2C hardware.