search

adafruit / Adafruit_nRF52_Arduino

Adafruit code for the Nordic nRF52 BLE SoC on Arduino
Other
606 stars 492 forks source link

Enable custom UART baudrate for nRF52 #712

Open deepindeep opened 2 years ago

deepindeep commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 */
deepindeep commented 2 years ago

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 commented 1 year ago

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.