Fixed syslink unhandled overrun bug #121
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit fixes a bug, whereby an overrun of the syslink UART buffer was not handled in the interrupt routine, causing constant retriggering and reentry, stalling the system. Overruns are caused when high CPU/interrupt load causes the syslink UART data buffer to not be processed fast enough. This bug was fixed by: 1. handling unhandled conditions in the ISR, and 2. reordering/simplifying instructions in the ISR to reduce latency before data read. As it is currently implemented, every received byte causes the CPU to be interrupted, and the byte to be sent to the syslink for processing (this is due to a variable packet size and hence unknown transaction length). A nicer implementation would be to fix the packet length at its maximum (32 bytes), and use DMA for the entire RX transaction. This would entirely avert the problem of an overrun causing packet loss. I assert (without evidence) that the time required to interrupt the CPU every byte, is less than the overhead due to fixing the packet size at its maximum length. Signed-off-by: Mike Hamer <mike@mikehamer.info>
|
Thanks a lot for the fix. I agree, sending fixed sized packet would make things easier and I actually though about doing it from the beginning but back then it would have been over-optimisation. It is possible to switch to fixed sized packet right now without changing the protocol: we just have to pad the packets with zeros until we reach the wanted size. Then on the receiving side we can "synch" the DMAs (ie. at the beginning we might end up having one packet in the middle of a transaction, this can be fixed for the following transactions). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes a bug, whereby an overrun of the syslink UART buffer was not handled in the interrupt routine, causing constant retriggering and reentry, stalling the system.
Overruns are caused when high CPU/interrupt load causes the syslink UART data buffer to not be processed fast enough.
This bug was fixed by:
As it is currently implemented, every received byte causes the CPU to be interrupted, and the byte to be sent to the syslink for processing (this is due to a variable packet size and hence unknown transaction length).
TODO: A nicer implementation would be to fix the packet length at its maximum (32 bytes), and use DMA for the entire RX transaction. This would entirely avert the problem of an overrun causing packet loss. I assert (without evidence) that the time required to interrupt the CPU every byte, is less than the overhead due to fixing the packet size at its maximum length.
Signed-off-by: Mike Hamer mike@mikehamer.info