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

Enable custom UART baudrate for nRF52 #712

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

deepindeep
Copy link

Though nRF52 platform supported csutom baudrated for UART,
it was supported in the uart.cpp core driver. It is enabled
to support the same in this patchset

Pradeep added 2 commits January 8, 2022 19:41
 though nRF52 platform supported csutom baudrated for UART,
it was supported in the uart.cpp core driver. It is enabled
to support the same in this patchset
@hathach
Copy link
Member

hathach commented Feb 18, 2022

Sorry for late response, I have been busy before and after the TET with lots of other works. This PR is great update. However, we still need some tweak to get it right.
I have run a simple test sketch to check the computed value and golden/expected value, there is a slight different, it is close but we should tweak the compute function to be EXACT

Here is the sketch and its output result
baud_test.ino.txt

Baud test
Baud: 1200, Expected: 0004F000, Actual: 0004F000 ... OK
Baud: 2400, Expected: 0009D000, Actual: 0009D000 ... OK
Baud: 4800, Expected: 0013B000, Actual: 0013B000 ... OK
Baud: 9600, Expected: 00275000, Actual: 00275000 ... OK
Baud: 14400, Expected: 003AF000, Actual: 003B0000 ... FAILED
Baud: 19200, Expected: 004EA000, Actual: 004EA000 ... OK
Baud: 28800, Expected: 0075C000, Actual: 0075F000 ... FAILED      
Baud: 31250, Expected: 00800000, Actual: 00800000 ... OK          
Baud: 38400, Expected: 009D0000, Actual: 009D5000 ... FAILED      
Baud: 56000, Expected: 00E50000, Actual: 00E56000 ... FAILED      
Baud: 57600, Expected: 00EB0000, Actual: 00EBF000 ... FAILED      
Baud: 76800, Expected: 013A9000, Actual: 013A9000 ... OK
Baud: 115200, Expected: 01D60000, Actual: 01D7E000 ... FAILED
Baud: 230400, Expected: 03B00000, Actual: 03AFB000 ... FAILED
Baud: 250000, Expected: 04000000, Actual: 04000000 ... OK
Baud: 460800, Expected: 07400000, Actual: 075F6000 ... FAILED
Baud: 921600, Expected: 0F000000, Actual: 0EBED000 ... FAILED
Baud: 1000000, Expected: 10000000, Actual: 0FFFF000 ... FAILED

The compute function in this PR used the suggestion by nordic employee here https://devzone.nordicsemi.com/f/nordic-q-a/391/uart-baudrate-register-values

@hathach
Copy link
Member

hathach commented Feb 18, 2022

The pre-computed values are also slightly different e.g 14401 instead of 14400. Since the default values are used by most of other user, using different baud register can cause drift e.g when communicating with other nrf board via uart running pre-computed value.

Could you mind explaining why would you need a custom baudrate ? and specifically which value that you need.

#define UARTE_BAUDRATE_BAUDRATE_Pos (0UL) /*!< Position of BAUDRATE field. */
#define UARTE_BAUDRATE_BAUDRATE_Msk (0xFFFFFFFFUL << UARTE_BAUDRATE_BAUDRATE_Pos) /*!< Bit mask of BAUDRATE field. */
#define UARTE_BAUDRATE_BAUDRATE_Baud1200 (0x0004F000UL) /*!< 1200 baud (actual rate: 1205) */
#define UARTE_BAUDRATE_BAUDRATE_Baud2400 (0x0009D000UL) /*!< 2400 baud (actual rate: 2396) */
#define UARTE_BAUDRATE_BAUDRATE_Baud4800 (0x0013B000UL) /*!< 4800 baud (actual rate: 4808) */
#define UARTE_BAUDRATE_BAUDRATE_Baud9600 (0x00275000UL) /*!< 9600 baud (actual rate: 9598) */
#define UARTE_BAUDRATE_BAUDRATE_Baud14400 (0x003AF000UL) /*!< 14400 baud (actual rate: 14401) */
#define UARTE_BAUDRATE_BAUDRATE_Baud19200 (0x004EA000UL) /*!< 19200 baud (actual rate: 19208) */
#define UARTE_BAUDRATE_BAUDRATE_Baud28800 (0x0075C000UL) /*!< 28800 baud (actual rate: 28777) */
#define UARTE_BAUDRATE_BAUDRATE_Baud31250 (0x00800000UL) /*!< 31250 baud */
#define UARTE_BAUDRATE_BAUDRATE_Baud38400 (0x009D0000UL) /*!< 38400 baud (actual rate: 38369) */
#define UARTE_BAUDRATE_BAUDRATE_Baud56000 (0x00E50000UL) /*!< 56000 baud (actual rate: 55944) */
#define UARTE_BAUDRATE_BAUDRATE_Baud57600 (0x00EB0000UL) /*!< 57600 baud (actual rate: 57554) */
#define UARTE_BAUDRATE_BAUDRATE_Baud76800 (0x013A9000UL) /*!< 76800 baud (actual rate: 76923) */
#define UARTE_BAUDRATE_BAUDRATE_Baud115200 (0x01D60000UL) /*!< 115200 baud (actual rate: 115108) */
#define UARTE_BAUDRATE_BAUDRATE_Baud230400 (0x03B00000UL) /*!< 230400 baud (actual rate: 231884) */
#define UARTE_BAUDRATE_BAUDRATE_Baud250000 (0x04000000UL) /*!< 250000 baud */
#define UARTE_BAUDRATE_BAUDRATE_Baud460800 (0x07400000UL) /*!< 460800 baud (actual rate: 457143) */
#define UARTE_BAUDRATE_BAUDRATE_Baud921600 (0x0F000000UL) /*!< 921600 baud (actual rate: 941176) */
#define UARTE_BAUDRATE_BAUDRATE_Baud1M (0x10000000UL) /*!< 1Mega baud */

