uart_set_baudrate() reports success for out-of-range baud rates #20

New issue

Closed

opened 2021-01-25 21:51:23 -06:00 by MustBeArt · 2 comments

MustBeArt commented

2021-01-25 21:51:23 -06:00

(Migrated from github.com)

For example, if I ask for 100 baud, I get an actual baud rate of about 621 (measured on hardware), but uart_set_baudrate() returns 100.

It should either calculate the actual baud rate programmed and return that, or else it should refuse to change the baudrate at all and return a flag value like 0, or the old baud rate. Or, perhaps, the API should change to not return a value at all.

Documentation for UART should list the range of valid baud rates for default/common value(s) of the peripheral clock (clk_peri) frequency. Ideally it should also include a description of how (or whether) to slow down clk_peri in order to achieve low baud rates, and possibly a pointer to PIO as an alternative.

Section 4.2.3.1 of the RP2040 data sheet details limits on the possible baud rates for a given UARTCLK frequency. The code in uart_set_baudrate() has a check for this, but only in an invalid_params_if() macro, which does nothing if asserts are not enabled. In release builds with asserts off, the function goes ahead and programs the baud rate incorrectly, but returns a baud rate approximately equal to the requested baud rate, falsely indicating success. For example, if I ask for 100 baud, I get an actual baud rate of about 621 (measured on hardware), but uart_set_baudrate() returns 100. It should either calculate the actual baud rate programmed and return that, or else it should refuse to change the baudrate at all and return a flag value like 0, or the old baud rate. Or, perhaps, the API should change to not return a value at all. Documentation for UART should list the range of valid baud rates for default/common value(s) of the peripheral clock (clk_peri) frequency. Ideally it should also include a description of how (or whether) to slow down clk_peri in order to achieve low baud rates, and possibly a pointer to PIO as an alternative.

kilograham commented

2021-01-27 10:54:10 -06:00

(Migrated from github.com)

Yeah, since this function returns a return code, I think the return code should valid; probably change this to return an int and return an error in this case

Rows
Columns