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

nrf/uart: Use formula instead of switch for baudrate calculation. #4004

Closed
wants to merge 1 commit into from

Conversation

aykevl
Copy link
Contributor

@aykevl aykevl commented Aug 2, 2018

This saves a bit of code:
nrf51: -176
nrf52: -152

Taken from this branch: https://github.com/aykevl/micropython/tree/nrf-hal-uart-raw

@aykevl aykevl requested a review from glennrub August 2, 2018 11:52
@glennrub
Copy link
Contributor

glennrub commented Aug 2, 2018

Looks good @aykevl. Have you done some sanity check or testing of this patch to verify that it is the correct value calculated on target? Does it work for both nrf51 and nrf52?

@aykevl
Copy link
Contributor Author

aykevl commented Aug 2, 2018

I haven't tested it now (only the REPL for the nrf52) but I have properly tested it in the past (that all baudrates match, and most likely on both nrf51 and nrf52). We discussed this a while (~months) back.
Would you rather have it re-tested now?

See this commit:
aykevl@fef3530

This saves a bit of code:
nrf51: -176
nrf52: -152
@aykevl
Copy link
Contributor Author

aykevl commented Aug 6, 2018

I have compared all previously listed baud rates with the computed value on the target and they all match exactly.
There is only one ambiguity: the nrf52 datasheet has two lists for the BAUDRATE that do not match entirely. This is because a slightly different rounding is used in both cases. This formula matches just one of the lists, not the other.
I haven't tested this on the nrf51 target now (but I did with an equivalent commit before). I don't know why it wouldn't work, however, as the nrf51 uses the exact same formula and this formula matches the datasheet.

And of course, the 115200 baudrate works correctly as I can use the REPL over it.

@glennrub is that enough testing? Or should I test the nrf51 as well?

@glennrub
Copy link
Contributor

glennrub commented Aug 6, 2018

@aykevl, when you say that nrf52 have two list, do you mean UART and UARTE baudrate numbers? Which of these are diverging?

@aykevl
Copy link
Contributor Author

aykevl commented Aug 6, 2018

Ah, that explains why there are two lists.
Apparently it's UARTE that has slightly different values. I wonder why? Compare e.g. the register value for 14400bps (0x3af000 vs 0x3b0000) - probably within the error tolerance but I wonder why they're different. Is this a datasheet error or is it really clocked just slightly different?

@dpgeorge
Copy link
Member

This looks good to me, and the testing that was done sounds sufficient. Merged in a293fa3

@dpgeorge dpgeorge closed this Jan 31, 2019
tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Jan 21, 2021
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.

None yet

3 participants