Copy link
Member

@hathach hathach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR generate different value than the pre-computed in the bit-field. It is close enough, and may or may not cause the issue. But I would rather stay on the safe side. Please let me know why would you need the custom baud in the first place. If there is a good reason behind, we would find an way to fit this in.

@deepindeep
Copy link
Author

deepindeep commented Mar 4, 2022

Hello @hathach ,
Thanks for taking time to review the patchset!

There are multiple reason that I would want to submit this change:

  1. The baud rate register values present in the baseline below is WRONG for few values and is inconsistent with the nRF52 header files released by nordic (https://github.com/NordicSemiconductor/nrfx/blob/master/mdk/nrf52_bitfields.h).

https://github.com/adafruit/Adafruit_nRF52_Arduino/blob/master/cores/nRF5/Uart.cpp:

#define UARTE_BAUDRATE_BAUDRATE_Pos (0UL) /*!< Position of BAUDRATE field. */
#define UARTE_BAUDRATE_BAUDRATE_Msk (0xFFFFFFFFUL << UARTE_BAUDRATE_BAUDRATE_Pos) /*!< Bit mask of BAUDRATE field. */
#define UARTE_BAUDRATE_BAUDRATE_Baud1200 (0x0004F000UL) /*!< 1200 baud (actual rate: 1205) */
#define UARTE_BAUDRATE_BAUDRATE_Baud2400 (0x0009D000UL) /*!< 2400 baud (actual rate: 2396) */
#define UARTE_BAUDRATE_BAUDRATE_Baud4800 (0x0013B000UL) /*!< 4800 baud (actual rate: 4808) */
#define UARTE_BAUDRATE_BAUDRATE_Baud9600 (0x00275000UL) /*!< 9600 baud (actual rate: 9598) */
#define UARTE_BAUDRATE_BAUDRATE_Baud14400 (0x003AF000UL) /*!< 14400 baud (actual rate: 14401) */
#define UARTE_BAUDRATE_BAUDRATE_Baud19200 (0x004EA000UL) /*!< 19200 baud (actual rate: 19208) */
#define UARTE_BAUDRATE_BAUDRATE_Baud28800 (0x0075C000UL) /*!< 28800 baud (actual rate: 28777) */
#define UARTE_BAUDRATE_BAUDRATE_Baud31250 (0x00800000UL) /*!< 31250 baud */
#define UARTE_BAUDRATE_BAUDRATE_Baud38400 (0x009D0000UL) /*!< 38400 baud (actual rate: 38369) */
#define UARTE_BAUDRATE_BAUDRATE_Baud56000 (0x00E50000UL) /*!< 56000 baud (actual rate: 55944) */
#define UARTE_BAUDRATE_BAUDRATE_Baud57600 (0x00EB0000UL) /*!< 57600 baud (actual rate: 57554) */
#define UARTE_BAUDRATE_BAUDRATE_Baud76800 (0x013A9000UL) /*!< 76800 baud (actual rate: 76923) */
#define UARTE_BAUDRATE_BAUDRATE_Baud115200 (0x01D60000UL) /*!< 115200 baud (actual rate: 115108) */
#define UARTE_BAUDRATE_BAUDRATE_Baud230400 (0x03B00000UL) /*!< 230400 baud (actual rate: 231884) */
#define UARTE_BAUDRATE_BAUDRATE_Baud250000 (0x04000000UL) /*!< 250000 baud */
#define UARTE_BAUDRATE_BAUDRATE_Baud460800 (0x07400000UL) /*!< 460800 baud (actual rate: 457143) */
#define UARTE_BAUDRATE_BAUDRATE_Baud921600 (0x0F000000UL) /*!< 921600 baud (actual rate: 941176) */
#define UARTE_BAUDRATE_BAUDRATE_Baud1M (0x10000000UL) /*!< 1Mega baud */

https://github.com/NordicSemiconductor/nrfx/blob/master/mdk/nrf52_bitfields.h:

/* Register: UART_BAUDRATE */
/* Description: Baud rate */

/* Bits 31..0 : Baud rate */
#define UART_BAUDRATE_BAUDRATE_Pos (0UL) /*!< Position of BAUDRATE field. */
#define UART_BAUDRATE_BAUDRATE_Msk (0xFFFFFFFFUL << UART_BAUDRATE_BAUDRATE_Pos) /*!< Bit mask of BAUDRATE field. */
#define UART_BAUDRATE_BAUDRATE_Baud1200 (0x0004F000UL) /*!< 1200 baud (actual rate: 1205) */
#define UART_BAUDRATE_BAUDRATE_Baud2400 (0x0009D000UL) /*!< 2400 baud (actual rate: 2396) */
#define UART_BAUDRATE_BAUDRATE_Baud4800 (0x0013B000UL) /*!< 4800 baud (actual rate: 4808) */
#define UART_BAUDRATE_BAUDRATE_Baud9600 (0x00275000UL) /*!< 9600 baud (actual rate: 9598) */
#define UART_BAUDRATE_BAUDRATE_Baud14400 (0x003B0000UL) /*!< 14400 baud (actual rate: 14414) */
#define UART_BAUDRATE_BAUDRATE_Baud19200 (0x004EA000UL) /*!< 19200 baud (actual rate: 19208) */
#define UART_BAUDRATE_BAUDRATE_Baud28800 (0x0075F000UL) /*!< 28800 baud (actual rate: 28829) */
#define UART_BAUDRATE_BAUDRATE_Baud31250 (0x00800000UL) /*!< 31250 baud */
#define UART_BAUDRATE_BAUDRATE_Baud38400 (0x009D5000UL) /*!< 38400 baud (actual rate: 38462) */
#define UART_BAUDRATE_BAUDRATE_Baud56000 (0x00E50000UL) /*!< 56000 baud (actual rate: 55944) */
#define UART_BAUDRATE_BAUDRATE_Baud57600 (0x00EBF000UL) /*!< 57600 baud (actual rate: 57762) */
#define UART_BAUDRATE_BAUDRATE_Baud76800 (0x013A9000UL) /*!< 76800 baud (actual rate: 76923) */
#define UART_BAUDRATE_BAUDRATE_Baud115200 (0x01D7E000UL) /*!< 115200 baud (actual rate: 115942) */
#define UART_BAUDRATE_BAUDRATE_Baud230400 (0x03AFB000UL) /*!< 230400 baud (actual rate: 231884) */
#define UART_BAUDRATE_BAUDRATE_Baud250000 (0x04000000UL) /*!< 250000 baud */
#define UART_BAUDRATE_BAUDRATE_Baud460800 (0x075F7000UL) /*!< 460800 baud (actual rate: 470588) */
#define UART_BAUDRATE_BAUDRATE_Baud921600 (0x0EBED000UL) /*!< 921600 baud (actual rate: 941176) */
#define UART_BAUDRATE_BAUDRATE_Baud1M (0x10000000UL) /*!< 1Mega baud */

With my patchset all the values are matching as per definition of nrf52_bitfields.h.

  1. The method void Uart::begin(unsigned long baudrate, uint16_t config) does not prevent user from sending any baudrate value as 'baudrate' argument. If a user sends baudrate that is undefined then the method would assign baudrate of UART_BAUDRATE_BAUDRATE_Baud1M which might not the user/caller intention.

  2. Many Arduino platforms such as Arduino Uno, Nano and even ESP32 support custom baudrates in their core files. When nRF52 has hardware support to do so, we should not be limiting this in firmware.

  3. I was trying to communicate with smart-cards where I wanted custom baud-rates like 13333, 10753 or 13513 bps. It was impossible to get the exact baud-rate with the existing baseline core file. This forced me to submit this patch set.

Please let me know if you have any other concerns in the commit.

@hathach
Copy link
Member

hathach commented Jun 21, 2023

super late response, due to various reason. Custom baudrate is great to have, however, common value still need to use the stock one from nordic defines.

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