Don't busy-wait when sending on UART

[smurfix]

Contribution description

Busy-waiting on UART transmission is not a good idea (unless called from interrupt …).

This commit modifies those UART drivers which already use a transmit interrupt+buffer to wait on a mutex when the buffer is full, instead of busy-looping until there's space.

Testing procedure

I verified that this scheme works on a bluepill board.

The others are untested.

[smurfix]

For short delays busy waiting will be faster than locking a mutex and switching context.

At the point where this patch takes effect, a "short delay" is unlikely, as we've already added UART_TXBUF_SIZE to the send buffer and are presumably about to add a bunch more, each of which we'd also have to wait for.

Better to have a single long(ish) delay (until the buffer is empty) and let other tasks proceed while we're waiting.

[smurfix]

What do the numbers look like in tests/periph_uart_nonblocking with / without this patch?

It takes eight ms or so longer, which is not a surprise. I taught the interrupt code to restart the sender halfway through instead of when the buffer is completely empty, which reduced the overhead to four ms.

That's with the plain "nonblocking" test. When I remove the delay from it, the overhead shrinks to ~800µs (total runtime: 58178 instead of 57365 µs). More importantly, though, I added a second thread that wakes up once a millisecond to increment a counter; that increases the overhead to 58234 µs, i.e. a mere half microsecond per task switch. Can't do much better than that IMHO.

I can add a config parameter to this PR that allows people to disable the task switching, if you think the overhead is too much. In any case, my project simply isn't going to work reliably without this. I'd rather not carry a customized RIOT in my project.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

[smurfix]

OK, so do you think that I should make this conditional, or …?

[benpicco]

Sorry for forgetting about this.

Just to recall correctly: The issue was that the UART drivers busy loop when the tsrb is full and the idea was to instead lock the thread instead until there is again room in the tsrb.

I agree that this makes sense. However, I don't think adding this logic to each UART driver individually is the best way to go about it.

How about adding a tsrb_add_blocking() instead that keep the logic contained in a common place?