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

Fix K20DX uart race condition #97

Merged
merged 4 commits into from
Jul 11, 2016

Conversation

c1728p9
Copy link
Contributor

@c1728p9 c1728p9 commented Apr 30, 2016

If a uart interrupt comes in at the right time then
write_buffer.cnt_out can get incremented twice. This causes the TX
interrupt to get stuck in a state where it is constantly transmitting
data.

This patch fixes that race condition by incrementing
write_buffer.cnt_out in just one place - the uart interrupt.

@c1728p9
Copy link
Contributor Author

c1728p9 commented Jun 22, 2016

@mbed-bot: TEST

@c1728p9 c1728p9 mentioned this pull request Jun 22, 2016
@c1728p9
Copy link
Contributor Author

c1728p9 commented Jul 8, 2016

Updated this PR to fix the KL26Z which has this problem as well. Also added a test to catch future regressions.

The bug causing the echo test does not effect the atsam3u2c and lpc11u35 due to how the TX interrupt is enabled/disabled. The race condition causing TX count to get corrupted could not be reproduced in the atsam3u2c and lpc11u35.

This PR does not comprehensively fix UART race conditions, since that would require an overhaul of each uart driver. Instead these are point fixes.

@c1728p9
Copy link
Contributor Author

c1728p9 commented Jul 10, 2016

@mbed-bot: TEST

@jupe
Copy link

jupe commented Jul 11, 2016

this might fix some problems what we have seen..

c1728p9 and others added 4 commits July 11, 2016 14:40
If a uart interrupt comes in at the right time then
write_buffer.cnt_out can get incremented twice.  This causes the TX
interrupt to get stuck in a state where it is constantly transmitting
data.

This patch fixes that race condition by incrementing
write_buffer.cnt_out in just one place - the uart interrupt.
When the mbed-os echo test runs it causes DAPLink to stop transmitting
characters to the device.  The echo test sends a string to the device,
reads the same string back and then repeats this sequence.   With
optimizations in mbed-host-tests version 0.2.15 the repeat happens much
faster, revealing this issue.

This problem occurs because tx_in_progress is set to 1 in the middle
of a a call to uart_write_data when UART1_RX_TX_IRQHandler is
processing a UART RX interrupt.  Because this flag is set, the
function uart_write_data does not enable the transmitter interrupt
as it should.  This causes DAPLink to stop sending characters.

This patch fixes this problem by checking that the write_buffer has
data to send and enabling the transmitter interrupt in a critical
section.  It also prevents interrupts that aren't enabled from
being processed to further avoid any race conditions.  Finally,
it removes tx_in_progress since this flag is no longer necessary.
Bring k20dx uart fixes into the kl26z:
-Fix UART bug causing hang on echo test
-Fix K20DX uart race condition
This patch adds a timing test which stress tests the UART by
randomizing timing.  The test is for catching race conditions in the
uart, specifically when the uart finishes or starts sending data.
@c1728p9
Copy link
Contributor Author

c1728p9 commented Jul 11, 2016

@mbed-bot: TEST

@c1728p9 c1728p9 merged commit 93ed9f3 into ARMmbed:master Jul 11, 2016
@c1728p9
Copy link
Contributor Author

c1728p9 commented Jul 11, 2016

This should fix #115

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

Successfully merging this pull request may close these issues.

2 participants