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

Faulty UART baudrate divisor formula #820

Open
KisImre opened this issue Jul 22, 2024 · 2 comments
Open

Faulty UART baudrate divisor formula #820

KisImre opened this issue Jul 22, 2024 · 2 comments

Comments

@KisImre
Copy link

KisImre commented Jul 22, 2024

The fractional part of the UART baudrate divisor can overflow for certain baudrates. In this example I picked the calculate_baudrate_dividers function and iterated though a range of baudrates while printing the integer and fractional divisor values if they change. In the output this is the interesting part:

baudrate=57861 divint=135 divfrac=1
baudrate=57868 divint=135 divfrac=0
baudrate=57871 divint=134 divfrac=64 <----
baudrate=57874 divint=134 divfrac=63
baudrate=57881 divint=134 divfrac=62

The function outputs fractional divisor value of 64 which gets truncated once it's written into the register, so the effective fractional value will be zero.

The issue is in this line. (baudrate_div >> 7, ((baudrate_div & 0x7F) + 1) / 2 The recommended +0.5 correction is only added to the fractional part but the overflowing carry bit is not added to the integer part. Adding 1 and dividing by 2 before splitting into integer and fractional parts would solve the issue.

I've tested the issue on a Raspberry Pico board using the uart example and changing the baudrate in line 94. I've measured the actual baudrate (with ~10Hz precision) using a logic analyzer and I got the following results:

baudrate=57861 -> 57870
baudrate=57868 -> 57870
baudrate=57871 -> 58300 <----
baudrate=57874 -> 57870
baudrate=57881 -> 57870

The issue occurs for each baudrate where the integer divisor is about to change, or more precisely, where (baudrate_div & 0x7f) == 0x7f.

@jannic
Copy link
Member

jannic commented Jul 23, 2024

Hi @KisImre!

Thanks for this great analysis. How did you even notice this? I'd guess that the difference is small enough that it doesn't impact the usability of the UART. (https://pdfserv.maximintegrated.com/en/an/AN2141.pdf)

Impressive that you didn't stop with pointing out the bug in the source code, but also confirmed it by measuring the actual timing on the wire.

Given the comment above that code ("Code inspired from the C SDK.") I wonder if the C SDK has the same issue. Did you check that? if it's present there as well, do you want to file a bug yourself, or should I do it?

@KisImre
Copy link
Author

KisImre commented Jul 23, 2024

I just noticed the affected line while I was tinkering with the UART and the (... + 0x7F) + 1 part seemed like a place where an overflow can happen. I put together the attached example to verify my theory and then tested it in the hardware just to see it in practice.

I guess it doesn't affect much on lower baudrates because the error is approx. 1 / divint. However, for a faulty baudrate in the 500000 baud range the error would be around 6% which is not good.

Now I tested the C SDK too, which indeed has the same bug. I will open bug report in their repo as well.

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

No branches or pull requests

2 participants